aboutsummaryrefslogtreecommitdiff
path: root/quantum
diff options
context:
space:
mode:
authorPascal Getreuer <[email protected]>2025-01-27 03:32:23 -0800
committerGitHub <[email protected]>2025-01-27 12:32:23 +0100
commit544ddde113657a932275399f577a422a809d2880 (patch)
tree61644740ae434fae3d93231409da1384c64adf95 /quantum
parentee63d39058deee162b794c62d7180d61f540914a (diff)
downloadqmk_firmware-544ddde113657a932275399f577a422a809d2880.tar.gz
qmk_firmware-544ddde113657a932275399f577a422a809d2880.zip
[Core] Add Chordal Hold, an "opposite hands rule" tap-hold option similar to Achordion, Bilateral Combinations. (#24560)
* 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.
Diffstat (limited to 'quantum')
-rw-r--r--quantum/action_tapping.c274
-rw-r--r--quantum/action_tapping.h65
2 files changed, 319 insertions, 20 deletions
diff --git a/quantum/action_tapping.c b/quantum/action_tapping.c
index 8f238490f2..9171970702 100644
--- a/quantum/action_tapping.c
+++ b/quantum/action_tapping.c
@@ -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");
}
diff --git a/quantum/action_tapping.h b/quantum/action_tapping.h
index 6b518b8298..c3c7b999ec 100644
--- a/quantum/action_tapping.h
+++ b/quantum/action_tapping.h
@@ -46,6 +46,71 @@ 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 DYNAMIC_TAPPING_TERM_ENABLE
extern uint16_t g_tapping_term;
#endif