From 347135bad9b650470ac554aea21d7640679b6aed Mon Sep 17 00:00:00 2001 From: precondition <57645186+precondition@users.noreply.github.com> Date: Wed, 10 May 2023 00:33:22 +0200 Subject: [PATCH 1/5] Tap a dummy keycode to prevent modifier flashes in RETRO_TAPPING and KEY_OVERRIDE --- quantum/action.c | 7 ++++++ quantum/action_util.c | 25 +++++++++++++++++++ quantum/action_util.h | 11 ++++++++ .../process_keycode/process_key_override.c | 9 +++++++ 4 files changed, 52 insertions(+) diff --git a/quantum/action.c b/quantum/action.c index 9a6bbcca1162..5a077a9b67ea 100644 --- a/quantum/action.c +++ b/quantum/action.c @@ -527,6 +527,13 @@ void process_action(keyrecord_t *record, action_t action) { unregister_code(action.key.code); } else { ac_dprintf("MODS_TAP: No tap: add_mods\n"); +# 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) { + neutralize_flashing_modifiers(get_mods()); + } +# endif unregister_mods(mods); } } diff --git a/quantum/action_util.c b/quantum/action_util.c index 361f410d2ddf..55b41d8ba92a 100644 --- a/quantum/action_util.c +++ b/quantum/action_util.c @@ -500,3 +500,28 @@ __attribute__((weak)) void oneshot_layer_changed_kb(uint8_t layer) { uint8_t has_anymod(void) { return bitpop(real_mods); } + +#ifdef DUMMY_MOD_NEUTRALIZER_KEYCODE +/** \brief Send a dummy keycode in between the register and unregister event of a modifier key, to neutralize the "flashing modifiers" phenomen. + * + * \param active_mods 8-bit packed bit-array describing the currenctly active modifiers (in the format GASCGASC). + * + * Certain QMK features like key overrides or retro tap must unregister a previously + * registered modifier before sending another keycode but this can trigger undesired + * keyboard shortcuts if the clean tap of a single modifier key is bound to an action + * on the host OS, as is for example the case for the left GUI key on Windows, which + * opens the Start Menu when tapped. + */ +void neutralize_flashing_modifiers(uint8_t active_mods) { + // In most scenarios, the flashing modifiers phenomenon is a problem + // only for a subset of modifier masks. + const static uint8_t mods_to_neutralize[] = MODS_TO_NEUTRALIZE; + const static uint8_t n_mods = ARRAY_SIZE(mods_to_neutralize); + for (uint8_t i = 0; i < n_mods; ++i) { + if (active_mods == mods_to_neutralize[i]) { + tap_code(DUMMY_MOD_NEUTRALIZER_KEYCODE); + break; + } + } +} +#endif diff --git a/quantum/action_util.h b/quantum/action_util.h index 02f6e9e6df2e..8a82c4b57a73 100644 --- a/quantum/action_util.h +++ b/quantum/action_util.h @@ -102,6 +102,17 @@ void use_oneshot_swaphands(void); void clear_oneshot_swaphands(void); #endif +#ifdef DUMMY_MOD_NEUTRALIZER_KEYCODE +# if !(QK_BASIC <= DUMMY_MOD_NEUTRALIZER_KEYCODE && DUMMY_MOD_NEUTRALIZER_KEYCODE <= QK_BASIC_MAX) +# error "DUMMY_MOD_NEUTRALIZER_KEYCODE must be a basic, unmodified, HID keycode!" +# endif +void neutralize_flashing_modifiers(uint8_t active_mods); +#endif +#ifndef MODS_TO_NEUTRALIZE +# define MODS_TO_NEUTRALIZE \ + { MOD_BIT(KC_LEFT_ALT), MOD_BIT(KC_LEFT_GUI) } +#endif + #ifdef __cplusplus } #endif diff --git a/quantum/process_keycode/process_key_override.c b/quantum/process_keycode/process_key_override.c index 17e490e67ae3..de628d3fec75 100644 --- a/quantum/process_keycode/process_key_override.c +++ b/quantum/process_keycode/process_key_override.c @@ -322,6 +322,15 @@ static bool try_activating_override(const uint16_t keycode, const uint8_t layer, clear_active_override(false); +#ifdef DUMMY_MOD_NEUTRALIZER_KEYCODE + // Send a dummy keycode before unregistering the modifier(s) + // so that suppressing the modifier(s) doesn't falsely get interpreted + // by the host OS as a tap of a modifier key. + // For example, unintended activations of the start menu on Windows when + // using a GUI+ key override with suppressed mods. + neutralize_flashing_modifiers(active_mods); +#endif + active_override = override; active_override_trigger_is_down = true; From 92d3047924a342d4ebc8f1f9eaf7abe8dee39636 Mon Sep 17 00:00:00 2001 From: precondition <57645186+precondition@users.noreply.github.com> Date: Fri, 19 May 2023 21:58:02 +0200 Subject: [PATCH 2/5] Document DUMMY_MOD_NEUTRALIZER_KEYCODE in retro tap and key override docs --- docs/feature_key_overrides.md | 27 +++++++++++++++++++++++++++ docs/tap_hold.md | 25 +++++++++++++++++++++++++ 2 files changed, 52 insertions(+) diff --git a/docs/feature_key_overrides.md b/docs/feature_key_overrides.md index 608eb001e4b8..ec7efd4c017e 100644 --- a/docs/feature_key_overrides.md +++ b/docs/feature_key_overrides.md @@ -225,3 +225,30 @@ The duration of the key repeat delay is controlled with the `KEY_OVERRIDE_REPEAT ## Difference to Combos :id=difference-to-combos Note that key overrides are very different from [combos](https://docs.qmk.fm/#/feature_combo). Combos require that you press down several keys almost _at the same time_ and can work with any combination of non-modifier keys. Key overrides work like keyboard shortcuts (e.g. `ctrl` + `z`): They take combinations of _multiple_ modifiers and _one_ non-modifier key to then perform some custom action. Key overrides are implemented with much care to behave just like normal keyboard shortcuts would in regards to the order of pressed keys, timing, and interacton with other pressed keys. There are a number of optional settings that can be used to really fine-tune the behavior of each key override as well. Using key overrides also does not delay key input for regular key presses, which inherently happens in combos and may be undesirable. + +## Solution to the problem of flashing modifiers :id=neutralize-flashing-modifiers + +If the programs you use bind an action to taps of modifier keys (e.g. tapping left GUI to bring up the applications menu or tapping left Alt to focus the menu bar), you may find that using key overrides with suppressed mods falsely triggers those actions. To counteract this, you can define a `DUMMY_MOD_NEUTRALIZER_KEYCODE` in `config.h` that will get sent in between the register and unregister events of a suppressed modifier. That way, the programs on your computer will no longer interpret the mod suppression induced by key overrides as a lone tap of a modifier key and will thus not falsely trigger the undesired action. + +Naturally, for this technique to be effective, you must choose a `DUMMY_MOD_NEUTRALIZER_KEYCODE` for which no keyboard shortcuts are bound to. Recommended values are: `KC_RIGHT_CTRL` or `KC_F18`. +Please note that `DUMMY_MOD_NEUTRALIZER_KEYCODE` must be a basic, unmodified, HID keycode so values like `KC_NO`, `KC_TRANSPARENT` or `KC_PIPE` aka `S(KC_BACKSLASH)` are not permitted. + +By default, only left Alt and left GUI are neutralized. If you want to change the list of applicable modifier masks, use the following in your `config.h`: + +```c +#define MODS_TO_NEUTRALIZE { , , ... } +``` + +Examples: + +```c +#define DUMMY_MOD_NEUTRALIZER_KEYCODE KC_RIGHT_CTRL + +// Neutralize left alt and left GUI (Default value) +#define MODS_TO_NEUTRALIZE { MOD_BIT(KC_LEFT_ALT), MOD_BIT(KC_LEFT_GUI) } + +// Neutralize left alt, left GUI, right GUI and left Control+Shift +#define MODS_TO_NEUTRALIZE { MOD_BIT(KC_LEFT_ALT), MOD_BIT(KC_LEFT_GUI), MOD_BIT(KC_RIGHT_GUI), MOD_BIT(KC_LEFT_CTRL)|MOD_BIT(KC_LEFT_SHIFT) } +``` + +!> Do not use `MOD_xxx` constants like `MOD_LSFT` or `MOD_RALT`, since they're 5-bit packed bit-arrays while `MODS_TO_NEUTRALIZE` expects a list of 8-bit packed bit-arrays. Use `MOD_BIT()` or `MOD_MASK_xxx` instead. diff --git a/docs/tap_hold.md b/docs/tap_hold.md index cdc1cfeca79e..e634a955ff8c 100644 --- a/docs/tap_hold.md +++ b/docs/tap_hold.md @@ -462,6 +462,31 @@ bool get_retro_tapping(uint16_t keycode, keyrecord_t *record) { } ``` +If the programs you use bind an action to taps of modifier keys (e.g. tapping left GUI to bring up the applications menu or tapping left Alt to focus the menu bar), you may find that using retro-tapping falsely triggers those actions. To counteract this, you can define a `DUMMY_MOD_NEUTRALIZER_KEYCODE` in `config.h` that will get sent in between the register and unregister events of a held mod-tap key. That way, the programs on your computer will no longer interpret the mod suppression induced by retro-tapping as a lone tap of a modifier key and will thus not falsely trigger the undesired action. + +Naturally, for this technique to be effective, you must choose a `DUMMY_MOD_NEUTRALIZER_KEYCODE` for which no keyboard shortcuts are bound to. Recommended values are: `KC_RIGHT_CTRL` or `KC_F18`. +Please note that `DUMMY_MOD_NEUTRALIZER_KEYCODE` must be a basic, unmodified, HID keycode so values like `KC_NO`, `KC_TRANSPARENT` or `KC_PIPE` aka `S(KC_BACKSLASH)` are not permitted. + +By default, only left Alt and left GUI are neutralized. If you want to change the list of applicable modifier masks, use the following in your `config.h`: + +```c +#define MODS_TO_NEUTRALIZE { , , ... } +``` + +Examples: + +```c +#define DUMMY_MOD_NEUTRALIZER_KEYCODE KC_RIGHT_CTRL + +// Neutralize left alt and left GUI (Default value) +#define MODS_TO_NEUTRALIZE { MOD_BIT(KC_LEFT_ALT), MOD_BIT(KC_LEFT_GUI) } + +// Neutralize left alt, left GUI, right GUI and left Control+Shift +#define MODS_TO_NEUTRALIZE { MOD_BIT(KC_LEFT_ALT), MOD_BIT(KC_LEFT_GUI), MOD_BIT(KC_RIGHT_GUI), MOD_BIT(KC_LEFT_CTRL)|MOD_BIT(KC_LEFT_SHIFT) } +``` + +!> Do not use `MOD_xxx` constants like `MOD_LSFT` or `MOD_RALT`, since they're 5-bit packed bit-arrays while `MODS_TO_NEUTRALIZE` expects a list of 8-bit packed bit-arrays. Use `MOD_BIT()` or `MOD_MASK_xxx` instead. + ### Retro Shift [Auto Shift,](feature_auto_shift.md) has its own version of `retro tapping` called `retro shift`. It is extremely similar to `retro tapping`, but holding the key past `AUTO_SHIFT_TIMEOUT` results in the value it sends being shifted. Other configurations also affect it differently; see [here](feature_auto_shift.md#retro-shift) for more information. From b048bc7cda99a1ed7e6370bd278718e1478703ba Mon Sep 17 00:00:00 2001 From: precondition <57645186+precondition@users.noreply.github.com> Date: Fri, 19 May 2023 23:13:13 +0200 Subject: [PATCH 3/5] Add unit tests for retro tap flashing mods neutralization --- .../retro_tapping/config.h | 5 +- .../retro_tapping/test_neutralization.cpp | 201 ++++++++++++++++++ 2 files changed, 205 insertions(+), 1 deletion(-) create mode 100644 tests/tap_hold_configurations/retro_tapping/test_neutralization.cpp diff --git a/tests/tap_hold_configurations/retro_tapping/config.h b/tests/tap_hold_configurations/retro_tapping/config.h index 4b38f2644b83..cc9f1624770e 100644 --- a/tests/tap_hold_configurations/retro_tapping/config.h +++ b/tests/tap_hold_configurations/retro_tapping/config.h @@ -18,4 +18,7 @@ #include "test_common.h" -#define RETRO_TAPPING \ No newline at end of file +#define RETRO_TAPPING +#define DUMMY_MOD_NEUTRALIZER_KEYCODE KC_RIGHT_CTRL +#define MODS_TO_NEUTRALIZE \ + { MOD_BIT(KC_LEFT_GUI) } diff --git a/tests/tap_hold_configurations/retro_tapping/test_neutralization.cpp b/tests/tap_hold_configurations/retro_tapping/test_neutralization.cpp new file mode 100644 index 000000000000..10d675a3b188 --- /dev/null +++ b/tests/tap_hold_configurations/retro_tapping/test_neutralization.cpp @@ -0,0 +1,201 @@ +/* Copyright 2023 Vladislav Kucheriavykh + * + * 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 . + */ + +#include "keyboard_report_util.hpp" +#include "keycode.h" +#include "test_common.hpp" +#include "action_tapping.h" +#include "test_keymap_key.hpp" + +using testing::_; +using testing::InSequence; + +class RetroTapNeutralization : public TestFixture {}; + +TEST_F(RetroTapNeutralization, neutralize_retro_tapped_left_gui_mod_tap) { + TestDriver driver; + InSequence s; + auto mod_tap_hold_key = KeymapKey(0, 7, 0, LGUI_T(KC_P)); + + set_keymap({mod_tap_hold_key}); + + EXPECT_NO_REPORT(driver); + mod_tap_hold_key.press(); + idle_for(TAPPING_TERM); + VERIFY_AND_CLEAR(driver); + + EXPECT_REPORT(driver, (KC_LGUI)); + run_one_scan_loop(); + VERIFY_AND_CLEAR(driver); + + EXPECT_REPORT(driver, (DUMMY_MOD_NEUTRALIZER_KEYCODE, KC_LGUI)); + EXPECT_REPORT(driver, (KC_LGUI)); + 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(RetroTapNeutralization, do_not_neutralize_retro_tapped_left_shift_mod_tap) { + TestDriver driver; + InSequence s; + auto mod_tap_hold_key = KeymapKey(0, 7, 0, LSFT_T(KC_P)); + + set_keymap({mod_tap_hold_key}); + + EXPECT_NO_REPORT(driver); + mod_tap_hold_key.press(); + idle_for(TAPPING_TERM); + VERIFY_AND_CLEAR(driver); + + EXPECT_REPORT(driver, (KC_LEFT_SHIFT)); + run_one_scan_loop(); + VERIFY_AND_CLEAR(driver); + + 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(RetroTapNeutralization, do_not_neutralize_retro_tapped_right_gui_mod_tap) { + TestDriver driver; + InSequence s; + auto mod_tap_hold_key = KeymapKey(0, 7, 0, RGUI_T(KC_P)); + + set_keymap({mod_tap_hold_key}); + + EXPECT_NO_REPORT(driver); + mod_tap_hold_key.press(); + idle_for(TAPPING_TERM); + VERIFY_AND_CLEAR(driver); + + EXPECT_REPORT(driver, (KC_RGUI)); + run_one_scan_loop(); + VERIFY_AND_CLEAR(driver); + + 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(RetroTapNeutralization, do_not_neutralize_retro_tapped_left_gui_shift_mod_tap) { + TestDriver driver; + InSequence s; + auto mod_tap_hold_key = KeymapKey(0, 7, 0, MT(MOD_LGUI | MOD_LSFT, KC_P)); + + set_keymap({mod_tap_hold_key}); + + EXPECT_NO_REPORT(driver); + mod_tap_hold_key.press(); + idle_for(TAPPING_TERM); + VERIFY_AND_CLEAR(driver); + + EXPECT_REPORT(driver, (KC_LSFT, KC_LGUI)); + run_one_scan_loop(); + VERIFY_AND_CLEAR(driver); + + 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(RetroTapNeutralization, do_not_neutralize_roll_of_regular_and_mod_tap_keys) { + 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_A); + + set_keymap({mod_tap_hold_key, regular_key}); + + /* Press mod-tap-hold key. */ + EXPECT_NO_REPORT(driver); + mod_tap_hold_key.press(); + run_one_scan_loop(); + VERIFY_AND_CLEAR(driver); + + /* Press regular key. */ + EXPECT_NO_REPORT(driver); + regular_key.press(); + run_one_scan_loop(); + VERIFY_AND_CLEAR(driver); + + /* Release regular key. */ + EXPECT_NO_REPORT(driver); + regular_key.release(); + run_one_scan_loop(); + VERIFY_AND_CLEAR(driver); + + /* Release mod-tap-hold key. */ + EXPECT_REPORT(driver, (KC_P)); + EXPECT_REPORT(driver, (KC_P, KC_A)); + EXPECT_REPORT(driver, (KC_P)); + EXPECT_EMPTY_REPORT(driver); + mod_tap_hold_key.release(); + run_one_scan_loop(); + VERIFY_AND_CLEAR(driver); + + /* Idle for tapping term of mod tap hold key. */ + idle_for(TAPPING_TERM - 3); + VERIFY_AND_CLEAR(driver); +} + +TEST_F(RetroTapNeutralization, do_not_neutralize_tap_regular_key_while_mod_tap_is_held) { + 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_A); + + set_keymap({mod_tap_hold_key, regular_key}); + + /* Press and hold mod-tap key. */ + EXPECT_REPORT(driver, (KC_LEFT_GUI)); + mod_tap_hold_key.press(); + idle_for(TAPPING_TERM + 1); + VERIFY_AND_CLEAR(driver); + + /* Press regular key. */ + EXPECT_REPORT(driver, (KC_A, KC_LEFT_GUI)); + regular_key.press(); + run_one_scan_loop(); + VERIFY_AND_CLEAR(driver); + + /* Release regular key. */ + EXPECT_REPORT(driver, (KC_LEFT_GUI)); + regular_key.release(); + run_one_scan_loop(); + VERIFY_AND_CLEAR(driver); + + /* Release mod-tap-hold key. */ + EXPECT_EMPTY_REPORT(driver); + mod_tap_hold_key.release(); + run_one_scan_loop(); + VERIFY_AND_CLEAR(driver); + + /* Idle for tapping term of mod tap hold key. */ + idle_for(TAPPING_TERM - 3); + VERIFY_AND_CLEAR(driver); +} From 6e7779fc6afe743723d4a499024a609f67739d93 Mon Sep 17 00:00:00 2001 From: precondition <57645186+precondition@users.noreply.github.com> Date: Wed, 31 May 2023 08:41:57 +0200 Subject: [PATCH 4/5] Set lowerbound to KC_A --- quantum/action_util.h | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/quantum/action_util.h b/quantum/action_util.h index 8a82c4b57a73..831caf3c0a16 100644 --- a/quantum/action_util.h +++ b/quantum/action_util.h @@ -103,7 +103,9 @@ void clear_oneshot_swaphands(void); #endif #ifdef DUMMY_MOD_NEUTRALIZER_KEYCODE -# if !(QK_BASIC <= DUMMY_MOD_NEUTRALIZER_KEYCODE && DUMMY_MOD_NEUTRALIZER_KEYCODE <= QK_BASIC_MAX) +// KC_A is used as the lowerbound instead of QK_BASIC because the range QK_BASIC...KC_A includes +// internal keycodes like KC_NO and KC_TRANSPARENT which are unsuitable for use with `tap_code(kc)`. +# if !(KC_A <= DUMMY_MOD_NEUTRALIZER_KEYCODE && DUMMY_MOD_NEUTRALIZER_KEYCODE <= QK_BASIC_MAX) # error "DUMMY_MOD_NEUTRALIZER_KEYCODE must be a basic, unmodified, HID keycode!" # endif void neutralize_flashing_modifiers(uint8_t active_mods); From 82691b22df8b06fb276d9ec1e9fcf28f6f500dbc Mon Sep 17 00:00:00 2001 From: precondition <57645186+precondition@users.noreply.github.com> Date: Sun, 11 Jun 2023 14:07:07 +0200 Subject: [PATCH 5/5] Fix spelling in docstring --- quantum/action_util.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/quantum/action_util.c b/quantum/action_util.c index 55b41d8ba92a..909dea059594 100644 --- a/quantum/action_util.c +++ b/quantum/action_util.c @@ -502,9 +502,9 @@ uint8_t has_anymod(void) { } #ifdef DUMMY_MOD_NEUTRALIZER_KEYCODE -/** \brief Send a dummy keycode in between the register and unregister event of a modifier key, to neutralize the "flashing modifiers" phenomen. +/** \brief Send a dummy keycode in between the register and unregister event of a modifier key, to neutralize the "flashing modifiers" phenomenon. * - * \param active_mods 8-bit packed bit-array describing the currenctly active modifiers (in the format GASCGASC). + * \param active_mods 8-bit packed bit-array describing the currently active modifiers (in the format GASCGASC). * * Certain QMK features like key overrides or retro tap must unregister a previously * registered modifier before sending another keycode but this can trigger undesired