fix inconsistencies with chained holdtap keys

kmk/modules/holdtap.py

@@ -45,6 +45,9 @@ class HoldTap(Module):
     def process_key(self, keyboard, key, is_pressed, int_coord):
         '''Handle holdtap being interrupted by another key press/release.'''
         current_key = key
+        send_buffer = False
+        append_buffer = False
+
         for key, state in self.key_states.items():
             if key == current_key:
                 continue
@@ -61,20 +64,28 @@ class HoldTap(Module):
                 self.ht_activate_on_interrupt(
                     key, keyboard, *state.args, **state.kwargs
                 )
-
                 keyboard._send_hid()
-
+                send_buffer = True
-                self.send_key_buffer(keyboard)
 
             if state.activated == ActivationType.INTERRUPTED:
                 current_key = keyboard._find_key_in_map(int_coord)
 
             # if interrupt on release: store interrupting keys until one of them
             # is released.
-            if key.meta.tap_interrupted:
+            if (
-                if is_pressed:
+                key.meta.tap_interrupted
-                    self.key_buffer.append((int_coord, current_key))
+                and is_pressed
-                    current_key = None
+                and not isinstance(current_key.meta, HoldTapKeyMeta)
+            ):
+                append_buffer = True
+
+        # apply changes with 'side-effects' on key_states or the loop behaviour
+        # outside the loop.
+        if append_buffer:
+            self.key_buffer.append((int_coord, current_key))
+            current_key = None
+        elif send_buffer:
+            self.send_key_buffer(keyboard)
 
         return current_key
 
@@ -105,24 +116,29 @@ class HoldTap(Module):
 
     def ht_released(self, key, keyboard, *args, **kwargs):
         '''On keyup, release mod or tap key.'''
-        if key in self.key_states:
+        if key not in self.key_states:
-            state = self.key_states[key]
+            return keyboard
-            keyboard.cancel_timeout(state.timeout_key)
+
-            if state.activated == ActivationType.HOLD_TIMEOUT:
+        state = self.key_states[key]
-                # release hold
+        keyboard.cancel_timeout(state.timeout_key)
-                self.ht_deactivate_hold(key, keyboard, *args, **kwargs)
+
-            elif state.activated == ActivationType.INTERRUPTED:
+        if state.activated == ActivationType.HOLD_TIMEOUT:
-                # release tap
+            # release hold
-                self.ht_deactivate_on_interrupt(key, keyboard, *args, **kwargs)
+            self.ht_deactivate_hold(key, keyboard, *args, **kwargs)
-            elif state.activated == ActivationType.PRESSED:
+        elif state.activated == ActivationType.INTERRUPTED:
-                # press and release tap because key released within tap time
+            # release tap
-                self.ht_activate_tap(key, keyboard, *args, **kwargs)
+            self.ht_deactivate_on_interrupt(key, keyboard, *args, **kwargs)
-                keyboard.set_timeout(
+        elif state.activated == ActivationType.PRESSED:
-                    False,
+            # press and release tap because key released within tap time
-                    lambda: self.ht_deactivate_tap(key, keyboard, *args, **kwargs),
+            self.ht_activate_tap(key, keyboard, *args, **kwargs)
-                )
+            keyboard.set_timeout(
-                self.send_key_buffer(keyboard)
+                False,
-            del self.key_states[key]
+                lambda: self.ht_deactivate_tap(key, keyboard, *args, **kwargs),
+            )
+            state.activated = ActivationType.RELEASED
+            self.send_key_buffer(keyboard)
+        del self.key_states[key]
+
         return keyboard
 
     def on_tap_time_expired(self, key, keyboard, *args, **kwargs):
@@ -145,9 +161,12 @@ class HoldTap(Module):
             del self.key_states[key]
 
     def send_key_buffer(self, keyboard):
-        for (int_coord, key) in self.key_buffer:
+        key_buffer = self.key_buffer
-            key.on_press(keyboard)
+        self.key_buffer = []
-            keyboard._send_hid()
+        for (int_coord, key) in key_buffer:
+            new_key = keyboard._find_key_in_map(int_coord)
+            keyboard.pre_process_key(new_key, True, int_coord)
+        keyboard._send_hid()
         self.key_buffer.clear()
 
     def ht_activate_hold(self, key, keyboard, *args, **kwargs):

tests/test_hold_tap.py

@@ -8,7 +8,7 @@ from tests.keyboard_test import KeyboardTest
 
 
 class TestHoldTap(unittest.TestCase):
-    def test_basic_kmk_keyboard(self):
+    def test_holdtap(self):
         keyboard = KeyboardTest(
             [Layers(), ModTap(), OneShot()],
             [
@@ -100,6 +100,168 @@ class TestHoldTap(unittest.TestCase):
             [{KC.N4}, {}],
         )
 
+        keyboard.test(
+            'LT after tap time nested -> hold behavior',
+            [
+                (0, True),
+                350,
+                (1, True),
+                350,
+                (3, True),
+                (3, False),
+                (1, False),
+                (0, False),
+            ],
+            [{KC.LCTL}, {KC.LCTL, KC.N4}, {KC.LCTL}, {}],
+        )
+
+    def test_holdtap_chain(self):
+        keyboard = KeyboardTest(
+            [ModTap()],
+            [
+                [
+                    KC.N0,
+                    KC.MT(KC.N1, KC.LCTL, tap_time=50),
+                    KC.MT(KC.N2, KC.LSFT, tap_interrupted=True, tap_time=50),
+                    KC.MT(
+                        KC.N3,
+                        KC.LALT,
+                        prefer_hold=False,
+                        tap_interrupted=True,
+                        tap_time=50,
+                    ),
+                ],
+            ],
+            debug_enabled=False,
+        )
+        # t_within = 40
+        t_after = 60
+
+        keyboard.test(
+            'chained 0',
+            [(1, True), (2, True), (0, True), (0, False), (2, False), (1, False)],
+            [
+                {KC.LCTL},
+                {KC.LCTL, KC.LSFT},
+                {KC.LCTL, KC.LSFT, KC.N0},
+                {KC.LCTL, KC.LSFT},
+                {KC.LCTL},
+                {},
+            ],
+        )
+
+        keyboard.test(
+            'chained 1',
+            [(2, True), (1, True), (0, True), (0, False), (1, False), (2, False)],
+            [
+                {KC.LCTL},
+                {KC.LCTL, KC.LSFT},
+                {KC.LCTL, KC.LSFT, KC.N0},
+                {KC.LCTL, KC.LSFT},
+                {KC.LSFT},
+                {},
+            ],
+        )
+
+        keyboard.test(
+            'chained 2',
+            [(1, True), (2, True), (0, True), (1, False), (2, False), (0, False)],
+            [
+                {KC.LCTL},
+                {KC.LCTL, KC.LSFT},
+                {KC.LCTL, KC.LSFT, KC.N0},
+                {KC.LSFT, KC.N0},
+                {KC.N0},
+                {},
+            ],
+        )
+
+        keyboard.test(
+            'chained 3',
+            [(1, True), (3, True), (0, True), (0, False), (1, False), (3, False)],
+            [
+                {KC.LCTL},
+                {KC.LCTL, KC.N3},
+                {KC.LCTL, KC.N3, KC.N0},
+                {KC.LCTL, KC.N3},
+                {KC.N3},
+                {},
+            ],
+        )
+
+        keyboard.test(
+            'chained 4',
+            [(1, True), (3, True), (0, True), (3, False), (1, False), (0, False)],
+            [{KC.LCTL}, {KC.LCTL, KC.N3, KC.N0}, {KC.LCTL, KC.N0}, {KC.N0}, {}],
+        )
+
+        keyboard.test(
+            'chained 5',
+            [(3, True), (1, True), (0, True), (0, False), (1, False), (3, False)],
+            [
+                {KC.LCTL},
+                {KC.LCTL, KC.N3},
+                {KC.LCTL, KC.N3, KC.N0},
+                {KC.LCTL, KC.N3},
+                {KC.N3},
+                {},
+            ],
+        )
+
+        keyboard.test(
+            'chained 6',
+            [
+                (3, True),
+                (1, True),
+                t_after,
+                (0, True),
+                (0, False),
+                (1, False),
+                (3, False),
+            ],
+            [
+                {KC.LCTL, KC.LALT},
+                {KC.LCTL, KC.LALT, KC.N0},
+                {KC.LCTL, KC.LALT},
+                {KC.LALT},
+                {},
+            ],
+        )
+
+        keyboard.test(
+            'chained 7',
+            [
+                (1, True),
+                (3, True),
+                t_after,
+                (0, True),
+                (0, False),
+                (1, False),
+                (3, False),
+            ],
+            [
+                {KC.LCTL},
+                {KC.LCTL, KC.LALT},
+                {KC.LCTL, KC.LALT, KC.N0},
+                {KC.LCTL, KC.LALT},
+                {KC.LALT},
+                {},
+            ],
+        )
+
+        keyboard.test(
+            'chained 8',
+            [(2, True), (3, True), (0, True), (0, False), (2, False), (3, False)],
+            [
+                {KC.LSFT},
+                {KC.LSFT, KC.N3},
+                {KC.LSFT, KC.N3, KC.N0},
+                {KC.LSFT, KC.N3},
+                {KC.N3},
+                {},
+            ],
+        )
+
         # TODO test TT
 
     def test_oneshot(self):