Apply global-quick-tap-ms PR zmkfirmware#1387

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

@@ -20,8 +20,11 @@ properties:
     default: -1
   quick_tap_ms: # deprecated
     type: int
-  global-quick-tap:
+  global-quick-tap: # deprecated
     type: boolean
+  global-quick-tap-ms:
+    type: int
+    default: -1
   flavor:
     type: string
     required: false

app/dts/bindings/zmk,combos.yaml

@@ -18,6 +18,9 @@ child-binding:
     timeout-ms:
       type: int
       default: 50
+    global-quick-tap-ms:
+      type: int
+      default: -1
     slow-release:
       type: boolean
     layers:

app/src/behaviors/behavior_hold_tap.c

@@ -57,7 +57,7 @@ struct behavior_hold_tap_config {
     char *hold_behavior_dev;
     char *tap_behavior_dev;
     int quick_tap_ms;
-    bool global_quick_tap;
+    int global_quick_tap_ms;
     enum flavor flavor;
     bool retro_tap;
     int32_t hold_trigger_key_positions_len;
@@ -96,7 +96,9 @@ struct last_tapped {
     int64_t timestamp;
 };
 
-struct last_tapped last_tapped = {INT32_MIN, INT64_MIN};
+// Set time stamp to large negative number initially for test suites, but not
+// int64 min since it will overflow if -1 is added
+struct last_tapped last_tapped = {INT32_MIN, INT32_MIN};
 
 static void store_last_tapped(int64_t timestamp) {
     if (timestamp > last_tapped.timestamp) {
@@ -111,10 +113,11 @@ static void store_last_hold_tapped(struct active_hold_tap *hold_tap) {
 }
 
 static bool is_quick_tap(struct active_hold_tap *hold_tap) {
-    if (hold_tap->config->global_quick_tap || last_tapped.position == hold_tap->position) {
-        return (last_tapped.timestamp + hold_tap->config->quick_tap_ms) > hold_tap->timestamp;
+    if ((last_tapped.timestamp + hold_tap->config->global_quick_tap_ms) > hold_tap->timestamp) {
+        return true;
     } else {
-        return false;
+        return (last_tapped.position == hold_tap->position) &&
+               (last_tapped.timestamp + hold_tap->config->quick_tap_ms) > hold_tap->timestamp;
     }
 }
 
@@ -700,7 +703,9 @@ static int behavior_hold_tap_init(const struct device *dev) {
         .hold_behavior_dev = DT_LABEL(DT_INST_PHANDLE_BY_IDX(n, bindings, 0)),                     \
         .tap_behavior_dev = DT_LABEL(DT_INST_PHANDLE_BY_IDX(n, bindings, 1)),                      \
         .quick_tap_ms = DT_INST_PROP(n, quick_tap_ms),                                             \
-        .global_quick_tap = DT_INST_PROP(n, global_quick_tap),                                     \
+        .global_quick_tap_ms = DT_INST_PROP(n, global_quick_tap)                                   \
+                                   ? DT_INST_PROP(n, quick_tap_ms)                                 \
+                                   : DT_INST_PROP(n, global_quick_tap_ms),                         \
         .flavor = DT_ENUM_IDX(DT_DRV_INST(n), flavor),                                             \
         .retro_tap = DT_INST_PROP(n, retro_tap),                                                   \
         .hold_trigger_key_positions = DT_INST_PROP(n, hold_trigger_key_positions),                 \

app/src/combo.c

@@ -15,6 +15,7 @@
 #include <zmk/behavior.h>
 #include <zmk/event_manager.h>
 #include <zmk/events/position_state_changed.h>
+#include <zmk/events/keycode_state_changed.h>
 #include <zmk/hid.h>
 #include <zmk/matrix.h>
 #include <zmk/keymap.h>
@@ -28,6 +29,7 @@ struct combo_cfg {
     int32_t key_position_len;
     struct zmk_behavior_binding behavior;
     int32_t timeout_ms;
+    int32_t global_quick_tap_ms;
     // if slow release is set, the combo releases when the last key is released.
     // otherwise, the combo releases when the first key is released.
     bool slow_release;
@@ -70,6 +72,17 @@ int active_combo_count = 0;
 struct k_work_delayable timeout_task;
 int64_t timeout_task_timeout_at;
 
+// this keeps track of the last non-combo, non-mod key tap
+int64_t last_tapped_timestamp = INT32_MIN;
+// this keeps track of the last time a combo was pressed
+int64_t last_combo_timestamp = INT32_MIN;
+
+static void store_last_tapped(int64_t timestamp) {
+    if (timestamp > last_combo_timestamp) {
+        last_tapped_timestamp = timestamp;
+    }
+}
+
 // Store the combo key pointer in the combos array, one pointer for each key position
 // The combos are sorted shortest-first, then by virtual-key-position.
 static int initialize_combo(struct combo_cfg *new_combo) {
@@ -120,6 +133,10 @@ static bool combo_active_on_layer(struct combo_cfg *combo, uint8_t layer) {
     return false;
 }
 
+static bool is_quick_tap(struct combo_cfg *combo, int64_t timestamp) {
+    return (last_tapped_timestamp + combo->global_quick_tap_ms) > timestamp;
+}
+
 static int setup_candidates_for_first_keypress(int32_t position, int64_t timestamp) {
     int number_of_combo_candidates = 0;
     uint8_t highest_active_layer = zmk_keymap_highest_layer_active();
@@ -128,7 +145,7 @@ static int setup_candidates_for_first_keypress(int32_t position, int64_t timesta
         if (combo == NULL) {
             return number_of_combo_candidates;
         }
-        if (combo_active_on_layer(combo, highest_active_layer)) {
+        if (combo_active_on_layer(combo, highest_active_layer) && !is_quick_tap(combo, timestamp)) {
             candidates[number_of_combo_candidates].combo = combo;
             candidates[number_of_combo_candidates].timeout_at = timestamp + combo->timeout_ms;
             number_of_combo_candidates++;
@@ -238,7 +255,7 @@ static int capture_pressed_key(const zmk_event_t *ev) {
         pressed_keys[i] = ev;
         return ZMK_EV_EVENT_CAPTURED;
     }
-    return 0;
+    return ZMK_EV_EVENT_BUBBLE;
 }
 
 const struct zmk_listener zmk_listener_combo;
@@ -270,6 +287,8 @@ static inline int press_combo_behavior(struct combo_cfg *combo, int32_t timestam
         .timestamp = timestamp,
     };
 
+    last_combo_timestamp = timestamp;
+
     return behavior_keymap_binding_pressed(&combo->behavior, event);
 }
 
@@ -399,7 +418,7 @@ static int position_state_down(const zmk_event_t *ev, struct zmk_position_state_
     if (candidates[0].combo == NULL) {
         num_candidates = setup_candidates_for_first_keypress(data->position, data->timestamp);
         if (num_candidates == 0) {
-            return 0;
+            return ZMK_EV_EVENT_BUBBLE;
         }
     } else {
         filter_timed_out_candidates(data->timestamp);
@@ -439,7 +458,7 @@ static int position_state_up(const zmk_event_t *ev, struct zmk_position_state_ch
         ZMK_EVENT_RAISE(ev);
         return ZMK_EV_EVENT_CAPTURED;
     }
-    return 0;
+    return ZMK_EV_EVENT_BUBBLE;
 }
 
 static void combo_timeout_handler(struct k_work *item) {
@@ -456,7 +475,7 @@ static void combo_timeout_handler(struct k_work *item) {
 static int position_state_changed_listener(const zmk_event_t *ev) {
     struct zmk_position_state_changed *data = as_zmk_position_state_changed(ev);
     if (data == NULL) {
-        return 0;
+        return ZMK_EV_EVENT_BUBBLE;
     }
 
     if (data->state) { // keydown
@@ -466,12 +485,31 @@ static int position_state_changed_listener(const zmk_event_t *ev) {
     }
 }
 
-ZMK_LISTENER(combo, position_state_changed_listener);
+static int keycode_state_changed_listener(const zmk_event_t *eh) {
+    struct zmk_keycode_state_changed *ev = as_zmk_keycode_state_changed(eh);
+    if (ev->state && !is_mod(ev->usage_page, ev->keycode)) {
+        store_last_tapped(ev->timestamp);
+    }
+    return ZMK_EV_EVENT_BUBBLE;
+}
+
+int behavior_combo_listener(const zmk_event_t *eh) {
+    if (as_zmk_position_state_changed(eh) != NULL) {
+        return position_state_changed_listener(eh);
+    } else if (as_zmk_keycode_state_changed(eh) != NULL) {
+        return keycode_state_changed_listener(eh);
+    }
+    return ZMK_EV_EVENT_BUBBLE;
+}
+
+ZMK_LISTENER(combo, behavior_combo_listener);
 ZMK_SUBSCRIPTION(combo, zmk_position_state_changed);
+ZMK_SUBSCRIPTION(combo, zmk_keycode_state_changed);
 
 #define COMBO_INST(n)                                                                              \
     static struct combo_cfg combo_config_##n = {                                                   \
         .timeout_ms = DT_PROP(n, timeout_ms),                                                      \
+        .global_quick_tap_ms = DT_PROP(n, global_quick_tap_ms),                                    \
         .key_positions = DT_PROP(n, key_positions),                                                \
         .key_position_len = DT_PROP_LEN(n, key_positions),                                         \
         .behavior = ZMK_KEYMAP_EXTRACT_BINDING(0, n),                                              \

app/tests/combo/global-quick-tap/events.patterns

@@ -0,0 +1 @@
+s/.*hid_listener_keycode_//p

app/tests/combo/global-quick-tap/keycode_events.snapshot

@@ -0,0 +1,14 @@
+pressed: usage_page 0x07 keycode 0x04 implicit_mods 0x00 explicit_mods 0x00
+released: usage_page 0x07 keycode 0x04 implicit_mods 0x00 explicit_mods 0x00
+pressed: usage_page 0x07 keycode 0x04 implicit_mods 0x00 explicit_mods 0x00
+pressed: usage_page 0x07 keycode 0x05 implicit_mods 0x00 explicit_mods 0x00
+released: usage_page 0x07 keycode 0x04 implicit_mods 0x00 explicit_mods 0x00
+released: usage_page 0x07 keycode 0x05 implicit_mods 0x00 explicit_mods 0x00
+pressed: usage_page 0x07 keycode 0x1B implicit_mods 0x00 explicit_mods 0x00
+released: usage_page 0x07 keycode 0x1B implicit_mods 0x00 explicit_mods 0x00
+pressed: usage_page 0x07 keycode 0x1B implicit_mods 0x00 explicit_mods 0x00
+released: usage_page 0x07 keycode 0x1B implicit_mods 0x00 explicit_mods 0x00
+pressed: usage_page 0x07 keycode 0x04 implicit_mods 0x00 explicit_mods 0x00
+released: usage_page 0x07 keycode 0x04 implicit_mods 0x00 explicit_mods 0x00
+pressed: usage_page 0x07 keycode 0x1C implicit_mods 0x00 explicit_mods 0x00
+released: usage_page 0x07 keycode 0x1C implicit_mods 0x00 explicit_mods 0x00

app/tests/combo/global-quick-tap/native_posix_64.keymap

@@ -0,0 +1,64 @@
+#include <dt-bindings/zmk/keys.h>
+#include <behaviors.dtsi>
+#include <dt-bindings/zmk/kscan-mock.h>
+
+/ {
+	combos {
+		compatible = "zmk,combos";
+		combo_one {
+			timeout-ms = <50>;
+			key-positions = <0 1>;
+			bindings = <&kp X>;
+			global-quick-tap-ms = <100>;
+		};
+
+		combo_two {
+			timeout-ms = <50>;
+			key-positions = <0 2>;
+			bindings = <&kp Y>;
+		};
+	};
+
+	keymap {
+		compatible = "zmk,keymap";
+		label ="Default keymap";
+
+		default_layer {
+			bindings = <
+				&kp A &kp B
+				&kp C &kp D
+			>;
+		};
+	};
+};
+
+&kscan {
+	events = <
+		/* Tap A  */
+		ZMK_MOCK_PRESS(0,0,10)
+		ZMK_MOCK_RELEASE(0,0,60)
+		/* Quick Tap A and B */
+		ZMK_MOCK_PRESS(0,0,10)
+		ZMK_MOCK_PRESS(0,1,10)
+		ZMK_MOCK_RELEASE(0,0,10)
+		ZMK_MOCK_RELEASE(0,1,200)
+		/* Combo One */
+		ZMK_MOCK_PRESS(0,0,10)
+		ZMK_MOCK_PRESS(0,1,10)
+		ZMK_MOCK_RELEASE(0,0,10)
+		ZMK_MOCK_RELEASE(0,1,10)
+		/* Combo One Again (shouldn't quick tap) */
+		ZMK_MOCK_PRESS(0,0,10)
+		ZMK_MOCK_PRESS(0,1,10)
+		ZMK_MOCK_RELEASE(0,0,10)
+		ZMK_MOCK_RELEASE(0,1,10)
+		/* Tap A  */
+		ZMK_MOCK_PRESS(0,0,10)
+		ZMK_MOCK_RELEASE(0,0,60)
+		/* Combo 2  */
+		ZMK_MOCK_PRESS(0,0,10)
+		ZMK_MOCK_PRESS(1,0,10)
+		ZMK_MOCK_RELEASE(0,0,10)
+		ZMK_MOCK_RELEASE(1,0,10)
+	>;
+};

app/tests/hold-tap/balanced/8-global-quick-tap/1-basic/native_posix_64.keymap

@@ -6,17 +6,17 @@
 &kscan {
 	events = <
 		/* tap */
-		ZMK_MOCK_PRESS(0,0,10) 
-		ZMK_MOCK_RELEASE(0,0,10) 
-		/* normal quick tap */
+		ZMK_MOCK_PRESS(0,0,10)
+		ZMK_MOCK_RELEASE(0,0,250)
+		/* quick tap */
 		ZMK_MOCK_PRESS(0,0,400)
 		ZMK_MOCK_RELEASE(0,0,400)
 		/* hold */
 		ZMK_MOCK_PRESS(0,0,400)
 		ZMK_MOCK_PRESS(1,0,10)
 		ZMK_MOCK_RELEASE(1,0,10)
 		ZMK_MOCK_RELEASE(0,0,400)
-		/* global quick tap */
+		/* min prior term */
 		ZMK_MOCK_PRESS(1,0,10)
 		ZMK_MOCK_PRESS(0,0,400)
 		ZMK_MOCK_RELEASE(1,0,10)

app/tests/hold-tap/balanced/8-global-quick-tap/behavior_keymap.dtsi

@@ -11,8 +11,8 @@
 			flavor = "balanced";
 			tapping-term-ms = <300>;
 			quick-tap-ms = <300>;
+			global-quick-tap-ms = <100>;
 			bindings = <&kp>, <&kp>;
-			global-quick-tap;
 		};
 	};
 

app/tests/hold-tap/hold-preferred/8-global-quick-tap/1-basic/native_posix_64.keymap

@@ -6,17 +6,17 @@
 &kscan {
 	events = <
 		/* tap */
-		ZMK_MOCK_PRESS(0,0,10) 
-		ZMK_MOCK_RELEASE(0,0,10) 
-		/* normal quick tap */
+		ZMK_MOCK_PRESS(0,0,10)
+		ZMK_MOCK_RELEASE(0,0,250)
+		/* quick tap */
 		ZMK_MOCK_PRESS(0,0,400)
 		ZMK_MOCK_RELEASE(0,0,400)
 		/* hold */
 		ZMK_MOCK_PRESS(0,0,10)
 		ZMK_MOCK_PRESS(1,0,10)
 		ZMK_MOCK_RELEASE(1,0,10)
 		ZMK_MOCK_RELEASE(0,0,400)
-		/* global quick tap */
+		/* min prior term */
 		ZMK_MOCK_PRESS(1,0,10)
 		ZMK_MOCK_PRESS(0,0,400)
 		ZMK_MOCK_RELEASE(1,0,10)

app/tests/hold-tap/hold-preferred/8-global-quick-tap/behavior_keymap.dtsi

@@ -11,8 +11,8 @@
 			flavor = "hold-preferred";
 			tapping-term-ms = <300>;
 			quick-tap-ms = <300>;
+			global-quick-tap-ms = <100>;
 			bindings = <&kp>, <&kp>;
-			global-quick-tap;
 		};
 	};
 

app/tests/hold-tap/tap-preferred/8-global-quick-tap/1-basic/native_posix_64.keymap

@@ -7,16 +7,16 @@
 	events = <
 		/* tap */
 		ZMK_MOCK_PRESS(0,0,10) 
-		ZMK_MOCK_RELEASE(0,0,10) 
-		/* normal quick tap */
+		ZMK_MOCK_RELEASE(0,0,250)
+		/* quick tap */
 		ZMK_MOCK_PRESS(0,0,400)
 		ZMK_MOCK_RELEASE(0,0,400)
 		/* hold */
 		ZMK_MOCK_PRESS(0,0,400)
 		ZMK_MOCK_PRESS(1,0,10)
 		ZMK_MOCK_RELEASE(1,0,10)
 		ZMK_MOCK_RELEASE(0,0,400)
-		/* global quick tap */
+		/* min prior term */
 		ZMK_MOCK_PRESS(1,0,10)
 		ZMK_MOCK_PRESS(0,0,400)
 		ZMK_MOCK_RELEASE(1,0,10)

app/tests/hold-tap/tap-preferred/8-global-quick-tap/2-double-hold/native_posix_64.keymap

@@ -6,7 +6,7 @@
 &kscan {
 	events = <
 		/* hold the first mod tap */
-		ZMK_MOCK_PRESS(0,0,400)
+		ZMK_MOCK_PRESS(0,0,10)
 		/* hold the second mod tap */
 		ZMK_MOCK_PRESS(0,1,400)
 		/* press the normal key */

app/tests/hold-tap/tap-preferred/8-global-quick-tap/behavior_keymap.dtsi

@@ -11,8 +11,8 @@
 			flavor = "tap-preferred";
 			tapping-term-ms = <300>;
 			quick-tap-ms = <300>;
+			global-quick-tap-ms = <100>;
 			bindings = <&kp>, <&kp>;
-			global-quick-tap;
 		};
 	};