From 06be6511a89e24ec3c154f22e731677d4a2842d2 Mon Sep 17 00:00:00 2001 From: Ciutadellla Date: Mon, 20 Nov 2023 07:46:24 +0100 Subject: [PATCH] Revert "fix(combos)Fix bug with overlapping combos timeouts (#1945)" This reverts commit aa4cb143bf2de89f06039fd9ba0b259f6d38fc5d. --- app/src/combo.c | 26 ++--- .../events.patterns | 1 - .../keycode_events.snapshot | 8 -- .../native_posix_64.keymap | 98 ------------------- 4 files changed, 7 insertions(+), 126 deletions(-) delete mode 100644 app/tests/combo/overlapping-combos-4-different-timeouts/events.patterns delete mode 100644 app/tests/combo/overlapping-combos-4-different-timeouts/keycode_events.snapshot delete mode 100644 app/tests/combo/overlapping-combos-4-different-timeouts/native_posix_64.keymap diff --git a/app/src/combo.c b/app/src/combo.c index 87a931439a13..90c89c15e737 100644 --- a/app/src/combo.c +++ b/app/src/combo.c @@ -204,34 +204,22 @@ static inline bool candidate_is_completely_pressed(struct combo_cfg *candidate) static int cleanup(); static int filter_timed_out_candidates(int64_t timestamp) { - int remaining_candidates = 0; + int num_candidates = 0; for (int i = 0; i < CONFIG_ZMK_COMBO_MAX_COMBOS_PER_KEY; i++) { struct combo_candidate *candidate = &candidates[i]; if (candidate->combo == NULL) { break; } if (candidate->timeout_at > timestamp) { - bool need_to_bubble_up = remaining_candidates != i; - if (need_to_bubble_up) { - // bubble up => reorder candidates so they're contiguous - candidates[remaining_candidates].combo = candidate->combo; - candidates[remaining_candidates].timeout_at = candidate->timeout_at; - // clear the previous location - candidates[i].combo = NULL; - candidates[i].timeout_at = 0; - } - - remaining_candidates++; + // reorder candidates so they're contiguous + candidates[num_candidates].combo = candidate->combo; + candidates[num_candidates].timeout_at = candidate->timeout_at; + num_candidates++; } else { candidate->combo = NULL; } } - - LOG_DBG( - "after filtering out timed out combo candidates: remaining_candidates=%d timestamp=%lld", - remaining_candidates, timestamp); - - return remaining_candidates; + return num_candidates; } static int clear_candidates() { @@ -461,7 +449,7 @@ static void combo_timeout_handler(struct k_work *item) { // timer was cancelled or rescheduled. return; } - if (filter_timed_out_candidates(timeout_task_timeout_at) == 0) { + if (filter_timed_out_candidates(timeout_task_timeout_at) < 2) { cleanup(); } update_timeout_task(); diff --git a/app/tests/combo/overlapping-combos-4-different-timeouts/events.patterns b/app/tests/combo/overlapping-combos-4-different-timeouts/events.patterns deleted file mode 100644 index 89015deee067..000000000000 --- a/app/tests/combo/overlapping-combos-4-different-timeouts/events.patterns +++ /dev/null @@ -1 +0,0 @@ -s/.*\(hid_listener_keycode_pressed\|filter_timed_out_candidates\): //p \ No newline at end of file diff --git a/app/tests/combo/overlapping-combos-4-different-timeouts/keycode_events.snapshot b/app/tests/combo/overlapping-combos-4-different-timeouts/keycode_events.snapshot deleted file mode 100644 index 8fe441ff46e3..000000000000 --- a/app/tests/combo/overlapping-combos-4-different-timeouts/keycode_events.snapshot +++ /dev/null @@ -1,8 +0,0 @@ -after filtering out timed out combo candidates: remaining_candidates=2 timestamp=71 -after filtering out timed out combo candidates: remaining_candidates=1 timestamp=81 -after filtering out timed out combo candidates: remaining_candidates=0 timestamp=91 -usage_page 0x07 keycode 0x04 implicit_mods 0x00 explicit_mods 0x00 -after filtering out timed out combo candidates: remaining_candidates=2 timestamp=143 -after filtering out timed out combo candidates: remaining_candidates=1 timestamp=153 -after filtering out timed out combo candidates: remaining_candidates=1 timestamp=159 -usage_page 0x07 keycode 0x1D implicit_mods 0x00 explicit_mods 0x00 diff --git a/app/tests/combo/overlapping-combos-4-different-timeouts/native_posix_64.keymap b/app/tests/combo/overlapping-combos-4-different-timeouts/native_posix_64.keymap deleted file mode 100644 index 8967207987b1..000000000000 --- a/app/tests/combo/overlapping-combos-4-different-timeouts/native_posix_64.keymap +++ /dev/null @@ -1,98 +0,0 @@ -#include -#include -#include - -#define kA 0 -#define kB 1 -#define kC 2 -#define kD 3 - -/ { - combos { - compatible = "zmk,combos"; - - // Intentionally out of order in the config, to make sure 'combo.c' handles it properly - combo_40 { - timeout-ms = <40>; - key-positions = ; - bindings = <&kp Z>; - }; - combo_20 { - timeout-ms = <20>; - key-positions = ; - bindings = <&kp X>; - }; - combo_30 { - timeout-ms = <30>; - key-positions = ; - bindings = <&kp Y>; - }; - - }; - - keymap { - compatible = "zmk,keymap"; - label ="Default keymap"; - - default_layer { - bindings = < - &kp A &kp B - &kp C &kp D - >; - }; - }; -}; - -#define press_A_and_wait(delay_next) \ - ZMK_MOCK_PRESS(0,0,delay_next) -#define press_B_and_wait(delay_next) \ - ZMK_MOCK_PRESS(0,1,delay_next) -#define press_C_and_wait(delay_next) \ - ZMK_MOCK_PRESS(1,0,delay_next) -#define press_D_and_wait(delay_next) \ - ZMK_MOCK_PRESS(1,1,delay_next) - -#define release_A_and_wait(delay_next) \ - ZMK_MOCK_RELEASE(0,0,delay_next) -#define release_D_and_wait(delay_next) \ - ZMK_MOCK_RELEASE(1,1,delay_next) - -&kscan { - events = < - /* Note: This starts at T+50 because the ZMK_MOCK_PRESS seems to launch the first event at T+(first wait duration). So in our case T+50 */ - - - - /*** First Phase: All 3 combos expire ***/ - - /* T+50+0= T+50: Press A and wait 50ms */ - press_A_and_wait(50) - - /* T+50+20= T+70: 'combo_20' should expire */ - /* T+50+30= T+80: 'combo_30' should expire */ - /* T+50+40= T+90: 'combo_40' should expire, and we should send the keycode 'A' */ - - /* T+50+50= T+100: We release A and wait 20ms */ - release_A_and_wait(20) - - - - /*** Second Phase: 2 combo expire, 1 combo triggers ***/ - - /* T+120+0= T+120: Press A and wait 35ms */ - press_A_and_wait(35) - - /* T+120+20= T+140: 'combo_20' should expire */ - /* T+120+30= T+150: 'combo_30' should expire */ - - /* T+120+35= T+155: We press 'D', this should trigger 'combo_40' and send the keycode 'Z'. We wait 15ms */ - press_D_and_wait(15) - - - - /*** Cleanup ***/ - /* T+120+50= T+170: We release both keys */ - release_A_and_wait(20) - release_D_and_wait(0) - >; -}; \ No newline at end of file