drashna/qmk_firmware
1
0
Fork 0
Code Activity Actions

[Core] Add Chordal Hold, an "opposite hands rule" tap-hold option similar to Achordion, Bilateral Combinations. (#24560)

Browse source 
* 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.
This commit is contained in:
Pascal Getreuer 2025-01-27 03:32:23 -08:00 • committed by GitHub
parent ee63d39058
commit 544ddde113
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
25 changed files with 3607 additions and 26 deletions
Download patch file Download diff file Expand all files Collapse all files

274
quantum/action_tapping.c
View file

@ -49,6 +49,45 @@ __attribute__((weak)) bool get_permissive_hold(uint16_t keycode, keyrecord_t *re
}
# endif
# if defined(CHORDAL_HOLD)
extern const char chordal_hold_layout[MATRIX_ROWS][MATRIX_COLS] PROGMEM;
# define REGISTERED_TAPS_SIZE 8
// Array of tap-hold keys that have been settled as tapped but not yet released.
static keypos_t registered_taps[REGISTERED_TAPS_SIZE] = {};
static uint8_t num_registered_taps = 0;
/** Adds `key` to the registered_taps array. */
static void registered_taps_add(keypos_t key);
/** Returns the index of `key` in registered_taps, or -1 if not found. */
static int8_t registered_tap_find(keypos_t key);
/** Removes index `i` from the registered_taps array. */
static void registered_taps_del_index(uint8_t i);
/** Logs the registered_taps array for debugging. */
static void debug_registered_taps(void);
/** \brief Finds which queued events should be held according to Chordal Hold.
*
* In a situation with multiple unsettled tap-hold key presses, scan the queue
* up until the first release, non-tap-hold, or one-shot event and find the
* latest event in the queue that settles as held according to
* get_chordal_hold().
*
* \return Index of the first tap, or equivalently, one past the latest hold.
*/
static uint8_t waiting_buffer_find_chordal_hold_tap(void);
/** Processes queued events up to and including `key` as tapped. */
static void waiting_buffer_chordal_hold_taps_until(keypos_t key);
/** \brief Processes and pops buffered events until the first tap-hold event. */
static void waiting_buffer_process_regular(void);
static bool is_mt_or_lt(uint16_t keycode) {
return IS_QK_MOD_TAP(keycode) || IS_QK_LAYER_TAP(keycode);
}
# endif // CHORDAL_HOLD
# ifdef HOLD_ON_OTHER_KEY_PRESS_PER_KEY
__attribute__((weak)) bool get_hold_on_other_key_press(uint16_t keycode, keyrecord_t *record) {
return false;
@ -166,6 +205,20 @@ void action_tapping_process(keyrecord_t record) {
bool process_tapping(keyrecord_t *keyp) {
const keyevent_t event = keyp->event;
# if defined(CHORDAL_HOLD)
if (!event.pressed) {
const int8_t i = registered_tap_find(event.key);
if (i != -1) {
// If a tap-hold key was previously settled as tapped, set its
// tap.count correspondingly on release.
keyp->tap.count = 1;
registered_taps_del_index(i);
ac_dprintf("Found tap release for [%d]\n", i);
debug_registered_taps();
}
}
# endif // CHORDAL_HOLD
// state machine is in the "reset" state, no tapping key is to be
// processed
if (IS_NOEVENT(tapping_key.event)) {
@ -188,7 +241,7 @@ bool process_tapping(keyrecord_t *keyp) {
return true;
}
# if (defined(AUTO_SHIFT_ENABLE) && defined(RETRO_SHIFT)) || defined(PERMISSIVE_HOLD_PER_KEY) || defined(HOLD_ON_OTHER_KEY_PRESS_PER_KEY)
# if (defined(AUTO_SHIFT_ENABLE) && defined(RETRO_SHIFT)) || defined(PERMISSIVE_HOLD_PER_KEY) || defined(CHORDAL_HOLD) || defined(HOLD_ON_OTHER_KEY_PRESS_PER_KEY)
TAP_DEFINE_KEYCODE;
# endif
@ -199,6 +252,7 @@ bool process_tapping(keyrecord_t *keyp) {
// early return for tick events
return true;
}
if (tapping_key.tap.count == 0) {
if (IS_TAPPING_RECORD(keyp) && !event.pressed) {
// first tap!
@ -212,6 +266,25 @@ bool process_tapping(keyrecord_t *keyp) {
// enqueue
return false;
}
# if defined(CHORDAL_HOLD)
else if (is_mt_or_lt(tapping_keycode) && !event.pressed && waiting_buffer_typed(event) && !get_chordal_hold(tapping_keycode, &tapping_key, get_record_keycode(keyp, false), keyp)) {
// Key release that is not a chord with the tapping key.
// Settle the tapping key and any other pending tap-hold
// keys preceding the press of this key as tapped.
ac_dprintf("Tapping: End. Chord considered a tap\n");
tapping_key.tap.count = 1;
registered_taps_add(tapping_key.event.key);
process_record(&tapping_key);
tapping_key = (keyrecord_t){0};
waiting_buffer_chordal_hold_taps_until(event.key);
debug_registered_taps();
debug_waiting_buffer();
// enqueue
return false;
}
# endif // CHORDAL_HOLD
/* Process a key typed within TAPPING_TERM
* This can register the key before settlement of tapping,
* useful for long TAPPING_TERM but may prevent fast typing.
@ -229,6 +302,22 @@ bool process_tapping(keyrecord_t *keyp) {
// clang-format on
ac_dprintf("Tapping: End. No tap. Interfered by typing key\n");
process_record(&tapping_key);
# if defined(CHORDAL_HOLD)
uint8_t first_tap = waiting_buffer_find_chordal_hold_tap();
ac_dprintf("first_tap = %u\n", first_tap);
if (first_tap < WAITING_BUFFER_SIZE) {
for (; waiting_buffer_tail != first_tap; waiting_buffer_tail = (waiting_buffer_tail + 1) % WAITING_BUFFER_SIZE) {
ac_dprintf("Processing [%u]\n", waiting_buffer_tail);
process_record(&waiting_buffer[waiting_buffer_tail]);
}
}
waiting_buffer_chordal_hold_taps_until(event.key);
debug_registered_taps();
debug_waiting_buffer();
# endif // CHORDAL_HOLD
tapping_key = (keyrecord_t){0};
debug_tapping_key();
// enqueue
@ -237,6 +326,19 @@ bool process_tapping(keyrecord_t *keyp) {
/* Process release event of a key pressed before tapping starts
* Without this unexpected repeating will occur with having fast repeating setting
* https://github.com/tmk/tmk_keyboard/issues/60
*
* NOTE: This workaround causes events to process out of order,
* e.g. in a rolled press of three tap-hold keys like
*
* "A down, B down, C down, A up, B up, C up"
*
* events are processed as
*
* "A down, B down, A up, B up, C down, C up"
*
* It seems incorrect to process keyp before the tapping key.
* This workaround is old, from 2013. This might no longer
* be needed for the original problem it was meant to address.
*/
else if (!event.pressed && !waiting_buffer_typed(event)) {
// Modifier/Layer should be retained till end of this tapping.
@ -271,19 +373,52 @@ bool process_tapping(keyrecord_t *keyp) {
// set interrupted flag when other key pressed during tapping
if (event.pressed) {
tapping_key.tap.interrupted = true;
if (TAP_GET_HOLD_ON_OTHER_KEY_PRESS
# if defined(CHORDAL_HOLD)
if (is_mt_or_lt(tapping_keycode) && !get_chordal_hold(tapping_keycode, &tapping_key, get_record_keycode(keyp, false), keyp)) {
// In process_action(), HOLD_ON_OTHER_KEY_PRESS
// will revert interrupted events to holds, so
// this needs to be set false.
tapping_key.tap.interrupted = false;
if (!is_tap_record(keyp)) {
ac_dprintf("Tapping: End. Chord considered a tap\n");
tapping_key.tap.count = 1;
registered_taps_add(tapping_key.event.key);
debug_registered_taps();
process_record(&tapping_key);
tapping_key = (keyrecord_t){0};
}
} else
# endif // CHORDAL_HOLD
if (TAP_GET_HOLD_ON_OTHER_KEY_PRESS
# if defined(AUTO_SHIFT_ENABLE) && defined(RETRO_SHIFT)
// Auto Shift cannot evaluate this early
// Retro Shift uses the hold action for all nested taps even without HOLD_ON_OTHER_KEY_PRESS, so this is fine to skip
&& !(MAYBE_RETRO_SHIFTING(event, keyp) && get_auto_shifted_key(get_record_keycode(keyp, false), keyp))
// Auto Shift cannot evaluate this early
// Retro Shift uses the hold action for all nested taps even without HOLD_ON_OTHER_KEY_PRESS, so this is fine to skip
&& !(MAYBE_RETRO_SHIFTING(event, keyp) && get_auto_shifted_key(get_record_keycode(keyp, false), keyp))
# endif
) {
) {
// Settle the tapping key as *held*, since
// HOLD_ON_OTHER_KEY_PRESS is enabled for this key.
ac_dprintf("Tapping: End. No tap. Interfered by pressed key\n");
process_record(&tapping_key);
tapping_key = (keyrecord_t){0};
# if defined(CHORDAL_HOLD)
if (waiting_buffer_tail != waiting_buffer_head && is_tap_record(&waiting_buffer[waiting_buffer_tail])) {
tapping_key = waiting_buffer[waiting_buffer_tail];
// Pop tail from the queue.
waiting_buffer_tail = (waiting_buffer_tail + 1) % WAITING_BUFFER_SIZE;
debug_waiting_buffer();
} else
# endif // CHORDAL_HOLD
{
tapping_key = (keyrecord_t){0};
}
debug_tapping_key();
// enqueue
return false;
# if defined(CHORDAL_HOLD)
waiting_buffer_process_regular();
# endif // CHORDAL_HOLD
}
}
// enqueue
@ -520,26 +655,125 @@ void waiting_buffer_scan_tap(void) {
}
}
/** \brief Tapping key debug print
*
* FIXME: Needs docs
*/
# ifdef CHORDAL_HOLD
__attribute__((weak)) bool get_chordal_hold(uint16_t tap_hold_keycode, keyrecord_t *tap_hold_record, uint16_t other_keycode, keyrecord_t *other_record) {
return get_chordal_hold_default(tap_hold_record, other_record);
}
bool get_chordal_hold_default(keyrecord_t *tap_hold_record, keyrecord_t *other_record) {
if (tap_hold_record->event.type != KEY_EVENT || other_record->event.type != KEY_EVENT) {
return true; // Return true on combos or other non-key events.
}
char tap_hold_hand = chordal_hold_handedness(tap_hold_record->event.key);
if (tap_hold_hand == '*') {
return true;
}
char other_hand = chordal_hold_handedness(other_record->event.key);
return other_hand == '*' || tap_hold_hand != other_hand;
}
__attribute__((weak)) char chordal_hold_handedness(keypos_t key) {
return (char)pgm_read_byte(&chordal_hold_layout[key.row][key.col]);
}
static void registered_taps_add(keypos_t key) {
if (num_registered_taps >= REGISTERED_TAPS_SIZE) {
ac_dprintf("TAPS OVERFLOW: CLEAR ALL STATES\n");
clear_keyboard();
num_registered_taps = 0;
}
registered_taps[num_registered_taps] = key;
++num_registered_taps;
}
static int8_t registered_tap_find(keypos_t key) {
for (int8_t i = 0; i < num_registered_taps; ++i) {
if (KEYEQ(registered_taps[i], key)) {
return i;
}
}
return -1;
}
static void registered_taps_del_index(uint8_t i) {
if (i < num_registered_taps) {
--num_registered_taps;
if (i < num_registered_taps) {
registered_taps[i] = registered_taps[num_registered_taps];
}
}
}
static void debug_registered_taps(void) {
ac_dprintf("registered_taps = { ");
for (int8_t i = 0; i < num_registered_taps; ++i) {
ac_dprintf("%02X%02X ", registered_taps[i].row, registered_taps[i].col);
}
ac_dprintf("}\n");
}
static uint8_t waiting_buffer_find_chordal_hold_tap(void) {
keyrecord_t *prev = &tapping_key;
uint16_t prev_keycode = get_record_keycode(&tapping_key, false);
uint8_t first_tap = WAITING_BUFFER_SIZE;
for (uint8_t i = waiting_buffer_tail; i != waiting_buffer_head; i = (i + 1) % WAITING_BUFFER_SIZE) {
keyrecord_t * cur = &waiting_buffer[i];
const uint16_t cur_keycode = get_record_keycode(cur, false);
if (!cur->event.pressed || !is_mt_or_lt(prev_keycode)) {
break;
} else if (get_chordal_hold(prev_keycode, prev, cur_keycode, cur)) {
first_tap = i; // Track one index past the latest hold.
}
prev = cur;
prev_keycode = cur_keycode;
}
return first_tap;
}
static void waiting_buffer_chordal_hold_taps_until(keypos_t key) {
while (waiting_buffer_tail != waiting_buffer_head) {
keyrecord_t *record = &waiting_buffer[waiting_buffer_tail];
ac_dprintf("waiting_buffer_chordal_hold_taps_until: processing [%u]\n", waiting_buffer_tail);
if (is_tap_record(record)) {
record->tap.count = 1;
registered_taps_add(record->event.key);
}
process_record(record);
waiting_buffer_tail = (waiting_buffer_tail + 1) % WAITING_BUFFER_SIZE;
if (KEYEQ(key, record->event.key) && record->event.pressed) {
break;
}
}
}
static void waiting_buffer_process_regular(void) {
for (; waiting_buffer_tail != waiting_buffer_head; waiting_buffer_tail = (waiting_buffer_tail + 1) % WAITING_BUFFER_SIZE) {
if (is_tap_record(&waiting_buffer[waiting_buffer_tail])) {
break; // Stop once a tap-hold key event is reached.
}
ac_dprintf("waiting_buffer_process_regular: processing [%u]\n", waiting_buffer_tail);
process_record(&waiting_buffer[waiting_buffer_tail]);
}
debug_waiting_buffer();
}
# endif // CHORDAL_HOLD
/** \brief Logs tapping key if ACTION_DEBUG is enabled. */
static void debug_tapping_key(void) {
ac_dprintf("TAPPING_KEY=");
debug_record(tapping_key);
ac_dprintf("\n");
}
/** \brief Waiting buffer debug print
*
* FIXME: Needs docs
*/
/** \brief Logs waiting buffer if ACTION_DEBUG is enabled. */
static void debug_waiting_buffer(void) {
ac_dprintf("{ ");
ac_dprintf("{");
for (uint8_t i = waiting_buffer_tail; i != waiting_buffer_head; i = (i + 1) % WAITING_BUFFER_SIZE) {
ac_dprintf("[%u]=", i);
ac_dprintf(" [%u]=", i);
debug_record(waiting_buffer[i]);
ac_dprintf(" ");
}
ac_dprintf("}\n");
}