qmk_firmware/tests/tap_hold_configurations
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
..
chordal_hold [Bug][Core] Fix for Chordal Hold: stuck mods when mod-taps are pressed in a stuttered sequence. (#24878) 2025-01-30 06:17:54 +11:00
default_mod_tap Improve test invocation, fix Retro Shift bugs, and add Auto+Retro Shift test cases (#15889) 2023-09-25 12:45:49 +10:00
flow_tap [Bug][Core] Fix for Flow Tap: fix handling of distinct taps and timer updates. (#25175) 2025-04-22 09:59:49 +02:00
hold_on_other_key_press Introduce VERIFY_AND_CLEAR shorthand (#19370) 2022-12-18 21:55:14 +01:00
permissive_hold Improve test invocation, fix Retro Shift bugs, and add Auto+Retro Shift test cases (#15889) 2023-09-25 12:45:49 +10:00
quick_tap Make IGNORE_MOD_TAP_INTERRUPT the default behaviour for mod-taps (#20211) 2023-04-03 15:32:47 +10:00
retro_tapping Retro Tapping Re-Write; Key Roll Fix (#23641) 2025-02-16 20:50:42 +11:00