* 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."
* Chordal Hold: restrict what chords settle as hold
* Chordal Hold: docs and further improvements
* Fix formatting.
* Doc rewording and minor edit.
* Support Chordal Hold of multiple tap-hold keys.
* Fix formatting.
* Simplification and additional test.
* Fix formatting.
* Tighten tests.
* Add test two_mod_taps_same_hand_hold_til_timeout.
* Revise handing of pairs of tap-hold keys.
* Generate a default chordal_hold_layout.
* Document chordal_hold_handedness().
* Add license notice to new and branched files in PR.
* Add `tapping.chordal_hold` property for info.json.
* Update docs/reference_info_json.md
* Revise "hand" jsonschema.
* Chordal Hold: Improved layout handedness heuristic.
This commit improves the heuristic used in generate-keyboard-c for
inferring key handedness from keyboard.json geometry data.
Heuristic summary:
1. If the layout is symmetric (e.g. most split keyboards), guess the
handedness based on the sign of (x - layout_x_midpoint).
2. Otherwise, if the layout has a key of >=6u width, it is probably the
spacebar. Form a dividing line through the spacebar, nearly vertical
but with a slight angle to follow typical row stagger.
3. Otherwise, assume handedness based on the widest horizontal
separation.
I have tested this strategy on a couple dozen keyboards and found it to
work reliably.
* Use Optional instead of `| None`.
* Refactor to avoid lambdas.
* Remove trailing comma in chordal_hold_layout.
* Minor docs edits.
* Revise to allow combining multiple same-hand mods.
This commit revises Chordal Hold as described in discussion in
https://github.com/qmk/qmk_firmware/pull/24560#discussion_r1894655238
1. In "RCTL_T(KC_A)↓, RSFT_T(KC_C)↓, RCTL_T(KC_A)↑" before the tapping
term, RCTL_T(KC_A) is settled as tapped.
2. In "RCTL_T(KC_A)↓, RSFT_T(KC_C)↓, RSFT_T(KC_C)↑", both RCTL_T(KC_A)
and RSFT_T(KC_C) are settled as tapped.
3. In "RCTL_T(KC_A)↓, RSFT_T(KC_C)↓, KC_U↓" (all keys on the same side),
both RCTL_T(KC_A) and RSFT_T(KC_C) are settled as tapped.
4. In "RCTL_T(KC_A)↓, RSFT_T(KC_C)↓, LSFT_T(KC_T)↓", with the third key
on the other side, we allow Permissive Hold or Hold On Other Keypress
to decide how/when to settle the keys.
5. In "RCTL_T(KC_A)↓, RSFT_T(KC_C)↓" held until the tapping term, the
keys are settled as held.
1–3 provide same-hand roll protection. 4–5 are for combining multiple
same-hand modifiers.
I've updated the unit tests and have been running it on my keyboard, for
a few hours so far, and all seems good. I really like this scheme. It
allows combining same-side mods, yet it also has roll protection on
streaks. For me, this feels like Achordion, but clearly better streak
handling and improved responsiveness.
* Fix formatting.
* Add a couple tests with LT keys.
* Remove stale use of CHORDAL_HOLD_LAYOUT.
* Fix misspelling lastest -> latest
* Handling tweak for LTs and tests.
* Fix formatting.
* More tests with LT keys.
* Fix formatting.
* Add keyevent for combo keyrecord
* Fix formatting
* Update quantum/process_keycode/process_combo.c
Co-authored-by: Sergey Vlasov <sigprof@gmail.com>
* Add combo unit-tests and hot-fix process_record_tap_hint
...as this function tries to lookup the combo keys passed in. This will
be refactored in a later pr.
---------
Co-authored-by: Sergey Vlasov <sigprof@gmail.com>
Co-authored-by: Stefan Kerkmann <karlk90@pm.me>
* Replace Tapping Force Hold feature with Quick Tap Term
* Replace keyboard level TAPPING_FORCE_HOLD with QUICK_TAP_TERM 0
* Deprecate force hold in info_config.json
* Before and after quick tap term unit tests
* Quick tap unit tests iteration
* Keymap config.h correction
* Remove TAPPING_FORCE_HOLD_PER_KEY macros that were missed
* Add two more test cases for quick tap
* Replace TAPPING_FORCE_HOLD with QUICK_TAP_TERM in configs #2
* Replace TAPPING_FORCE_HOLD_PER_KEY with QUICK_TAP_TERM_PER_KEY in configs #2
* Add function declaration for get_quick_tap_term
Co-authored-by: Stefan Kerkmann <karlk90@pm.me>
* Make process_tapping more readable
Move most #ifdefs into conditionally defined macros to make the logic
easier to follow.
* Retain momentary layers until the end of tapping
This allows mod-tap and layer-tap keys on layers to behave as expected.
Bug: https://github.com/qmk/qmk_firmware/issues/17281
* Add tests for delayed mod/layer release while tapping
Mods and layer key release is delayed while tapping is in progress to
ensure that the tap is registered with the modifier state and on the
layer where the key was first pressed.
Signed-off-by: Felix Kuehling <felix.kuehling@gmail.com>
* Add GET_TAPPING_TERM macro to reduce duplicate code
The macro gives the right tapping term depending on whether per-key
tapping terms and/or dynamic tapping terms are enabled. Unnecessary
function calls and variable resolution are avoided.
Fixes#16472.
* Use GET_TAPPING_TERM for Cirque trackpads
Co-authored-by: Stefan Kerkmann <karlk90@pm.me>
* New feature: `DYNAMIC_TAPPING_TERM_ENABLE`
3 new quantum keys to configure the tapping term on the fly.
* Replace sprintf call in tapping_term_report by get_u16_str
* Replace tab with 4 spaces
* Revert "Short term bodge for firmware size bloat (#14144)"
This reverts commit a8d6547346.
* Revert "Tidy up quantum.c now some of tmk_core has been merged (#14083)"
This reverts commit c4dbf4bf01.
