From f513cc810550e797b724772e90151a398952dbe8 Mon Sep 17 00:00:00 2001 From: Dean Scarff Date: Sun, 26 Jul 2020 01:23:38 +1000 Subject: [PATCH 01/11] Auto shift: support repeats and early registration (#7048) Instead of waiting for the next record to tap the autoshiftable key, expose a matrix_scan function that will check the timeout and register the key immediately if the timeout period has elapsed. This means the user gets immediate feedback about their press and doesn't have to guess whether it's been long enough. Additionally, don't unregister the keypress immediately. This means that auto-shifted keys can be held down. Removes the 'autoshift_on' function (which had external linkage but was not exposed in the header). Fixes qmk/qmk_firmware#7048 --- quantum/process_keycode/process_auto_shift.c | 166 ++++++++++++++----- quantum/process_keycode/process_auto_shift.h | 1 + 2 files changed, 129 insertions(+), 38 deletions(-) diff --git a/quantum/process_keycode/process_auto_shift.c b/quantum/process_keycode/process_auto_shift.c index b1267922ce77..c47e186afd7b 100644 --- a/quantum/process_keycode/process_auto_shift.c +++ b/quantum/process_keycode/process_auto_shift.c @@ -16,47 +16,152 @@ #ifdef AUTO_SHIFT_ENABLE +# include # include # include "process_auto_shift.h" -static bool autoshift_enabled = true; static uint16_t autoshift_time = 0; static uint16_t autoshift_timeout = AUTO_SHIFT_TIMEOUT; static uint16_t autoshift_lastkey = KC_NO; +static struct { + // Whether autoshift is enabled. + bool enabled : 1; + // Whether the auto-shifted keypress has been registered. + bool registered : 1; + // Whether autoshift is currently "holding" the shift key. + bool holding_shift : 1; +} autoshift_flags = {true, false, false}; + +/** \brief Resets the autoshift state and releases the shift key */ +static void autoshift_flush(void) { + if (autoshift_flags.holding_shift) { + // Release the shift key if it was simulated. + unregister_code(KC_LSFT); + } + autoshift_lastkey = KC_NO; + autoshift_flags.holding_shift = false; + autoshift_flags.registered = false; +} -void autoshift_flush(void) { - if (autoshift_lastkey != KC_NO) { - uint16_t elapsed = timer_elapsed(autoshift_time); +/** \brief Record the press of an autoshiftable key + * + * \return Whether the record should be further processed. + */ +static bool autoshift_press(uint16_t keycode, keyrecord_t *record) { + if (!autoshift_flags.enabled) { + return true; + } + + autoshift_flush(); + +# ifndef AUTO_SHIFT_MODIFIERS + if (get_mods()) { + return true; + } +# endif + + // Record the keycode so we can simulate it later. + autoshift_time = record->event.time; + autoshift_lastkey = keycode; - if (elapsed > autoshift_timeout) { - tap_code16(LSFT(autoshift_lastkey)); - } else { - tap_code(autoshift_lastkey); +# if !defined(NO_ACTION_ONESHOT) && !defined(NO_ACTION_TAPPING) + clear_oneshot_layer_state(ONESHOT_OTHER_KEY_PRESSED); +# endif + return false; +} + +/** \brief Registers an autoshiftable key under the right conditions + * + * If the autoshift delay has elapsed and no shift has already been registered, + * register a shift and the key. + */ +void autoshift_check_timeout(uint16_t now) { + const uint16_t elapsed = TIMER_DIFF_16(now, autoshift_time); + if (elapsed >= autoshift_timeout) { + // The timeout has expired, so simulate the keypress. + if (!(get_mods() & (MOD_BIT(KC_LSFT) | MOD_BIT(KC_RSFT)))) { + // Simulate pressing the shift key. + register_code(KC_LSFT); + autoshift_flags.holding_shift = true; } + register_code(autoshift_lastkey); + autoshift_flags.registered = true; - autoshift_time = 0; - autoshift_lastkey = KC_NO; +# if TAP_CODE_DELAY > 0 + wait_ms(TAP_CODE_DELAY); +# endif } } -void autoshift_on(uint16_t keycode) { - autoshift_time = timer_read(); - autoshift_lastkey = keycode; +/** \brief Registers an autoshiftable key under the right conditions + * + * If the autoshift delay has elapsed and no shift has already been registered, + * register a shift and the key. + * + * If the autoshift key is released before the delay has elapsed, register the + * key without a shift. + */ +static void autoshift_check_record(uint16_t keycode, keyrecord_t *record) { + if (autoshift_lastkey == KC_NO) { + return; + } + + // Process the release of the autoshiftable key. + if (keycode == autoshift_lastkey && !record->event.pressed) { + // Time since the initial press was recorded. + const uint16_t elapsed = TIMER_DIFF_16(record->event.time, autoshift_time); + if (elapsed < autoshift_timeout) { + // Auto-shiftable key is being released before the shift timeout; + // simulate the original press then let the usual processing take + // care of the release. + register_code(keycode); +# if TAP_CODE_DELAY > 0 + wait_ms(TAP_CODE_DELAY); +# endif + } + autoshift_flush(); + } else if (keycode == KC_LSFT && record->event.pressed) { + // If the user presses shift, auto-shift will not hold shift for + // them. + autoshift_flags.holding_shift = false; + } else { + // Use an unrelated event as an opportunity to check if the autoshift + // timeout has expired. + autoshift_check_timeout(record->event.time); + } +} + +/** \brief Simulates auto-shifted key presses + * + * Can be called from \c matrix_scan_user so that auto-shifted keys are sent + * immediately after the timeout has expired, rather than waiting for the key + * to be released. + */ +void autoshift_matrix_scan(void) { + if (autoshift_lastkey == KC_NO || autoshift_flags.registered) { + return; + } + + autoshift_check_timeout(timer_read()); } void autoshift_toggle(void) { - if (autoshift_enabled) { - autoshift_enabled = false; + if (autoshift_flags.enabled) { + autoshift_flags.enabled = false; autoshift_flush(); } else { - autoshift_enabled = true; + autoshift_flags.enabled = true; } } -void autoshift_enable(void) { autoshift_enabled = true; } +void autoshift_enable(void) { + autoshift_flags.enabled = true; + autoshift_flush(); +} + void autoshift_disable(void) { - autoshift_enabled = false; + autoshift_flags.enabled = false; autoshift_flush(); } @@ -70,7 +175,7 @@ void autoshift_timer_report(void) { } # endif -bool get_autoshift_state(void) { return autoshift_enabled; } +bool get_autoshift_state(void) { return autoshift_flags.enabled; } uint16_t get_autoshift_timeout(void) { return autoshift_timeout; } @@ -102,6 +207,7 @@ bool process_auto_shift(uint16_t keycode, keyrecord_t *record) { autoshift_timer_report(); return true; # endif + # ifndef NO_AUTO_SHIFT_ALPHA case KC_A ... KC_Z: # endif @@ -113,30 +219,14 @@ bool process_auto_shift(uint16_t keycode, keyrecord_t *record) { case KC_MINUS ... KC_SLASH: case KC_NONUS_BSLASH: # endif - autoshift_flush(); - if (!autoshift_enabled) return true; - -# ifndef AUTO_SHIFT_MODIFIERS - if (get_mods()) { - return true; - } -# endif - autoshift_on(keycode); - - // We need some extra handling here for OSL edge cases -# if !defined(NO_ACTION_ONESHOT) && !defined(NO_ACTION_TAPPING) - clear_oneshot_layer_state(ONESHOT_OTHER_KEY_PRESSED); -# endif - return false; + return autoshift_press(keycode, record); default: - autoshift_flush(); - return true; + break; } - } else { - autoshift_flush(); } + autoshift_check_record(keycode, record); return true; } diff --git a/quantum/process_keycode/process_auto_shift.h b/quantum/process_keycode/process_auto_shift.h index e86c4658e98d..5b2718f11ca6 100644 --- a/quantum/process_keycode/process_auto_shift.h +++ b/quantum/process_keycode/process_auto_shift.h @@ -30,3 +30,4 @@ void autoshift_toggle(void); bool get_autoshift_state(void); uint16_t get_autoshift_timeout(void); void set_autoshift_timeout(uint16_t timeout); +void autoshift_matrix_scan(void); From 0b83ff45c6f3059d5a1a99c7fada32f2c1a2f730 Mon Sep 17 00:00:00 2001 From: Dean Scarff Date: Sun, 26 Jul 2020 03:25:20 +1000 Subject: [PATCH 02/11] Add auto shift matrix scan to quantum --- quantum/quantum.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/quantum/quantum.c b/quantum/quantum.c index 1234b70c30a0..3ac0ed87161a 100644 --- a/quantum/quantum.c +++ b/quantum/quantum.c @@ -58,6 +58,10 @@ float bell_song[][2] = SONG(TERMINAL_SOUND); # endif #endif +#ifdef AUTO_SHIFT_ENABLE +# include "process_auto_shift.h" +#endif + static void do_code16(uint16_t code, void (*f)(uint8_t)) { switch (code) { case QK_MODS ... QK_MODS_MAX: @@ -671,6 +675,10 @@ void matrix_scan_quantum() { dip_switch_read(false); #endif +#ifdef AUTO_SHIFT_ENABLE + autoshift_matrix_scan(); +#endif + matrix_scan_kb(); } From 65fe438b8071dc5feb4014bc9774b810daac0660 Mon Sep 17 00:00:00 2001 From: Dean Scarff Date: Sun, 26 Jul 2020 02:10:45 +1000 Subject: [PATCH 03/11] Support tap-then-hold for auto shift Make auto-shift keys like dual-function keys insofar as they can be repeated by tapping then immediately (within TAPPING_TERM) holding again. --- quantum/process_keycode/process_auto_shift.c | 44 ++++++++++++++------ 1 file changed, 32 insertions(+), 12 deletions(-) diff --git a/quantum/process_keycode/process_auto_shift.c b/quantum/process_keycode/process_auto_shift.c index c47e186afd7b..217e558ef815 100644 --- a/quantum/process_keycode/process_auto_shift.c +++ b/quantum/process_keycode/process_auto_shift.c @@ -27,19 +27,26 @@ static uint16_t autoshift_lastkey = KC_NO; static struct { // Whether autoshift is enabled. bool enabled : 1; + // Whether the last autoshifted key was released after the timeout. This + // is used to replicate the last key for a tap-then-hold. + bool lastshifted : 1; + // Whether an auto-shiftable key is currently being pressed. + bool in_progress : 1; // Whether the auto-shifted keypress has been registered. bool registered : 1; // Whether autoshift is currently "holding" the shift key. bool holding_shift : 1; -} autoshift_flags = {true, false, false}; +} autoshift_flags = {true, false, false, false, false}; /** \brief Resets the autoshift state and releases the shift key */ -static void autoshift_flush(void) { +static void autoshift_flush(uint16_t now) { if (autoshift_flags.holding_shift) { // Release the shift key if it was simulated. unregister_code(KC_LSFT); } - autoshift_lastkey = KC_NO; + // Roll the autoshift_time forward for detecting tap-and-hold. + autoshift_time = now; + autoshift_flags.in_progress = false; autoshift_flags.holding_shift = false; autoshift_flags.registered = false; } @@ -53,7 +60,7 @@ static bool autoshift_press(uint16_t keycode, keyrecord_t *record) { return true; } - autoshift_flush(); + autoshift_flush(record->event.time); # ifndef AUTO_SHIFT_MODIFIERS if (get_mods()) { @@ -61,9 +68,20 @@ static bool autoshift_press(uint16_t keycode, keyrecord_t *record) { } # endif + const uint16_t elapsed = TIMER_DIFF_16(record->event.time, autoshift_time); + +# ifndef TAPPING_FORCE_HOLD + if (elapsed < TAPPING_TERM && keycode == autoshift_lastkey && !autoshift_flags.lastshifted) { + // Allow a tap-then-hold to hold the unshifted key. + autoshift_lastkey = KC_NO; + return true; + } +# endif + // Record the keycode so we can simulate it later. - autoshift_time = record->event.time; - autoshift_lastkey = keycode; + autoshift_time = record->event.time; + autoshift_lastkey = keycode; + autoshift_flags.in_progress = true; # if !defined(NO_ACTION_ONESHOT) && !defined(NO_ACTION_TAPPING) clear_oneshot_layer_state(ONESHOT_OTHER_KEY_PRESSED); @@ -83,6 +101,7 @@ void autoshift_check_timeout(uint16_t now) { if (!(get_mods() & (MOD_BIT(KC_LSFT) | MOD_BIT(KC_RSFT)))) { // Simulate pressing the shift key. register_code(KC_LSFT); + autoshift_flags.lastshifted = true; autoshift_flags.holding_shift = true; } register_code(autoshift_lastkey); @@ -103,7 +122,7 @@ void autoshift_check_timeout(uint16_t now) { * key without a shift. */ static void autoshift_check_record(uint16_t keycode, keyrecord_t *record) { - if (autoshift_lastkey == KC_NO) { + if (!autoshift_flags.in_progress) { return; } @@ -112,6 +131,7 @@ static void autoshift_check_record(uint16_t keycode, keyrecord_t *record) { // Time since the initial press was recorded. const uint16_t elapsed = TIMER_DIFF_16(record->event.time, autoshift_time); if (elapsed < autoshift_timeout) { + autoshift_flags.lastshifted = false; // Auto-shiftable key is being released before the shift timeout; // simulate the original press then let the usual processing take // care of the release. @@ -120,7 +140,7 @@ static void autoshift_check_record(uint16_t keycode, keyrecord_t *record) { wait_ms(TAP_CODE_DELAY); # endif } - autoshift_flush(); + autoshift_flush(record->event.time); } else if (keycode == KC_LSFT && record->event.pressed) { // If the user presses shift, auto-shift will not hold shift for // them. @@ -139,7 +159,7 @@ static void autoshift_check_record(uint16_t keycode, keyrecord_t *record) { * to be released. */ void autoshift_matrix_scan(void) { - if (autoshift_lastkey == KC_NO || autoshift_flags.registered) { + if (!autoshift_flags.in_progress || autoshift_flags.registered) { return; } @@ -149,7 +169,7 @@ void autoshift_matrix_scan(void) { void autoshift_toggle(void) { if (autoshift_flags.enabled) { autoshift_flags.enabled = false; - autoshift_flush(); + autoshift_flush(0); } else { autoshift_flags.enabled = true; } @@ -157,12 +177,12 @@ void autoshift_toggle(void) { void autoshift_enable(void) { autoshift_flags.enabled = true; - autoshift_flush(); + autoshift_flush(0); } void autoshift_disable(void) { autoshift_flags.enabled = false; - autoshift_flush(); + autoshift_flush(0); } # ifndef AUTO_SHIFT_NO_SETUP From 1ab316f2871b6920aa79973b3058a34c1f8a601a Mon Sep 17 00:00:00 2001 From: Dean Scarff Date: Sun, 26 Jul 2020 03:35:13 +1000 Subject: [PATCH 04/11] Flush auto-shifted key When processing a different keypress record, register the outstanding key from the auto-shift. --- quantum/process_keycode/process_auto_shift.c | 32 ++++++++++++++------ 1 file changed, 23 insertions(+), 9 deletions(-) diff --git a/quantum/process_keycode/process_auto_shift.c b/quantum/process_keycode/process_auto_shift.c index 217e558ef815..934ba94c9946 100644 --- a/quantum/process_keycode/process_auto_shift.c +++ b/quantum/process_keycode/process_auto_shift.c @@ -40,7 +40,13 @@ static struct { /** \brief Resets the autoshift state and releases the shift key */ static void autoshift_flush(uint16_t now) { - if (autoshift_flags.holding_shift) { + if (autoshift_flags.in_progress && !autoshift_flags.registered) { + // Register the key. + register_code(autoshift_lastkey); +# if TAP_CODE_DELAY > 0 + wait_ms(TAP_CODE_DELAY); +# endif + } else if (autoshift_flags.holding_shift) { // Release the shift key if it was simulated. unregister_code(KC_LSFT); } @@ -93,6 +99,8 @@ static bool autoshift_press(uint16_t keycode, keyrecord_t *record) { * * If the autoshift delay has elapsed and no shift has already been registered, * register a shift and the key. + * + * \return True if the timeout had elapsed. */ void autoshift_check_timeout(uint16_t now) { const uint16_t elapsed = TIMER_DIFF_16(now, autoshift_time); @@ -120,6 +128,8 @@ void autoshift_check_timeout(uint16_t now) { * * If the autoshift key is released before the delay has elapsed, register the * key without a shift. + * + * If another key is pressed, register the key. */ static void autoshift_check_record(uint16_t keycode, keyrecord_t *record) { if (!autoshift_flags.in_progress) { @@ -136,19 +146,23 @@ static void autoshift_check_record(uint16_t keycode, keyrecord_t *record) { // simulate the original press then let the usual processing take // care of the release. register_code(keycode); + autoshift_flags.registered = true; # if TAP_CODE_DELAY > 0 wait_ms(TAP_CODE_DELAY); # endif } autoshift_flush(record->event.time); - } else if (keycode == KC_LSFT && record->event.pressed) { - // If the user presses shift, auto-shift will not hold shift for - // them. - autoshift_flags.holding_shift = false; - } else { - // Use an unrelated event as an opportunity to check if the autoshift - // timeout has expired. - autoshift_check_timeout(record->event.time); + } else if (record->event.pressed) { + if (keycode == KC_LSFT) { + // If the user presses shift, auto-shift will not hold shift for + // them. + autoshift_flags.holding_shift = false; + } else if (!autoshift_flags.registered) { + // If the key isn't registered yet, it means the timeout hasn't + // elapsed, so register the unshifted key. + autoshift_flush(0); + autoshift_lastkey = KC_NO; + } } } From b0a3cd47574f38eb9c21b8ae3ec8bc22ff0ce706 Mon Sep 17 00:00:00 2001 From: Dean Scarff Date: Sun, 26 Jul 2020 05:23:50 +1000 Subject: [PATCH 05/11] Don't register extra keys on release --- quantum/process_keycode/process_auto_shift.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/quantum/process_keycode/process_auto_shift.c b/quantum/process_keycode/process_auto_shift.c index 934ba94c9946..cafbb913f619 100644 --- a/quantum/process_keycode/process_auto_shift.c +++ b/quantum/process_keycode/process_auto_shift.c @@ -140,7 +140,7 @@ static void autoshift_check_record(uint16_t keycode, keyrecord_t *record) { if (keycode == autoshift_lastkey && !record->event.pressed) { // Time since the initial press was recorded. const uint16_t elapsed = TIMER_DIFF_16(record->event.time, autoshift_time); - if (elapsed < autoshift_timeout) { + if (!autoshift_flags.registered && elapsed < autoshift_timeout) { autoshift_flags.lastshifted = false; // Auto-shiftable key is being released before the shift timeout; // simulate the original press then let the usual processing take From d2e4383976116f1bb48d05aaaeece224b5c68c6f Mon Sep 17 00:00:00 2001 From: Dean Scarff Date: Sun, 26 Jul 2020 06:32:12 +1000 Subject: [PATCH 06/11] Fix flaky unshifting --- quantum/process_keycode/process_auto_shift.c | 36 +++++++++++++------- 1 file changed, 23 insertions(+), 13 deletions(-) diff --git a/quantum/process_keycode/process_auto_shift.c b/quantum/process_keycode/process_auto_shift.c index cafbb913f619..2de17dacff79 100644 --- a/quantum/process_keycode/process_auto_shift.c +++ b/quantum/process_keycode/process_auto_shift.c @@ -38,7 +38,7 @@ static struct { bool holding_shift : 1; } autoshift_flags = {true, false, false, false, false}; -/** \brief Resets the autoshift state and releases the shift key */ +/** \brief Resets the autoshift state */ static void autoshift_flush(uint16_t now) { if (autoshift_flags.in_progress && !autoshift_flags.registered) { // Register the key. @@ -46,17 +46,22 @@ static void autoshift_flush(uint16_t now) { # if TAP_CODE_DELAY > 0 wait_ms(TAP_CODE_DELAY); # endif - } else if (autoshift_flags.holding_shift) { - // Release the shift key if it was simulated. - unregister_code(KC_LSFT); } // Roll the autoshift_time forward for detecting tap-and-hold. autoshift_time = now; autoshift_flags.in_progress = false; - autoshift_flags.holding_shift = false; autoshift_flags.registered = false; } +/** \brief Releases the shift key if it was held by auto-shift */ +static void autoshift_flush_shift(void) { + if (autoshift_flags.holding_shift) { + // Release the shift key if it was simulated. + unregister_code(KC_LSFT); + } + autoshift_flags.holding_shift = false; +} + /** \brief Record the press of an autoshiftable key * * \return Whether the record should be further processed. @@ -66,16 +71,14 @@ static bool autoshift_press(uint16_t keycode, keyrecord_t *record) { return true; } + const uint16_t elapsed = TIMER_DIFF_16(record->event.time, autoshift_time); autoshift_flush(record->event.time); # ifndef AUTO_SHIFT_MODIFIERS - if (get_mods()) { + if (get_mods() ^ (autoshift_flags.holding_shift ? MOD_BIT(KC_LSFT) : 0)) { return true; } # endif - - const uint16_t elapsed = TIMER_DIFF_16(record->event.time, autoshift_time); - # ifndef TAPPING_FORCE_HOLD if (elapsed < TAPPING_TERM && keycode == autoshift_lastkey && !autoshift_flags.lastshifted) { // Allow a tap-then-hold to hold the unshifted key. @@ -85,7 +88,6 @@ static bool autoshift_press(uint16_t keycode, keyrecord_t *record) { # endif // Record the keycode so we can simulate it later. - autoshift_time = record->event.time; autoshift_lastkey = keycode; autoshift_flags.in_progress = true; @@ -138,15 +140,16 @@ static void autoshift_check_record(uint16_t keycode, keyrecord_t *record) { // Process the release of the autoshiftable key. if (keycode == autoshift_lastkey && !record->event.pressed) { + autoshift_flush_shift(); // Time since the initial press was recorded. const uint16_t elapsed = TIMER_DIFF_16(record->event.time, autoshift_time); if (!autoshift_flags.registered && elapsed < autoshift_timeout) { - autoshift_flags.lastshifted = false; // Auto-shiftable key is being released before the shift timeout; // simulate the original press then let the usual processing take // care of the release. register_code(keycode); - autoshift_flags.registered = true; + autoshift_flags.lastshifted = false; + autoshift_flags.registered = true; # if TAP_CODE_DELAY > 0 wait_ms(TAP_CODE_DELAY); # endif @@ -159,9 +162,13 @@ static void autoshift_check_record(uint16_t keycode, keyrecord_t *record) { autoshift_flags.holding_shift = false; } else if (!autoshift_flags.registered) { // If the key isn't registered yet, it means the timeout hasn't - // elapsed, so register the unshifted key. + // elapsed, so register the key without additional shifting. autoshift_flush(0); autoshift_lastkey = KC_NO; + } else { + // The key is registered; flush any shift state for the + // non-autoshiftable key. + autoshift_flush_shift(); } } } @@ -183,6 +190,7 @@ void autoshift_matrix_scan(void) { void autoshift_toggle(void) { if (autoshift_flags.enabled) { autoshift_flags.enabled = false; + autoshift_flush_shift(); autoshift_flush(0); } else { autoshift_flags.enabled = true; @@ -191,11 +199,13 @@ void autoshift_toggle(void) { void autoshift_enable(void) { autoshift_flags.enabled = true; + autoshift_flush_shift(); autoshift_flush(0); } void autoshift_disable(void) { autoshift_flags.enabled = false; + autoshift_flush_shift(); autoshift_flush(0); } From ac7dfafb65980dd816c48cf925fcf8e905097a95 Mon Sep 17 00:00:00 2001 From: Dean Scarff Date: Sun, 26 Jul 2020 04:43:44 +1000 Subject: [PATCH 07/11] Update docs for auto shift Update the auto shift docs to reflect that repeats now work. --- docs/feature_auto_shift.md | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/docs/feature_auto_shift.md b/docs/feature_auto_shift.md index b21a7690d9e4..cee78f0f6fba 100644 --- a/docs/feature_auto_shift.md +++ b/docs/feature_auto_shift.md @@ -15,25 +15,25 @@ problem. When you tap a key, it stays depressed for a short period of time before it is then released. This depressed time is a different length for everyone. Auto Shift defines a constant `AUTO_SHIFT_TIMEOUT` which is typically set to twice your -normal pressed state time. When you press a key, a timer starts and then stops -when you release the key. If the time depressed is greater than or equal to the -`AUTO_SHIFT_TIMEOUT`, then a shifted version of the key is emitted. If the time -is less than the `AUTO_SHIFT_TIMEOUT` time, then the normal state is emitted. +normal pressed state time. When you press a key, a timer starts, and if you +have not released the key after the `AUTO_SHIFT_TIMEOUT` period, then a shifted +version of the key is emitted. If the time is less than the `AUTO_SHIFT_TIMEOUT` +time, or you press another key, then the normal state is emitted. + +If you hold the key down, it will repeat the shifted key. If you want to repeat +the normal key, then tap it once then immediately (within `TAPPING_TERM`) hold +it down again. The ability to repeat the normal key like this will be disabled +if `TAPPING_FORCE_HOLD` is set. ## Are There Limitations to Auto Shift? Yes, unfortunately. -1. Key repeat will cease to work. For example, before if you wanted 20 'a' - characters, you could press and hold the 'a' key for a second or two. This no - longer works with Auto Shift because it is timing your depressed time instead - of emitting a depressed key state to your operating system. -2. You will have characters that are shifted when you did not intend on shifting, and - other characters you wanted shifted, but were not. This simply comes down to - practice. As we get in a hurry, we think we have hit the key long enough - for a shifted version, but we did not. On the other hand, we may think we are - tapping the keys, but really we have held it for a little longer than - anticipated. +You will have characters that are shifted when you did not intend on shifting, and +other characters you wanted shifted, but were not. This simply comes down to +practice. As we get in a hurry, we think we have hit the key long enough for a +shifted version, but we did not. On the other hand, we may think we are tapping +the keys, but really we have held it for a little longer than anticipated. ## How Do I Enable Auto Shift? From e576132a445aa0058fa6a43ed40f1083c23180f8 Mon Sep 17 00:00:00 2001 From: IsaacElenbaas Date: Mon, 27 Jul 2020 20:47:23 -0400 Subject: [PATCH 08/11] Cleanup and refactoring, mostly prepared for custom values --- quantum/process_keycode/process_auto_shift.c | 212 ++++++++----------- 1 file changed, 90 insertions(+), 122 deletions(-) diff --git a/quantum/process_keycode/process_auto_shift.c b/quantum/process_keycode/process_auto_shift.c index 2de17dacff79..04d93890bf2f 100644 --- a/quantum/process_keycode/process_auto_shift.c +++ b/quantum/process_keycode/process_auto_shift.c @@ -27,38 +27,18 @@ static uint16_t autoshift_lastkey = KC_NO; static struct { // Whether autoshift is enabled. bool enabled : 1; - // Whether the last autoshifted key was released after the timeout. This + // Whether the last auto-shifted key was released after the timeout. This // is used to replicate the last key for a tap-then-hold. bool lastshifted : 1; - // Whether an auto-shiftable key is currently being pressed. + // Whether an auto-shiftable key has been pressed but not processed. bool in_progress : 1; // Whether the auto-shifted keypress has been registered. - bool registered : 1; - // Whether autoshift is currently "holding" the shift key. bool holding_shift : 1; -} autoshift_flags = {true, false, false, false, false}; +} autoshift_flags = {true, false, false, false}; -/** \brief Resets the autoshift state */ -static void autoshift_flush(uint16_t now) { - if (autoshift_flags.in_progress && !autoshift_flags.registered) { - // Register the key. - register_code(autoshift_lastkey); -# if TAP_CODE_DELAY > 0 - wait_ms(TAP_CODE_DELAY); -# endif - } - // Roll the autoshift_time forward for detecting tap-and-hold. - autoshift_time = now; - autoshift_flags.in_progress = false; - autoshift_flags.registered = false; -} - -/** \brief Releases the shift key if it was held by auto-shift */ +/** \brief Releases the shift key if it was held by autoshift */ static void autoshift_flush_shift(void) { - if (autoshift_flags.holding_shift) { - // Release the shift key if it was simulated. - unregister_code(KC_LSFT); - } + del_weak_mods(MOD_BIT(KC_LSFT)); autoshift_flags.holding_shift = false; } @@ -71,24 +51,35 @@ static bool autoshift_press(uint16_t keycode, keyrecord_t *record) { return true; } - const uint16_t elapsed = TIMER_DIFF_16(record->event.time, autoshift_time); - autoshift_flush(record->event.time); - # ifndef AUTO_SHIFT_MODIFIERS - if (get_mods() ^ (autoshift_flags.holding_shift ? MOD_BIT(KC_LSFT) : 0)) { + if (get_mods() & (~MOD_BIT(KC_LSFT))) { return true; } # endif -# ifndef TAPPING_FORCE_HOLD - if (elapsed < TAPPING_TERM && keycode == autoshift_lastkey && !autoshift_flags.lastshifted) { - // Allow a tap-then-hold to hold the unshifted key. - autoshift_lastkey = KC_NO; - return true; +# ifndef AUTO_SHIFT_NO_REPEAT + const uint16_t elapsed = TIMER_DIFF_16(record->event.time, autoshift_time); +# ifndef AUTO_SHIFT_NO_AUTO_REPEAT + if (!autoshift_flags.lastshifted) { +# endif + if (elapsed < TAPPING_TERM && keycode == autoshift_lastkey) { + // Allow a tap-then-hold for keyrepeat. + if (!autoshift_flags.lastshifted) { + register_code(autoshift_lastkey); + } else { + // Simulate pressing the shift key. + add_weak_mods(MOD_BIT(KC_LSFT)); + register_code(autoshift_lastkey); + } + return false; + } +# ifndef AUTO_SHIFT_NO_AUTO_REPEAT } +# endif # endif // Record the keycode so we can simulate it later. autoshift_lastkey = keycode; + autoshift_time = record->event.time; autoshift_flags.in_progress = true; # if !defined(NO_ACTION_ONESHOT) && !defined(NO_ACTION_TAPPING) @@ -99,114 +90,82 @@ static bool autoshift_press(uint16_t keycode, keyrecord_t *record) { /** \brief Registers an autoshiftable key under the right conditions * - * If the autoshift delay has elapsed and no shift has already been registered, - * register a shift and the key. - * - * \return True if the timeout had elapsed. - */ -void autoshift_check_timeout(uint16_t now) { - const uint16_t elapsed = TIMER_DIFF_16(now, autoshift_time); - if (elapsed >= autoshift_timeout) { - // The timeout has expired, so simulate the keypress. - if (!(get_mods() & (MOD_BIT(KC_LSFT) | MOD_BIT(KC_RSFT)))) { - // Simulate pressing the shift key. - register_code(KC_LSFT); - autoshift_flags.lastshifted = true; - autoshift_flags.holding_shift = true; - } - register_code(autoshift_lastkey); - autoshift_flags.registered = true; - -# if TAP_CODE_DELAY > 0 - wait_ms(TAP_CODE_DELAY); -# endif - } -} - -/** \brief Registers an autoshiftable key under the right conditions - * - * If the autoshift delay has elapsed and no shift has already been registered, - * register a shift and the key. + * If the autoshift delay has elapsed, register a shift and the key. * * If the autoshift key is released before the delay has elapsed, register the * key without a shift. - * - * If another key is pressed, register the key. */ -static void autoshift_check_record(uint16_t keycode, keyrecord_t *record) { - if (!autoshift_flags.in_progress) { - return; - } +static void autoshift_end(uint16_t keycode, uint16_t now, bool matrix_trigger) { + // Called on key down with KC_NO, auto-shifted key up, and timeout. + if (autoshift_flags.in_progress) { + // Process the auto-shiftable key. + autoshift_flags.in_progress = false; - // Process the release of the autoshiftable key. - if (keycode == autoshift_lastkey && !record->event.pressed) { - autoshift_flush_shift(); // Time since the initial press was recorded. - const uint16_t elapsed = TIMER_DIFF_16(record->event.time, autoshift_time); - if (!autoshift_flags.registered && elapsed < autoshift_timeout) { - // Auto-shiftable key is being released before the shift timeout; - // simulate the original press then let the usual processing take - // care of the release. - register_code(keycode); + const uint16_t elapsed = TIMER_DIFF_16(now, autoshift_time); + if (elapsed < autoshift_timeout) { + register_code(autoshift_lastkey); autoshift_flags.lastshifted = false; - autoshift_flags.registered = true; -# if TAP_CODE_DELAY > 0 - wait_ms(TAP_CODE_DELAY); + } else { + // Simulate pressing the shift key. + add_weak_mods(MOD_BIT(KC_LSFT)); + register_code(autoshift_lastkey); + autoshift_flags.lastshifted = true; +# if !defined(AUTO_SHIFT_NO_REPEAT) && !defined(AUTO_SHIFT_NO_AUTO_REPEAT) + if (matrix_trigger) { + // Prevents release. + return; + } # endif } - autoshift_flush(record->event.time); - } else if (record->event.pressed) { - if (keycode == KC_LSFT) { - // If the user presses shift, auto-shift will not hold shift for - // them. - autoshift_flags.holding_shift = false; - } else if (!autoshift_flags.registered) { - // If the key isn't registered yet, it means the timeout hasn't - // elapsed, so register the key without additional shifting. - autoshift_flush(0); - autoshift_lastkey = KC_NO; - } else { - // The key is registered; flush any shift state for the - // non-autoshiftable key. + +# if TAP_CODE_DELAY > 0 + wait_ms(TAP_CODE_DELAY); +# endif + unregister_code(autoshift_lastkey); + autoshift_flush_shift(); + } else { + // Release after keyrepeat. + unregister_code(keycode); + if (keycode == autoshift_lastkey) { + // This will only fire when the key was the last auto-shiftable + // pressed. That prevents aaaaBBBB then releasing a from unshifting + // later Bs (if B wasn't auto-shiftable). autoshift_flush_shift(); } } + // Roll the autoshift_time forward for detecting tap-and-hold. + autoshift_time = now; } -/** \brief Simulates auto-shifted key presses +/** \brief Simulates auto-shifted key releases when timeout is hit * * Can be called from \c matrix_scan_user so that auto-shifted keys are sent * immediately after the timeout has expired, rather than waiting for the key * to be released. */ void autoshift_matrix_scan(void) { - if (!autoshift_flags.in_progress || autoshift_flags.registered) { - return; + if (autoshift_flags.in_progress) { + const uint16_t now = timer_read(); + const uint16_t elapsed = TIMER_DIFF_16(now, autoshift_time); + if (elapsed >= autoshift_timeout) { + autoshift_end(autoshift_lastkey, now, true); + } } - - autoshift_check_timeout(timer_read()); } void autoshift_toggle(void) { - if (autoshift_flags.enabled) { - autoshift_flags.enabled = false; + autoshift_flags.enabled = !autoshift_flags.enabled; + if (!autoshift_flags.enabled) { autoshift_flush_shift(); - autoshift_flush(0); - } else { - autoshift_flags.enabled = true; } } -void autoshift_enable(void) { - autoshift_flags.enabled = true; - autoshift_flush_shift(); - autoshift_flush(0); -} +void autoshift_enable(void) { autoshift_flags.enabled = true; } void autoshift_disable(void) { autoshift_flags.enabled = false; autoshift_flush_shift(); - autoshift_flush(0); } # ifndef AUTO_SHIFT_NO_SETUP @@ -227,11 +186,18 @@ void set_autoshift_timeout(uint16_t timeout) { autoshift_timeout = timeout; } bool process_auto_shift(uint16_t keycode, keyrecord_t *record) { if (record->event.pressed) { + if (autoshift_flags.in_progress) { + // Evaluate previous key if there is one. Doing this elsewhere is + // more complicated and easier to break. + autoshift_end(KC_NO, record->event.time, false); + } + // For pressing another key while keyrepeating shifted autoshift. + autoshift_flush_shift(); + switch (keycode) { case KC_ASTG: autoshift_toggle(); return true; - case KC_ASON: autoshift_enable(); return true; @@ -251,26 +217,28 @@ bool process_auto_shift(uint16_t keycode, keyrecord_t *record) { autoshift_timer_report(); return true; # endif + } + } + switch (keycode) { # ifndef NO_AUTO_SHIFT_ALPHA - case KC_A ... KC_Z: + case KC_A ... KC_Z: # endif # ifndef NO_AUTO_SHIFT_NUMERIC - case KC_1 ... KC_0: + case KC_1 ... KC_0: # endif # ifndef NO_AUTO_SHIFT_SPECIAL - case KC_TAB: - case KC_MINUS ... KC_SLASH: - case KC_NONUS_BSLASH: + case KC_TAB: + case KC_MINUS ... KC_SLASH: + case KC_NONUS_BSLASH: # endif + if (record->event.pressed) { return autoshift_press(keycode, record); - - default: - break; - } + } else { + autoshift_end(keycode, record->event.time, false); + return false; + } } - - autoshift_check_record(keycode, record); return true; } From 451fcc7a990908a126fa72c356ce42234b15e582 Mon Sep 17 00:00:00 2001 From: IsaacElenbaas Date: Thu, 30 Jul 2020 10:45:28 -0400 Subject: [PATCH 09/11] Docs and changed defines to not be breaking change --- docs/feature_auto_shift.md | 22 ++++++++++++++++---- quantum/process_keycode/process_auto_shift.c | 4 ++-- 2 files changed, 20 insertions(+), 6 deletions(-) diff --git a/docs/feature_auto_shift.md b/docs/feature_auto_shift.md index cee78f0f6fba..8e04d9dd38cf 100644 --- a/docs/feature_auto_shift.md +++ b/docs/feature_auto_shift.md @@ -20,10 +20,11 @@ have not released the key after the `AUTO_SHIFT_TIMEOUT` period, then a shifted version of the key is emitted. If the time is less than the `AUTO_SHIFT_TIMEOUT` time, or you press another key, then the normal state is emitted. -If you hold the key down, it will repeat the shifted key. If you want to repeat -the normal key, then tap it once then immediately (within `TAPPING_TERM`) hold -it down again. The ability to repeat the normal key like this will be disabled -if `TAPPING_FORCE_HOLD` is set. +If `AUTO_SHIFT_REPEAT` is defined, there is keyrepeat support. Holding the key +down will repeat the shifted key, though this can be disabled with +`AUTO_SHIFT_NO_AUTO_REPEAT`. If you want to repeat the normal key, then tap it +once then immediately (within `TAPPING_TERM`) hold it down again (this works +with the shifted value as well if auto-repeat is disabled). ## Are There Limitations to Auto Shift? @@ -35,6 +36,11 @@ practice. As we get in a hurry, we think we have hit the key long enough for a shifted version, but we did not. On the other hand, we may think we are tapping the keys, but really we have held it for a little longer than anticipated. +Additionally, with keyrepeat the desired shift state can get mixed up. It will +always 'belong' to the last key pressed. For example, keyrepeating a capital +and then tapping something lowercase (whether or not it's an Auto Shift key) +will result in the capital's *key* still being held, but shift not. + ## How Do I Enable Auto Shift? Add to your `rules.mk` in the keymap folder: @@ -103,6 +109,14 @@ Do not Auto Shift numeric keys, zero through nine. Do not Auto Shift alpha characters, which include A through Z. +### AUTO_SHIFT_REPEAT (simple define) + +Enables keyrepeat. + +### AUTO_SHIFT_NO_AUTO_REPEAT (simple define) + +Disables automatically keyrepeating when `AUTO_SHIFT_TIMEOUT` is exceeded. + ## Using Auto Shift Setup This will enable you to define three keys temporarily to increase, decrease and report your `AUTO_SHIFT_TIMEOUT`. diff --git a/quantum/process_keycode/process_auto_shift.c b/quantum/process_keycode/process_auto_shift.c index 04d93890bf2f..fd5634e9dc68 100644 --- a/quantum/process_keycode/process_auto_shift.c +++ b/quantum/process_keycode/process_auto_shift.c @@ -56,7 +56,7 @@ static bool autoshift_press(uint16_t keycode, keyrecord_t *record) { return true; } # endif -# ifndef AUTO_SHIFT_NO_REPEAT +# ifdef AUTO_SHIFT_REPEAT const uint16_t elapsed = TIMER_DIFF_16(record->event.time, autoshift_time); # ifndef AUTO_SHIFT_NO_AUTO_REPEAT if (!autoshift_flags.lastshifted) { @@ -111,7 +111,7 @@ static void autoshift_end(uint16_t keycode, uint16_t now, bool matrix_trigger) { add_weak_mods(MOD_BIT(KC_LSFT)); register_code(autoshift_lastkey); autoshift_flags.lastshifted = true; -# if !defined(AUTO_SHIFT_NO_REPEAT) && !defined(AUTO_SHIFT_NO_AUTO_REPEAT) +# if defined(AUTO_SHIFT_REPEAT) && !defined(AUTO_SHIFT_NO_AUTO_REPEAT) if (matrix_trigger) { // Prevents release. return; From be23535f61b3fd94da3390094b9854e85d340b6c Mon Sep 17 00:00:00 2001 From: IsaacElenbaas Date: Thu, 30 Jul 2020 22:22:51 -0400 Subject: [PATCH 10/11] Fixed shift not being released and removed unnecessary flush_shift. --- quantum/process_keycode/process_auto_shift.c | 19 ++++++------------- 1 file changed, 6 insertions(+), 13 deletions(-) diff --git a/quantum/process_keycode/process_auto_shift.c b/quantum/process_keycode/process_auto_shift.c index fd5634e9dc68..bff864edaf00 100644 --- a/quantum/process_keycode/process_auto_shift.c +++ b/quantum/process_keycode/process_auto_shift.c @@ -36,12 +36,6 @@ static struct { bool holding_shift : 1; } autoshift_flags = {true, false, false, false}; -/** \brief Releases the shift key if it was held by autoshift */ -static void autoshift_flush_shift(void) { - del_weak_mods(MOD_BIT(KC_LSFT)); - autoshift_flags.holding_shift = false; -} - /** \brief Record the press of an autoshiftable key * * \return Whether the record should be further processed. @@ -123,7 +117,7 @@ static void autoshift_end(uint16_t keycode, uint16_t now, bool matrix_trigger) { wait_ms(TAP_CODE_DELAY); # endif unregister_code(autoshift_lastkey); - autoshift_flush_shift(); + del_weak_mods(MOD_BIT(KC_LSFT)); } else { // Release after keyrepeat. unregister_code(keycode); @@ -131,9 +125,10 @@ static void autoshift_end(uint16_t keycode, uint16_t now, bool matrix_trigger) { // This will only fire when the key was the last auto-shiftable // pressed. That prevents aaaaBBBB then releasing a from unshifting // later Bs (if B wasn't auto-shiftable). - autoshift_flush_shift(); + del_weak_mods(MOD_BIT(KC_LSFT)); } } + send_keyboard_report(); // del_weak_mods doesn't send one. // Roll the autoshift_time forward for detecting tap-and-hold. autoshift_time = now; } @@ -156,16 +151,14 @@ void autoshift_matrix_scan(void) { void autoshift_toggle(void) { autoshift_flags.enabled = !autoshift_flags.enabled; - if (!autoshift_flags.enabled) { - autoshift_flush_shift(); - } + del_weak_mods(MOD_BIT(KC_LSFT)); } void autoshift_enable(void) { autoshift_flags.enabled = true; } void autoshift_disable(void) { autoshift_flags.enabled = false; - autoshift_flush_shift(); + del_weak_mods(MOD_BIT(KC_LSFT)); } # ifndef AUTO_SHIFT_NO_SETUP @@ -192,7 +185,7 @@ bool process_auto_shift(uint16_t keycode, keyrecord_t *record) { autoshift_end(KC_NO, record->event.time, false); } // For pressing another key while keyrepeating shifted autoshift. - autoshift_flush_shift(); + del_weak_mods(MOD_BIT(KC_LSFT)); switch (keycode) { case KC_ASTG: From 4c7f736e0115bae90fd61a765d79c658c3abefa0 Mon Sep 17 00:00:00 2001 From: Dean Scarff Date: Thu, 26 Nov 2020 11:11:38 +1100 Subject: [PATCH 11/11] Don't trust record->event.time Manual testing has shown event.time is not reliable on some keyboards. Perform a timer_read() instead. --- quantum/process_keycode/process_auto_shift.c | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/quantum/process_keycode/process_auto_shift.c b/quantum/process_keycode/process_auto_shift.c index bff864edaf00..a2d315408b09 100644 --- a/quantum/process_keycode/process_auto_shift.c +++ b/quantum/process_keycode/process_auto_shift.c @@ -40,7 +40,7 @@ static struct { * * \return Whether the record should be further processed. */ -static bool autoshift_press(uint16_t keycode, keyrecord_t *record) { +static bool autoshift_press(uint16_t keycode, uint16_t now, keyrecord_t *record) { if (!autoshift_flags.enabled) { return true; } @@ -51,7 +51,7 @@ static bool autoshift_press(uint16_t keycode, keyrecord_t *record) { } # endif # ifdef AUTO_SHIFT_REPEAT - const uint16_t elapsed = TIMER_DIFF_16(record->event.time, autoshift_time); + const uint16_t elapsed = TIMER_DIFF_16(now, autoshift_time); # ifndef AUTO_SHIFT_NO_AUTO_REPEAT if (!autoshift_flags.lastshifted) { # endif @@ -73,7 +73,7 @@ static bool autoshift_press(uint16_t keycode, keyrecord_t *record) { // Record the keycode so we can simulate it later. autoshift_lastkey = keycode; - autoshift_time = record->event.time; + autoshift_time = now; autoshift_flags.in_progress = true; # if !defined(NO_ACTION_ONESHOT) && !defined(NO_ACTION_TAPPING) @@ -178,11 +178,15 @@ uint16_t get_autoshift_timeout(void) { return autoshift_timeout; } void set_autoshift_timeout(uint16_t timeout) { autoshift_timeout = timeout; } bool process_auto_shift(uint16_t keycode, keyrecord_t *record) { + // Note that record->event.time isn't reliable, see: + // https://github.com/qmk/qmk_firmware/pull/9826#issuecomment-733559550 + const uint16_t now = timer_read(); + if (record->event.pressed) { if (autoshift_flags.in_progress) { // Evaluate previous key if there is one. Doing this elsewhere is // more complicated and easier to break. - autoshift_end(KC_NO, record->event.time, false); + autoshift_end(KC_NO, now, false); } // For pressing another key while keyrepeating shifted autoshift. del_weak_mods(MOD_BIT(KC_LSFT)); @@ -226,9 +230,9 @@ bool process_auto_shift(uint16_t keycode, keyrecord_t *record) { case KC_NONUS_BSLASH: # endif if (record->event.pressed) { - return autoshift_press(keycode, record); + return autoshift_press(keycode, now, record); } else { - autoshift_end(keycode, record->event.time, false); + autoshift_end(keycode, now, false); return false; } }