Adapt TapDance plugin to KeyEvent handlers

Signed-off-by: Michael Richters <gedankenexperimenter@gmail.com>

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

@@ -19,6 +19,7 @@
 #include <Kaleidoscope-FocusSerial.h>
 #include "kaleidoscope/keyswitch_state.h"
 #include "kaleidoscope/layers.h"
+#include "kaleidoscope/KeyEventTracker.h"
 
 namespace kaleidoscope {
 namespace plugin {
@@ -26,153 +27,124 @@ namespace plugin {
 // --- config ---
 
 uint16_t TapDance::time_out = 200;
-KeyAddr TapDance::release_addr_ = KeyAddr{KeyAddr::invalid_state};
+uint8_t TapDance::tap_count_ = 0;
+
+KeyEventTracker TapDance::event_tracker_;
 
 // --- api ---
 void TapDance::actionKeys(uint8_t tap_count,
-                          ActionType tap_dance_action,
+                          ActionType action,
                           uint8_t max_keys,
                           const Key tap_keys[]) {
+  if (event_queue_.isEmpty())
+    return;
+
   if (tap_count > max_keys)
     tap_count = max_keys;
 
-  Key key = tap_keys[tap_count - 1].readFromProgmem();
+  KeyEvent event = event_queue_.event(0);
+  event.key = tap_keys[tap_count - 1].readFromProgmem();
 
-  switch (tap_dance_action) {
+  if (action == Interrupt || action == Timeout) {
-  case Tap:
+    event_queue_.shift();
-    break;
+    Runtime.handleKeyswitchEvent(event);
-  case Interrupt:
+  } else if (action == Tap && tap_count == max_keys) {
-  case Timeout:
+    tap_count_ = 0;
-    if (event_queue_.isEmpty())
+    event_queue_.clear();
-      break;
+    Runtime.handleKeyswitchEvent(event);
-    {
-      KeyAddr td_addr = event_queue_.addr(0);
-      bool key_released = (live_keys[td_addr] == Key_Transparent);
-      handleKeyswitchEvent(key, td_addr, IS_PRESSED | INJECTED);
-      if (key_released)
-        release_addr_ = td_addr;
-    }
-    break;
-  case Hold:
-  case Release:
-    break;
   }
 }
 
 
+void TapDance::flushQueue(KeyAddr ignored_addr) {
+  while (! event_queue_.isEmpty()) {
+    KeyEvent queued_event = event_queue_.event(0);
+    event_queue_.shift();
+    if (queued_event.addr != ignored_addr)
+      Runtime.handleKeyswitchEvent(queued_event);
+  }
+}
+
 // --- hooks ---
 
 EventHandlerResult TapDance::onNameQuery() {
   return ::Focus.sendName(F("TapDance"));
 }
 
-EventHandlerResult TapDance::onKeyswitchEvent(Key &key,
+EventHandlerResult TapDance::onKeyswitchEvent(KeyEvent &event) {
-                                              KeyAddr key_addr,
+  // If the plugin has already processed and released this event, ignore it.
-                                              uint8_t key_state) {
+  // There's no need to update the event tracker explicitly.
-  if (key_state & INJECTED)
+  if (event_tracker_.shouldIgnore(event)) {
+    // We should never get an event that's in our queue here, but just in case
+    // some other plugin sends one, abort.
+    if (event_queue_.shouldAbort(event))
+      return EventHandlerResult::ABORT;
     return EventHandlerResult::OK;
+  }
 
-  if (event_queue_.isEmpty()) {
+  // If event.addr is not a physical key, ignore it; some other plugin injected it.
-    if (keyToggledOn(key_state) && isTapDanceKey(key)) {
+  if (! event.addr.isValid() || (event.state & INJECTED) != 0) {
-      // Begin a new TapDance sequence:
-      uint8_t td_id = key.getRaw() - ranges::TD_FIRST;
-      tapDanceAction(td_id, key_addr, 1, Tap);
-      event_queue_.append(key_addr, key_state);
-      return EventHandlerResult::EVENT_CONSUMED;
-    }
     return EventHandlerResult::OK;
   }
 
-  uint8_t td_count = event_queue_.length();
+  if (keyToggledOff(event.state)) {
+    if (event_queue_.isEmpty())
+      return EventHandlerResult::OK;
+    event_queue_.append(event);
+    return EventHandlerResult::ABORT;
+  }
+
+  if (event_queue_.isEmpty() && !isTapDanceKey(event.key))
+    return EventHandlerResult::OK;
+
   KeyAddr td_addr = event_queue_.addr(0);
-  Key     td_key = Layer.lookup(td_addr);
+  Key     td_key = Layer.lookupOnActiveLayer(td_addr);
   uint8_t td_id = td_key.getRaw() - ranges::TD_FIRST;
 
-  if (keyToggledOn(key_state)) {
+  if (! event_queue_.isEmpty() &&
-    if (key_addr == td_addr) {
+      event.addr != event_queue_.addr(0)) {
-      // The same TapDance key was pressed again; continue the sequence:
+    // Interrupt: Call `tapDanceAction()` first, so it will have access to the
-      tapDanceAction(td_id, td_addr, ++td_count, Tap);
+    // TapDance key press event that needs to be sent, then flush the queue.
-    } else {
+    tapDanceAction(td_id, td_addr, tap_count_, Interrupt);
-      // A different key was pressed; interrupt the sequeunce:
+    flushQueue();
-      tapDanceAction(td_id, td_addr, td_count, Interrupt);
+    tap_count_ = 0;
-      event_queue_.clear();
+    // If the event isn't another TapDance key, let it proceed. If it is, fall
-      // If the sequence was interrupted by another TapDance key, start the new
+    // through to the next block, which handles "Tap" actions.
-      // sequence:
+    if (! isTapDanceKey(event.key))
-      if (isTapDanceKey(Layer.lookup(key_addr))) {
+      return EventHandlerResult::OK;
-        td_id = key.getRaw() - ranges::TD_FIRST;
-        tapDanceAction(td_id, key_addr, 1, Tap);
-      }
-    }
-    // Any key that toggles on while a TapDance sequence is live gets added to
-    // the queue. If it interrupted the queue, we need to hold off on processing
-    // that event until the next cycle to guarantee that the events appear in
-    // order on the host.
-    event_queue_.append(key_addr, key_state);
-    if (isTapDanceKey(key))
-      return EventHandlerResult::EVENT_CONSUMED;
-    return EventHandlerResult::ABORT;
-  } else if (keyIsPressed(key_state)) {
-    // Until a key press event has been released from the queue, its "hold
-    // event" must be suppressed every cycle.
-    for (uint8_t i{0}; i < event_queue_.length(); ++i) {
-      if (event_queue_.addr(i) == key_addr) {
-        return EventHandlerResult::ABORT;
-      }
-    }
   }
-  // We always indicate that other plugins don't need to handle TapDance keys,
+
-  // but we do allow them to show up as active keys when they're held. This way,
+  // Tap: First flush the queue, ignoring the previous press and release events
-  // when one times out, if it's not being held any longer, we can send the
+  // for the TapDance key, then add the new tap to the queue (it becomes the
-  // release event.
+  // first entry).
-  if (isTapDanceKey(key))
+  flushQueue(event.addr);
-    return EventHandlerResult::EVENT_CONSUMED;
+  event_queue_.append(event);
-  // This key is being held, but is not in the queue, or it toggled off, but is
+  tapDanceAction(td_id, td_addr, ++tap_count_, Tap);
-  // not (currently) a TapDance key.
+  return EventHandlerResult::ABORT;
-  return EventHandlerResult::OK;
 }
 
 EventHandlerResult TapDance::afterEachCycle() {
-  if (release_addr_.isValid()) {
+  // If there's no active TapDance sequence, there's nothing to do.
-    handleKeyswitchEvent(Key_NoKey, release_addr_, WAS_PRESSED | INJECTED);
-    release_addr_ = KeyAddr{KeyAddr::invalid_state};
-  }
-  Key event_key;
-  // Purge any non-TapDance key events from the front of the queue.
-  while (! event_queue_.isEmpty()) {
-    KeyAddr event_addr = event_queue_.addr(0);
-    event_key = Layer.lookup(event_addr);
-    if (isTapDanceKey(event_key)) {
-      break;
-    }
-    handleKeyswitchEvent(event_key, event_addr, IS_PRESSED | INJECTED);
-    event_queue_.shift();
-  }
-
   if (event_queue_.isEmpty())
     return EventHandlerResult::OK;
 
   // The first event in the queue is now guaranteed to be a TapDance key.
-  uint8_t td_id = event_key.getRaw() - ranges::TD_FIRST;
   KeyAddr td_addr = event_queue_.addr(0);
+  Key     td_key = Layer.lookupOnActiveLayer(td_addr);
+  uint8_t td_id = td_key.getRaw() - ranges::TD_FIRST;
 
   // Check for timeout
-  uint8_t td_count = event_queue_.length();
+  uint16_t start_time = event_queue_.timestamp(0);
-  uint16_t start_time = event_queue_.timestamp(td_count - 1);
   if (Runtime.hasTimeExpired(start_time, time_out)) {
-    tapDanceAction(td_id, td_addr, td_count, Timeout);
+    tapDanceAction(td_id, td_addr, tap_count_, Timeout);
-    event_queue_.clear();
+    flushQueue();
-    // There's still a race condition here, but it's a minor one. If a TapDance
+    tap_count_ = 0;
-    // sequence times out in the `afterEachCycle()` handler, then another key
-    // toggles on in the following scan cycle, and that key is handled first,
-    // the two events could show up out of order on the host. The probability of
-    // this happening is low, and event-driven Kaleidoscope will fix it
-    // completely, so I'm willing to accept the risk for now.
   }
   return EventHandlerResult::OK;
 }
 
-}
+}  // namespace plugin
-}
+}  // namespace kaleidoscope
 
 __attribute__((weak)) void tapDanceAction(uint8_t tap_dance_index, KeyAddr key_addr, uint8_t tap_count,
                                           kaleidoscope::plugin::TapDance::ActionType tap_dance_action) {

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

@@ -20,7 +20,9 @@
 #include "kaleidoscope/Runtime.h"
 #include "kaleidoscope/LiveKeys.h"
 #include <Kaleidoscope-Ranges.h>
+#include "kaleidoscope/KeyAddr.h"
 #include "kaleidoscope/KeyAddrEventQueue.h"
+#include "kaleidoscope/KeyEventTracker.h"
 
 #define TD(n) Key(kaleidoscope::ranges::TD_FIRST + n)
 
@@ -49,7 +51,7 @@ class TapDance : public kaleidoscope::Plugin {
   void actionKeys(uint8_t tap_count, ActionType tap_dance_action, uint8_t max_keys, const Key tap_keys[]);
 
   EventHandlerResult onNameQuery();
-  EventHandlerResult onKeyswitchEvent(Key &mapped_key, KeyAddr key_addr, uint8_t keyState);
+  EventHandlerResult onKeyswitchEvent(KeyEvent &event);
   EventHandlerResult afterEachCycle();
 
   static constexpr bool isTapDanceKey(Key key) {
@@ -64,12 +66,17 @@ class TapDance : public kaleidoscope::Plugin {
   // The event queue stores a series of press and release events.
   KeyAddrEventQueue<queue_capacity_> event_queue_;
 
-  static KeyAddr release_addr_;
+  static KeyEventTracker event_tracker_;
+
+  // The number of taps in the current TapDance sequence.
+  static uint8_t tap_count_;
+
+  void flushQueue(KeyAddr ignored_addr = KeyAddr::none());
 
 };
-}
 
-}
+}  // namespace plugin
+}  // namespace kaleidoscope
 
 void tapDanceAction(uint8_t tap_dance_index, KeyAddr key_addr, uint8_t tap_count,
                     kaleidoscope::plugin::TapDance::ActionType tap_dance_action);