From 4cacab68bfe7af34b416016febb2dcb362ed257f Mon Sep 17 00:00:00 2001 From: Kasimir Pihlasviita Date: Sun, 5 Mar 2023 19:08:09 +0200 Subject: [PATCH 1/9] Remove handling of multiple taps --- quantum/action.c | 5 ----- 1 file changed, 5 deletions(-) diff --git a/quantum/action.c b/quantum/action.c index 7d3f40a95083..b5b7534b94ac 100644 --- a/quantum/action.c +++ b/quantum/action.c @@ -447,8 +447,6 @@ void process_action(keyrecord_t *record, action_t action) { clear_oneshot_mods(); set_oneshot_locked_mods(mods | get_oneshot_locked_mods()); # endif - } else { - register_mods(mods | get_oneshot_mods()); } } else { if (tap_count == 0) { @@ -465,9 +463,6 @@ void process_action(keyrecord_t *record, action_t action) { } else if (tap_count == ONESHOT_TAP_TOGGLE) { // Toggle Oneshot Layer # endif - } else { - unregister_mods(mods); - clear_oneshot_mods(); } } } From f447da4309226432ba447dd5d79e619940260080 Mon Sep 17 00:00:00 2001 From: Kasimir Pihlasviita Date: Sun, 5 Mar 2023 19:10:26 +0200 Subject: [PATCH 2/9] Leave OSMs intact on press-and-hold --- quantum/action.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/quantum/action.c b/quantum/action.c index b5b7534b94ac..e2be5bc5e1ce 100644 --- a/quantum/action.c +++ b/quantum/action.c @@ -436,7 +436,7 @@ void process_action(keyrecord_t *record, action_t action) { if (event.pressed) { if (tap_count == 0) { ac_dprintf("MODS_TAP: Oneshot: 0\n"); - register_mods(mods | get_oneshot_mods()); + register_mods(mods); } else if (tap_count == 1) { ac_dprintf("MODS_TAP: Oneshot: start\n"); set_oneshot_mods(mods | get_oneshot_mods()); @@ -450,7 +450,6 @@ void process_action(keyrecord_t *record, action_t action) { } } else { if (tap_count == 0) { - clear_oneshot_mods(); unregister_mods(mods); } else if (tap_count == 1) { // Retain Oneshot mods From a8c9d0e4a2ae1471227363fc8c7603bb29c46173 Mon Sep 17 00:00:00 2001 From: Kasimir Pihlasviita Date: Sun, 5 Mar 2023 19:17:03 +0200 Subject: [PATCH 3/9] Remove redundant outer conditional block --- quantum/action.c | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/quantum/action.c b/quantum/action.c index e2be5bc5e1ce..713bbae5349c 100644 --- a/quantum/action.c +++ b/quantum/action.c @@ -451,16 +451,11 @@ void process_action(keyrecord_t *record, action_t action) { } else { if (tap_count == 0) { unregister_mods(mods); - } else if (tap_count == 1) { - // Retain Oneshot mods # if defined(ONESHOT_TAP_TOGGLE) && ONESHOT_TAP_TOGGLE > 1 - if (mods & get_mods()) { - unregister_mods(mods); - clear_oneshot_mods(); - set_oneshot_locked_mods(~mods & get_oneshot_locked_mods()); - } - } else if (tap_count == ONESHOT_TAP_TOGGLE) { - // Toggle Oneshot Layer + } else if (tap_count == 1 && (mods & get_mods())) { + unregister_mods(mods); + clear_oneshot_mods(); + set_oneshot_locked_mods(~mods & get_oneshot_locked_mods()); # endif } } From 15fde8aa995498d20d249f775a967c99d24715cb Mon Sep 17 00:00:00 2001 From: Kasimir Pihlasviita Date: Sun, 5 Mar 2023 19:22:24 +0200 Subject: [PATCH 4/9] Add function del_oneshot_locked_mods() --- quantum/action.c | 3 ++- quantum/action_util.c | 6 ++++++ quantum/action_util.h | 1 + 3 files changed, 9 insertions(+), 1 deletion(-) diff --git a/quantum/action.c b/quantum/action.c index 713bbae5349c..be306da567f8 100644 --- a/quantum/action.c +++ b/quantum/action.c @@ -451,11 +451,12 @@ void process_action(keyrecord_t *record, action_t action) { } else { if (tap_count == 0) { unregister_mods(mods); + del_oneshot_locked_mods(mods); # if defined(ONESHOT_TAP_TOGGLE) && ONESHOT_TAP_TOGGLE > 1 } else if (tap_count == 1 && (mods & get_mods())) { unregister_mods(mods); + del_oneshot_locked_mods(mods); clear_oneshot_mods(); - set_oneshot_locked_mods(~mods & get_oneshot_locked_mods()); # endif } } diff --git a/quantum/action_util.c b/quantum/action_util.c index 7f7d32887b6a..dea903e3d915 100644 --- a/quantum/action_util.c +++ b/quantum/action_util.c @@ -58,6 +58,12 @@ void clear_oneshot_locked_mods(void) { oneshot_locked_mods_changed_kb(oneshot_locked_mods); } } +void del_oneshot_locked_mods(uint8_t mods) { + if (oneshot_locked_mods & mods) { + oneshot_locked_mods &= ~mods; + oneshot_locked_mods_changed_kb(oneshot_locked_mods); + } +} # if (defined(ONESHOT_TIMEOUT) && (ONESHOT_TIMEOUT > 0)) static uint16_t oneshot_time = 0; bool has_oneshot_mods_timed_out(void) { diff --git a/quantum/action_util.h b/quantum/action_util.h index 6f1f09c4bd1c..2558bc9842f9 100644 --- a/quantum/action_util.h +++ b/quantum/action_util.h @@ -66,6 +66,7 @@ bool has_oneshot_mods_timed_out(void); uint8_t get_oneshot_locked_mods(void); void set_oneshot_locked_mods(uint8_t mods); void clear_oneshot_locked_mods(void); +void del_oneshot_locked_mods(uint8_t mods); typedef enum { ONESHOT_PRESSED = 0b01, ONESHOT_OTHER_KEY_PRESSED = 0b10, ONESHOT_START = 0b11, ONESHOT_TOGGLED = 0b100 } oneshot_fullfillment_t; void set_oneshot_layer(uint8_t layer, uint8_t state); From a6f6a4a18c8f786ba1b817beacf363e3f71464af Mon Sep 17 00:00:00 2001 From: Kasimir Pihlasviita Date: Sun, 5 Mar 2023 19:26:33 +0200 Subject: [PATCH 5/9] Fix clearing OSMs --- quantum/action.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/quantum/action.c b/quantum/action.c index be306da567f8..df339f2510ec 100644 --- a/quantum/action.c +++ b/quantum/action.c @@ -444,19 +444,20 @@ void process_action(keyrecord_t *record, action_t action) { } else if (tap_count == ONESHOT_TAP_TOGGLE) { ac_dprintf("MODS_TAP: Toggling oneshot"); register_mods(mods); - clear_oneshot_mods(); + del_oneshot_mods(mods); set_oneshot_locked_mods(mods | get_oneshot_locked_mods()); # endif } } else { if (tap_count == 0) { unregister_mods(mods); + del_oneshot_mods(mods); del_oneshot_locked_mods(mods); # if defined(ONESHOT_TAP_TOGGLE) && ONESHOT_TAP_TOGGLE > 1 } else if (tap_count == 1 && (mods & get_mods())) { unregister_mods(mods); + del_oneshot_mods(mods); del_oneshot_locked_mods(mods); - clear_oneshot_mods(); # endif } } From 7b58a55f9c218b4cdbf89ac09e3524cc0f1dde07 Mon Sep 17 00:00:00 2001 From: Kasimir Pihlasviita Date: Sun, 5 Mar 2023 19:33:28 +0200 Subject: [PATCH 6/9] Improve readability --- quantum/action.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/quantum/action.c b/quantum/action.c index df339f2510ec..783fdbc2c3af 100644 --- a/quantum/action.c +++ b/quantum/action.c @@ -435,11 +435,12 @@ void process_action(keyrecord_t *record, action_t action) { } else { if (event.pressed) { if (tap_count == 0) { + // Not a tap, but a hold: register the held mod ac_dprintf("MODS_TAP: Oneshot: 0\n"); register_mods(mods); } else if (tap_count == 1) { ac_dprintf("MODS_TAP: Oneshot: start\n"); - set_oneshot_mods(mods | get_oneshot_mods()); + add_oneshot_mods(mods); # if defined(ONESHOT_TAP_TOGGLE) && ONESHOT_TAP_TOGGLE > 1 } else if (tap_count == ONESHOT_TAP_TOGGLE) { ac_dprintf("MODS_TAP: Toggling oneshot"); @@ -450,6 +451,7 @@ void process_action(keyrecord_t *record, action_t action) { } } else { if (tap_count == 0) { + // Release hold: unregister the held mod and its variants unregister_mods(mods); del_oneshot_mods(mods); del_oneshot_locked_mods(mods); From 1cac38145543f0b7b4ac26f6a147f75d3117c399 Mon Sep 17 00:00:00 2001 From: Kasimir Pihlasviita Date: Sun, 5 Mar 2023 19:40:00 +0200 Subject: [PATCH 7/9] Add add_oneshot_locked_mods() --- quantum/action.c | 2 +- quantum/action_util.c | 6 ++++++ quantum/action_util.h | 1 + 3 files changed, 8 insertions(+), 1 deletion(-) diff --git a/quantum/action.c b/quantum/action.c index 783fdbc2c3af..3dd7863e1ddd 100644 --- a/quantum/action.c +++ b/quantum/action.c @@ -446,7 +446,7 @@ void process_action(keyrecord_t *record, action_t action) { ac_dprintf("MODS_TAP: Toggling oneshot"); register_mods(mods); del_oneshot_mods(mods); - set_oneshot_locked_mods(mods | get_oneshot_locked_mods()); + add_oneshot_locked_mods(mods); # endif } } else { diff --git a/quantum/action_util.c b/quantum/action_util.c index dea903e3d915..034c2ecaae1b 100644 --- a/quantum/action_util.c +++ b/quantum/action_util.c @@ -46,6 +46,12 @@ static uint8_t oneshot_locked_mods = 0; uint8_t get_oneshot_locked_mods(void) { return oneshot_locked_mods; } +void add_oneshot_locked_mods(uint8_t mods) { + if ((oneshot_locked_mods & mods) != mods) { + oneshot_locked_mods |= mods; + oneshot_locked_mods_changed_kb(oneshot_locked_mods); + } +} void set_oneshot_locked_mods(uint8_t mods) { if (mods != oneshot_locked_mods) { oneshot_locked_mods = mods; diff --git a/quantum/action_util.h b/quantum/action_util.h index 2558bc9842f9..d1f41f6a11d9 100644 --- a/quantum/action_util.h +++ b/quantum/action_util.h @@ -64,6 +64,7 @@ void clear_oneshot_mods(void); bool has_oneshot_mods_timed_out(void); uint8_t get_oneshot_locked_mods(void); +void add_oneshot_locked_mods(uint8_t mods); void set_oneshot_locked_mods(uint8_t mods); void clear_oneshot_locked_mods(void); void del_oneshot_locked_mods(uint8_t mods); From 1d670f0492d9f5821afff08251588c20bbf608b0 Mon Sep 17 00:00:00 2001 From: Kasimir Pihlasviita Date: Wed, 22 Mar 2023 16:19:04 +0200 Subject: [PATCH 8/9] Add tests --- tests/basic/test_one_shot_keys.cpp | 145 +++++++++++++++++++++++++++++ 1 file changed, 145 insertions(+) diff --git a/tests/basic/test_one_shot_keys.cpp b/tests/basic/test_one_shot_keys.cpp index 2401c2c837a5..edb1dff5fd11 100644 --- a/tests/basic/test_one_shot_keys.cpp +++ b/tests/basic/test_one_shot_keys.cpp @@ -160,6 +160,151 @@ INSTANTIATE_TEST_CASE_P( )); // clang-format on +TEST_F(OneShot, OSMChainingTwoOSMs) { + TestDriver driver; + InSequence s; + KeymapKey osm_key1 = KeymapKey{0, 0, 0, OSM(MOD_LSFT), KC_LSFT}; + KeymapKey osm_key2 = KeymapKey{0, 0, 1, OSM(MOD_LCTL), KC_LCTL}; + KeymapKey regular_key = KeymapKey{0, 1, 0, KC_A}; + + set_keymap({osm_key1, osm_key2, regular_key}); + + /* Press and release OSM1 */ + EXPECT_NO_REPORT(driver); + osm_key1.press(); + run_one_scan_loop(); + osm_key1.release(); + run_one_scan_loop(); + VERIFY_AND_CLEAR(driver); + + /* Press and relesea OSM2 */ + EXPECT_NO_REPORT(driver); + osm_key2.press(); + run_one_scan_loop(); + osm_key2.release(); + run_one_scan_loop(); + VERIFY_AND_CLEAR(driver); + + /* Press and hold regular key */ + EXPECT_REPORT(driver, (osm_key1.report_code, osm_key2.report_code, regular_key.report_code)).Times(1); + regular_key.press(); + run_one_scan_loop(); + VERIFY_AND_CLEAR(driver); + + /* Release regular key */ + EXPECT_EMPTY_REPORT(driver); + regular_key.release(); + run_one_scan_loop(); + VERIFY_AND_CLEAR(driver); +} + +TEST_F(OneShot, OSMDoubleTapNotLockingOSMs) { + TestDriver driver; + InSequence s; + KeymapKey osm_key1 = KeymapKey{0, 0, 0, OSM(MOD_LSFT), KC_LSFT}; + KeymapKey osm_key2 = KeymapKey{0, 0, 1, OSM(MOD_LCTL), KC_LCTL}; + KeymapKey regular_key = KeymapKey{0, 1, 0, KC_A}; + + set_keymap({osm_key1, osm_key2, regular_key}); + + /* Press and release OSM1 */ + EXPECT_NO_REPORT(driver); + osm_key1.press(); + run_one_scan_loop(); + osm_key1.release(); + run_one_scan_loop(); + VERIFY_AND_CLEAR(driver); + + /* Press and release OSM2 twice */ + EXPECT_NO_REPORT(driver); + osm_key2.press(); + run_one_scan_loop(); + osm_key2.release(); + run_one_scan_loop(); + osm_key2.press(); + run_one_scan_loop(); + osm_key2.release(); + run_one_scan_loop(); + VERIFY_AND_CLEAR(driver); + + /* Press and hold regular key */ + EXPECT_REPORT(driver, (osm_key1.report_code, osm_key2.report_code, regular_key.report_code)).Times(1); + regular_key.press(); + run_one_scan_loop(); + VERIFY_AND_CLEAR(driver); + + /* Release regular key */ + EXPECT_EMPTY_REPORT(driver); + regular_key.release(); + run_one_scan_loop(); + VERIFY_AND_CLEAR(driver); + + /* Press and hold regular key */ + EXPECT_REPORT(driver, (regular_key.report_code)).Times(1); + regular_key.press(); + run_one_scan_loop(); + VERIFY_AND_CLEAR(driver); + + /* Release regular key */ + EXPECT_EMPTY_REPORT(driver); + regular_key.release(); + run_one_scan_loop(); + VERIFY_AND_CLEAR(driver); +} + +TEST_F(OneShot, OSMHoldNotLockingOSMs) { + TestDriver driver; + InSequence s; + KeymapKey osm_key1 = KeymapKey{0, 0, 0, OSM(MOD_LSFT), KC_LSFT}; + KeymapKey osm_key2 = KeymapKey{0, 0, 1, OSM(MOD_LCTL), KC_LCTL}; + KeymapKey regular_key = KeymapKey{0, 1, 0, KC_A}; + + set_keymap({osm_key1, osm_key2, regular_key}); + + /* Press and release OSM1 */ + EXPECT_NO_REPORT(driver); + osm_key1.press(); + run_one_scan_loop(); + osm_key1.release(); + run_one_scan_loop(); + VERIFY_AND_CLEAR(driver); + + /* Press and hold OSM2 */ + EXPECT_NO_REPORT(driver); + osm_key2.press(); + run_one_scan_loop(); + //osm_key2.release(); + //run_one_scan_loop(); + VERIFY_AND_CLEAR(driver); + + /* Press and hold regular key */ + EXPECT_NO_REPORT(driver); + regular_key.press(); + run_one_scan_loop(); + regular_key.release(); + run_one_scan_loop(); + VERIFY_AND_CLEAR(driver); + + /* Release regular key */ + EXPECT_REPORT(driver, (osm_key1.report_code, osm_key2.report_code, regular_key.report_code)).Times(1); + EXPECT_EMPTY_REPORT(driver); + osm_key2.release(); + run_one_scan_loop(); + VERIFY_AND_CLEAR(driver); + + /* Press and hold regular key */ + EXPECT_REPORT(driver, (regular_key.report_code)).Times(1); + regular_key.press(); + run_one_scan_loop(); + VERIFY_AND_CLEAR(driver); + + /* Release regular key */ + EXPECT_EMPTY_REPORT(driver); + regular_key.release(); + run_one_scan_loop(); + VERIFY_AND_CLEAR(driver); +} + TEST_F(OneShot, OSLWithAdditionalKeypress) { TestDriver driver; InSequence s; From 8b8f7f3bc5a7418f56f79f460250b3c63bfac2ca Mon Sep 17 00:00:00 2001 From: Kasimir Pihlasviita Date: Wed, 22 Mar 2023 16:42:31 +0200 Subject: [PATCH 9/9] Fix tests --- tests/basic/test_one_shot_keys.cpp | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/tests/basic/test_one_shot_keys.cpp b/tests/basic/test_one_shot_keys.cpp index edb1dff5fd11..2a3434bf1616 100644 --- a/tests/basic/test_one_shot_keys.cpp +++ b/tests/basic/test_one_shot_keys.cpp @@ -185,7 +185,7 @@ TEST_F(OneShot, OSMChainingTwoOSMs) { run_one_scan_loop(); VERIFY_AND_CLEAR(driver); - /* Press and hold regular key */ + /* Press regular key */ EXPECT_REPORT(driver, (osm_key1.report_code, osm_key2.report_code, regular_key.report_code)).Times(1); regular_key.press(); run_one_scan_loop(); @@ -227,7 +227,7 @@ TEST_F(OneShot, OSMDoubleTapNotLockingOSMs) { run_one_scan_loop(); VERIFY_AND_CLEAR(driver); - /* Press and hold regular key */ + /* Press regular key */ EXPECT_REPORT(driver, (osm_key1.report_code, osm_key2.report_code, regular_key.report_code)).Times(1); regular_key.press(); run_one_scan_loop(); @@ -239,7 +239,7 @@ TEST_F(OneShot, OSMDoubleTapNotLockingOSMs) { run_one_scan_loop(); VERIFY_AND_CLEAR(driver); - /* Press and hold regular key */ + /* Press regular key */ EXPECT_REPORT(driver, (regular_key.report_code)).Times(1); regular_key.press(); run_one_scan_loop(); @@ -270,29 +270,28 @@ TEST_F(OneShot, OSMHoldNotLockingOSMs) { VERIFY_AND_CLEAR(driver); /* Press and hold OSM2 */ - EXPECT_NO_REPORT(driver); + EXPECT_REPORT(driver, (osm_key1.report_code, osm_key2.report_code)).Times(1); osm_key2.press(); run_one_scan_loop(); - //osm_key2.release(); - //run_one_scan_loop(); + idle_for(TAPPING_TERM); VERIFY_AND_CLEAR(driver); - /* Press and hold regular key */ - EXPECT_NO_REPORT(driver); + /* Press and release regular key */ + EXPECT_REPORT(driver, (osm_key1.report_code, osm_key2.report_code, regular_key.report_code)).Times(1); + EXPECT_REPORT(driver, (osm_key2.report_code)).Times(1); regular_key.press(); run_one_scan_loop(); regular_key.release(); run_one_scan_loop(); VERIFY_AND_CLEAR(driver); - /* Release regular key */ - EXPECT_REPORT(driver, (osm_key1.report_code, osm_key2.report_code, regular_key.report_code)).Times(1); + /* Release OSM2 */ EXPECT_EMPTY_REPORT(driver); osm_key2.release(); run_one_scan_loop(); VERIFY_AND_CLEAR(driver); - /* Press and hold regular key */ + /* Press regular key */ EXPECT_REPORT(driver, (regular_key.report_code)).Times(1); regular_key.press(); run_one_scan_loop();