revert refactor, simplify tests

app/dts/bindings/behaviors/zmk,behavior-hold-tap.yaml

@@ -28,7 +28,6 @@ properties:
       - "hold-preferred"
       - "balanced"
       - "tap-preferred"
-      - "tap-positionally-preferred"
   retro-tap:
     type: boolean
   hold-trigger-key-positions:

app/src/behaviors/behavior_hold_tap.c

@@ -34,7 +34,6 @@ enum flavor {
     FLAVOR_HOLD_PREFERRED,
     FLAVOR_BALANCED,
     FLAVOR_TAP_PREFERRED,
-    FLAVOR_TAP_POSITIONALLY_PREFERRED,
 };
 
 enum status {
@@ -73,6 +72,9 @@ struct active_hold_tap {
     const struct behavior_hold_tap_config *config;
     struct k_delayed_work work;
     bool work_is_cancelled;
+
+    // initialized to -1, which is to be interpreted as "no other key has been pressed yet"
+    int32_t position_of_first_other_key_pressed;
 };
 
 // The undecided hold tap is the hold tap that needs to be decided before
@@ -209,6 +211,7 @@ static struct active_hold_tap *store_hold_tap(uint32_t position, uint32_t param_
         active_hold_taps[i].param_hold = param_hold;
         active_hold_taps[i].param_tap = param_tap;
         active_hold_taps[i].timestamp = timestamp;
+        active_hold_taps[i].position_of_first_other_key_pressed = -1;
         return &active_hold_taps[i];
     }
     return NULL;
@@ -274,40 +277,6 @@ static void decide_hold_preferred(struct active_hold_tap *hold_tap, enum decisio
     }
 }
 
-static bool does_other_key_down_position_trigger_hold(struct active_hold_tap *hold_tap,
-                                                      int32_t other_key_down_position) {
-    for (int i = 0; i < hold_tap->config->hold_trigger_key_positions_len; i++) {
-        if (hold_tap->config->hold_trigger_key_positions[i] == other_key_down_position) {
-            return true;
-        }
-    }
-    return false;
-}
-
-static void decide_tap_positionally_preferred(struct active_hold_tap *hold_tap,
-                                              enum decision_moment event,
-                                              int32_t other_key_down_position) {
-    switch (event) {
-    case HT_KEY_UP:
-        hold_tap->status = STATUS_TAP;
-        return;
-    case HT_OTHER_KEY_DOWN:
-        hold_tap->status =
-            does_other_key_down_position_trigger_hold(hold_tap, other_key_down_position)
-                ? STATUS_HOLD_INTERRUPT
-                : STATUS_TAP;
-        return;
-    case HT_TIMER_EVENT:
-        hold_tap->status = STATUS_TAP;
-        return;
-    case HT_QUICK_TAP:
-        hold_tap->status = STATUS_TAP;
-        return;
-    default:
-        return;
-    }
-}
-
 static inline const char *flavor_str(enum flavor flavor) {
     switch (flavor) {
     case FLAVOR_HOLD_PREFERRED:
@@ -316,8 +285,6 @@ static inline const char *flavor_str(enum flavor flavor) {
         return "balanced";
     case FLAVOR_TAP_PREFERRED:
         return "tap-preferred";
-    case FLAVOR_TAP_POSITIONALLY_PREFERRED:
-        return "tap-positionally-preferred";
     default:
         return "UNKNOWN FLAVOR";
     }
@@ -398,13 +365,41 @@ static int release_binding(struct active_hold_tap *hold_tap) {
     return behavior_keymap_binding_released(&binding, event);
 }
 
-static void decide_hold_tap(struct active_hold_tap *hold_tap, enum decision_moment decision_moment,
+static bool is_first_other_key_pressed_trigger_key(struct active_hold_tap *hold_tap) {
-                            int32_t other_key_down_pos) {
+    for (int i = 0; i < hold_tap->config->hold_trigger_key_positions_len; i++) {
-    if (decision_moment == HT_OTHER_KEY_DOWN && other_key_down_pos < 0) {
+        if (hold_tap->config->hold_trigger_key_positions[i] ==
-        LOG_DBG("ERROR other_key_down_pos must be a valid key position if decision_moment == "
+            hold_tap->position_of_first_other_key_pressed) {
-                "HT_OTHER_KEY_DOWN");
+            return true;
+        }
+    }
+    return false;
 }
 
+// Force a tap decision if the positional conditions for a hold decision are not met.
+static void decide_positional_hold(struct active_hold_tap *hold_tap) {
+    // Only force a tap decision if the positional hold/tap feature is enabled.
+    if (!(hold_tap->config->hold_trigger_key_positions_len > 0)) {
+        return;
+    }
+
+    // Only force a tap decision if another key was pressed after
+    // the hold/tap key.
+    if (hold_tap->position_of_first_other_key_pressed == -1) {
+        return;
+    }
+
+    // Only force a tap decision if the first other key to be pressed
+    // (after the hold/tap key) is not one of the trigger keys.
+    if (is_first_other_key_pressed_trigger_key(hold_tap)) {
+        return;
+    }
+
+    // Since the positional key conditions have failed, force a TAP decision.
+    hold_tap->status = STATUS_TAP;
+}
+
+static void decide_hold_tap(struct active_hold_tap *hold_tap,
+                            enum decision_moment decision_moment) {
     if (hold_tap->status != STATUS_UNDECIDED) {
         return;
     }
@@ -414,6 +409,8 @@ static void decide_hold_tap(struct active_hold_tap *hold_tap, enum decision_mome
         return;
     }
 
+    // If the hold-tap behavior is still undecided, attempt to decide it.
+    if (hold_tap->status == STATUS_UNDECIDED) {
         switch (hold_tap->config->flavor) {
         case FLAVOR_HOLD_PREFERRED:
             decide_hold_preferred(hold_tap, decision_moment);
@@ -421,14 +418,15 @@ static void decide_hold_tap(struct active_hold_tap *hold_tap, enum decision_mome
             decide_balanced(hold_tap, decision_moment);
         case FLAVOR_TAP_PREFERRED:
             decide_tap_preferred(hold_tap, decision_moment);
-    case FLAVOR_TAP_POSITIONALLY_PREFERRED:
+        }
-        decide_tap_positionally_preferred(hold_tap, decision_moment, other_key_down_pos);
     }
 
     if (hold_tap->status == STATUS_UNDECIDED) {
         return;
     }
 
+    decide_positional_hold(hold_tap);
+
     // Since the hold-tap has been decided, clean up undecided_hold_tap and
     // execute the decided behavior.
     LOG_DBG("%d decided %s (%s decision moment %s)", hold_tap->position,
@@ -491,7 +489,7 @@ static int on_hold_tap_binding_pressed(struct zmk_behavior_binding *binding,
     undecided_hold_tap = hold_tap;
 
     if (is_quick_tap(hold_tap)) {
-        decide_hold_tap(hold_tap, HT_QUICK_TAP, -1);
+        decide_hold_tap(hold_tap, HT_QUICK_TAP);
     }
 
     // if this behavior was queued we have to adjust the timer to only
@@ -514,10 +512,10 @@ static int on_hold_tap_binding_released(struct zmk_behavior_binding *binding,
     // We insert a timer event before the TH_KEY_UP event to verify.
     int work_cancel_result = k_delayed_work_cancel(&hold_tap->work);
     if (event.timestamp > (hold_tap->timestamp + hold_tap->config->tapping_term_ms)) {
-        decide_hold_tap(hold_tap, HT_TIMER_EVENT, -1);
+        decide_hold_tap(hold_tap, HT_TIMER_EVENT);
     }
 
-    decide_hold_tap(hold_tap, HT_KEY_UP, -1);
+    decide_hold_tap(hold_tap, HT_KEY_UP);
     decide_retro_tap(hold_tap);
     release_binding(hold_tap);
 
@@ -549,6 +547,14 @@ static int position_state_changed_listener(const zmk_event_t *eh) {
         return ZMK_EV_EVENT_BUBBLE;
     }
 
+    // Store the position of pressed key for positional hold-tap purposes.
+    if ((ev->state) // i.e. key pressed (not released)
+        && (undecided_hold_tap->position_of_first_other_key_pressed ==
+            -1) // i.e. no other key has been pressed yet
+    ) {
+        undecided_hold_tap->position_of_first_other_key_pressed = ev->position;
+    }
+
     if (undecided_hold_tap->position == ev->position) {
         if (ev->state) { // keydown
             LOG_ERR("hold-tap listener should be called before before most other listeners!");
@@ -564,7 +570,7 @@ static int position_state_changed_listener(const zmk_event_t *eh) {
     // have run out.
     if (ev->timestamp >
         (undecided_hold_tap->timestamp + undecided_hold_tap->config->tapping_term_ms)) {
-        decide_hold_tap(undecided_hold_tap, HT_TIMER_EVENT, -1);
+        decide_hold_tap(undecided_hold_tap, HT_TIMER_EVENT);
     }
 
     if (!ev->state && find_captured_keydown_event(ev->position) == NULL) {
@@ -578,8 +584,7 @@ static int position_state_changed_listener(const zmk_event_t *eh) {
     LOG_DBG("%d capturing %d %s event", undecided_hold_tap->position, ev->position,
             ev->state ? "down" : "up");
     capture_event(eh);
-    decide_hold_tap(undecided_hold_tap, ev->state ? HT_OTHER_KEY_DOWN : HT_OTHER_KEY_UP,
+    decide_hold_tap(undecided_hold_tap, ev->state ? HT_OTHER_KEY_DOWN : HT_OTHER_KEY_UP);
-                    ev->state ? ev->position : -1);
     return ZMK_EV_EVENT_CAPTURED;
 }
 
@@ -625,7 +630,7 @@ void behavior_hold_tap_timer_work_handler(struct k_work *item) {
     if (hold_tap->work_is_cancelled) {
         clear_hold_tap(hold_tap);
     } else {
-        decide_hold_tap(hold_tap, HT_TIMER_EVENT, -1);
+        decide_hold_tap(hold_tap, HT_TIMER_EVENT);
     }
 }
 
@@ -658,14 +663,7 @@ static struct behavior_hold_tap_data behavior_hold_tap_data;
     };                                                                                             \
     DEVICE_DT_INST_DEFINE(n, behavior_hold_tap_init, device_pm_control_nop,                        \
                           &behavior_hold_tap_data, &behavior_hold_tap_config_##n, APPLICATION,     \
-                          CONFIG_KERNEL_INIT_PRIORITY_DEFAULT, &behavior_hold_tap_driver_api);     \
+                          CONFIG_KERNEL_INIT_PRIORITY_DEFAULT, &behavior_hold_tap_driver_api);
-    BUILD_ASSERT((DT_ENUM_IDX(DT_DRV_INST(n), flavor) == 4 &&                                      \
-                  !(DT_INST_PROP_LEN(n, hold_trigger_key_positions) > 0)),                         \
-                 "'hold-trigger-key-positions' must be non-empty if hold-tap flavor is "           \
-                 "'tap-positionally-preferred'");                                                  \
-    BUILD_ASSERT((DT_INST_PROP_LEN(n, hold_trigger_key_positions) > 0)                             \
-                 && DT_ENUM_IDX(DT_DRV_INST(n), flavor) != 4 ),                                    \
-                 "hold-tap flavor must be 'tap-positionally-preferred' if 'hold-trigger-key-positions' is non-empty");
 
 DT_INST_FOREACH_STATUS_OKAY(KP_INST)
 

app/tests/hold-tap/balanced/7-positional/2-dn-timer-up/keycode_events.snapshot

@@ -0,0 +1,5 @@
+ht_binding_pressed: 0 new undecided hold_tap
+ht_decide: 0 decided hold-timer (balanced decision moment timer)
+kp_pressed: usage_page 0x07 keycode 0xe1 implicit_mods 0x00 explicit_mods 0x00
+kp_released: usage_page 0x07 keycode 0xe1 implicit_mods 0x00 explicit_mods 0x00
+ht_binding_released: 0 cleaning up hold-tap

app/tests/hold-tap/balanced/7-positional/4a-dn-ntgdn-timer-ntgup-up/keycode_events.snapshot

@@ -1,7 +1,7 @@
-kp_pressed: usage_page 0x07 keycode 0x07 implicit_mods 0x00 explicit_mods 0x00
 ht_binding_pressed: 0 new undecided hold_tap
-kp_released: usage_page 0x07 keycode 0x07 implicit_mods 0x00 explicit_mods 0x00
+ht_decide: 0 decided tap (balanced decision moment timer)
-ht_decide: 0 decided tap (tap-preferred decision moment timer)
 kp_pressed: usage_page 0x07 keycode 0x09 implicit_mods 0x00 explicit_mods 0x00
+kp_pressed: usage_page 0x07 keycode 0x08 implicit_mods 0x00 explicit_mods 0x00
+kp_released: usage_page 0x07 keycode 0x08 implicit_mods 0x00 explicit_mods 0x00
 kp_released: usage_page 0x07 keycode 0x09 implicit_mods 0x00 explicit_mods 0x00
 ht_binding_released: 0 cleaning up hold-tap

app/tests/hold-tap/balanced/7-positional/4a-dn-ntgdn-timer-ntgup-up/native_posix.keymap

@@ -5,10 +5,10 @@
 
 &kscan {
 	events = <
-		ZMK_MOCK_PRESS(0,0,100)
+		ZMK_MOCK_PRESS(0,0,200)
-		ZMK_MOCK_PRESS(1,0,100)
+		ZMK_MOCK_PRESS(1,1,200) // non trigger key
-		ZMK_MOCK_RELEASE(1,0,200)
 		/* timer fires */ 
+		ZMK_MOCK_RELEASE(1,1,10)
 		ZMK_MOCK_RELEASE(0,0,10)
 	>;
 };

app/tests/hold-tap/balanced/7-positional/4a-dn-tgdn-timer-tgup-up/keycode_events.snapshot

@@ -1,5 +1,5 @@
 ht_binding_pressed: 0 new undecided hold_tap
-ht_decide: 0 decided hold-timer (tap-preferred decision moment timer)
+ht_decide: 0 decided hold-timer (balanced decision moment timer)
 kp_pressed: usage_page 0x07 keycode 0xe1 implicit_mods 0x00 explicit_mods 0x00
 kp_pressed: usage_page 0x07 keycode 0x07 implicit_mods 0x00 explicit_mods 0x00
 kp_released: usage_page 0x07 keycode 0x07 implicit_mods 0x00 explicit_mods 0x00

app/tests/hold-tap/balanced/7-positional/4a-dn-tgdn-timer-tgup-up/native_posix.keymap

@@ -6,7 +6,7 @@
 &kscan {
 	events = <
 		ZMK_MOCK_PRESS(0,0,200)
-		ZMK_MOCK_PRESS(1,0,200)
+		ZMK_MOCK_PRESS(1,0,200) // trigger key
 		/* timer fires */ 
 		ZMK_MOCK_RELEASE(1,0,10)
 		ZMK_MOCK_RELEASE(0,0,10)

app/tests/hold-tap/balanced/7-positional/behavior_keymap.dtsi

@@ -4,15 +4,15 @@
 
 / {
 	behaviors {
-		ht_tpp: behavior_hold_tap_tap_positionally_preferred {
+		ht_bal: behavior_hold_tap_balanced {
 			compatible = "zmk,behavior-hold-tap";
-			label = "HOLD_TAP_TAP_POSITIONALLY_PREFERRED";
+			label = "HOLD_TAP_BALANCED";
 			#binding-cells = <2>;
-			flavor = "tap-preferred";
+			flavor = "balanced";
 			tapping-term-ms = <300>;
 			quick-tap-ms = <200>;
 			bindings = <&kp>, <&kp>;
-			hold-trigger-key-positions = <1 2 3>;
+            hold-trigger-key-positions = <2>;
 		};
 	};
 
@@ -22,8 +22,8 @@
 
 		default_layer {
 			bindings = <
-				&ht_tpp LEFT_SHIFT F &kp J
+				&ht_bal LEFT_SHIFT F      &ht_bal LEFT_CONTROL J
-				&kp D &kp RIGHT_CONTROL>;
+				&kp D      &kp E>;
 		};
 	};
 };

app/tests/hold-tap/balanced/7-positional/tgdn-dn-ntgdn-timer-ntgup-tgup-up/keycode_events.snapshot

@@ -1,7 +1,9 @@
-ht_binding_pressed: 0 new undecided hold_tap
-ht_decide: 0 decided tap (tap-preferred decision moment timer)
-kp_pressed: usage_page 0x07 keycode 0x09 implicit_mods 0x00 explicit_mods 0x00
 kp_pressed: usage_page 0x07 keycode 0x07 implicit_mods 0x00 explicit_mods 0x00
+ht_binding_pressed: 0 new undecided hold_tap
+ht_decide: 0 decided tap (balanced decision moment timer)
+kp_pressed: usage_page 0x07 keycode 0x09 implicit_mods 0x00 explicit_mods 0x00
+kp_pressed: usage_page 0x07 keycode 0x08 implicit_mods 0x00 explicit_mods 0x00
+kp_released: usage_page 0x07 keycode 0x08 implicit_mods 0x00 explicit_mods 0x00
 kp_released: usage_page 0x07 keycode 0x07 implicit_mods 0x00 explicit_mods 0x00
 kp_released: usage_page 0x07 keycode 0x09 implicit_mods 0x00 explicit_mods 0x00
 ht_binding_released: 0 cleaning up hold-tap

app/tests/hold-tap/balanced/7-positional/tgdn-dn-ntgdn-timer-ntgup-tgup-up/native_posix.keymap

@@ -5,10 +5,12 @@
 
 &kscan {
 	events = <
+		ZMK_MOCK_PRESS(1,0,10) // trigger key
 		ZMK_MOCK_PRESS(0,0,10)
-		ZMK_MOCK_PRESS(1,0,10)
+		ZMK_MOCK_PRESS(1,1,400) // not trigger key
+		/* timer fires */ 
+		ZMK_MOCK_RELEASE(1,1,10)
 		ZMK_MOCK_RELEASE(1,0,10)
 		ZMK_MOCK_RELEASE(0,0,10)
-		/* timer */ 
 	>;
 };

app/tests/hold-tap/hold-preferred/7-positional/2-dn-timer-up/keycode_events.snapshot

@@ -0,0 +1,5 @@
+ht_binding_pressed: 0 new undecided hold_tap
+ht_decide: 0 decided hold-timer (hold-preferred decision moment timer)
+kp_pressed: usage_page 0x07 keycode 0xe1 implicit_mods 0x00 explicit_mods 0x00
+kp_released: usage_page 0x07 keycode 0xe1 implicit_mods 0x00 explicit_mods 0x00
+ht_binding_released: 0 cleaning up hold-tap

app/tests/hold-tap/hold-preferred/7-positional/4a-dn-ntgdn-timer-ntgup-up/keycode_events.snapshot

@@ -0,0 +1,7 @@
+ht_binding_pressed: 0 new undecided hold_tap
+ht_decide: 0 decided tap (hold-preferred decision moment other-key-down)
+kp_pressed: usage_page 0x07 keycode 0x09 implicit_mods 0x00 explicit_mods 0x00
+kp_pressed: usage_page 0x07 keycode 0x08 implicit_mods 0x00 explicit_mods 0x00
+kp_released: usage_page 0x07 keycode 0x08 implicit_mods 0x00 explicit_mods 0x00
+kp_released: usage_page 0x07 keycode 0x09 implicit_mods 0x00 explicit_mods 0x00
+ht_binding_released: 0 cleaning up hold-tap

app/tests/hold-tap/hold-preferred/7-positional/4a-dn-ntgdn-timer-ntgup-up/native_posix.keymap

@@ -5,10 +5,10 @@
 
 &kscan {
 	events = <
-		ZMK_MOCK_PRESS(0,0,100)
+		ZMK_MOCK_PRESS(0,0,200)
-		ZMK_MOCK_PRESS(1,0,100)
+		ZMK_MOCK_PRESS(1,1,200) // non trigger key
-		ZMK_MOCK_RELEASE(1,0,200)
 		/* timer fires */ 
+		ZMK_MOCK_RELEASE(1,1,10)
 		ZMK_MOCK_RELEASE(0,0,10)
 	>;
 };

app/tests/hold-tap/hold-preferred/7-positional/4a-dn-tgdn-timer-tgup-up/keycode_events.snapshot

@@ -0,0 +1,7 @@
+ht_binding_pressed: 0 new undecided hold_tap
+ht_decide: 0 decided hold-interrupt (hold-preferred decision moment other-key-down)
+kp_pressed: usage_page 0x07 keycode 0xe1 implicit_mods 0x00 explicit_mods 0x00
+kp_pressed: usage_page 0x07 keycode 0x07 implicit_mods 0x00 explicit_mods 0x00
+kp_released: usage_page 0x07 keycode 0x07 implicit_mods 0x00 explicit_mods 0x00
+kp_released: usage_page 0x07 keycode 0xe1 implicit_mods 0x00 explicit_mods 0x00
+ht_binding_released: 0 cleaning up hold-tap

app/tests/hold-tap/hold-preferred/7-positional/4a-dn-tgdn-timer-tgup-up/native_posix.keymap

@@ -6,7 +6,7 @@
 &kscan {
 	events = <
 		ZMK_MOCK_PRESS(0,0,200)
-		ZMK_MOCK_PRESS(1,0,200)
+		ZMK_MOCK_PRESS(1,0,200) // trigger key
 		/* timer fires */ 
 		ZMK_MOCK_RELEASE(1,0,10)
 		ZMK_MOCK_RELEASE(0,0,10)

app/tests/hold-tap/hold-preferred/7-positional/behavior_keymap.dtsi

@@ -0,0 +1,29 @@
+#include <dt-bindings/zmk/keys.h>
+#include <behaviors.dtsi>
+#include <dt-bindings/zmk/kscan_mock.h>
+
+/ {
+	behaviors {
+		ht_hold: behavior_hold_hold_tap {
+			compatible = "zmk,behavior-hold-tap";
+			label = "hold_hold_tap";
+			#binding-cells = <2>;
+			flavor = "hold-preferred";
+			tapping-term-ms = <300>;
+			quick-tap-ms = <200>;
+			bindings = <&kp>, <&kp>;
+            hold-trigger-key-positions = <2>;
+		};
+	};
+
+	keymap {
+		compatible = "zmk,keymap";
+		label ="Default keymap";
+
+		default_layer {
+			bindings = <
+				&ht_hold LEFT_SHIFT F      &ht_hold LEFT_CONTROL J
+				&kp D      &kp E>;
+		};
+	};
+};

app/tests/hold-tap/hold-preferred/7-positional/tgdn-dn-ntgdn-timer-ntgup-tgup-up/keycode_events.snapshot

@@ -1,7 +1,9 @@
 kp_pressed: usage_page 0x07 keycode 0x07 implicit_mods 0x00 explicit_mods 0x00
 ht_binding_pressed: 0 new undecided hold_tap
-kp_released: usage_page 0x07 keycode 0x07 implicit_mods 0x00 explicit_mods 0x00
+ht_decide: 0 decided tap (hold-preferred decision moment other-key-down)
-ht_decide: 0 decided tap (tap-preferred decision moment key-up)
 kp_pressed: usage_page 0x07 keycode 0x09 implicit_mods 0x00 explicit_mods 0x00
+kp_pressed: usage_page 0x07 keycode 0x08 implicit_mods 0x00 explicit_mods 0x00
+kp_released: usage_page 0x07 keycode 0x08 implicit_mods 0x00 explicit_mods 0x00
+kp_released: usage_page 0x07 keycode 0x07 implicit_mods 0x00 explicit_mods 0x00
 kp_released: usage_page 0x07 keycode 0x09 implicit_mods 0x00 explicit_mods 0x00
 ht_binding_released: 0 cleaning up hold-tap

app/tests/hold-tap/hold-preferred/7-positional/tgdn-dn-ntgdn-timer-ntgup-tgup-up/native_posix.keymap

@@ -0,0 +1,16 @@
+#include <dt-bindings/zmk/keys.h>
+#include <behaviors.dtsi>
+#include <dt-bindings/zmk/kscan_mock.h>
+#include "../behavior_keymap.dtsi"
+
+&kscan {
+	events = <
+		ZMK_MOCK_PRESS(1,0,10) // trigger key
+		ZMK_MOCK_PRESS(0,0,10)
+		ZMK_MOCK_PRESS(1,1,400) // not trigger key
+		/* timer fires */ 
+		ZMK_MOCK_RELEASE(1,1,10)
+		ZMK_MOCK_RELEASE(1,0,10)
+		ZMK_MOCK_RELEASE(0,0,10)
+	>;
+};

app/tests/hold-tap/tap-positionally-preferred-disabled/1-dn-up/keycode_events.snapshot

@@ -1,5 +0,0 @@
-ht_binding_pressed: 0 new undecided hold_tap
-ht_decide: 0 decided tap (tap-preferred decision moment key-up)
-kp_pressed: usage_page 0x07 keycode 0x09 implicit_mods 0x00 explicit_mods 0x00
-kp_released: usage_page 0x07 keycode 0x09 implicit_mods 0x00 explicit_mods 0x00
-ht_binding_released: 0 cleaning up hold-tap

app/tests/hold-tap/tap-positionally-preferred-disabled/2-dn-timer-up/keycode_events.snapshot

@@ -1,5 +0,0 @@
-ht_binding_pressed: 0 new undecided hold_tap
-ht_decide: 0 decided tap (tap-preferred decision moment timer)
-kp_pressed: usage_page 0x07 keycode 0x09 implicit_mods 0x00 explicit_mods 0x00
-kp_released: usage_page 0x07 keycode 0x09 implicit_mods 0x00 explicit_mods 0x00
-ht_binding_released: 0 cleaning up hold-tap

app/tests/hold-tap/tap-positionally-preferred-disabled/3c-kcdn-dn-kcup-up/native_posix.keymap

@@ -1,13 +0,0 @@
-#include <dt-bindings/zmk/keys.h>
-#include <behaviors.dtsi>
-#include <dt-bindings/zmk/kscan_mock.h>
-#include "../behavior_keymap.dtsi"
-
-&kscan {
-	events = <
-		ZMK_MOCK_PRESS(1,0,10) /*d*/
-		ZMK_MOCK_PRESS(0,0,100)  /*mt f-shift */
-		ZMK_MOCK_RELEASE(1,0,10)
-		ZMK_MOCK_RELEASE(0,0,10) 
-	>;
-};

app/tests/hold-tap/tap-positionally-preferred-disabled/3d-kcdn-dn-kcup-timer-up/native_posix.keymap

@@ -1,13 +0,0 @@
-#include <dt-bindings/zmk/keys.h>
-#include <behaviors.dtsi>
-#include <dt-bindings/zmk/kscan_mock.h>
-#include "../behavior_keymap.dtsi"
-
-&kscan {
-	events = <
-		ZMK_MOCK_PRESS(1,0,10) /* d */
-		ZMK_MOCK_PRESS(0,0,100) /* mt f-shift */
-		ZMK_MOCK_RELEASE(1,0,400)
-		ZMK_MOCK_RELEASE(0,0,10) 
-	>;
-};

app/tests/hold-tap/tap-positionally-preferred-disabled/4c-dn-kcdn-kcup-up/keycode_events.snapshot

@@ -1,7 +0,0 @@
-ht_binding_pressed: 0 new undecided hold_tap
-ht_decide: 0 decided tap (tap-preferred decision moment key-up)
-kp_pressed: usage_page 0x07 keycode 0x09 implicit_mods 0x00 explicit_mods 0x00
-kp_pressed: usage_page 0x07 keycode 0x07 implicit_mods 0x00 explicit_mods 0x00
-kp_released: usage_page 0x07 keycode 0x07 implicit_mods 0x00 explicit_mods 0x00
-kp_released: usage_page 0x07 keycode 0x09 implicit_mods 0x00 explicit_mods 0x00
-ht_binding_released: 0 cleaning up hold-tap

app/tests/hold-tap/tap-positionally-preferred-disabled/4d-dn-kcdn-timer-up-kcup/keycode_events.snapshot

@@ -1,7 +0,0 @@
-ht_binding_pressed: 0 new undecided hold_tap
-ht_decide: 0 decided tap (tap-preferred decision moment key-up)
-kp_pressed: usage_page 0x07 keycode 0x09 implicit_mods 0x00 explicit_mods 0x00
-kp_pressed: usage_page 0x07 keycode 0x07 implicit_mods 0x00 explicit_mods 0x00
-kp_released: usage_page 0x07 keycode 0x09 implicit_mods 0x00 explicit_mods 0x00
-ht_binding_released: 0 cleaning up hold-tap
-kp_released: usage_page 0x07 keycode 0x07 implicit_mods 0x00 explicit_mods 0x00

app/tests/hold-tap/tap-positionally-preferred-disabled/4d-dn-kcdn-timer-up-kcup/native_posix.keymap

@@ -1,14 +0,0 @@
-#include <dt-bindings/zmk/keys.h>
-#include <behaviors.dtsi>
-#include <dt-bindings/zmk/kscan_mock.h>
-#include "../behavior_keymap.dtsi"
-
-&kscan {
-	events = <
-		ZMK_MOCK_PRESS(0,0,100)
-		ZMK_MOCK_PRESS(1,0,100)
-		ZMK_MOCK_RELEASE(0,0,200)
-		/* timer fires */
-		ZMK_MOCK_RELEASE(1,0,10)
-	>;
-};

app/tests/hold-tap/tap-positionally-preferred-enabled/1-dn-up/keycode_events.snapshot

@@ -1,5 +0,0 @@
-ht_binding_pressed: 0 new undecided hold_tap
-ht_decide: 0 decided tap (tap-preferred decision moment key-up)
-kp_pressed: usage_page 0x07 keycode 0x09 implicit_mods 0x00 explicit_mods 0x00
-kp_released: usage_page 0x07 keycode 0x09 implicit_mods 0x00 explicit_mods 0x00
-ht_binding_released: 0 cleaning up hold-tap

app/tests/hold-tap/tap-positionally-preferred-enabled/1-dn-up/native_posix.keymap

@@ -1,11 +0,0 @@
-#include <dt-bindings/zmk/keys.h>
-#include <behaviors.dtsi>
-#include <dt-bindings/zmk/kscan_mock.h>
-#include "../behavior_keymap.dtsi"
-
-&kscan {
-	events = <
-		ZMK_MOCK_PRESS(0,0,10) 
-		ZMK_MOCK_RELEASE(0,0,10) 
-	>;
-};

app/tests/hold-tap/tap-positionally-preferred-enabled/2-dn-timer-up/keycode_events.snapshot

@@ -1,5 +0,0 @@
-ht_binding_pressed: 0 new undecided hold_tap
-ht_decide: 0 decided tap (tap-preferred decision moment timer)
-kp_pressed: usage_page 0x07 keycode 0x09 implicit_mods 0x00 explicit_mods 0x00
-kp_released: usage_page 0x07 keycode 0x09 implicit_mods 0x00 explicit_mods 0x00
-ht_binding_released: 0 cleaning up hold-tap

app/tests/hold-tap/tap-positionally-preferred-enabled/3c-kcdn-dn-kcup-up/keycode_events.snapshot

@@ -1,7 +0,0 @@
-kp_pressed: usage_page 0x07 keycode 0x07 implicit_mods 0x00 explicit_mods 0x00
-ht_binding_pressed: 0 new undecided hold_tap
-kp_released: usage_page 0x07 keycode 0x07 implicit_mods 0x00 explicit_mods 0x00
-ht_decide: 0 decided tap (tap-preferred decision moment key-up)
-kp_pressed: usage_page 0x07 keycode 0x09 implicit_mods 0x00 explicit_mods 0x00
-kp_released: usage_page 0x07 keycode 0x09 implicit_mods 0x00 explicit_mods 0x00
-ht_binding_released: 0 cleaning up hold-tap

app/tests/hold-tap/tap-positionally-preferred-enabled/3c-kcdn-dn-kcup-up/native_posix.keymap

@@ -1,13 +0,0 @@
-#include <dt-bindings/zmk/keys.h>
-#include <behaviors.dtsi>
-#include <dt-bindings/zmk/kscan_mock.h>
-#include "../behavior_keymap.dtsi"
-
-&kscan {
-	events = <
-		ZMK_MOCK_PRESS(1,0,10) /*d*/
-		ZMK_MOCK_PRESS(0,0,100)  /*mt f-shift */
-		ZMK_MOCK_RELEASE(1,0,10)
-		ZMK_MOCK_RELEASE(0,0,10) 
-	>;
-};

app/tests/hold-tap/tap-positionally-preferred-enabled/3d-kcdn-dn-kcup-timer-up/native_posix.keymap

@@ -1,13 +0,0 @@
-#include <dt-bindings/zmk/keys.h>
-#include <behaviors.dtsi>
-#include <dt-bindings/zmk/kscan_mock.h>
-#include "../behavior_keymap.dtsi"
-
-&kscan {
-	events = <
-		ZMK_MOCK_PRESS(1,0,10) /* d */
-		ZMK_MOCK_PRESS(0,0,100) /* mt f-shift */
-		ZMK_MOCK_RELEASE(1,0,400)
-		ZMK_MOCK_RELEASE(0,0,10) 
-	>;
-};

app/tests/hold-tap/tap-positionally-preferred-enabled/4a-dn-kcdn-timer-kcup-up/events.patterns

@@ -1,4 +0,0 @@
-s/.*hid_listener_keycode/kp/p
-s/.*mo_keymap_binding/mo/p
-s/.*on_hold_tap_binding/ht_binding/p
-s/.*decide_hold_tap/ht_decide/p

app/tests/hold-tap/tap-positionally-preferred-enabled/4b-dn-kcdn-kcup-timer-up/events.patterns

@@ -1,4 +0,0 @@
-s/.*hid_listener_keycode/kp/p
-s/.*mo_keymap_binding/mo/p
-s/.*on_hold_tap_binding/ht_binding/p
-s/.*decide_hold_tap/ht_decide/p

app/tests/hold-tap/tap-positionally-preferred-enabled/4c-dn-kcdn-kcup-up/events.patterns

@@ -1,4 +0,0 @@
-s/.*hid_listener_keycode/kp/p
-s/.*mo_keymap_binding/mo/p
-s/.*on_hold_tap_binding/ht_binding/p
-s/.*decide_hold_tap/ht_decide/p

app/tests/hold-tap/tap-positionally-preferred-enabled/4c-dn-kcdn-kcup-up/keycode_events.snapshot

@@ -1,7 +0,0 @@
-ht_binding_pressed: 0 new undecided hold_tap
-ht_decide: 0 decided tap (tap-preferred decision moment key-up)
-kp_pressed: usage_page 0x07 keycode 0x09 implicit_mods 0x00 explicit_mods 0x00
-kp_pressed: usage_page 0x07 keycode 0x07 implicit_mods 0x00 explicit_mods 0x00
-kp_released: usage_page 0x07 keycode 0x07 implicit_mods 0x00 explicit_mods 0x00
-kp_released: usage_page 0x07 keycode 0x09 implicit_mods 0x00 explicit_mods 0x00
-ht_binding_released: 0 cleaning up hold-tap

app/tests/hold-tap/tap-positionally-preferred-enabled/4d-dn-kcdn-timer-up-kcup/events.patterns

@@ -1,4 +0,0 @@
-s/.*hid_listener_keycode/kp/p
-s/.*mo_keymap_binding/mo/p
-s/.*on_hold_tap_binding/ht_binding/p
-s/.*decide_hold_tap/ht_decide/p

app/tests/hold-tap/tap-positionally-preferred-enabled/4d-dn-kcdn-timer-up-kcup/keycode_events.snapshot

@@ -1,7 +0,0 @@
-ht_binding_pressed: 0 new undecided hold_tap
-ht_decide: 0 decided tap (tap-preferred decision moment key-up)
-kp_pressed: usage_page 0x07 keycode 0x09 implicit_mods 0x00 explicit_mods 0x00
-kp_pressed: usage_page 0x07 keycode 0x07 implicit_mods 0x00 explicit_mods 0x00
-kp_released: usage_page 0x07 keycode 0x09 implicit_mods 0x00 explicit_mods 0x00
-ht_binding_released: 0 cleaning up hold-tap
-kp_released: usage_page 0x07 keycode 0x07 implicit_mods 0x00 explicit_mods 0x00

app/tests/hold-tap/tap-positionally-preferred-enabled/4d-dn-kcdn-timer-up-kcup/native_posix.keymap

@@ -1,14 +0,0 @@
-#include <dt-bindings/zmk/keys.h>
-#include <behaviors.dtsi>
-#include <dt-bindings/zmk/kscan_mock.h>
-#include "../behavior_keymap.dtsi"
-
-&kscan {
-	events = <
-		ZMK_MOCK_PRESS(0,0,100)
-		ZMK_MOCK_PRESS(1,0,100)
-		ZMK_MOCK_RELEASE(0,0,200)
-		/* timer fires */
-		ZMK_MOCK_RELEASE(1,0,10)
-	>;
-};

app/tests/hold-tap/tap-preferred/7-positional/2-dn-timer-up/keycode_events.snapshot

@@ -0,0 +1,5 @@
+ht_binding_pressed: 0 new undecided hold_tap
+ht_decide: 0 decided hold-timer (tap-preferred decision moment timer)
+kp_pressed: usage_page 0x07 keycode 0xe1 implicit_mods 0x00 explicit_mods 0x00
+kp_released: usage_page 0x07 keycode 0xe1 implicit_mods 0x00 explicit_mods 0x00
+ht_binding_released: 0 cleaning up hold-tap

app/tests/hold-tap/tap-preferred/7-positional/2-dn-timer-up/native_posix.keymap

@@ -5,7 +5,7 @@
 
 &kscan {
 	events = <
-		ZMK_MOCK_PRESS(0,0,10) 
+		ZMK_MOCK_PRESS(0,0,500) 
 		ZMK_MOCK_RELEASE(0,0,10) 
 	>;
 };

app/tests/hold-tap/tap-preferred/7-positional/4a-dn-ntgdn-timer-ntgup-up/keycode_events.snapshot

@@ -1,7 +1,7 @@
-kp_pressed: usage_page 0x07 keycode 0x07 implicit_mods 0x00 explicit_mods 0x00
 ht_binding_pressed: 0 new undecided hold_tap
-kp_released: usage_page 0x07 keycode 0x07 implicit_mods 0x00 explicit_mods 0x00
 ht_decide: 0 decided tap (tap-preferred decision moment timer)
 kp_pressed: usage_page 0x07 keycode 0x09 implicit_mods 0x00 explicit_mods 0x00
+kp_pressed: usage_page 0x07 keycode 0x08 implicit_mods 0x00 explicit_mods 0x00
+kp_released: usage_page 0x07 keycode 0x08 implicit_mods 0x00 explicit_mods 0x00
 kp_released: usage_page 0x07 keycode 0x09 implicit_mods 0x00 explicit_mods 0x00
 ht_binding_released: 0 cleaning up hold-tap

app/tests/hold-tap/tap-preferred/7-positional/4a-dn-ntgdn-timer-ntgup-up/native_posix.keymap

@@ -0,0 +1,14 @@
+#include <dt-bindings/zmk/keys.h>
+#include <behaviors.dtsi>
+#include <dt-bindings/zmk/kscan_mock.h>
+#include "../behavior_keymap.dtsi"
+
+&kscan {
+	events = <
+		ZMK_MOCK_PRESS(0,0,200)
+		ZMK_MOCK_PRESS(1,1,200) // non trigger key
+		/* timer fires */ 
+		ZMK_MOCK_RELEASE(1,1,10)
+		ZMK_MOCK_RELEASE(0,0,10)
+	>;
+};

app/tests/hold-tap/tap-preferred/7-positional/4a-dn-tgdn-timer-tgup-up/native_posix.keymap

@@ -5,10 +5,10 @@
 
 &kscan {
 	events = <
-		ZMK_MOCK_PRESS(0,0,10)
+		ZMK_MOCK_PRESS(0,0,200)
-		ZMK_MOCK_PRESS(1,0,10)
+		ZMK_MOCK_PRESS(1,0,200) // trigger key
+		/* timer fires */ 
 		ZMK_MOCK_RELEASE(1,0,10)
 		ZMK_MOCK_RELEASE(0,0,10)
-		/* timer */ 
 	>;
 };

app/tests/hold-tap/tap-preferred/7-positional/behavior_keymap.dtsi

@@ -4,15 +4,15 @@
 
 / {
 	behaviors {
-		ht_tpp: behavior_hold_tap_tap_positionally_preferred {
+		tp: behavior_tap_preferred {
 			compatible = "zmk,behavior-hold-tap";
-			label = "HOLD_TAP_TAP_POSITIONALLY_PREFERRED";
+			label = "MOD_TAP";
 			#binding-cells = <2>;
 			flavor = "tap-preferred";
 			tapping-term-ms = <300>;
 			quick-tap-ms = <200>;
 			bindings = <&kp>, <&kp>;
-			hold-trigger-key-positions = <99>;
+            hold-trigger-key-positions = <2>;
 		};
 	};
 
@@ -22,8 +22,8 @@
 
 		default_layer {
 			bindings = <
-				&ht_tpp LEFT_SHIFT F &kp J
+				&tp LEFT_SHIFT F      &tp LEFT_CONTROL J
-				&kp D &kp RIGHT_CONTROL>;
+				&kp D      &kp E>;
 		};
 	};
 };

app/tests/hold-tap/tap-preferred/7-positional/tgdn-dn-ntgdn-timer-ntgup-tgup-up/keycode_events.snapshot

@@ -1,7 +1,9 @@
+kp_pressed: usage_page 0x07 keycode 0x07 implicit_mods 0x00 explicit_mods 0x00
 ht_binding_pressed: 0 new undecided hold_tap
 ht_decide: 0 decided tap (tap-preferred decision moment timer)
 kp_pressed: usage_page 0x07 keycode 0x09 implicit_mods 0x00 explicit_mods 0x00
-kp_pressed: usage_page 0x07 keycode 0x07 implicit_mods 0x00 explicit_mods 0x00
+kp_pressed: usage_page 0x07 keycode 0x08 implicit_mods 0x00 explicit_mods 0x00
+kp_released: usage_page 0x07 keycode 0x08 implicit_mods 0x00 explicit_mods 0x00
 kp_released: usage_page 0x07 keycode 0x07 implicit_mods 0x00 explicit_mods 0x00
 kp_released: usage_page 0x07 keycode 0x09 implicit_mods 0x00 explicit_mods 0x00
 ht_binding_released: 0 cleaning up hold-tap

app/tests/hold-tap/tap-preferred/7-positional/tgdn-dn-ntgdn-timer-ntgup-tgup-up/native_posix.keymap

@@ -0,0 +1,16 @@
+#include <dt-bindings/zmk/keys.h>
+#include <behaviors.dtsi>
+#include <dt-bindings/zmk/kscan_mock.h>
+#include "../behavior_keymap.dtsi"
+
+&kscan {
+	events = <
+		ZMK_MOCK_PRESS(1,0,10) // trigger key
+		ZMK_MOCK_PRESS(0,0,10)
+		ZMK_MOCK_PRESS(1,1,400) // not trigger key
+		/* timer fires */ 
+		ZMK_MOCK_RELEASE(1,1,10)
+		ZMK_MOCK_RELEASE(1,0,10)
+		ZMK_MOCK_RELEASE(0,0,10)
+	>;
+};

docs/docs/behaviors/hold-tap.md

@@ -26,7 +26,6 @@ We call this the 'hold-preferred' flavor of hold-taps. While this flavor may wor
 - The 'hold-preferred' flavor triggers the hold behavior when the `tapping-term-ms` has expired or another key is pressed.
 - The 'balanced' flavor will trigger the hold behavior when the `tapping-term-ms` has expired or another key is pressed and released.
 - The 'tap-preferred' flavor triggers the hold behavior when the `tapping-term-ms` has expired. It triggers the tap behavior when another key is pressed.
-- The 'tap-positionally-preferred' flavor triggers the hold behavior if and only if a key at one of the `hold-trigger-key-positions` is pressed before `tapping-term-ms` expires. See the below section on `hold-trigger-key-positions` for more details.
 
 When the hold-tap key is released and the hold behavior has not been triggered, the tap behavior will trigger.
 
@@ -60,50 +59,47 @@ For example, if you press `&mt LEFT_SHIFT A` and then release it without pressin
 };
 ```
 
-#### `hold-trigger-key-positions`
+#### Positional hold-tap and `hold-trigger-key-positions`
 
-- `hold-trigger-key-positions` is used exclusively with the `tap-positionally-preferred` flavor, and is required for the `tap-positionally-preferred` flavor.
+- Including `hold-trigger-key-postions` in your hold-tap definition turns on the positional hold-tap feature.
-- Specifies which key positions are eligible for triggering a hold behavior.
+- With positional hold-tap enabled, if you press any key **NOT** listed in `hold-trigger-key-positions` before `tapping-term-ms` expires, it will produce a tap.
-- If and only if a key at one of the `hold-trigger-key-positions` is pressed before `tapping-term-ms` expires, will 'tap-positionally-preferred' produce a hold behavior.
+- In all other situations, positional hold-tap will not modify the behavior of your hold-tap.
-- `hold-trigger-key-positions` is an array of key positions indexes. Key positions are numbered / indexed according to the keys in your keymap, starting at 0. So, if the first key in your keymap is Q, this key is in position 0. The next key (possibly W) will have position 1, et cetera.
+- Positional hold-tap is useful with home-row modifiers. If you have a home-row modifier key in the left hand for example, by including only keys positions from the right hand in `hold-trigger-key-positions`, you will only get hold behaviors during cross-hand key combinations.
-
+- Note that `hold-trigger-key-postions` is an array of key position indexes. Key positions are numbered according to your keymap, starting with 0. So if the first key in your keymap is Q, this key is in position 0. The next key (probably W) will be in position 1, et cetera.
-* See the following example:
+- See the following example, which uses a hold-tap behavior definition, configured with the `hold-preferrred` flavor, and with positional hold-tap enabled:
 
 ```
 #include <dt-bindings/zmk/keys.h>
 #include <behaviors.dtsi>
-
 / {
 	behaviors {
-		tpht: tap_positional_hold_tap {
+		pht: positional_hold_tap {
 			compatible = "zmk,behavior-hold-tap";
-			label = "TAP_POSITIONAL_HOLD_TAP";
+			label = "POSITIONAL_HOLD_TAP";
 			#binding-cells = <2>;
-			flavor = "tap-positionally-preferred";
+			flavor = "hold-preferred";
-			tapping-term-ms = <200>;
+			tapping-term-ms = <400>;
 			quick-tap-ms = <200>;
 			bindings = <&kp>, <&kp>;
 			hold-trigger-key-positions = <1>;    // <---[[the W key]]
 		};
 	};
-
 	keymap {
 		compatible = "zmk,keymap";
 		label ="Default keymap";
 		default_layer {
 			bindings = <
 				//  position 0         position 1       position 2
-				&tpht LEFT_SHIFT Q        &kp W            &kp E
+				&pht LEFT_SHIFT Q        &kp W            &kp E
 			>;
 		};
 	};
 };
 ```
 
-- The sequence `(tpht_down, W_down, W_up, tpht_up)` produces `W`. The `tap-positionally-preferred` flavor produces a **hold** behavior because the first key pressed after the hold-tap key is the `W key`, which is in position 1, which **IS** included in `hold-trigger-key-positions`.
+- The sequence `(pht_down, E_down, E_up, pht_up)` produces `qe`. The normal hold behavior (LEFT_SHIFT) **IS** modified into a tap behavior (Q) by positional hold-tap because the first key pressed after the hold-tap key is the `E key`, which is in position 2, which **is NOT** included in `hold-trigger-key-positions`.
-- Meanwhile the sequence `(tpht_down, E_down, E_up, tpht_up)` produces `qe`. The `tap-positionally-preferred` flavor produces a **tap** behavior because the first key pressed after the hold-tap key is the `E key`, which is in position 2, which is **NOT** included in `hold-trigger-key-positions`.
+- The sequence `(pht_down, W_down, W_up, pht_up)` produces `W`. The normal hold behavior (LEFT_SHIFT) **is NOT** modified into a tap behavior (Q) by positional hold-tap because the first key pressed after the hold-tap key is the `W key`, which is in position 1, which **IS** included in `hold-trigger-key-positions`.
-- If the `LEFT_SHIFT / Q key` is held by itself for longer than `tapping-term-ms`, a **tap** behavior is produced.
+- If the `LEFT_SHIFT / Q key` is held by itself for longer than `tapping-term-ms`, a hold behavior is produced. This is because positional hold-tap only modifies the behavior of a hold-tap if another key is pressed before the `tapping-term-ms` period expires.
-- The `tap-positionally-preferred` flavor is useful with home-row modifiers. With a home-row-modifier key in the left hand, by only including keys positions from the right hand in `hold-trigger-key-positions`, hold behaviors will only occur with cross-hand key combinations.
 
 #### Home row mods
 

docs/docs/development/tests.md

@@ -3,10 +3,10 @@ title: Tests
 sidebar_label: Tests
 ---
 
-Running tests requires [native posix support](posix-board.md). Any folder under `/app/tests`
+- Running tests requires [native posix support](posix-board.md).
-containing `native_posix.keymap` will be selected when running `west test`.
+- Any folder under `/app/tests` containing `native_posix.keymap` will be selected when running `west test`.
-
+- Run tests from within the `/zmk/app` directory.
-Run a single test with `west test <testname>`, like `west test tests/toggle-layer/normal`.
+- Run a single test with `west test <testname>`, like `west test tests/toggle-layer/normal`.
 
 ## Creating a New Test Set