Merge pull request #1111 from gedankenexperimenter/tapdance-hold

plugins/Kaleidoscope-TapDance/README.md

@@ -32,20 +32,22 @@ timer expires, then the tap-dance key will trigger an action first, perform it,
 and only then will the firmware continue handling the interrupting key press.
 This is to preserve the order of keys pressed.
 
-In both of these cases, the [`tapDanceAction`][tdaction] will be called, with
-`tapDanceIndex` set to the index of the tap-dance action (as set in the keymap),
-the `tapCount`, and `tapDanceAction` set to either
-`kaleidoscope::plugin::TapDance::Interrupt`, or
-`kaleidoscope::plugin::TapDance::Timeout`. If we continue holding the key, then
-as long as it is held, the same function will be called with `tapDanceAction`
-set to `kaleidoscope::plugin::TapDance::Hold`. When the key is released, after
-either an `Interrupt` or `Timeout` action was triggered, the function will be
-called with `tapDanceAction` set to `kaleidoscope::plugin::TapDance::Release`.
+In both of these cases, the user-defined `tapDanceAction()` function will be
+called, with `tap_dance_index` set to the index of the tap-dance action (as set
+in the keymap), the `tap_count`, and `tap_dance_action` set to one of the
+following values:
+
+- `kaleidoscope::plugin::TapDance::Hold`, if the tap-dance key is still being
+  held when its timeout expires.
+- `kaleidoscope::plugin::TapDance::Timeout`, if the tap-dance key has been
+  released when its timeout expires.
+- `kaleidoscope::plugin::TapDance::Interrupt`, if another key is pressed before
+  the tap-dance key's timeout expires.
 
 These actions allow us to create sophisticated tap-dance setups, where one can
 tap a key twice and hold it, and have it repeat, for example.
 
-There is one additional value the `tapDanceAction` parameter can take:
+There is one additional value the `tap_dance_action` parameter can take:
 `kaleidoscope::plugin::TapDance::Tap`. It is called with this argument for each
 and every tap, even if no action is to be triggered yet. This is so that we can
 have a way to do some side-effects, like light up LEDs to show progress, and so

plugins/Kaleidoscope-TapDance/src/kaleidoscope/plugin/TapDance.cpp

@@ -45,7 +45,7 @@ void TapDance::actionKeys(uint8_t tap_count,
   KeyEvent event = event_queue_.event(0);
   event.key = tap_keys[tap_count - 1].readFromProgmem();
 
-  if (action == Interrupt || action == Timeout) {
+  if (action == Interrupt || action == Timeout || action == Hold) {
     event_queue_.shift();
     Runtime.handleKeyswitchEvent(event);
   } else if (action == Tap && tap_count == max_keys) {
@@ -136,7 +136,20 @@ EventHandlerResult TapDance::afterEachCycle() {
   // Check for timeout
   uint16_t start_time = event_queue_.timestamp(0);
   if (Runtime.hasTimeExpired(start_time, time_out)) {
-    tapDanceAction(td_id, td_addr, tap_count_, Timeout);
+    // We start with the assumption that the TapDance key is still being held.
+    ActionType action = Hold;
+    // Now we search for a release event for the TapDance key, starting from the
+    // second event in the queue (the first one being its press event).
+    for (uint8_t i{1}; i < event_queue_.length(); ++i) {
+      // It should be safe to assume that if we find a second event for the same
+      // address, it's a release, so we skip the test for it.
+      if (event_queue_.addr(i) == td_addr) {
+        action = Timeout;
+        // We don't need to bother breaking here because this is basically
+        // guaranteed to be the last event in the queue.
+      }
+    }
+    tapDanceAction(td_id, td_addr, tap_count_, action);
     flushQueue();
     tap_count_ = 0;
   }

plugins/Kaleidoscope-TapDance/src/kaleidoscope/plugin/TapDance.h

@@ -36,13 +36,13 @@ namespace kaleidoscope {
 namespace plugin {
 class TapDance : public kaleidoscope::Plugin {
  public:
-  typedef enum {
+  enum ActionType {
     Tap,
     Hold,
     Interrupt,
     Timeout,
     Release,
-  } ActionType;
+  };
 
   TapDance(void) {}
 

tests/plugins/TapDance/basic/basic.ino

@@ -46,8 +46,13 @@ void tapDanceAction(uint8_t tap_dance_index,
                     kaleidoscope::plugin::TapDance::ActionType tap_dance_action) {
   switch (tap_dance_index) {
   case 0:
-    return tapDanceActionKeys(tap_count, tap_dance_action,
-                              Key_A, Key_B, Key_C);
+    if (tap_dance_action == TapDance.Hold) {
+      return tapDanceActionKeys(tap_count, tap_dance_action,
+                                Key_A, Key_H, Key_C);
+    } else {
+      return tapDanceActionKeys(tap_count, tap_dance_action,
+                                Key_A, Key_B, Key_C);
+    }
   default:
     break;
   }

tests/plugins/TapDance/basic/test.ktest

@@ -89,7 +89,7 @@ RUN 10 ms
 PRESS TD_0
 RUN 1 cycle
 RUN 25 ms
-EXPECT keyboard-report Key_B # The report should contain `B`
+EXPECT keyboard-report Key_H # The report should contain `H`
 
 RUN 10 ms
 RELEASE TD_0