Refactor RapidFire (#1026)

* Fix RapidFireKey argument expansion
* Add basic unit test
* Fix calling keyboard.tap() from within a scheduled task
* Simplify code and memory management

kmk/modules/rapidfire.py

@@ -1,7 +1,16 @@
+from micropython import const
+
 from random import randint
 
 from kmk.keys import Key, make_argumented_key
 from kmk.modules import Module
+from kmk.utils import Debug
+
+debug = Debug(__name__)
+
+_INACTIVE = const(0)
+_HOLD = const(1)
+_ACTIVE = const(2)
 
 
 class RapidFireKey(Key):
@@ -13,7 +22,7 @@ def __init__(
         enable_interval_randomization=False,
         randomization_magnitude=15,
         toggle=False,
-        *kwargs,
+        **kwargs,
     ):
         super().__init__(**kwargs)
         self.key = key
@@ -22,13 +31,11 @@ def __init__(
         self.enable_interval_randomization = enable_interval_randomization
         self.randomization_magnitude = randomization_magnitude
         self.toggle = toggle
+        self._state = _INACTIVE
+        self._timeout = None
 
 
 class RapidFire(Module):
-    _active_keys = {}
-    _toggled_keys = []
-    _waiting_keys = []
-
     def __init__(self):
         make_argumented_key(
             names=('RF',),
@@ -37,51 +44,57 @@ def __init__(self):
             on_release=self._rf_released,
         )
 
-    def _get_repeat(self, key):
+    def _on_timer_timeout(self, key, keyboard):
+        if key._state == _HOLD:
+            key._state = _ACTIVE
+            keyboard.remove_key(key.key)
+            key._timeout = keyboard.set_timeout(
+                1, lambda: self._on_timer_timeout(key, keyboard)
+            )
+            return
+
+        keyboard.add_key(key.key)
+        keyboard.set_timeout(1, lambda: keyboard.remove_key(key.key))
+
+        interval = key.interval
         if key.enable_interval_randomization:
-            return key.interval + randint(
+            interval += randint(
                 -key.randomization_magnitude, key.randomization_magnitude
             )
-        return key.interval
-
-    def _on_timer_timeout(self, key, keyboard):
-        keyboard.tap_key(key.key)
-        if key in self._waiting_keys:
-            self._waiting_keys.remove(key)
-        if key.toggle and key not in self._toggled_keys:
-            self._toggled_keys.append(key)
-        self._active_keys[key] = keyboard.set_timeout(
-            self._get_repeat(key), lambda: self._on_timer_timeout(key, keyboard)
+        key._timeout = keyboard.set_timeout(
+            interval, lambda: self._on_timer_timeout(key, keyboard)
         )
 
+        if debug.enabled:
+            debug(key.key, ' @', interval, 'ms')
+
     def _rf_pressed(self, key, keyboard, *args, **kwargs):
-        if key in self._toggled_keys:
-            self._toggled_keys.remove(key)
+        if key._state == _ACTIVE:
             self._deactivate_key(key, keyboard)
             return
-        if key.timeout > 0:
-            keyboard.tap_key(key.key)
-            self._waiting_keys.append(key)
-            self._active_keys[key] = keyboard.set_timeout(
-                key.timeout, lambda: self._on_timer_timeout(key, keyboard)
-            )
-        else:
-            self._on_timer_timeout(key, keyboard)
+
+        keyboard.add_key(key.key)
+        key._state = _HOLD
+        key._timeout = keyboard.set_timeout(
+            key.timeout, lambda: self._on_timer_timeout(key, keyboard)
+        )
 
     def _rf_released(self, key, keyboard, *args, **kwargs):
-        if key not in self._active_keys:
-            return
-        if key in self._toggled_keys:
-            if key not in self._waiting_keys:
+        if key._state == _ACTIVE:
+            if key.toggle:
                 return
-            self._toggled_keys.remove(key)
-        if key in self._waiting_keys:
-            self._waiting_keys.remove(key)
+            key._state = _INACTIVE
+        elif key._state == _INACTIVE:
+            return
+        else:
+            keyboard.remove_key(key.key)
+
         self._deactivate_key(key, keyboard)
 
     def _deactivate_key(self, key, keyboard):
-        keyboard.cancel_timeout(self._active_keys[key])
-        self._active_keys.pop(key)
+        keyboard.cancel_timeout(key._timeout)
+        key._state = _INACTIVE
+        key._timeout = None
 
     def during_bootup(self, keyboard):
         return

tests/test_rapidfire.py

@@ -0,0 +1,94 @@
+import unittest
+
+from kmk.keys import KC
+from kmk.modules.rapidfire import RapidFire
+from tests.keyboard_test import KeyboardTest
+
+t_interval = 4 * KeyboardTest.loop_delay_ms
+t_timeout = 10 * KeyboardTest.loop_delay_ms
+t_hold = t_timeout + 5 * t_interval + KeyboardTest.loop_delay_ms
+
+
+class TestKeyRepeat(unittest.TestCase):
+    @classmethod
+    def setUpClass(cls):
+        KC.clear()
+
+        cls.keyboard = KeyboardTest(
+            [RapidFire()],
+            [
+                [
+                    KC.RF(KC.N0, interval=t_interval, timeout=t_timeout),
+                    KC.RF(KC.N1, interval=t_interval, timeout=t_timeout, toggle=True),
+                ],
+            ],
+            debug_enabled=False,
+        )
+
+    def test_rapidfire(self):
+        self.keyboard.test(
+            '',
+            [(0, True), (0, False)],
+            [{KC.N0}, {}],
+        )
+
+        self.keyboard.test(
+            '',
+            [(0, True), t_timeout // 2, (0, False)],
+            [{KC.N0}, {}],
+        )
+
+        self.keyboard.test(
+            '',
+            [(0, True), t_timeout + t_interval // 2, (0, False)],
+            [{KC.N0}, {}, {KC.N0}, {}],
+        )
+
+        self.keyboard.test(
+            '',
+            [(0, True), t_timeout + (3 * t_interval) // 2, (0, False)],
+            [{KC.N0}, {}, {KC.N0}, {}, {KC.N0}, {}],
+        )
+
+        self.keyboard.test(
+            '',
+            [(0, True), t_hold, (0, False)],
+            [
+                {KC.N0},
+                {},
+                {KC.N0},
+                {},
+                {KC.N0},
+                {},
+                {KC.N0},
+                {},
+                {KC.N0},
+                {},
+                {KC.N0},
+                {},
+            ],
+        )
+
+        self.keyboard.test(
+            '',
+            [
+                (1, True),
+                t_timeout + t_interval // 2,
+                (1, False),
+                3 * t_interval,
+                (1, True),
+                (1, False),
+            ],
+            [
+                {KC.N1},
+                {},
+                {KC.N1},
+                {},
+                {KC.N1},
+                {},
+                {KC.N1},
+                {},
+                {KC.N1},
+                {},
+            ],
+        )