From e924a0cd36b7f377fd194a112635c38c034f4d2b Mon Sep 17 00:00:00 2001 From: Pascal Getreuer Date: Fri, 15 Nov 2024 20:08:52 -0800 Subject: [PATCH] Revise handing of pairs of tap-hold keys. --- docs/tap_hold.md | 106 ++++++++++++-- quantum/action_tapping.c | 128 ++++++++--------- .../hold_on_other_key_press/test_tap_hold.cpp | 62 ++++++--- .../permissive_hold/test_tap_hold.cpp | 129 +++++++++++++----- 4 files changed, 300 insertions(+), 125 deletions(-) diff --git a/docs/tap_hold.md b/docs/tap_hold.md index 84d0a17ab88..0ade6a86737 100644 --- a/docs/tap_hold.md +++ b/docs/tap_hold.md @@ -560,26 +560,106 @@ bool get_chordal_hold(uint16_t tap_hold_keycode, keyrecord_t* tap_hold_record, uint16_t other_keycode, keyrecord_t* other_record) { // Exceptionally allow some one-handed chords for hotkeys. switch (tap_hold_keycode) { - case LCTL_T(KC_A): - if (other_keycode == KC_C || other_keycode == KC_V) { - return true; - } - break; + case LCTL_T(KC_Z): + if (other_keycode == KC_C || other_keycode == KC_V) { + return true; + } + break; - case RCTL_T(KC_SCLN): - if (other_keycode == KC_N) { - return true; - } - break; + case RCTL_T(KC_SLSH): + if (other_keycode == KC_N) { + return true; + } + break; } // Otherwise defer to the opposite hands rule. return get_chordal_hold_default(tap_hold_record, other_record); } ``` -As shown above, the function may call `get_chordal_hold_default(tap_hold_record, -other_record)` to get the default tap vs. hold decision according to the -opposite hands rule. +As shown in the last line above, you may use +`get_chordal_hold_default(tap_hold_record, other_record)` to get the default tap +vs. hold decision according to the opposite hands rule. + +If you use home row mods, you may want to produce a hotkey like Ctrl+Shift+V by +holding Ctrl and Shift mod-taps on one hand while tapping `KC_V` with the other +hand, say: + +- `RCTL_T(KC_K)` Down +- `RSFT_T(KC_L)` Down (on the same hand as `RCTL_T(KC_K)`) +- `KC_V` Down +- `KC_V` Up +- `RCTL_T(KC_K)` Up +- `RSFT_T(KC_L)` Up + +However, supposing `RCTL_T(KC_K)` and `RSFT_T(KC_L)` are on the same hand, +Chordal Hold by default considers `RCTL_T(KC_K)` tapped, producing "`kV`" +instead of the desired Ctrl+Shift+V. + +To address this, `get_chordal_hold()` may be defined to allow chords between any +pair of mod-tap keys with + +```c +bool get_chordal_hold(uint16_t tap_hold_keycode, keyrecord_t* tap_hold_record, + uint16_t other_keycode, keyrecord_t* other_record) { + // Allow hold between any pair of mod-tap keys. + if (IS_QK_MOD_TAP(tap_hold_keycode) && IS_QK_MOD_TAP(other_keycode)) { + return true; + } + + // Otherwise defer to the opposite hands rule. + return get_chordal_hold_default(tap_hold_record, other_record); +} +``` + +Or to allow one-handed chords of specific mod-taps but not others, use: + +```c +bool get_chordal_hold(uint16_t tap_hold_keycode, keyrecord_t* tap_hold_record, + uint16_t other_keycode, keyrecord_t* other_record) { + switch (tap_hold_keycode) { + case RCTL_T(KC_K): + if (other_keycode == RSFT_T(KC_L)) { + // Allow hold in "RCTL_T(KC_K) down, RSFT_T(KC_L) down". + return true; + } + break; + + case RSFT_T(KC_L): + if (other_keycode == RCTL_T(KC_K)) { + // Allow hold in "RSFT_T(KC_L) down, RCTL_T(KC_K) down". + return true; + } + break; + } + // Otherwise defer to the opposite hands rule. + return get_chordal_hold_default(tap_hold_record, other_record); +} +``` + +Above, two exceptions are defined, one where `RCTL_T(KC_K)` is pressed first and +another where `RSFT_T(KC_L)` is held first, such that Ctrl+Shift+V could be done +by holding the mod-taps in either order. For yet finer control, you could choose +to define an exception for one order but not the other: + +```c +bool get_chordal_hold(uint16_t tap_hold_keycode, keyrecord_t* tap_hold_record, + uint16_t other_keycode, keyrecord_t* other_record) { + switch (tap_hold_keycode) { + case RCTL_T(KC_K): + if (other_keycode == RSFT_T(KC_L)) { + // Allow hold in "RCTL_T(KC_K) down, RSFT_T(KC_L), down". + return true; + } + break; + + // ... but RSFT_T(KC_L) is considered tapped in + // "RSFT_T(KC_L) down, RCTL_T(KC_K) down". + } + // Otherwise defer to the opposite hands rule. + return get_chordal_hold_default(tap_hold_record, other_record); +} +``` ## Retro Tapping diff --git a/quantum/action_tapping.c b/quantum/action_tapping.c index 49ac47107dc..53a3413c385 100644 --- a/quantum/action_tapping.c +++ b/quantum/action_tapping.c @@ -50,12 +50,25 @@ __attribute__((weak)) bool get_permissive_hold(uint16_t keycode, keyrecord_t *re # endif # ifdef CHORDAL_HOLD +# define REGISTERED_TAPS_SIZE 8 +// Array of tap-hold keys that have been settled as tapped but not yet released. +static keypos_t registered_taps[REGISTERED_TAPS_SIZE] = {}; +static uint8_t num_registered_taps = 0; + +/** Adds `key` to the registered_taps array. */ +static void registered_taps_add(keypos_t key); +/** Returns the index of `key` in registered_taps, or -1 if not found. */ +static int8_t registered_tap_find(keypos_t key); +/** Removes index `i` from the registered_taps array. */ +static void registered_taps_del_index(uint8_t i); +/** Logs the registered_taps array for debugging. */ +static void debug_registered_taps(void); +/** Processes and pops buffered events until the first tap-hold event. */ +static void waiting_buffer_process_regular(void); + static bool is_one_shot(uint16_t keycode) { return IS_QK_ONE_SHOT_MOD(keycode) || IS_QK_ONE_SHOT_LAYER(keycode); } -static uint8_t waiting_buffer_find_chordal_hold(void); -static void waiting_buffer_process_until(uint8_t new_tail); -static void waiting_buffer_process_regular(void); # endif // CHORDAL_HOLD # ifdef HOLD_ON_OTHER_KEY_PRESS_PER_KEY @@ -175,6 +188,20 @@ void action_tapping_process(keyrecord_t record) { bool process_tapping(keyrecord_t *keyp) { const keyevent_t event = keyp->event; +# ifdef CHORDAL_HOLD + if (!event.pressed) { + const int8_t i = registered_tap_find(event.key); + if (i != -1) { + // If a tap-hold key was previously settled as tapped, set its + // tap.count correspondingly on release. + keyp->tap.count = 1; + registered_taps_del_index(i); + ac_dprintf("CHORDAL_HOLD: Found tap release for [%d]\n", i); + debug_registered_taps(); + } + } +# endif // CHORDAL_HOLD + // state machine is in the "reset" state, no tapping key is to be // processed if (IS_NOEVENT(tapping_key.event)) { @@ -222,28 +249,6 @@ bool process_tapping(keyrecord_t *keyp) { // enqueue return false; } -# ifdef CHORDAL_HOLD - else if (!TAP_GET_RETRO_TAPPING(keyp) && waiting_buffer_tail != waiting_buffer_head && is_tap_record(&waiting_buffer[waiting_buffer_tail]) && !event.pressed && waiting_buffer_typed(event)) { - /* (!event.pressed || !is_tap_record(keyp)) && !is_one_shot(get_record_keycode(keyp, false))) { */ - const uint8_t new_tail = waiting_buffer_find_chordal_hold(); - ac_dprintf("Chordal hold: new tail = %u\n", new_tail); - - // Settle and process the tapping key and waiting events - // preceding first_tap as *held*. - if (new_tail < WAITING_BUFFER_SIZE) { - ac_dprintf("Tapping: End. No tap. Chord considered held\n"); - process_record(&tapping_key); - tapping_key = (keyrecord_t){0}; - debug_tapping_key(); - - waiting_buffer_process_until(new_tail); - waiting_buffer_process_regular(); - } - - // enqueue - return false; - } -# endif /* Process a key typed within TAPPING_TERM * This can register the key before settlement of tapping, * useful for long TAPPING_TERM but may prevent fast typing. @@ -324,17 +329,15 @@ bool process_tapping(keyrecord_t *keyp) { // this needs to be set false. tapping_key.tap.interrupted = false; - if (!is_tap_record(keyp)) { - // Settle the tapping key as *tapped*, since it - // is not considered a held chord with keyp. - ac_dprintf("Tapping: End. Chord considered a tap\n"); - tapping_key.tap.count = 1; - process_record(&tapping_key); - debug_tapping_key(); + ac_dprintf("Tapping: End. Chord considered a tap\n"); + tapping_key.tap.count = 1; + registered_taps_add(tapping_key.event.key); + debug_registered_taps(); + process_record(&tapping_key); + tapping_key = (keyrecord_t){0}; - // Process regular keys in the waiting buffer. - waiting_buffer_process_regular(); - } + // Process regular keys in the waiting buffer. + waiting_buffer_process_regular(); } else # endif if (TAP_GET_HOLD_ON_OTHER_KEY_PRESS @@ -634,44 +637,43 @@ __attribute__((weak)) char chordal_hold_handedness_user(keypos_t key) { # endif } -/** \brief Finds which queued events should be held according to Chordal Hold. - * - * In a situation with multiple unsettled tap-hold key presses, scan the queue - * for a successive pair of keys that settles as held according to - * get_chordal_hold(). - * - * \return Buffer index of the hold, or WAITING_BUFFER_SIZE if none is found. - */ -static uint8_t waiting_buffer_find_chordal_hold(void) { - keyrecord_t *prev = &tapping_key; - uint16_t prev_keycode = get_record_keycode(&tapping_key, false); +static void registered_taps_add(keypos_t key) { + if (num_registered_taps >= REGISTERED_TAPS_SIZE) { + ac_dprintf("TAPS OVERFLOW: CLEAR ALL STATES\n"); + clear_keyboard(); + num_registered_taps = 0; + } - for (uint8_t i = waiting_buffer_tail; i != waiting_buffer_head; i = (i + 1) % WAITING_BUFFER_SIZE) { - keyrecord_t * cur = &waiting_buffer[i]; - const uint16_t cur_keycode = get_record_keycode(cur, false); + registered_taps[num_registered_taps] = key; + ++num_registered_taps; +} - if (get_chordal_hold(prev_keycode, prev, cur_keycode, cur)) { +static int8_t registered_tap_find(keypos_t key) { + for (int8_t i = 0; i < num_registered_taps; ++i) { + if (KEYEQ(registered_taps[i], key)) { return i; } - - prev = cur; - prev_keycode = cur_keycode; } - - return WAITING_BUFFER_SIZE; + return -1; } -/** \brief Processes and pops buffered events preceding `new_tail`. */ -static void waiting_buffer_process_until(uint8_t new_tail) { - for (; waiting_buffer_tail != new_tail; waiting_buffer_tail = (waiting_buffer_tail + 1) % WAITING_BUFFER_SIZE) { - ac_dprintf("waiting_buffer_process_until: processing [%u]\n", waiting_buffer_tail); - process_record(&waiting_buffer[waiting_buffer_tail]); +static void registered_taps_del_index(uint8_t i) { + if (i < num_registered_taps) { + --num_registered_taps; + if (i < num_registered_taps) { + registered_taps[i] = registered_taps[num_registered_taps]; + } } - - debug_waiting_buffer(); } -/** \brief Processes and pops buffered events until the first tap-hold event. */ +static void debug_registered_taps(void) { + ac_dprintf("registered_taps = { "); + for (int8_t i = 0; i < num_registered_taps; ++i) { + ac_dprintf("%02X%02X ", registered_taps[i].row, registered_taps[i].col); + } + ac_dprintf("}\n"); +} + static void waiting_buffer_process_regular(void) { for (; waiting_buffer_tail != waiting_buffer_head; waiting_buffer_tail = (waiting_buffer_tail + 1) % WAITING_BUFFER_SIZE) { if (is_tap_record(&waiting_buffer[waiting_buffer_tail])) { diff --git a/tests/tap_hold_configurations/chordal_hold/hold_on_other_key_press/test_tap_hold.cpp b/tests/tap_hold_configurations/chordal_hold/hold_on_other_key_press/test_tap_hold.cpp index e551a75dee3..cb08e5c3fbf 100644 --- a/tests/tap_hold_configurations/chordal_hold/hold_on_other_key_press/test_tap_hold.cpp +++ b/tests/tap_hold_configurations/chordal_hold/hold_on_other_key_press/test_tap_hold.cpp @@ -211,13 +211,15 @@ TEST_F(ChordalHoldHoldOnOtherKeyPress, two_mod_taps_same_hand_hold_til_timeout) EXPECT_NO_REPORT(driver); mod_tap_key1.press(); run_one_scan_loop(); + VERIFY_AND_CLEAR(driver); + + EXPECT_REPORT(driver, (KC_A)); mod_tap_key2.press(); run_one_scan_loop(); VERIFY_AND_CLEAR(driver); // Continue holding til the tapping term. - EXPECT_REPORT(driver, (KC_RIGHT_CTRL)); - EXPECT_REPORT(driver, (KC_RIGHT_CTRL, KC_RIGHT_SHIFT)); + EXPECT_REPORT(driver, (KC_A, KC_RIGHT_SHIFT)); idle_for(TAPPING_TERM); VERIFY_AND_CLEAR(driver); @@ -279,12 +281,14 @@ TEST_F(ChordalHoldHoldOnOtherKeyPress, two_mod_taps_nested_press_same_hand) { EXPECT_NO_REPORT(driver); mod_tap_key1.press(); run_one_scan_loop(); + VERIFY_AND_CLEAR(driver); + + EXPECT_REPORT(driver, (KC_A)); mod_tap_key2.press(); run_one_scan_loop(); VERIFY_AND_CLEAR(driver); // Release mod-tap keys. - EXPECT_REPORT(driver, (KC_A)); EXPECT_REPORT(driver, (KC_A, KC_B)); EXPECT_REPORT(driver, (KC_A)); EXPECT_EMPTY_REPORT(driver); @@ -308,8 +312,14 @@ TEST_F(ChordalHoldHoldOnOtherKeyPress, three_mod_taps_same_hand_streak_roll) { EXPECT_NO_REPORT(driver); mod_tap_key1.press(); run_one_scan_loop(); + VERIFY_AND_CLEAR(driver); + + EXPECT_REPORT(driver, (KC_A)); mod_tap_key2.press(); run_one_scan_loop(); + VERIFY_AND_CLEAR(driver); + + EXPECT_REPORT(driver, (KC_A, KC_B)); mod_tap_key3.press(); run_one_scan_loop(); VERIFY_AND_CLEAR(driver); @@ -327,8 +337,6 @@ TEST_F(ChordalHoldHoldOnOtherKeyPress, three_mod_taps_same_hand_streak_roll) { // However, due to a workaround for https://github.com/tmk/tmk_keyboard/issues/60, // the events are processed out of order, with the first two keys released // before pressing KC_C. - EXPECT_REPORT(driver, (KC_A)); - EXPECT_REPORT(driver, (KC_A, KC_B)); EXPECT_REPORT(driver, (KC_B)); EXPECT_EMPTY_REPORT(driver); EXPECT_REPORT(driver, (KC_C)); @@ -399,9 +407,20 @@ TEST_F(ChordalHoldHoldOnOtherKeyPress, three_mod_taps_same_hand_streak_orders) { run_one_scan_loop(); VERIFY_AND_CLEAR(driver); + // NOTE: The correct order of events should be + // EXPECT_REPORT(driver, (KC_A)); + // EXPECT_REPORT(driver, (KC_A, KC_B)); + // EXPECT_REPORT(driver, (KC_A, KC_B, KC_C)); + // EXPECT_REPORT(driver, (KC_A, KC_C)); + // EXPECT_REPORT(driver, (KC_A)); + // EXPECT_EMPTY_REPORT(driver); + // + // However, due to a workaround for https://github.com/tmk/tmk_keyboard/issues/60, + // the events are processed out of order, with the first two keys released + // before pressing KC_C. EXPECT_REPORT(driver, (KC_A)); EXPECT_REPORT(driver, (KC_A, KC_B)); - EXPECT_REPORT(driver, (KC_A, KC_B, KC_C)); + EXPECT_REPORT(driver, (KC_A)); EXPECT_REPORT(driver, (KC_A, KC_C)); EXPECT_REPORT(driver, (KC_A)); EXPECT_EMPTY_REPORT(driver); @@ -423,7 +442,7 @@ TEST_F(ChordalHoldHoldOnOtherKeyPress, three_mod_taps_same_hand_streak_orders) { VERIFY_AND_CLEAR(driver); } -TEST_F(ChordalHoldHoldOnOtherKeyPress, three_mod_taps_two_held_one_tapped) { +TEST_F(ChordalHoldHoldOnOtherKeyPress, three_mod_taps_two_left_one_right) { TestDriver driver; InSequence s; auto mod_tap_key1 = KeymapKey(0, 1, 0, SFT_T(KC_A)); @@ -433,25 +452,30 @@ TEST_F(ChordalHoldHoldOnOtherKeyPress, three_mod_taps_two_held_one_tapped) { set_keymap({mod_tap_key1, mod_tap_key2, mod_tap_key3}); // Press mod-tap keys. - EXPECT_REPORT(driver, (KC_LEFT_SHIFT)); - EXPECT_REPORT(driver, (KC_LEFT_SHIFT, KC_LEFT_CTRL)); + EXPECT_NO_REPORT(driver); mod_tap_key1.press(); run_one_scan_loop(); + VERIFY_AND_CLEAR(driver); + + EXPECT_REPORT(driver, (KC_A)); mod_tap_key2.press(); run_one_scan_loop(); + VERIFY_AND_CLEAR(driver); + + EXPECT_REPORT(driver, (KC_A, KC_LEFT_CTRL)); mod_tap_key3.press(); run_one_scan_loop(); VERIFY_AND_CLEAR(driver); // Release key 3. - EXPECT_REPORT(driver, (KC_LEFT_SHIFT, KC_LEFT_CTRL, KC_C)); - EXPECT_REPORT(driver, (KC_LEFT_SHIFT, KC_LEFT_CTRL)); + EXPECT_REPORT(driver, (KC_A, KC_LEFT_CTRL, KC_C)); + EXPECT_REPORT(driver, (KC_A, KC_LEFT_CTRL)); mod_tap_key3.release(); run_one_scan_loop(); VERIFY_AND_CLEAR(driver); // Release key 2, then key 1. - EXPECT_REPORT(driver, (KC_LEFT_SHIFT)); + EXPECT_REPORT(driver, (KC_A)); EXPECT_EMPTY_REPORT(driver); mod_tap_key2.release(); run_one_scan_loop(); @@ -460,20 +484,24 @@ TEST_F(ChordalHoldHoldOnOtherKeyPress, three_mod_taps_two_held_one_tapped) { VERIFY_AND_CLEAR(driver); // Press mod-tap keys. - EXPECT_REPORT(driver, (KC_LEFT_SHIFT)); - EXPECT_REPORT(driver, (KC_LEFT_SHIFT, KC_LEFT_CTRL)); - idle_for(TAPPING_TERM); + EXPECT_NO_REPORT(driver); mod_tap_key1.press(); run_one_scan_loop(); + VERIFY_AND_CLEAR(driver); + + EXPECT_REPORT(driver, (KC_A)); mod_tap_key2.press(); run_one_scan_loop(); + VERIFY_AND_CLEAR(driver); + + EXPECT_REPORT(driver, (KC_A, KC_LEFT_CTRL)); mod_tap_key3.press(); run_one_scan_loop(); VERIFY_AND_CLEAR(driver); // Release key 3. - EXPECT_REPORT(driver, (KC_LEFT_SHIFT, KC_LEFT_CTRL, KC_C)); - EXPECT_REPORT(driver, (KC_LEFT_SHIFT, KC_LEFT_CTRL)); + EXPECT_REPORT(driver, (KC_A, KC_LEFT_CTRL, KC_C)); + EXPECT_REPORT(driver, (KC_A, KC_LEFT_CTRL)); mod_tap_key3.release(); run_one_scan_loop(); VERIFY_AND_CLEAR(driver); diff --git a/tests/tap_hold_configurations/chordal_hold/permissive_hold/test_tap_hold.cpp b/tests/tap_hold_configurations/chordal_hold/permissive_hold/test_tap_hold.cpp index bcc4a6f91dd..3a79dbf35aa 100644 --- a/tests/tap_hold_configurations/chordal_hold/permissive_hold/test_tap_hold.cpp +++ b/tests/tap_hold_configurations/chordal_hold/permissive_hold/test_tap_hold.cpp @@ -216,13 +216,15 @@ TEST_F(ChordalHoldPermissiveHold, two_mod_taps_same_hand_hold_til_timeout) { EXPECT_NO_REPORT(driver); mod_tap_key1.press(); run_one_scan_loop(); + VERIFY_AND_CLEAR(driver); + + EXPECT_REPORT(driver, (KC_A)); mod_tap_key2.press(); run_one_scan_loop(); VERIFY_AND_CLEAR(driver); // Continue holding til the tapping term. - EXPECT_REPORT(driver, (KC_RIGHT_CTRL)); - EXPECT_REPORT(driver, (KC_RIGHT_CTRL, KC_RIGHT_SHIFT)); + EXPECT_REPORT(driver, (KC_A, KC_RIGHT_SHIFT)); idle_for(TAPPING_TERM); VERIFY_AND_CLEAR(driver); @@ -280,12 +282,14 @@ TEST_F(ChordalHoldPermissiveHold, two_mod_taps_nested_press_same_hand) { EXPECT_NO_REPORT(driver); mod_tap_key1.press(); run_one_scan_loop(); + VERIFY_AND_CLEAR(driver); + + EXPECT_REPORT(driver, (KC_A)); mod_tap_key2.press(); run_one_scan_loop(); VERIFY_AND_CLEAR(driver); // Release mod-tap keys. - EXPECT_REPORT(driver, (KC_A)); EXPECT_REPORT(driver, (KC_A, KC_B)); EXPECT_REPORT(driver, (KC_A)); mod_tap_key2.release(); @@ -310,8 +314,14 @@ TEST_F(ChordalHoldPermissiveHold, three_mod_taps_same_hand_streak_roll) { EXPECT_NO_REPORT(driver); mod_tap_key1.press(); run_one_scan_loop(); + VERIFY_AND_CLEAR(driver); + + EXPECT_REPORT(driver, (KC_A)); mod_tap_key2.press(); run_one_scan_loop(); + VERIFY_AND_CLEAR(driver); + + EXPECT_REPORT(driver, (KC_A, KC_B)); mod_tap_key3.press(); run_one_scan_loop(); VERIFY_AND_CLEAR(driver); @@ -319,8 +329,6 @@ TEST_F(ChordalHoldPermissiveHold, three_mod_taps_same_hand_streak_roll) { // Release keys 1, 2, 3. // // NOTE: The correct order of events should be - // EXPECT_REPORT(driver, (KC_A)); - // EXPECT_REPORT(driver, (KC_A, KC_B)); // EXPECT_REPORT(driver, (KC_A, KC_B, KC_C)); // EXPECT_REPORT(driver, (KC_B, KC_C)); // EXPECT_REPORT(driver, (KC_C)); @@ -329,8 +337,6 @@ TEST_F(ChordalHoldPermissiveHold, three_mod_taps_same_hand_streak_roll) { // However, due to a workaround for https://github.com/tmk/tmk_keyboard/issues/60, // the events are processed out of order, with the first two keys released // before pressing KC_C. - EXPECT_REPORT(driver, (KC_A)); - EXPECT_REPORT(driver, (KC_A, KC_B)); EXPECT_REPORT(driver, (KC_B)); EXPECT_EMPTY_REPORT(driver); EXPECT_REPORT(driver, (KC_C)); @@ -357,15 +363,19 @@ TEST_F(ChordalHoldPermissiveHold, three_mod_taps_same_hand_streak_orders) { EXPECT_NO_REPORT(driver); mod_tap_key1.press(); run_one_scan_loop(); + VERIFY_AND_CLEAR(driver); + + EXPECT_REPORT(driver, (KC_A)); mod_tap_key2.press(); run_one_scan_loop(); + VERIFY_AND_CLEAR(driver); + + EXPECT_REPORT(driver, (KC_A, KC_B)); mod_tap_key3.press(); run_one_scan_loop(); VERIFY_AND_CLEAR(driver); // Release keys 3, 2, 1. - EXPECT_REPORT(driver, (KC_A)); - EXPECT_REPORT(driver, (KC_A, KC_B)); EXPECT_REPORT(driver, (KC_A, KC_B, KC_C)); EXPECT_REPORT(driver, (KC_A, KC_B)); EXPECT_REPORT(driver, (KC_A)); @@ -383,15 +393,19 @@ TEST_F(ChordalHoldPermissiveHold, three_mod_taps_same_hand_streak_orders) { idle_for(TAPPING_TERM); mod_tap_key1.press(); run_one_scan_loop(); + VERIFY_AND_CLEAR(driver); + + EXPECT_REPORT(driver, (KC_A)); mod_tap_key2.press(); run_one_scan_loop(); + VERIFY_AND_CLEAR(driver); + + EXPECT_REPORT(driver, (KC_A, KC_B)); mod_tap_key3.press(); run_one_scan_loop(); VERIFY_AND_CLEAR(driver); // Release keys 3, 1, 2. - EXPECT_REPORT(driver, (KC_A)); - EXPECT_REPORT(driver, (KC_A, KC_B)); EXPECT_REPORT(driver, (KC_A, KC_B, KC_C)); EXPECT_REPORT(driver, (KC_A, KC_B)); EXPECT_REPORT(driver, (KC_B)); @@ -409,16 +423,29 @@ TEST_F(ChordalHoldPermissiveHold, three_mod_taps_same_hand_streak_orders) { idle_for(TAPPING_TERM); mod_tap_key1.press(); run_one_scan_loop(); + VERIFY_AND_CLEAR(driver); + + EXPECT_REPORT(driver, (KC_A)); mod_tap_key2.press(); run_one_scan_loop(); + VERIFY_AND_CLEAR(driver); + + EXPECT_REPORT(driver, (KC_A, KC_B)); mod_tap_key3.press(); run_one_scan_loop(); VERIFY_AND_CLEAR(driver); // Release keys 2, 3, 1. + // + // NOTE: The correct order of events should be + // EXPECT_REPORT(driver, (KC_A, KC_B, KC_C)); + // EXPECT_REPORT(driver, (KC_A, KC_C)); + // EXPECT_REPORT(driver, (KC_A)); + // EXPECT_EMPTY_REPORT(driver); + // + // However, due to a workaround for https://github.com/tmk/tmk_keyboard/issues/60, + // the events are processed out of order. EXPECT_REPORT(driver, (KC_A)); - EXPECT_REPORT(driver, (KC_A, KC_B)); - EXPECT_REPORT(driver, (KC_A, KC_B, KC_C)); EXPECT_REPORT(driver, (KC_A, KC_C)); EXPECT_REPORT(driver, (KC_A)); EXPECT_EMPTY_REPORT(driver); @@ -447,8 +474,13 @@ TEST_F(ChordalHoldPermissiveHold, three_mod_taps_opposite_hands_roll) { EXPECT_NO_REPORT(driver); mod_tap_key1.press(); run_one_scan_loop(); + VERIFY_AND_CLEAR(driver); + + EXPECT_REPORT(driver, (KC_A)); mod_tap_key2.press(); run_one_scan_loop(); + VERIFY_AND_CLEAR(driver); + mod_tap_key3.press(); run_one_scan_loop(); VERIFY_AND_CLEAR(driver); @@ -456,7 +488,6 @@ TEST_F(ChordalHoldPermissiveHold, three_mod_taps_opposite_hands_roll) { // Release keys 1, 2, 3. // // NOTE: The correct order of events should be - // EXPECT_REPORT(driver, (KC_A)); // EXPECT_REPORT(driver, (KC_A, KC_B)); // EXPECT_REPORT(driver, (KC_A, KC_B, KC_C)); // EXPECT_REPORT(driver, (KC_B, KC_C)); @@ -468,8 +499,7 @@ TEST_F(ChordalHoldPermissiveHold, three_mod_taps_opposite_hands_roll) { // before pressing KC_C. // Release keys 1, 2, 3. - EXPECT_REPORT(driver, (KC_A)); - EXPECT_REPORT(driver, (KC_A, KC_B)); + EXPECT_EMPTY_REPORT(driver); EXPECT_REPORT(driver, (KC_B)); EXPECT_EMPTY_REPORT(driver); EXPECT_REPORT(driver, (KC_C)); @@ -483,7 +513,7 @@ TEST_F(ChordalHoldPermissiveHold, three_mod_taps_opposite_hands_roll) { VERIFY_AND_CLEAR(driver); } -TEST_F(ChordalHoldPermissiveHold, three_mod_taps_two_held_one_tapped) { +TEST_F(ChordalHoldPermissiveHold, three_mod_taps_two_left_one_right) { TestDriver driver; InSequence s; auto mod_tap_key1 = KeymapKey(0, 1, 0, SFT_T(KC_A)); @@ -496,26 +526,33 @@ TEST_F(ChordalHoldPermissiveHold, three_mod_taps_two_held_one_tapped) { EXPECT_NO_REPORT(driver); mod_tap_key1.press(); run_one_scan_loop(); + VERIFY_AND_CLEAR(driver); + + EXPECT_REPORT(driver, (KC_A)); mod_tap_key2.press(); run_one_scan_loop(); + VERIFY_AND_CLEAR(driver); + + EXPECT_NO_REPORT(driver); mod_tap_key3.press(); run_one_scan_loop(); VERIFY_AND_CLEAR(driver); // Release key 3. - EXPECT_REPORT(driver, (KC_LEFT_SHIFT)); - EXPECT_REPORT(driver, (KC_LEFT_SHIFT, KC_LEFT_CTRL)); - EXPECT_REPORT(driver, (KC_LEFT_SHIFT, KC_LEFT_CTRL, KC_C)); - EXPECT_REPORT(driver, (KC_LEFT_SHIFT, KC_LEFT_CTRL)); + EXPECT_REPORT(driver, (KC_A, KC_LEFT_CTRL)); + EXPECT_REPORT(driver, (KC_A, KC_LEFT_CTRL, KC_C)); + EXPECT_REPORT(driver, (KC_A, KC_LEFT_CTRL)); mod_tap_key3.release(); run_one_scan_loop(); VERIFY_AND_CLEAR(driver); // Release key 2, then key 1. - EXPECT_REPORT(driver, (KC_LEFT_SHIFT)); - EXPECT_EMPTY_REPORT(driver); + EXPECT_REPORT(driver, (KC_A)); mod_tap_key2.release(); run_one_scan_loop(); + VERIFY_AND_CLEAR(driver); + + EXPECT_EMPTY_REPORT(driver); mod_tap_key1.release(); run_one_scan_loop(); VERIFY_AND_CLEAR(driver); @@ -525,26 +562,32 @@ TEST_F(ChordalHoldPermissiveHold, three_mod_taps_two_held_one_tapped) { idle_for(TAPPING_TERM); mod_tap_key1.press(); run_one_scan_loop(); + VERIFY_AND_CLEAR(driver); + + EXPECT_REPORT(driver, (KC_A)); mod_tap_key2.press(); run_one_scan_loop(); + + EXPECT_NO_REPORT(driver); mod_tap_key3.press(); run_one_scan_loop(); VERIFY_AND_CLEAR(driver); // Release key 3. - EXPECT_REPORT(driver, (KC_LEFT_SHIFT)); - EXPECT_REPORT(driver, (KC_LEFT_SHIFT, KC_LEFT_CTRL)); - EXPECT_REPORT(driver, (KC_LEFT_SHIFT, KC_LEFT_CTRL, KC_C)); - EXPECT_REPORT(driver, (KC_LEFT_SHIFT, KC_LEFT_CTRL)); + EXPECT_REPORT(driver, (KC_A, KC_LEFT_CTRL)); + EXPECT_REPORT(driver, (KC_A, KC_LEFT_CTRL, KC_C)); + EXPECT_REPORT(driver, (KC_A, KC_LEFT_CTRL)); mod_tap_key3.release(); run_one_scan_loop(); VERIFY_AND_CLEAR(driver); // Release key 1, then key 2. EXPECT_REPORT(driver, (KC_LEFT_CTRL)); - EXPECT_EMPTY_REPORT(driver); mod_tap_key1.release(); run_one_scan_loop(); + VERIFY_AND_CLEAR(driver); + + EXPECT_EMPTY_REPORT(driver); mod_tap_key2.release(); run_one_scan_loop(); VERIFY_AND_CLEAR(driver); @@ -570,10 +613,21 @@ TEST_F(ChordalHoldPermissiveHold, three_mod_taps_one_held_two_tapped) { VERIFY_AND_CLEAR(driver); // Release keys 3, 2, 1. + // + // NOTE: The correct order of events should be + // EXPECT_REPORT(driver, (KC_LEFT_SHIFT)); + // EXPECT_REPORT(driver, (KC_LEFT_SHIFT, KC_B)); + // EXPECT_REPORT(driver, (KC_LEFT_SHIFT, KC_B, KC_C)); + // EXPECT_REPORT(driver, (KC_LEFT_SHIFT, KC_B)); + // EXPECT_REPORT(driver, (KC_LEFT_SHIFT)); + // EXPECT_EMPTY_REPORT(driver); + // + // However, due to a workaround for https://github.com/tmk/tmk_keyboard/issues/60, + // the events are processed out of order. EXPECT_REPORT(driver, (KC_LEFT_SHIFT)); EXPECT_REPORT(driver, (KC_LEFT_SHIFT, KC_B)); - EXPECT_REPORT(driver, (KC_LEFT_SHIFT, KC_B, KC_C)); - EXPECT_REPORT(driver, (KC_LEFT_SHIFT, KC_B)); + EXPECT_REPORT(driver, (KC_LEFT_SHIFT)); + EXPECT_REPORT(driver, (KC_LEFT_SHIFT, KC_C)); EXPECT_REPORT(driver, (KC_LEFT_SHIFT)); EXPECT_EMPTY_REPORT(driver); mod_tap_key3.release(); @@ -596,10 +650,21 @@ TEST_F(ChordalHoldPermissiveHold, three_mod_taps_one_held_two_tapped) { VERIFY_AND_CLEAR(driver); // Release keys 3, 1, 2. + // + // NOTE: The correct order of events should be + // EXPECT_REPORT(driver, (KC_LEFT_SHIFT)); + // EXPECT_REPORT(driver, (KC_LEFT_SHIFT, KC_B)); + // EXPECT_REPORT(driver, (KC_LEFT_SHIFT, KC_B, KC_C)); + // EXPECT_REPORT(driver, (KC_LEFT_SHIFT, KC_B)); + // EXPECT_REPORT(driver, (KC_B)); + // EXPECT_EMPTY_REPORT(driver); + // + // However, due to a workaround for https://github.com/tmk/tmk_keyboard/issues/60, + // the events are processed out of order. EXPECT_REPORT(driver, (KC_LEFT_SHIFT)); EXPECT_REPORT(driver, (KC_LEFT_SHIFT, KC_B)); - EXPECT_REPORT(driver, (KC_LEFT_SHIFT, KC_B, KC_C)); - EXPECT_REPORT(driver, (KC_LEFT_SHIFT, KC_B)); + EXPECT_REPORT(driver, (KC_B)); + EXPECT_REPORT(driver, (KC_B, KC_C)); EXPECT_REPORT(driver, (KC_B)); EXPECT_EMPTY_REPORT(driver); mod_tap_key3.release();