qmk_firmware/quantum/action_tapping.h
Pascal Getreuer 73e2ef486a
[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."
2025-04-22 09:59:49 +02:00

191 lines
6.7 KiB
C

/*
Copyright 2013 Jun Wako <wakojun@gmail.com>
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 2 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/>.
*/
#pragma once
/* period of tapping(ms) */
#ifndef TAPPING_TERM
# define TAPPING_TERM 200
#endif
/* period of quick tap(ms) */
#if !defined(QUICK_TAP_TERM) || QUICK_TAP_TERM > TAPPING_TERM
# define QUICK_TAP_TERM TAPPING_TERM
#endif
/* tap count needed for toggling a feature */
#ifndef TAPPING_TOGGLE
# define TAPPING_TOGGLE 5
#endif
#define WAITING_BUFFER_SIZE 8
#ifndef NO_ACTION_TAPPING
uint16_t get_record_keycode(keyrecord_t *record, bool update_layer_cache);
uint16_t get_event_keycode(keyevent_t event, bool update_layer_cache);
void action_tapping_process(keyrecord_t record);
#endif
uint16_t get_tapping_term(uint16_t keycode, keyrecord_t *record);
uint16_t get_quick_tap_term(uint16_t keycode, keyrecord_t *record);
bool get_permissive_hold(uint16_t keycode, keyrecord_t *record);
bool get_retro_tapping(uint16_t keycode, keyrecord_t *record);
bool get_hold_on_other_key_press(uint16_t keycode, keyrecord_t *record);
#ifdef CHORDAL_HOLD
/**
* Callback to say when a key chord before the tapping term may be held.
*
* In keymap.c, define the callback
*
* bool get_chordal_hold(uint16_t tap_hold_keycode,
* keyrecord_t* tap_hold_record,
* uint16_t other_keycode,
* keyrecord_t* other_record) {
* // Conditions...
* }
*
* This callback is called when:
*
* 1. `tap_hold_keycode` is pressed.
* 2. `other_keycode` is pressed while `tap_hold_keycode` is still held,
* provided `other_keycode` is *not* also a tap-hold key and it is pressed
* before the tapping term.
*
* If false is returned, this has the effect of immediately settling the
* tap-hold key as tapped. If true is returned, the tap-hold key is still
* unsettled, and may be settled as held depending on configuration and
* subsequent events.
*
* @param tap_hold_keycode Keycode of the tap-hold key.
* @param tap_hold_record Record from the tap-hold press event.
* @param other_keycode Keycode of the other key.
* @param other_record Record from the other key's press event.
* @return True if the tap-hold key may be considered held; false if tapped.
*/
bool get_chordal_hold(uint16_t tap_hold_keycode, keyrecord_t *tap_hold_record, uint16_t other_keycode, keyrecord_t *other_record);
/**
* Default "opposite hands rule" for whether a key chord may settle as held.
*
* This function returns true when the tap-hold key and other key are on
* "opposite hands." In detail, handedness of the two keys are compared. If
* handedness values differ, or if either handedness is '*', the function
* returns true, indicating that it may be held. Otherwise, it returns false,
* in which case the tap-hold key is immediately settled at tapped.
*
* @param tap_hold_record Record of the active tap-hold key press.
* @param other_record Record of the other, interrupting key press.
* @return True if the tap-hold key may be considered held; false if tapped.
*/
bool get_chordal_hold_default(keyrecord_t *tap_hold_record, keyrecord_t *other_record);
/**
* Gets the handedness of a key.
*
* This function returns:
* 'L' for keys pressed by the left hand,
* 'R' for keys on the right hand,
* '*' for keys exempt from the "opposite hands rule." This could be used
* perhaps on thumb keys or keys that might be pressed by either hand.
*
* @param key A key matrix position.
* @return Handedness value.
*/
char chordal_hold_handedness(keypos_t key);
extern const char chordal_hold_layout[MATRIX_ROWS][MATRIX_COLS] PROGMEM;
#endif
#ifdef FLOW_TAP_TERM
/**
* Callback to specify the keys where Flow Tap is enabled.
*
* Flow Tap is constrained to certain keys by the following rule: this callback
* is called for both the tap-hold key *and* the key press immediately preceding
* it. If the callback returns true for both keycodes, Flow Tap is enabled.
*
* The default implementation of this callback corresponds to
*
* bool is_flow_tap_key(uint16_t keycode) {
* switch (get_tap_keycode(keycode)) {
* case KC_SPC:
* case KC_A ... KC_Z:
* case KC_DOT:
* case KC_COMM:
* case KC_SCLN:
* case KC_SLSH:
* return true;
* }
* return false;
* }
*
* @param keycode Keycode of the key.
* @return Whether to enable Flow Tap for this key.
*/
bool is_flow_tap_key(uint16_t keycode);
/**
* Callback to customize Flow Tap filtering.
*
* Flow Tap acts only when key events are closer together than this time.
*
* Return a time of 0 to disable filtering. In this way, Flow Tap may be
* disabled for certain tap-hold keys, or when following certain previous keys.
*
* The default implementation of this callback is
*
* uint16_t get_flow_tap_term(uint16_t keycode, keyrecord_t* record,
* uint16_t prev_keycode) {
* if (is_flow_tap_key(keycode) && is_flow_tap_key(prev_keycode)) {
* return g_flow_tap_term;
* }
* return 0;
* }
*
* NOTE: If both `is_flow_tap_key()` and `get_flow_tap_term()` are defined, then
* `get_flow_tap_term()` takes precedence.
*
* @param keycode Keycode of the tap-hold key.
* @param record keyrecord_t of the tap-hold event.
* @param prev_keycode Keycode of the previously pressed key.
* @return Time in milliseconds.
*/
uint16_t get_flow_tap_term(uint16_t keycode, keyrecord_t *record, uint16_t prev_keycode);
/** Updates the Flow Tap last key and timer. */
void flow_tap_update_last_event(keyrecord_t *record);
#endif // FLOW_TAP_TERM
#ifdef DYNAMIC_TAPPING_TERM_ENABLE
extern uint16_t g_tapping_term;
#endif
#if defined(TAPPING_TERM_PER_KEY) && !defined(NO_ACTION_TAPPING)
# define GET_TAPPING_TERM(keycode, record) get_tapping_term(keycode, record)
#elif defined(DYNAMIC_TAPPING_TERM_ENABLE) && !defined(NO_ACTION_TAPPING)
# define GET_TAPPING_TERM(keycode, record) g_tapping_term
#else
# define GET_TAPPING_TERM(keycode, record) (TAPPING_TERM)
#endif
#ifdef QUICK_TAP_TERM_PER_KEY
# define GET_QUICK_TAP_TERM(keycode, record) get_quick_tap_term(keycode, record)
#else
# define GET_QUICK_TAP_TERM(keycode, record) (QUICK_TAP_TERM)
#endif