make TapDance a module (#281)

* extract tapdance logic into a module

* clean out old tapdance code

* canonicalize key variable names

* split _process_tap_dance into td_pressed and td_released

* implement consistent argument order

* update documentation

* implement Module.process_key for key interception and modification

* fix tapdance realesing instead of pressing

* fix: default parameters in key handler

* cleanup holdtap

* add error handling to modules process_key

* fix: key released too late

Tapped keys didn't release on a "key released" event, but waited for a
timeout. Resulted in, for example, modifiers applying to keys after the
modifier was released.

* fix lint/formatting

* fix tap_time reference in modtap + minimal documentation

* fix lint

docs/modtap.md

@@ -4,7 +4,10 @@ added to the modules list.
 
 ```python
 from kmk.modules.modtap import ModTap
-keyboard.modules.append(ModTap())
+modtap = ModTap()
+# optional: set a custom tap timeout in ms
+# modtap.tap_time = 300
+keyboard.modules.append(modtap)
 ```
 
 ## Keycodes

docs/modules.md

@@ -13,3 +13,4 @@ put on your keyboard
 when tapped, and modifier when held.
 - [Power](power.md): Power saving features. This is mostly useful when on battery power.
 - [Split](split_keyboards.md): Keyboards split in two. Seems ergonomic!
+- [TapDance](tapdance.md): Different key actions depending on how often it is pressed.

docs/tapdance.md

@@ -23,6 +23,8 @@ keymap somewhere. The only limits on how many keys can go in the sequence are,
 theoretically, the amount of RAM your MCU/board has, and how fast you can mash
 the physical key. Here's your chance to use all that button-mash video game
 experience you've built up over the years.
+[//]: # (The button mashing part has been 'fixed' by a timeout refresh per)
+[//]: # (button press. The comedic sentiment is worth keeping though.)
 
 **NOTE**: Currently our basic tap dance implementation has some limitations that
 are planned to be worked around "eventually", but for now are noteworthy:
@@ -31,16 +33,23 @@ are planned to be worked around "eventually", but for now are noteworthy:
   currently "undefined" at best, and will probably crash your keyboard. For now,
   we strongly recommend avoiding `KC.MO` (or any other layer switch keys that
   use momentary switch behavior - `KC.LM`, `KC.LT`, and `KC.TT`)
+[//]: # (This also doesn't seem to be the case anymore; as long as the layer)
+[//]: # (is transparent to the tap dance key.)
+[//]: # (At least KC.MO is working as intended, other momentary actions haven't)
+[//]: # (been tested.)
 
 Here's an example of all this in action:
 
 ```python
 from kmk.keycodes import KC
-from kmk.macros.simple import send_string
+from kmk.handlers.sequences import send_string
+from kmk.modules.tapdance import TapDance
 
 keyboard = KMKKeyboard()
 
-keyboard.tap_time = 750
+tapdance = TapDance()
+tapdance.tap_time = 750
+keyboard.modules.append(tapdance)
 
 EXAMPLE_TD = KC.TD(
     KC.A,  # Tap once for "a"

kmk/handlers/stock.py

@@ -105,14 +105,6 @@ def uc_mode_pressed(key, keyboard, *args, **kwargs):
     return keyboard
 
 
-def td_pressed(key, keyboard, *args, **kwargs):
-    return keyboard._process_tap_dance(key, True)
-
-
-def td_released(key, keyboard, *args, **kwargs):
-    return keyboard._process_tap_dance(key, False)
-
-
 def hid_switch(key, keyboard, *args, **kwargs):
     keyboard.hid_type, keyboard.secondary_hid_type = (
         keyboard.secondary_hid_type,

kmk/keys.py

@@ -3,11 +3,7 @@ from micropython import const
 
 import kmk.handlers.stock as handlers
 from kmk.consts import UnicodeMode
-from kmk.key_validators import (
+from kmk.key_validators import key_seq_sleep_validator, unicode_mode_key_validator
-    key_seq_sleep_validator,
-    tap_dance_key_validator,
-    unicode_mode_key_validator,
-)
 from kmk.types import AttrDict, UnicodeModeKeyMeta
 
 DEBUG_OUTPUT = False
@@ -167,13 +163,6 @@ class KeyAttrDict(AttrDict):
                 names=('UC_MODE',),
                 on_press=handlers.uc_mode_pressed,
             )
-        elif key in ('TAP_DANCE', 'TD'):
-            make_argumented_key(
-                validator=tap_dance_key_validator,
-                names=('TAP_DANCE', 'TD'),
-                on_press=handlers.td_pressed,
-                on_release=handlers.td_released,
-            )
         elif key in ('HID_SWITCH', 'HID'):
             make_key(names=('HID_SWITCH', 'HID'), on_press=handlers.hid_switch)
         else:
@@ -417,7 +406,7 @@ class Key:
     def __repr__(self):
         return 'Key(code={}, has_modifiers={})'.format(self.code, self.has_modifiers)
 
-    def on_press(self, state, coord_int, coord_raw):
+    def on_press(self, state, coord_int=None, coord_raw=None):
         if hasattr(self, '_pre_press_handlers'):
             for fn in self._pre_press_handlers:
                 if not fn(self, state, KC, coord_int, coord_raw):
@@ -431,7 +420,7 @@ class Key:
 
         return ret
 
-    def on_release(self, state, coord_int, coord_raw):
+    def on_release(self, state, coord_int=None, coord_raw=None):
         if hasattr(self, '_pre_release_handlers'):
             for fn in self._pre_release_handlers:
                 if not fn(self, state, KC, coord_int, coord_raw):

kmk/kmk_keyboard.py

@@ -4,7 +4,6 @@ from kmk.consts import KMK_RELEASE, UnicodeMode
 from kmk.hid import BLEHID, USBHID, AbstractHID, HIDModes
 from kmk.keys import KC
 from kmk.matrix import MatrixScanner, intify_coordinate
-from kmk.types import TapDanceKeyMeta
 
 
 class Sandbox:
@@ -29,7 +28,6 @@ class KMKKeyboard:
     uart_buffer = []
 
     unicode_mode = UnicodeMode.NOOP
-    tap_time = 300
 
     modules = []
     extensions = []
@@ -61,9 +59,6 @@ class KMKKeyboard:
     active_layers = [0]
 
     _timeouts = {}
-    _tapping = False
-    _tap_dance_counts = {}
-    _tap_side_effects = {}
 
     # on some M4 setups (such as klardotsh/klarank_feather_m4, CircuitPython
     # 6.0rc1) this runs out of RAM every cycle and takes down the board. no
@@ -76,23 +71,18 @@ class KMKKeyboard:
             'diode_orientation={} '
             'matrix_scanner={} '
             'unicode_mode={} '
-            'tap_time={} '
             '_hid_helper={} '
             'keys_pressed={} '
             'coordkeys_pressed={} '
             'hid_pending={} '
             'active_layers={} '
             'timeouts={} '
-            'tapping={} '
-            'tap_dance_counts={} '
-            'tap_side_effects={}'
             ')'
         ).format(
             self.debug_enabled,
             self.diode_orientation,
             self.matrix_scanner,
             self.unicode_mode,
-            self.tap_time,
             self._hid_helper,
             # internal state
             self.keys_pressed,
@@ -100,9 +90,6 @@ class KMKKeyboard:
             self.hid_pending,
             self.active_layers,
             self._timeouts,
-            self._tapping,
-            self._tap_dance_counts,
-            self._tap_side_effects,
         )
 
     def _print_debug_cycle(self, init=False):
@@ -171,16 +158,22 @@ class KMKKeyboard:
             print('MatrixUndefinedCoordinate(col={} row={})'.format(col, row))
             return self
 
-        return self.process_key(self.current_key, is_pressed, int_coord, (row, col))
+        for module in self.modules:
+            try:
+                if module.process_key(self, self.current_key, is_pressed) is None:
+                    break
+            except Exception as err:
+                if self.debug_enabled:
+                    print('Failed to run process_key function in module: ', err, module)
+        else:
+            self.process_key(self.current_key, is_pressed, int_coord, (row, col))
+        return self
 
     def process_key(self, key, is_pressed, coord_int=None, coord_raw=None):
-        if self._tapping and isinstance(key.meta, TapDanceKeyMeta):
+        if is_pressed:
-            self._process_tap_dance(key, is_pressed)
+            key.on_press(self, coord_int, coord_raw)
         else:
-            if is_pressed:
+            key.on_release(self, coord_int, coord_raw)
-                key.on_press(self, coord_int, coord_raw)
-            else:
-                key.on_release(self, coord_int, coord_raw)
 
         return self
 
@@ -199,85 +192,6 @@ class KMKKeyboard:
 
         return self
 
-    def _process_tap_dance(self, changed_key, is_pressed):
-        if is_pressed:
-            if not isinstance(changed_key.meta, TapDanceKeyMeta):
-                # If we get here, changed_key is not a TapDanceKey and thus
-                # the user kept typing elsewhere (presumably).  End ALL of the
-                # currently outstanding tap dance runs.
-                for k, v in self._tap_dance_counts.items():
-                    if v:
-                        self._end_tap_dance(k)
-
-                return self
-
-            if (
-                changed_key not in self._tap_dance_counts
-                or not self._tap_dance_counts[changed_key]
-            ):
-                self._tap_dance_counts[changed_key] = 1
-                self._tapping = True
-            else:
-                self.cancel_timeout(self._tap_timeout)
-                self._tap_dance_counts[changed_key] += 1
-
-            if changed_key not in self._tap_side_effects:
-                self._tap_side_effects[changed_key] = None
-
-            self._tap_timeout = self.set_timeout(
-                self.tap_time, lambda: self._end_tap_dance(changed_key, hold=True)
-            )
-        else:
-            if not isinstance(changed_key.meta, TapDanceKeyMeta):
-                return self
-
-            has_side_effects = self._tap_side_effects[changed_key] is not None
-            hit_max_defined_taps = self._tap_dance_counts[changed_key] == len(
-                changed_key.meta.codes
-            )
-
-            if has_side_effects or hit_max_defined_taps:
-                self._end_tap_dance(changed_key)
-
-            self.cancel_timeout(self._tap_timeout)
-            self._tap_timeout = self.set_timeout(
-                self.tap_time, lambda: self._end_tap_dance(changed_key)
-            )
-
-        return self
-
-    def _end_tap_dance(self, td_key, hold=False):
-        v = self._tap_dance_counts[td_key] - 1
-
-        if v < 0:
-            return self
-
-        if td_key in self.keys_pressed:
-            key_to_press = td_key.meta.codes[v]
-            self.add_key(key_to_press)
-            self._tap_side_effects[td_key] = key_to_press
-        elif self._tap_side_effects[td_key]:
-            self.remove_key(self._tap_side_effects[td_key])
-            self._tap_side_effects[td_key] = None
-            self._cleanup_tap_dance(td_key)
-        elif hold is False:
-            if td_key.meta.codes[v] in self.keys_pressed:
-                self.remove_key(td_key.meta.codes[v])
-            else:
-                self.tap_key(td_key.meta.codes[v])
-            self._cleanup_tap_dance(td_key)
-        else:
-            self.add_key(td_key.meta.codes[v])
-
-        self.hid_pending = True
-
-        return self
-
-    def _cleanup_tap_dance(self, td_key):
-        self._tap_dance_counts[td_key] = 0
-        self._tapping = any(count > 0 for count in self._tap_dance_counts.values())
-        return self
-
     def set_timeout(self, after_ticks, callback):
         if after_ticks is False:
             # We allow passing False as an implicit "run this on the next process timeouts cycle"

kmk/modules/__init__.py

@@ -38,3 +38,6 @@ class Module:
 
     def on_powersave_disable(self, keyboard):
         raise NotImplementedError
+
+    def process_key(self, keyboard, key, is_pressed):
+        return key

kmk/modules/holdtap.py

@@ -1,6 +1,7 @@
 from micropython import const
 
 from kmk.modules import Module
+from kmk.types import ModTapKeyMeta
 
 
 class ActivationType:
@@ -18,6 +19,8 @@ class HoldTapKeyState:
 
 
 class HoldTap(Module):
+    tap_time = 300
+
     def __init__(self):
         self.key_states = {}
 
@@ -28,8 +31,12 @@ class HoldTap(Module):
         return
 
     def after_matrix_scan(self, keyboard):
+        return
+
+    def process_key(self, keyboard, key, is_pressed):
         '''Before other key down decide to send tap kc down.'''
-        if self.matrix_detected_press(keyboard):
+        current_key = key
+        if is_pressed and not isinstance(key.meta, ModTapKeyMeta):
             for key, state in self.key_states.items():
                 if state.activated == ActivationType.NOT_ACTIVATED:
                     # press tap because interrupted by other key
@@ -39,7 +46,7 @@ class HoldTap(Module):
                     )
                     if keyboard.hid_pending:
                         keyboard._send_hid()
-        return
+        return current_key
 
     def before_hid_send(self, keyboard):
         return
@@ -53,16 +60,10 @@ class HoldTap(Module):
     def on_powersave_disable(self, keyboard):
         return
 
-    def matrix_detected_press(self, keyboard):
-        return (keyboard.matrix_update is not None and keyboard.matrix_update[2]) or (
-            keyboard.secondary_matrix_update is not None
-            and keyboard.secondary_matrix_update[2]
-        )
-
     def ht_pressed(self, key, keyboard, *args, **kwargs):
         '''Do nothing yet, action resolves when key is released, timer expires or other key is pressed.'''
         timeout_key = keyboard.set_timeout(
-            keyboard.tap_time,
+            self.tap_time,
             lambda: self.on_tap_time_expired(key, keyboard, *args, **kwargs),
         )
         self.key_states[key] = HoldTapKeyState(timeout_key, *args, **kwargs)

kmk/modules/tapdance.py

@@ -0,0 +1,124 @@
+from kmk.key_validators import tap_dance_key_validator
+from kmk.keys import make_argumented_key
+from kmk.modules import Module
+from kmk.types import TapDanceKeyMeta
+
+
+class TapDance(Module):
+    # User-configurable
+    tap_time = 300
+
+    # Internal State
+    _tapping = False
+    _tap_dance_counts = {}
+    _tap_timeout = None
+    _tap_side_effects = {}
+
+    def __init__(self):
+        make_argumented_key(
+            validator=tap_dance_key_validator,
+            names=('TAP_DANCE', 'TD'),
+            on_press=self.td_pressed,
+            on_release=self.td_released,
+        )
+
+    def during_bootup(self, keyboard):
+        return
+
+    def before_matrix_scan(self, keyboard):
+        return
+
+    def after_matrix_scan(self, keyboard):
+        return
+
+    def before_hid_send(self, keyboard):
+        return
+
+    def after_hid_send(self, keyboard):
+        return
+
+    def on_powersave_enable(self, keyboard):
+        return
+
+    def on_powersave_disable(self, keyboard):
+        return
+
+    def process_key(self, keyboard, key, is_pressed):
+        if self._tapping and is_pressed and not isinstance(key.meta, TapDanceKeyMeta):
+            for k, v in self._tap_dance_counts.items():
+                if v:
+                    self._end_tap_dance(k, keyboard, hold=True)
+                    keyboard.hid_pending = True
+                    keyboard._send_hid()
+                    keyboard.set_timeout(
+                        False, lambda: keyboard.process_key(key, is_pressed, None, None)
+                    )
+                    return None
+
+        return key
+
+    def td_pressed(self, key, keyboard, *args, **kwargs):
+        if key not in self._tap_dance_counts or not self._tap_dance_counts[key]:
+            self._tap_dance_counts[key] = 1
+            self._tapping = True
+        else:
+            keyboard.cancel_timeout(self._tap_timeout)
+            self._tap_dance_counts[key] += 1
+
+        if key not in self._tap_side_effects:
+            self._tap_side_effects[key] = None
+
+        self._tap_timeout = keyboard.set_timeout(
+            self.tap_time, lambda: self._end_tap_dance(key, keyboard, hold=True)
+        )
+
+        return self
+
+    def td_released(self, key, keyboard, *args, **kwargs):
+        has_side_effects = self._tap_side_effects[key] is not None
+        hit_max_defined_taps = self._tap_dance_counts[key] == len(key.meta.codes)
+
+        keyboard.cancel_timeout(self._tap_timeout)
+        if has_side_effects or hit_max_defined_taps:
+            self._end_tap_dance(key, keyboard)
+        else:
+            self._tap_timeout = keyboard.set_timeout(
+                self.tap_time, lambda: self._end_tap_dance(key, keyboard)
+            )
+
+        return self
+
+    def _end_tap_dance(self, key, keyboard, hold=False):
+        v = self._tap_dance_counts[key] - 1
+
+        if v < 0:
+            return self
+
+        if key in keyboard.keys_pressed:
+            key_to_press = key.meta.codes[v]
+            keyboard.add_key(key_to_press)
+            self._tap_side_effects[key] = key_to_press
+        elif self._tap_side_effects[key]:
+            keyboard.remove_key(self._tap_side_effects[key])
+            self._tap_side_effects[key] = None
+            self._cleanup_tap_dance(key)
+        elif hold is False:
+            if key.meta.codes[v] in keyboard.keys_pressed:
+                keyboard.remove_key(key.meta.codes[v])
+            else:
+                keyboard.tap_key(key.meta.codes[v])
+            self._cleanup_tap_dance(key)
+        else:
+            key_to_press = key.meta.codes[v]
+            keyboard.add_key(key_to_press)
+            self._tap_side_effects[key] = key_to_press
+            self._tapping = 0
+
+        keyboard.hid_pending = True
+
+        return self
+
+    def _cleanup_tap_dance(self, key):
+        self._tap_dance_counts[key] = 0
+        self._tapping = any(count > 0 for count in self._tap_dance_counts.values())
+        return self