summaryrefslogtreecommitdiff
path: root/quantum
diff options
context:
space:
mode:
authorPascal Getreuer <50221757+getreuer@users.noreply.github.com>2025-04-22 00:59:49 -0700
committerGitHub <noreply@github.com>2025-04-22 09:59:49 +0200
commit73e2ef486ab7acf3763695cb3a3cf691451cdff3 (patch)
tree42fa19653935e7e2c7cc720accaf5ba17641d141 /quantum
parentb5f8f4d6a201e6a67d1ee483fe6d3327d98cb052 (diff)
[Bug][Core] Fix for Flow Tap: fix handling of distinct taps and timer updates. (#25175)
* Flow Tap bug fix. As reported by @amarz45 and @mwpardue, there is a bug where if two tap-hold keys are pressed in distinct taps back to back, then Flow Tap is not applied on the second tap-hold key, but it should be. In a related bug reported by @NikGovorov, if a tap-hold key is held followed by a tap of a tap-hold key, then Flow Tap updates its timer on the release of the held tap-hold key, but it should be ignored. The problem common to both these bugs is that I incorrectly assumed `tapping_key` is cleared to noevent once it is released, when actually `tapping_key` is still maintained for `TAPPING_TERM` ms after release (for Quick Tap). This commit fixes that. Thanks to @amarz45, @mwpardue, and @NikGovorov for reporting! Details: * Logic for converting the current tap-hold event to a tap is extracted to `flow_tap_key_if_within_term()`, which is now invoked also in the post-release "interfered with other tap key" case. This fixes the distinct taps bug. * The Flow Tap timer is now updated at the beginning of each call to `process_record()`, provided that there is no unsettled tap-hold key at that time and that the record is not for a mod or layer switch key. By moving this update logic to `process_record()`, it is conceptually simpler and more robust. * Unit tests extended to cover the reported scenarios. * Fix formatting. * Revision to fix @NikGovorov's scenario. The issue is that when another key is pressed while a layer-tap hasn't been settled yet, the `prev_keycode` remembers the keycode from before the layer switched. This can then enable Flow Tap for the following key when it shouldn't, or vice versa. Thanks to @NikGovorov for reporting! This commit revises Flow Tap in the following ways: * The previous key and timer are both updated from `process_record()`. This is slightly later in the sequence of processing than before, and by this point, a just-settled layer-tap should have taken effect so that the keycode from the correct layer is remembered. * The Flow Tap previous key and timer are updated now also on key release events, except for releases of modifiers and held layer switches. * The Flow Tap previous key and timer are now updated together, for simplicity. This makes the logic easier to think about. * A few additional unit tests, including @NikGovorov's scenario as "layer_tap_ignored_with_disabled_key_complex."
Diffstat (limited to 'quantum')
-rw-r--r--quantum/action.c3
-rw-r--r--quantum/action_tapping.c105
-rw-r--r--quantum/action_tapping.h3
3 files changed, 79 insertions, 32 deletions
diff --git a/quantum/action.c b/quantum/action.c
index eb0dbc7022..dd82c9ec99 100644
--- a/quantum/action.c
+++ b/quantum/action.c
@@ -281,6 +281,9 @@ void process_record(keyrecord_t *record) {
281 if (IS_NOEVENT(record->event)) { 281 if (IS_NOEVENT(record->event)) {
282 return; 282 return;
283 } 283 }
284#ifdef FLOW_TAP_TERM
285 flow_tap_update_last_event(record);
286#endif // FLOW_TAP_TERM
284 287
285 if (!process_record_quantum(record)) { 288 if (!process_record_quantum(record)) {
286#ifndef NO_ACTION_ONESHOT 289#ifndef NO_ACTION_ONESHOT
diff --git a/quantum/action_tapping.c b/quantum/action_tapping.c
index 312c639169..3e391d1526 100644
--- a/quantum/action_tapping.c
+++ b/quantum/action_tapping.c
@@ -6,6 +6,7 @@
6#include "action_tapping.h" 6#include "action_tapping.h"
7#include "action_util.h" 7#include "action_util.h"
8#include "keycode.h" 8#include "keycode.h"
9#include "quantum_keycodes.h"
9#include "timer.h" 10#include "timer.h"
10 11
11#ifndef NO_ACTION_TAPPING 12#ifndef NO_ACTION_TAPPING
@@ -102,10 +103,10 @@ __attribute__((weak)) bool get_hold_on_other_key_press(uint16_t keycode, keyreco
102# endif 103# endif
103 104
104# if defined(FLOW_TAP_TERM) 105# if defined(FLOW_TAP_TERM)
105static uint32_t last_input = 0; 106static uint32_t flow_tap_prev_time = 0;
106static uint16_t prev_keycode = KC_NO; 107static uint16_t flow_tap_prev_keycode = KC_NO;
107 108
108uint16_t get_flow_tap_term(uint16_t keycode, keyrecord_t *record, uint16_t prev_keycode); 109static bool flow_tap_key_if_within_term(keyrecord_t *record);
109# endif // defined(FLOW_TAP_TERM) 110# endif // defined(FLOW_TAP_TERM)
110 111
111static keyrecord_t tapping_key = {}; 112static keyrecord_t tapping_key = {};
@@ -157,19 +158,6 @@ void action_tapping_process(keyrecord_t record) {
157 } 158 }
158 } 159 }
159 if (IS_EVENT(record.event)) { 160 if (IS_EVENT(record.event)) {
160# if defined(FLOW_TAP_TERM)
161 const uint16_t keycode = get_record_keycode(&record, false);
162 // Track the previous key press.
163 if (record.event.pressed) {
164 prev_keycode = keycode;
165 }
166 // If there is no unsettled tap-hold key, update last input time. Ignore
167 // mod keys in this update to allow for chording multiple mods for
168 // hotkeys like "Ctrl+Shift+arrow".
169 if (IS_NOEVENT(tapping_key.event) && !IS_MODIFIER_KEYCODE(keycode)) {
170 last_input = timer_read32();
171 }
172# endif // defined(FLOW_TAP_TERM)
173 ac_dprintf("\n"); 161 ac_dprintf("\n");
174 } 162 }
175} 163}
@@ -252,22 +240,8 @@ bool process_tapping(keyrecord_t *keyp) {
252 // into the "pressed" tapping key state 240 // into the "pressed" tapping key state
253 241
254# if defined(FLOW_TAP_TERM) 242# if defined(FLOW_TAP_TERM)
255 const uint16_t keycode = get_record_keycode(keyp, false); 243 if (flow_tap_key_if_within_term(keyp)) {
256 if (is_mt_or_lt(keycode)) { 244 return true;
257 const uint32_t idle_time = timer_elapsed32(last_input);
258 uint16_t term = get_flow_tap_term(keycode, keyp, prev_keycode);
259 if (term > 500) {
260 term = 500;
261 }
262 if (idle_time < 500 && idle_time < term) {
263 debug_event(keyp->event);
264 ac_dprintf(" within flow tap term (%u < %u) considered a tap\n", (int16_t)idle_time, term);
265 keyp->tap.count = 1;
266 registered_taps_add(keyp->event.key);
267 debug_registered_taps();
268 process_record(keyp);
269 return true;
270 }
271 } 245 }
272# endif // defined(FLOW_TAP_TERM) 246# endif // defined(FLOW_TAP_TERM)
273 247
@@ -582,6 +556,13 @@ bool process_tapping(keyrecord_t *keyp) {
582 return true; 556 return true;
583 } else if (is_tap_record(keyp)) { 557 } else if (is_tap_record(keyp)) {
584 // Sequential tap can be interfered with other tap key. 558 // Sequential tap can be interfered with other tap key.
559# if defined(FLOW_TAP_TERM)
560 if (flow_tap_key_if_within_term(keyp)) {
561 tapping_key = (keyrecord_t){0};
562 debug_tapping_key();
563 return true;
564 }
565# endif // defined(FLOW_TAP_TERM)
585 ac_dprintf("Tapping: Start with interfering other tap.\n"); 566 ac_dprintf("Tapping: Start with interfering other tap.\n");
586 tapping_key = *keyp; 567 tapping_key = *keyp;
587 waiting_buffer_scan_tap(); 568 waiting_buffer_scan_tap();
@@ -809,6 +790,66 @@ static void waiting_buffer_process_regular(void) {
809# endif // CHORDAL_HOLD 790# endif // CHORDAL_HOLD
810 791
811# ifdef FLOW_TAP_TERM 792# ifdef FLOW_TAP_TERM
793void flow_tap_update_last_event(keyrecord_t *record) {
794 // Don't update while a tap-hold key is unsettled.
795 if (waiting_buffer_tail != waiting_buffer_head || (tapping_key.event.pressed && tapping_key.tap.count == 0)) {
796 return;
797 }
798 const uint16_t keycode = get_record_keycode(record, false);
799 // Ignore releases of modifiers and held layer switches.
800 if (!record->event.pressed) {
801 switch (keycode) {
802 case MODIFIER_KEYCODE_RANGE:
803 case QK_MOMENTARY ... QK_MOMENTARY_MAX:
804 case QK_LAYER_TAP_TOGGLE ... QK_LAYER_TAP_TOGGLE_MAX:
805# ifndef NO_ACTION_ONESHOT // Ignore one-shot keys.
806 case QK_ONE_SHOT_MOD ... QK_ONE_SHOT_MOD_MAX:
807 case QK_ONE_SHOT_LAYER ... QK_ONE_SHOT_LAYER_MAX:
808# endif // NO_ACTION_ONESHOT
809# ifdef TRI_LAYER_ENABLE // Ignore Tri Layer keys.
810 case QK_TRI_LAYER_LOWER:
811 case QK_TRI_LAYER_UPPER:
812# endif // TRI_LAYER_ENABLE
813 return;
814 case QK_MODS ... QK_MODS_MAX:
815 if (QK_MODS_GET_BASIC_KEYCODE(keycode) == KC_NO) {
816 return;
817 }
818 break;
819 case QK_MOD_TAP ... QK_MOD_TAP_MAX:
820 case QK_LAYER_TAP ... QK_LAYER_TAP_MAX:
821 if (record->tap.count == 0) {
822 return;
823 }
824 break;
825 }
826 }
827
828 flow_tap_prev_keycode = keycode;
829 flow_tap_prev_time = timer_read32();
830}
831
832static bool flow_tap_key_if_within_term(keyrecord_t *record) {
833 const uint16_t keycode = get_record_keycode(record, false);
834 if (is_mt_or_lt(keycode)) {
835 const uint32_t idle_time = timer_elapsed32(flow_tap_prev_time);
836 uint16_t term = get_flow_tap_term(keycode, record, flow_tap_prev_keycode);
837 if (term > 500) {
838 term = 500;
839 }
840 if (idle_time < 500 && idle_time < term) {
841 debug_event(record->event);
842 ac_dprintf(" within flow tap term (%u < %u) considered a tap\n", (int16_t)idle_time, term);
843 record->tap.count = 1;
844 registered_taps_add(record->event.key);
845 debug_registered_taps();
846 process_record(record);
847 return true;
848 }
849 }
850 return false;
851}
852
812// By default, enable Flow Tap for the keys in the main alphas area and Space. 853// By default, enable Flow Tap for the keys in the main alphas area and Space.
813// This should work reasonably even if the layout is remapped on the host to an 854// This should work reasonably even if the layout is remapped on the host to an
814// alt layout or international layout (e.g. Dvorak or AZERTY), where these same 855// alt layout or international layout (e.g. Dvorak or AZERTY), where these same
diff --git a/quantum/action_tapping.h b/quantum/action_tapping.h
index 2af000ad73..0cf4aa1200 100644
--- a/quantum/action_tapping.h
+++ b/quantum/action_tapping.h
@@ -166,6 +166,9 @@ bool is_flow_tap_key(uint16_t keycode);
166 * @return Time in milliseconds. 166 * @return Time in milliseconds.
167 */ 167 */
168uint16_t get_flow_tap_term(uint16_t keycode, keyrecord_t *record, uint16_t prev_keycode); 168uint16_t get_flow_tap_term(uint16_t keycode, keyrecord_t *record, uint16_t prev_keycode);
169
170/** Updates the Flow Tap last key and timer. */
171void flow_tap_update_last_event(keyrecord_t *record);
169#endif // FLOW_TAP_TERM 172#endif // FLOW_TAP_TERM
170 173
171#ifdef DYNAMIC_TAPPING_TERM_ENABLE 174#ifdef DYNAMIC_TAPPING_TERM_ENABLE