Retro Tapping Re-Write; Key Roll Fix (#23641)

2 files changed, 446 insertions, 12 deletions

diff --git a/quantum/action.c b/quantum/action.c
index 395340d9d7..be85192d25 100644
--- a/quantum/action.c
+++ b/quantum/action.c
@@ -47,7 +47,12 @@ along with this program.  If not, see <http://www.gnu.org/licenses/>.
 int tp_buttons;
 
 #if defined(RETRO_TAPPING) || defined(RETRO_TAPPING_PER_KEY) || (defined(AUTO_SHIFT_ENABLE) && defined(RETRO_SHIFT))
-int retro_tapping_counter = 0;
+bool     retro_tap_primed   = false;
+uint16_t retro_tap_curr_key = 0;
+#    if !(defined(AUTO_SHIFT_ENABLE) && defined(RETRO_SHIFT))
+uint8_t retro_tap_curr_mods = 0;
+uint8_t retro_tap_next_mods = 0;
+#    endif
 #endif
 
 #if defined(AUTO_SHIFT_ENABLE) && defined(RETRO_SHIFT) && !defined(NO_ACTION_TAPPING)
@@ -77,7 +82,13 @@ void action_exec(keyevent_t event) {
         debug_event(event);
         ac_dprintf("\n");
 #if defined(RETRO_TAPPING) || defined(RETRO_TAPPING_PER_KEY) || (defined(AUTO_SHIFT_ENABLE) && defined(RETRO_SHIFT))
-        retro_tapping_counter++;
+        uint16_t event_keycode = get_event_keycode(event, false);
+        if (event.pressed) {
+            retro_tap_primed   = false;
+            retro_tap_curr_key = event_keycode;
+        } else if (retro_tap_curr_key == event_keycode) {
+            retro_tap_primed = true;
+        }
 #endif
     }
 
@@ -531,7 +542,8 @@ void process_action(keyrecord_t *record, action_t action) {
 #    if defined(RETRO_TAPPING) && defined(DUMMY_MOD_NEUTRALIZER_KEYCODE)
                             // Send a dummy keycode to neutralize flashing modifiers
                             // if the key was held and then released with no interruptions.
-                            if (retro_tapping_counter == 2) {
+                            uint16_t ev_kc = get_event_keycode(event, false);
+                            if (retro_tap_primed && retro_tap_curr_key == ev_kc) {
                                 neutralize_flashing_modifiers(get_mods());
                             }
 #    endif
@@ -829,30 +841,44 @@ void process_action(keyrecord_t *record, action_t action) {
 
 #ifndef NO_ACTION_TAPPING
 #    if defined(RETRO_TAPPING) || defined(RETRO_TAPPING_PER_KEY) || (defined(AUTO_SHIFT_ENABLE) && defined(RETRO_SHIFT))
-    if (!is_tap_action(action)) {
-        retro_tapping_counter = 0;
-    } else {
+    if (is_tap_action(action)) {
         if (event.pressed) {
             if (tap_count > 0) {
-                retro_tapping_counter = 0;
+                retro_tap_primed = false;
+            } else {
+#        if !(defined(AUTO_SHIFT_ENABLE) && defined(RETRO_SHIFT))
+                retro_tap_curr_mods = retro_tap_next_mods;
+                retro_tap_next_mods = get_mods();
+#        endif
             }
         } else {
+            uint16_t event_keycode = get_event_keycode(event, false);
+#        if !(defined(AUTO_SHIFT_ENABLE) && defined(RETRO_SHIFT))
+            uint8_t curr_mods = get_mods();
+#        endif
             if (tap_count > 0) {
-                retro_tapping_counter = 0;
-            } else {
+                retro_tap_primed = false;
+            } else if (retro_tap_curr_key == event_keycode) {
                 if (
 #        ifdef RETRO_TAPPING_PER_KEY
-                    get_retro_tapping(get_event_keycode(record->event, false), record) &&
+                    get_retro_tapping(event_keycode, record) &&
 #        endif
-                    retro_tapping_counter == 2) {
+                    retro_tap_primed) {
 #        if defined(AUTO_SHIFT_ENABLE) && defined(RETRO_SHIFT)
                     process_auto_shift(action.layer_tap.code, record);
 #        else
+                    register_mods(retro_tap_curr_mods);
+                    wait_ms(TAP_CODE_DELAY);
                     tap_code(action.layer_tap.code);
+                    wait_ms(TAP_CODE_DELAY);
+                    unregister_mods(retro_tap_curr_mods);
 #        endif
                 }
-                retro_tapping_counter = 0;
+                retro_tap_primed = false;
             }
+#        if !(defined(AUTO_SHIFT_ENABLE) && defined(RETRO_SHIFT))
+            retro_tap_next_mods = curr_mods;
+#        endif
         }
     }
 #    endif
diff --git a/tests/tap_hold_configurations/retro_tapping/test_key_roll.cpp b/tests/tap_hold_configurations/retro_tapping/test_key_roll.cpp
new file mode 100644
index 0000000000..afcbde9937
--- /dev/null
+++ b/tests/tap_hold_configurations/retro_tapping/test_key_roll.cpp
@@ -0,0 +1,408 @@
+/* Copyright 2024 John Rigoni
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include "keyboard_report_util.hpp"
+#include "keycode.h"
+#include "keycodes.h"
+#include "test_common.hpp"
+#include "action_tapping.h"
+#include "test_keymap_key.hpp"
+
+using testing::_;
+using testing::InSequence;
+
+class RetroTapKeyRoll : public TestFixture {};
+
+TEST_F(RetroTapKeyRoll, regular_to_left_gui_mod_over_tap_term) {
+    TestDriver driver;
+    InSequence s;
+    auto       mod_tap_hold_key = KeymapKey(0, 1, 0, LGUI_T(KC_P));
+    auto       regular_key      = KeymapKey(0, 2, 0, KC_B);
+
+    set_keymap({mod_tap_hold_key, regular_key});
+
+    EXPECT_REPORT(driver, (KC_B));
+    regular_key.press();
+    run_one_scan_loop();
+    VERIFY_AND_CLEAR(driver);
+
+    EXPECT_NO_REPORT(driver);
+    mod_tap_hold_key.press();
+    idle_for(TAPPING_TERM);
+    VERIFY_AND_CLEAR(driver);
+
+    EXPECT_REPORT(driver, (KC_B, KC_LEFT_GUI));
+    run_one_scan_loop();
+    VERIFY_AND_CLEAR(driver);
+
+    EXPECT_REPORT(driver, (KC_LEFT_GUI));
+    regular_key.release();
+    run_one_scan_loop();
+    VERIFY_AND_CLEAR(driver);
+
+    EXPECT_REPORT(driver, (KC_LEFT_GUI, DUMMY_MOD_NEUTRALIZER_KEYCODE));
+    EXPECT_REPORT(driver, (KC_LEFT_GUI));
+    EXPECT_EMPTY_REPORT(driver);
+    EXPECT_REPORT(driver, (KC_P));
+    EXPECT_EMPTY_REPORT(driver);
+    mod_tap_hold_key.release();
+    run_one_scan_loop();
+    VERIFY_AND_CLEAR(driver);
+}
+
+TEST_F(RetroTapKeyRoll, regular_to_mod_over_tap_term) {
+    TestDriver driver;
+    InSequence s;
+    auto       mod_tap_hold_key = KeymapKey(0, 1, 0, SFT_T(KC_A));
+    auto       regular_key      = KeymapKey(0, 2, 0, KC_B);
+
+    set_keymap({mod_tap_hold_key, regular_key});
+
+    EXPECT_REPORT(driver, (KC_B));
+    regular_key.press();
+    run_one_scan_loop();
+    VERIFY_AND_CLEAR(driver);
+
+    EXPECT_REPORT(driver, (KC_B, KC_LEFT_SHIFT));
+    mod_tap_hold_key.press();
+    idle_for(TAPPING_TERM);
+    run_one_scan_loop();
+    VERIFY_AND_CLEAR(driver);
+
+    EXPECT_REPORT(driver, (KC_LEFT_SHIFT));
+    regular_key.release();
+    run_one_scan_loop();
+    VERIFY_AND_CLEAR(driver);
+
+    EXPECT_EMPTY_REPORT(driver);
+    EXPECT_REPORT(driver, (KC_A));
+    EXPECT_EMPTY_REPORT(driver);
+    mod_tap_hold_key.release();
+    run_one_scan_loop();
+    VERIFY_AND_CLEAR(driver);
+}
+
+TEST_F(RetroTapKeyRoll, regular_to_mod_under_tap_term) {
+    TestDriver driver;
+    InSequence s;
+    auto       mod_tap_hold_key = KeymapKey(0, 1, 0, SFT_T(KC_A));
+    auto       regular_key      = KeymapKey(0, 2, 0, KC_B);
+
+    set_keymap({mod_tap_hold_key, regular_key});
+
+    EXPECT_REPORT(driver, (KC_B));
+    regular_key.press();
+    run_one_scan_loop();
+    VERIFY_AND_CLEAR(driver);
+
+    EXPECT_NO_REPORT(driver);
+    mod_tap_hold_key.press();
+    run_one_scan_loop();
+    VERIFY_AND_CLEAR(driver);
+
+    EXPECT_EMPTY_REPORT(driver);
+    regular_key.release();
+    run_one_scan_loop();
+    VERIFY_AND_CLEAR(driver);
+
+    EXPECT_REPORT(driver, (KC_A));
+    EXPECT_EMPTY_REPORT(driver);
+    mod_tap_hold_key.release();
+    run_one_scan_loop();
+    VERIFY_AND_CLEAR(driver);
+}
+
+TEST_F(RetroTapKeyRoll, mod_under_tap_term_to_regular) {
+    TestDriver driver;
+    InSequence s;
+    auto       mod_tap_hold_key = KeymapKey(0, 1, 0, LGUI_T(KC_P));
+    auto       regular_key      = KeymapKey(0, 2, 0, KC_B);
+
+    set_keymap({mod_tap_hold_key, regular_key});
+
+    EXPECT_NO_REPORT(driver);
+    mod_tap_hold_key.press();
+    run_one_scan_loop();
+    VERIFY_AND_CLEAR(driver);
+
+    EXPECT_NO_REPORT(driver);
+    regular_key.press();
+    run_one_scan_loop();
+    VERIFY_AND_CLEAR(driver);
+
+    EXPECT_REPORT(driver, (KC_P));
+    EXPECT_REPORT(driver, (KC_B, KC_P));
+    EXPECT_REPORT(driver, (KC_B));
+    mod_tap_hold_key.release();
+    run_one_scan_loop();
+    VERIFY_AND_CLEAR(driver);
+
+    EXPECT_EMPTY_REPORT(driver);
+    regular_key.release();
+    run_one_scan_loop();
+    VERIFY_AND_CLEAR(driver);
+}
+
+TEST_F(RetroTapKeyRoll, mod_over_tap_term_to_regular) {
+    TestDriver driver;
+    InSequence s;
+    auto       mod_tap_hold_key = KeymapKey(0, 1, 0, SFT_T(KC_A));
+    auto       regular_key      = KeymapKey(0, 2, 0, KC_B);
+
+    set_keymap({mod_tap_hold_key, regular_key});
+
+    EXPECT_REPORT(driver, (KC_LEFT_SHIFT));
+    mod_tap_hold_key.press();
+    idle_for(TAPPING_TERM);
+    run_one_scan_loop();
+    VERIFY_AND_CLEAR(driver);
+
+    EXPECT_REPORT(driver, (KC_LEFT_SHIFT, KC_B));
+    regular_key.press();
+    run_one_scan_loop();
+    VERIFY_AND_CLEAR(driver);
+
+    EXPECT_REPORT(driver, (KC_B));
+    mod_tap_hold_key.release();
+    run_one_scan_loop();
+    VERIFY_AND_CLEAR(driver);
+
+    EXPECT_EMPTY_REPORT(driver);
+    regular_key.release();
+    run_one_scan_loop();
+    VERIFY_AND_CLEAR(driver);
+}
+
+TEST_F(RetroTapKeyRoll, mod_under_tap_term_to_mod_under_tap_term) {
+    TestDriver driver;
+    InSequence s;
+    auto       mod_tap_hold_gui   = KeymapKey(0, 1, 0, LGUI_T(KC_P));
+    auto       mod_tap_hold_lshft = KeymapKey(0, 2, 0, SFT_T(KC_A));
+
+    set_keymap({mod_tap_hold_gui, mod_tap_hold_lshft});
+
+    EXPECT_NO_REPORT(driver);
+    mod_tap_hold_lshft.press();
+    run_one_scan_loop();
+    VERIFY_AND_CLEAR(driver);
+
+    EXPECT_NO_REPORT(driver);
+    mod_tap_hold_gui.press();
+    run_one_scan_loop();
+    VERIFY_AND_CLEAR(driver);
+
+    EXPECT_REPORT(driver, (KC_A));
+    EXPECT_EMPTY_REPORT(driver);
+    mod_tap_hold_lshft.release();
+    run_one_scan_loop();
+    VERIFY_AND_CLEAR(driver);
+
+    EXPECT_REPORT(driver, (KC_P));
+    EXPECT_EMPTY_REPORT(driver);
+    mod_tap_hold_gui.release();
+    run_one_scan_loop();
+    VERIFY_AND_CLEAR(driver);
+}
+
+TEST_F(RetroTapKeyRoll, mod_over_tap_term_to_mod_under_tap_term) {
+    TestDriver driver;
+    InSequence s;
+    auto       mod_tap_hold_gui   = KeymapKey(0, 1, 0, LGUI_T(KC_P));
+    auto       mod_tap_hold_lshft = KeymapKey(0, 2, 0, SFT_T(KC_A));
+
+    set_keymap({mod_tap_hold_gui, mod_tap_hold_lshft});
+
+    EXPECT_REPORT(driver, (KC_LEFT_SHIFT));
+    mod_tap_hold_lshft.press();
+    idle_for(TAPPING_TERM);
+    run_one_scan_loop();
+    VERIFY_AND_CLEAR(driver);
+
+    EXPECT_NO_REPORT(driver);
+    mod_tap_hold_gui.press();
+    run_one_scan_loop();
+    VERIFY_AND_CLEAR(driver);
+
+    EXPECT_NO_REPORT(driver);
+    mod_tap_hold_lshft.release();
+    run_one_scan_loop();
+    VERIFY_AND_CLEAR(driver);
+
+    EXPECT_REPORT(driver, (KC_LEFT_SHIFT, KC_P));
+    EXPECT_REPORT(driver, (KC_P));
+    EXPECT_EMPTY_REPORT(driver);
+    mod_tap_hold_gui.release();
+    run_one_scan_loop();
+    VERIFY_AND_CLEAR(driver);
+}
+
+TEST_F(RetroTapKeyRoll, mod_under_tap_term_to_mod_over_tap_term) {
+    TestDriver driver;
+    InSequence s;
+    auto       mod_tap_hold_gui   = KeymapKey(0, 1, 0, LGUI_T(KC_P));
+    auto       mod_tap_hold_lshft = KeymapKey(0, 2, 0, SFT_T(KC_A));
+
+    set_keymap({mod_tap_hold_gui, mod_tap_hold_lshft});
+
+    EXPECT_NO_REPORT(driver);
+    mod_tap_hold_lshft.press();
+    run_one_scan_loop();
+    VERIFY_AND_CLEAR(driver);
+
+    EXPECT_REPORT(driver, (KC_LEFT_SHIFT));
+    EXPECT_REPORT(driver, (KC_LEFT_SHIFT, KC_LEFT_GUI));
+    mod_tap_hold_gui.press();
+    idle_for(TAPPING_TERM);
+    run_one_scan_loop();
+    VERIFY_AND_CLEAR(driver);
+
+    EXPECT_REPORT(driver, (KC_LEFT_GUI));
+    mod_tap_hold_lshft.release();
+    run_one_scan_loop();
+    VERIFY_AND_CLEAR(driver);
+
+    EXPECT_REPORT(driver, (KC_LEFT_GUI, DUMMY_MOD_NEUTRALIZER_KEYCODE));
+    EXPECT_REPORT(driver, (KC_LEFT_GUI));
+    EXPECT_EMPTY_REPORT(driver);
+    EXPECT_REPORT(driver, (KC_LEFT_SHIFT));
+    EXPECT_REPORT(driver, (KC_P, KC_LEFT_SHIFT));
+    EXPECT_REPORT(driver, (KC_LEFT_SHIFT));
+    EXPECT_EMPTY_REPORT(driver);
+    mod_tap_hold_gui.release();
+    run_one_scan_loop();
+    VERIFY_AND_CLEAR(driver);
+}
+
+TEST_F(RetroTapKeyRoll, mod_under_tap_term_to_mod_over_tap_term_offset) {
+    TestDriver driver;
+    InSequence s;
+    auto       mod_tap_hold_gui   = KeymapKey(0, 1, 0, LGUI_T(KC_P));
+    auto       mod_tap_hold_lshft = KeymapKey(0, 2, 0, SFT_T(KC_A));
+
+    set_keymap({mod_tap_hold_gui, mod_tap_hold_lshft});
+
+    EXPECT_NO_REPORT(driver);
+    mod_tap_hold_lshft.press();
+    run_one_scan_loop();
+    VERIFY_AND_CLEAR(driver);
+
+    EXPECT_NO_REPORT(driver);
+    mod_tap_hold_gui.press();
+    run_one_scan_loop();
+    VERIFY_AND_CLEAR(driver);
+
+    EXPECT_REPORT(driver, (KC_A));
+    EXPECT_EMPTY_REPORT(driver);
+    mod_tap_hold_lshft.release();
+    run_one_scan_loop();
+    VERIFY_AND_CLEAR(driver);
+
+    EXPECT_REPORT(driver, (KC_LEFT_GUI));
+    EXPECT_REPORT(driver, (KC_LEFT_GUI, DUMMY_MOD_NEUTRALIZER_KEYCODE));
+    EXPECT_REPORT(driver, (KC_LEFT_GUI));
+    EXPECT_EMPTY_REPORT(driver);
+    EXPECT_REPORT(driver, (KC_P));
+    EXPECT_EMPTY_REPORT(driver);
+    idle_for(TAPPING_TERM);
+    mod_tap_hold_gui.release();
+    run_one_scan_loop();
+    VERIFY_AND_CLEAR(driver);
+}
+
+TEST_F(RetroTapKeyRoll, mod_over_tap_term_to_mod_over_tap_term) {
+    TestDriver driver;
+    InSequence s;
+    auto       mod_tap_hold_gui   = KeymapKey(0, 1, 0, LGUI_T(KC_P));
+    auto       mod_tap_hold_lshft = KeymapKey(0, 2, 0, SFT_T(KC_A));
+
+    set_keymap({mod_tap_hold_gui, mod_tap_hold_lshft});
+
+    EXPECT_REPORT(driver, (KC_LEFT_SHIFT));
+    mod_tap_hold_lshft.press();
+    idle_for(TAPPING_TERM);
+    run_one_scan_loop();
+    VERIFY_AND_CLEAR(driver);
+
+    EXPECT_REPORT(driver, (KC_LEFT_SHIFT, KC_LEFT_GUI));
+    mod_tap_hold_gui.press();
+    idle_for(TAPPING_TERM);
+    run_one_scan_loop();
+    VERIFY_AND_CLEAR(driver);
+
+    EXPECT_REPORT(driver, (KC_LEFT_GUI));
+    mod_tap_hold_lshft.release();
+    run_one_scan_loop();
+    VERIFY_AND_CLEAR(driver);
+
+    EXPECT_REPORT(driver, (KC_LEFT_GUI, DUMMY_MOD_NEUTRALIZER_KEYCODE));
+    EXPECT_REPORT(driver, (KC_LEFT_GUI));
+    EXPECT_EMPTY_REPORT(driver);
+    EXPECT_REPORT(driver, (KC_LEFT_SHIFT));
+    EXPECT_REPORT(driver, (KC_P, KC_LEFT_SHIFT));
+    EXPECT_REPORT(driver, (KC_LEFT_SHIFT));
+    EXPECT_EMPTY_REPORT(driver);
+    mod_tap_hold_gui.release();
+    run_one_scan_loop();
+    VERIFY_AND_CLEAR(driver);
+}
+
+TEST_F(RetroTapKeyRoll, mod_to_mod_to_mod) {
+    TestDriver driver;
+    InSequence s;
+    auto       mod_tap_hold_lalt  = KeymapKey(0, 1, 0, LALT_T(KC_R));
+    auto       mod_tap_hold_lshft = KeymapKey(0, 2, 0, SFT_T(KC_A));
+    auto       mod_tap_hold_lctrl = KeymapKey(0, 3, 0, LCTL_T(KC_C));
+
+    set_keymap({mod_tap_hold_lalt, mod_tap_hold_lshft, mod_tap_hold_lctrl});
+
+    EXPECT_REPORT(driver, (KC_LEFT_ALT));
+    mod_tap_hold_lalt.press();
+    idle_for(TAPPING_TERM);
+    run_one_scan_loop();
+    VERIFY_AND_CLEAR(driver);
+
+    EXPECT_REPORT(driver, (KC_LEFT_SHIFT, KC_LEFT_ALT));
+    mod_tap_hold_lshft.press();
+    idle_for(TAPPING_TERM);
+    run_one_scan_loop();
+    VERIFY_AND_CLEAR(driver);
+
+    EXPECT_REPORT(driver, (KC_LEFT_SHIFT));
+    mod_tap_hold_lalt.release();
+    run_one_scan_loop();
+    VERIFY_AND_CLEAR(driver);
+
+    EXPECT_REPORT(driver, (KC_LEFT_CTRL, KC_LEFT_SHIFT));
+    EXPECT_NO_REPORT(driver);
+    mod_tap_hold_lctrl.press();
+    idle_for(TAPPING_TERM);
+    run_one_scan_loop();
+    VERIFY_AND_CLEAR(driver);
+
+    EXPECT_REPORT(driver, (KC_LEFT_CTRL));
+    mod_tap_hold_lshft.release();
+    run_one_scan_loop();
+    VERIFY_AND_CLEAR(driver);
+
+    EXPECT_EMPTY_REPORT(driver);
+    EXPECT_REPORT(driver, (KC_LEFT_SHIFT));
+    EXPECT_REPORT(driver, (KC_C, KC_LEFT_SHIFT));
+    EXPECT_REPORT(driver, (KC_LEFT_SHIFT));
+    EXPECT_EMPTY_REPORT(driver);
+    mod_tap_hold_lctrl.release();
+    run_one_scan_loop();
+    VERIFY_AND_CLEAR(driver);
+}