Achordion: pass hold events to process_record.

Fixes #11

Previously when Achordion settles a key as held, it would implement the
hold action by calling `register_mods()` or `layer_on()` directly.
However, this prevents subsequent handlers from responding to the event,
as is needed for instance for the tap-dance-like trick described in

https://github.com/filterpaper/qmk_userspace#tap-hold-macros

This commit revises Achordion to pass hold events to `process_record()`.

Additionally, this commit fixes a bug where Achordion would recursively
receive and block its own tap release events.

Thanks to @jweickm for reporting this.

Author: getreuer

features/achordion.c

@@ -23,39 +23,50 @@ static keyrecord_t tap_hold_record;
 static uint16_t tap_hold_keycode = KC_NO;
 // Timeout timer. When it expires, the key is considered held.
 static uint16_t hold_timer = 0;
-// Mods applied if holding a mod-tap.
-static uint8_t hold_mods = 0;
-// Layer activated if holding a layer-tap.
-static uint8_t hold_layer = 0;
-// Whether the tap-hold decision has been "settled" yet.
-static bool settled = false;
-
-// Applies the mods or layer switch for the tap-hold key's hold action.
-static void apply_hold_action(void) {
-  if (QK_MOD_TAP <= tap_hold_keycode && tap_hold_keycode <= QK_MOD_TAP_MAX) {
-    // Extract mods from mod-tap keycode.
-    hold_mods = (tap_hold_keycode >> 8) & 0xf;
-    if ((tap_hold_keycode & 0x1000) != 0) { hold_mods <<= 4; }
-    register_mods(hold_mods);
-  } else {
-    // Extract layer from layer-tap keycode.
-    hold_layer = (tap_hold_keycode >> 8) & 0xf;
-    layer_on(hold_layer);
-  }
+// Eagerly applied mods, if any.
+static uint8_t eager_mods = 0;
+
+// Achordion's current state.
+enum {
+  // A tap-hold key is pressed, but hasn't yet been settled as tapped or held.
+  STATE_UNSETTLED,
+  // Achordion is inactive.
+  STATE_RELEASED,
+  // Active tap-hold key has been settled as tapped.
+  STATE_TAPPING,
+  // Active tap-hold key has been settled as held.
+  STATE_HOLDING,
+  // This state is set while calling `process_record()`, which will recursively
+  // call `process_achordion()`. This state is checked so that we don't process
+  // events generated by Achordion and potentially create an infinite loop.
+  STATE_RECURSING,
+};
+static uint8_t achordion_state = STATE_RELEASED;
+
+// Calls `process_record()` with state set to RECURSING.
+static void recursively_process_record(keyrecord_t* record, uint8_t state) {
+  achordion_state = STATE_RECURSING;
+  process_record(record);
+  achordion_state = state;
 }
 
-// Clears the mods or layer set by `apply_hold_action()`.
-static void clear_hold_action(void) {
-  if (hold_mods) {
-    unregister_mods(hold_mods);
-    hold_mods = 0;
-  } else if (hold_layer) {
-    layer_off(hold_layer);
-    hold_layer = 0;
-  }
+// Sends hold press event and settles the active tap-hold key as held.
+static void settle_as_hold(void) {
+  eager_mods = 0;
+  // Create hold press event.
+  recursively_process_record(&tap_hold_record, STATE_HOLDING);
+}
+
+// Clears eagerly-applied mods.
+static void clear_eager_mods(void) {
+  unregister_mods(eager_mods);
+  eager_mods = 0;
 }
 
 bool process_achordion(uint16_t keycode, keyrecord_t* record) {
+  // Don't process events that Achordion generated.
+  if (achordion_state == STATE_RECURSING) { return true; }
+
   // Determine whether the current event is for a mod-tap or layer-tap key.
   const bool is_mt = QK_MOD_TAP <= keycode && keycode <= QK_MOD_TAP_MAX;
   const bool is_tap_hold =
@@ -64,22 +75,28 @@ bool process_achordion(uint16_t keycode, keyrecord_t* record) {
   const bool is_physical_pos = (record->event.key.row < 254
                              && record->event.key.col < 254);
 
-  if (tap_hold_keycode == KC_NO) {
-    if (record->event.pressed && is_physical_pos
-        && is_tap_hold && record->tap.count == 0) {
+  if (achordion_state == STATE_RELEASED) {
+    if (is_tap_hold && record->tap.count == 0 &&
+        record->event.pressed && is_physical_pos) {
       // A tap-hold key is pressed and considered by QMK as "held".
       const uint16_t timeout = achordion_timeout(keycode);
       if (timeout > 0) {
-        settled = false;
+        achordion_state = STATE_UNSETTLED;
         // Save info about this key.
         tap_hold_keycode = keycode;
         tap_hold_record = *record;
         hold_timer = record->event.time + timeout;
 
-        // For an "eager" mod, apply it immediately.
-        if (is_mt && achordion_eager_mod((keycode >> 8) & 0x1f)) {
-          apply_hold_action();
+        if (is_mt) {  // Apply mods immediately if they are "eager."
+          uint8_t mod = (tap_hold_keycode >> 8) & 0x1f;
+          if (achordion_eager_mod(mod)) {
+            eager_mods = ((mod & 0x10) == 0) ? mod : (mod << 4);
+            register_mods(eager_mods);
+          }
         }
+
+        dprintf("Achordion: Key 0x%04X pressed.%s\n",
+            keycode, eager_mods ? " Set eager mods." : "");
         return false;  // Skip default handling.
       }
     }
@@ -89,14 +106,25 @@ bool process_achordion(uint16_t keycode, keyrecord_t* record) {
 
   if (keycode == tap_hold_keycode && !record->event.pressed) {
     // The active tap-hold key is being released.
-    tap_hold_keycode = KC_NO;
-    clear_hold_action();
+    if (achordion_state == STATE_HOLDING) {
+      dprintln("Achordion: Key released. Plumbing hold release.");
+      tap_hold_record.event.pressed = false;
+      // Plumb hold release event.
+      recursively_process_record(&tap_hold_record, STATE_RELEASED);
+    } else {
+      dprintf("Achordion: Key released.%s\n",
+          eager_mods ? " Clearing eager mods." : "");
+      if (is_mt) {
+        clear_eager_mods();
+      }
+    }
+
+    achordion_state = STATE_RELEASED;
     return false;
   }
 
-  if (!settled && record->event.pressed) {
+  if (achordion_state == STATE_UNSETTLED && record->event.pressed) {
     // Press event occurred on a key other than the active tap-hold key.
-    settled = true;
 
     // If the other key is *also* a tap-hold key and considered by QMK to be
     // held, then we settle the active key as held. This way, things like
@@ -107,35 +135,42 @@ bool process_achordion(uint16_t keycode, keyrecord_t* record) {
     // tap-hold key as tapped vs. held. We implement the tap or hold by plumbing
     // events back into the handling pipeline so that QMK features and other
     // user code can see them. This is done by calling `process_record()`, which
-    // in turn calls most handlers including `process_record_user()`. Note that
-    // this makes this function recursive, as it is called by
-    // `process_record_user()`, so care is needed. We set `settled = true` above
-    // to prevent infinite loops.
+    // in turn calls most handlers including `process_record_user()`.
     if (!is_physical_pos || (is_tap_hold && record->tap.count == 0) ||
         achordion_chord(tap_hold_keycode, &tap_hold_record, keycode, record)) {
-      apply_hold_action();
+      dprintln("Achordion: Plumbing hold press.");
+      settle_as_hold();
     } else {
-      clear_hold_action();  // Clear in case mod was eagerly applied.
+      clear_eager_mods();  // Clear in case eager mods were set.
 
+      dprintln("Achordion: Plumbing tap press.");
       tap_hold_record.tap.count = 1;  // Revise event as a tap.
-      process_record(&tap_hold_record);  // Create tap press event.
+      tap_hold_record.tap.interrupted = true;
+      // Plumb tap press event.
+      recursively_process_record(&tap_hold_record, STATE_TAPPING);
+
 #if TAP_CODE_DELAY > 0
       wait_ms(TAP_CODE_DELAY);
 #endif  // TAP_CODE_DELAY > 0
+
+      dprintln("Achordion: Plumbing tap release.");
       tap_hold_record.event.pressed = false;
-      process_record(&tap_hold_record);  // Create tap release event.
+      // Plumb tap release event.
+      recursively_process_record(&tap_hold_record, STATE_TAPPING);
     }
-    process_record(record);  // Re-process the event.
+
+    recursively_process_record(record, achordion_state);  // Re-process event.
     return false;  // Block the original event.
   }
+
   return true;
 }
 
 void achordion_task(void) {
-  if (!settled && timer_expired(timer_read(), hold_timer)) {
-    // Timeout expired, settle the key as held.
-    settled = true;
-    apply_hold_action();
+  if (achordion_state == STATE_UNSETTLED &&
+      timer_expired(timer_read(), hold_timer)) {
+    dprintln("Achordion: Timeout. Plumbing hold press.");
+    settle_as_hold();  // Timeout expired, settle the key as held.
   }
 }
 

features/achordion.h

@@ -48,6 +48,10 @@
 
 #include "quantum.h"
 
+#ifdef __cplusplus
+extern "C" {
+#endif
+
 // Call this function from `process_record_user()` as
 //
 //   bool process_record_user(uint16_t keycode, keyrecord_t* record) {
@@ -112,9 +116,14 @@ uint16_t achordion_timeout(uint16_t tap_hold_keycode);
 //     }
 //   }
 //
-// NOTE: `mod` should be compared with `MOD_` prefixed codes, not `KC_`.
+// NOTE: `mod` should be compared with `MOD_` prefixed codes, not `KC_` codes.
 bool achordion_eager_mod(uint8_t mod);
 
 // Returns true if the args come from keys on opposite hands.
 bool achordion_opposite_hands(const keyrecord_t* tap_hold_record,
                               const keyrecord_t* other_record);
+
+#ifdef __cplusplus
+}
+#endif
+