Chordal Hold: docs and further improvements

docs/tap_hold.md

@@ -425,6 +425,167 @@ uint16_t get_quick_tap_term(uint16_t keycode, keyrecord_t *record) {
 If `QUICK_TAP_TERM` is set higher than `TAPPING_TERM`, it will default to `TAPPING_TERM`.
 :::
 
+## Chordal Hold
+
+Chordal Hold is intended to be used together with either Permissive Hold or Hold
+On Other Key Press. Chordal Hold is enabled by adding to your `config.h`:
+
+```c
+#define CHORDAL_HOLD
+```
+
+Chordal Hold implements, by default, an "opposite hands" rule. Suppose a
+tap-hold key is pressed and then, before the tapping term, another key is
+pressed. With Chordal Hold, the tap-hold key is settled as tapped if the two
+keys are on the same hand. This behavior may be useful to avoid accidental
+modifier activation with mod-taps, particularly in rolled keypresses when using
+home row mods.
+
+Notes:
+
+* In the case that the keys are on opposite hands, Chordal Hold alone does not
+  yet settle the tap-hold key. Chordal Hold may be used in combination with Hold
+  On Other Key Press or Permissive Hold to determine the behavior. With Hold On
+  Other Key Press, an opposite hands chord is settled immediately as held. Or
+  with Permissive Hold, an opposite hands chord is settled as held provided the
+  other key is pressed and released (nested press) before releasing the tap-hold
+  key.
+
+* Chordal Hold has no effect after the tapping term. 
+
+* Chordal Hold has no effect when the other key is also a tap-hold key. This is
+  so that multiple tap-hold keys may be held on the same hand, which is common
+  to do with home row mods.
+
+* Combos are exempt from the opposite hands rule, since "handedness" is
+  ill-defined in this case. Even so, Chordal Hold's behavior involving combos
+  may be customized through the `get_chordal_hold()` callback.
+
+An example of a sequence that is affected by “chordal hold”: 
+
+- `SFT_T(KC_A)` Down
+- `KC_C` Down
+- `KC_C` Up
+- `SFT_T(KC_A)` Up
+
+```
+                         TAPPING_TERM   
+  +---------------------------|--------+
+  | +----------------------+  |        |
+  | | SFT_T(KC_A)          |  |        |
+  | +----------------------+  |        |
+  |    +--------------+       |        |
+  |    | KC_C         |       |        |
+  |    +--------------+       |        |
+  +---------------------------|--------+
+```
+
+If the two keys are on the same hand, then this will produce `ac` with 
+`SFT_T(KC_A)` settled as tapped the moment that `KC_C` is pressed. 
+
+If the two keys are on opposite hands and the `HOLD_ON_OTHER_KEY_PRESS`
+option enabled, this will produce `C` with `SFT_T(KC_A)` settled as held the
+moment that `KC_C` is pressed.
+
+Or if the two keys are on opposite hands and the `PERMISSIVE_HOLD` option is
+enabled, this will produce `C` with `SFT_T(KC_A)` settled as held the
+moment that `KC_C` is released.
+
+### Chordal Hold Handedness
+
+Determining whether keys are on the same or opposite hands involves defining the
+"handedness" of each key position. By default, if nothing is specified,
+handedness is guessed based on keyboard matrix dimensions. If this is
+inaccurate, handedness may be specified in several ways.
+
+The easiest way to specify handedness is by `chordal_hold_layout`. Define in
+config.h:
+
+```c
+#define CHORDAL_HOLD_LAYOUT
+```
+
+Then in keymap.c, define `chordal_hold_layout` in the following form:
+
+```c
+const char chordal_hold_layout[MATRIX_ROWS][MATRIX_COLS] PROGMEM =
+    LAYOUT(
+        'L', 'L', 'L', 'L', 'L', 'L',  'R', 'R', 'R', 'R', 'R', 'R', 
+        'L', 'L', 'L', 'L', 'L', 'L',  'R', 'R', 'R', 'R', 'R', 'R', 
+        'L', 'L', 'L', 'L', 'L', 'L',  'R', 'R', 'R', 'R', 'R', 'R', 
+                       'L', 'L', 'L',  'R', 'R', 'R'
+    );
+```
+
+Use the same `LAYOUT` macro as used to define your keymap layers. Each entry is
+a character, either `'L'` for left, `'R'` for right, or `'*'` to exempt keys
+from the "opposite hands rule." When a key has `'*'` handedness, pressing it
+with either hand results in a hold. This could be used perhaps on thumb keys or
+other places where you want to allow same-hand chords.
+
+Alternatively, handedness may be defined functionally with
+`chordal_hold_handedness_user()`. For example, in keymap.c define:
+
+```c
+char chordal_hold_handedness_user(keypos_t key) {
+    if (key.col == 0 || key.col == MATRIX_COLS - 1) {
+        return '*';  // Exempt the outer columns.
+    }
+    // On split keyboards, typically, the first half of the rows are on the
+    // left, and the other half are on the right.
+    return key.row < MATRIX_ROWS / 2 ? 'L' : 'R';
+}
+```
+
+Given the matrix position of a key, the function should return `'L'`, `'R'`, or
+`'*'`. Adapt the logic in this function according to the keyboard's matrix.
+Similarly at the keyboard level, `chordal_hold_handedness_kb()` may be defined
+to specify handedness.
+
+::: warning
+Note the matrix may have irregularities around larger keys, around the edges of
+the board, and around thumb clusters. You may find it helpful to use [this
+debugging example](faq_debug#which-matrix-position-is-this-keypress) to
+correspond physical keys to matrix positions.
+:::
+
+
+### Per-chord customization
+
+Beyond the per-key configuration possible through handedness, Chordal Hold may
+be configured at a *per-chord* granularity for detailed tuning. In keymap.c,
+define `get_chordal_hold()`. Returning true settles the chord as held, while
+returning false settles as tapped.
+
+For example:
+
+```c
+bool get_chordal_hold(uint16_t tap_hold_keycode, keyrecord_t* tap_hold_record,
+                      uint16_t other_keycode, keyrecord_t* other_record) {
+    // Exceptionally allow some one-handed chords for hotkeys.
+    switch (tap_hold_keycode) {
+        case LCTL_T(KC_A):
+           if (other_keycode == KC_C || other_keycode == KC_V) {
+               return true;
+           }
+           break;
+
+        case RCTL_T(KC_SCLN):
+           if (other_keycode == KC_N) {
+               return true;
+           }
+           break;
+    }
+    // Otherwise defer to the opposite hands rule.
+    return get_chordal_hold_default(tap_hold_record, other_record);
+}
+```
+
+As shown above, the function may call `get_chordal_hold_default(tap_hold_record,
+other_record)` to get the default tap vs. hold decision according to the
+opposite hands rule.
+
+
 ## Retro Tapping
 
 To enable `retro tapping`, add the following to your `config.h`:

quantum/action_tapping.c

@@ -50,40 +50,41 @@ __attribute__((weak)) bool get_permissive_hold(uint16_t keycode, keyrecord_t *re
 #    endif
 
 #    ifdef CHORDAL_HOLD
-__attribute__((weak)) bool get_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) {
-        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(
+bool get_chordal_hold_default(keyrecord_t *tap_hold_record, keyrecord_t *other_record) {
-        keyrecord_t* tap_hold_record, keyrecord_t* other_record) {
+    if (tap_hold_record->event.type != KEY_EVENT || other_record->event.type != KEY_EVENT) {
-    uint8_t tap_hold_hand = chordal_hold_handedness_user(tap_hold_record->event.key);
+        return true; // Return true on combos or other non-key events.
-    if (tap_hold_hand == 0) {
+    }
+
+    char tap_hold_hand = chordal_hold_handedness_user(tap_hold_record->event.key);
+    if (tap_hold_hand == '*') {
         return true;
     }
-    uint8_t other_hand = chordal_hold_handedness_user(other_record->event.key);
+    char other_hand = chordal_hold_handedness_user(other_record->event.key);
-    return other_hand == 0 || tap_hold_hand != other_hand;
+    return other_hand == '*' || tap_hold_hand != other_hand;
 }
 
-__attribute__((weak)) uint8_t chordal_hold_handedness_kb(keypos_t key) {
+__attribute__((weak)) char chordal_hold_handedness_kb(keypos_t key) {
 #        if defined(SPLIT_KEYBOARD) || ((MATRIX_ROWS) > (MATRIX_COLS))
-#pragma message "Inferred handedness rows"
+#            pragma message "Inferred handedness rows"
     // If the keyboard is split or if MATRIX_ROWS > MATRIX_COLS, assume that the
     // first half of the rows are left and the latter half are right.
-    return (key.row < (MATRIX_ROWS) / 2) ? /*left*/ 1 : /*right*/ 2;
+    return (key.row < (MATRIX_ROWS) / 2) ? /*left*/ 'L' : /*right*/ 'R';
 #        else
-#pragma message "Inferred handedness cols"
+#            pragma message "Inferred handedness cols"
     // Otherwise, assume the first half of the cols are left, others are right.
-    return (key.col < (MATRIX_COLS) / 2) ? /*left*/ 1 : /*right*/ 2;
+    return (key.col < (MATRIX_COLS) / 2) ? /*left*/ 'L' : /*right*/ 'R';
 #        endif
 }
 
-__attribute__((weak)) uint8_t chordal_hold_handedness_user(keypos_t key) {
+__attribute__((weak)) char chordal_hold_handedness_user(keypos_t key) {
 #        if defined(CHORDAL_HOLD_LAYOUT)
 #            pragma message "Using chordal_hold_layout"
     // If given, read handedness from `chordal_hold_layout` array.
-    return pgm_read_byte(&chordal_hold_layout[key.row][key.col]);
+    return (char)pgm_read_byte(&chordal_hold_layout[key.row][key.col]);
 #        else
     return chordal_hold_handedness_kb(key);
 #        endif
@@ -314,18 +315,17 @@ bool process_tapping(keyrecord_t *keyp) {
                         tapping_key.tap.interrupted = true;
 
 #    if defined(CHORDAL_HOLD)
-                        if (!is_tap_record(keyp) &&
+                        if (!is_tap_record(keyp) && !get_chordal_hold(tapping_keycode, &tapping_key, get_record_keycode(keyp, true), keyp)) {
-                            !get_chordal_hold(tapping_keycode, &tapping_key,
+                            // Settle the tapping key as *tapped*, since it
-                                    get_record_keycode(keyp, true), keyp)) {
+                            // is not considered a held chord with keyp.
-                            // Settle the tapping key as *tapped*, since it is
+                            ac_dprintf("Tapping: End. Chord considered a tap\n");
-                            // not considered a held chord with keyp.
-                            ac_dprintf("Tapping: End. Tap in non-chord\n");
                             tapping_key.tap.count = 1;
-                            // In process_action(), HOLD_ON_OTHER_KEY_PRESS will
+                            // In process_action(), HOLD_ON_OTHER_KEY_PRESS
-                            // revert interrupted events to holds, so this needs
+                            // will revert interrupted events to holds, so
-                            // to be set false.
+                            // this needs to be set false.
                             tapping_key.tap.interrupted = false;
                             process_record(&tapping_key);
+                            debug_tapping_key();
                         } else
 #    endif
                             if (TAP_GET_HOLD_ON_OTHER_KEY_PRESS
@@ -341,8 +341,6 @@ bool process_tapping(keyrecord_t *keyp) {
                             process_record(&tapping_key);
                             tapping_key = (keyrecord_t){0};
                             debug_tapping_key();
-                            // enqueue
-                            return false;
                         }
                     }
                     // enqueue

quantum/action_tapping.h

@@ -48,7 +48,7 @@ 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 is considered held.
+ * Callback to say when a key chord before the tapping term may be held.
  *
  * In keymap.c, define the callback
  *
@@ -66,27 +66,27 @@ bool     get_hold_on_other_key_press(uint16_t keycode, keyrecord_t *record);
  *     provided `other_keycode` is *not* also a tap-hold key and it is pressed
  *     before the tapping term.
  *
- * Returning true indicates that the tap-hold key should be considered held, or
+ * If false is returned, this has the effect of immediately settling the
- * false to consider it tapped.
+ * 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 is considered held; false if tapped.
+ * @return True if the tap-hold key may be considered held; false if tapped.
  */
-bool get_chordal_hold(
+bool get_chordal_hold(uint16_t tap_hold_keycode, keyrecord_t *tap_hold_record, uint16_t other_keycode, keyrecord_t *other_record);
-    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 should settle as held.
+ * 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 zero, the function
+ * handedness values differ, or if either handedness is '*', the function
- * returns true, indicating a hold. Otherwise, it returns false, indicating that
+ * returns true, indicating that it may be held. Otherwise, it returns false,
- * the tap-hold key should settle as tapped.
+ * in which case the tap-hold key is immediately settled at tapped.
  *
  * "Handedness" is determined as follows, in order of decending precedence:
  * 1. `chordal_hold_handedness_user()`, if defined.
@@ -96,29 +96,28 @@ bool get_chordal_hold(
  *
  * @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 is considered held; false if tapped.
+ * @return True if the tap-hold key may be considered held; false if tapped.
  */
-bool get_chordal_hold_default(
+bool get_chordal_hold_default(keyrecord_t *tap_hold_record, keyrecord_t *other_record);
-    keyrecord_t* tap_hold_record, keyrecord_t* other_record);
 
 /**
  * Keyboard-level callback to determine handedness of a key.
  *
  * This function should return:
- *   1 for keys pressed by the left hand,
+ *   'L' for keys pressed by the left hand,
- *   2 for keys on the right hand,
+ *   'R' for keys on the right hand,
- *   0 for keys exempt from the "opposite hands rule." This could be used
+ *   '*' 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.
  */
-uint8_t chordal_hold_handedness_kb(keypos_t key);
+char chordal_hold_handedness_kb(keypos_t key);
 /** User callback to determine handedness of a key. */
-uint8_t chordal_hold_handedness_user(keypos_t key);
+char chordal_hold_handedness_user(keypos_t key);
 
 #    ifdef CHORDAL_HOLD_LAYOUT
-extern const uint8_t chordal_hold_layout[MATRIX_ROWS][MATRIX_COLS] PROGMEM;
+extern const char chordal_hold_layout[MATRIX_ROWS][MATRIX_COLS] PROGMEM;
 #    endif
 #endif
 

tests/tap_hold_configurations/chordal_hold/config.h

@@ -1,21 +0,0 @@
-/* Copyright 2022 Vladislav Kucheriavykh
- *
- * 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
-
-#include "test_common.h"
-#define CHORDAL_HOLD
-

tests/tap_hold_configurations/chordal_hold/test.mk

@@ -1,18 +0,0 @@
-# Copyright 2022 Vladislav Kucheriavykh
-#
-# 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/>.
-
-# --------------------------------------------------------------------------------
-# Keep this file, even if it is empty, as a marker that this folder contains tests
-# --------------------------------------------------------------------------------

tests/tap_hold_configurations/chordal_hold/test_tap_hold.cpp

@@ -1,167 +0,0 @@
-/* Copyright 2022 Vladislav Kucheriavykh
- *
- * 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/>.
- */
-
-#include "keyboard_report_util.hpp"
-#include "keycode.h"
-#include "test_common.hpp"
-#include "action_tapping.h"
-#include "test_fixture.hpp"
-#include "test_keymap_key.hpp"
-
-using testing::_;
-using testing::InSequence;
-
-class ChordalHold : public TestFixture {};
-
-TEST_F(ChordalHold, chord_with_mod_tap_settled_as_hold) {
-    TestDriver driver;
-    InSequence s;
-    // Mod-tap key on the left hand.
-    auto       mod_tap_hold_key = KeymapKey(0, 1, 0, SFT_T(KC_P));
-    // Regular key on the right hand.
-    auto       regular_key      = KeymapKey(0, MATRIX_COLS - 1, 0, KC_A);
-
-    set_keymap({mod_tap_hold_key, regular_key});
-
-    /* Press mod-tap-hold key. */
-    EXPECT_NO_REPORT(driver);
-    mod_tap_hold_key.press();
-    run_one_scan_loop();
-    VERIFY_AND_CLEAR(driver);
-
-    /* Press regular key. */
-    EXPECT_REPORT(driver, (KC_LEFT_SHIFT));
-    EXPECT_REPORT(driver, (KC_LEFT_SHIFT, KC_A));
-    regular_key.press();
-    idle_for(TAPPING_TERM);
-    VERIFY_AND_CLEAR(driver);
-
-    /* Release regular key. */
-    EXPECT_REPORT(driver, (KC_LEFT_SHIFT));
-    regular_key.release();
-    run_one_scan_loop();
-    VERIFY_AND_CLEAR(driver);
-
-    /* Release mod-tap-hold key. */
-    EXPECT_EMPTY_REPORT(driver);
-    mod_tap_hold_key.release();
-    run_one_scan_loop();
-    VERIFY_AND_CLEAR(driver);
-}
-
-TEST_F(ChordalHold, non_chord_with_mod_tap_settled_as_tap) {
-    TestDriver driver;
-    InSequence s;
-    // Mod-tap key and regular key both on the left hand.
-    auto       mod_tap_hold_key = KeymapKey(0, 1, 0, SFT_T(KC_P));
-    auto       regular_key      = KeymapKey(0, 2, 0, KC_A);
-
-    set_keymap({mod_tap_hold_key, regular_key});
-
-    /* Press mod-tap-hold key. */
-    EXPECT_NO_REPORT(driver);
-    mod_tap_hold_key.press();
-    run_one_scan_loop();
-    VERIFY_AND_CLEAR(driver);
-
-    /* Press regular key. */
-    EXPECT_REPORT(driver, (KC_P));
-    EXPECT_REPORT(driver, (KC_P, KC_A));
-    regular_key.press();
-    run_one_scan_loop();
-    VERIFY_AND_CLEAR(driver);
-
-    /* Release regular key. */
-    EXPECT_REPORT(driver, (KC_P));
-    regular_key.release();
-    idle_for(TAPPING_TERM);
-    VERIFY_AND_CLEAR(driver);
-
-    /* Release mod-tap-hold key. */
-    EXPECT_EMPTY_REPORT(driver);
-    mod_tap_hold_key.release();
-    run_one_scan_loop();
-    VERIFY_AND_CLEAR(driver);
-}
-
-TEST_F(ChordalHold, tap_mod_tap_key) {
-    TestDriver driver;
-    InSequence s;
-    auto mod_tap_key = KeymapKey(0, 1, 0, SFT_T(KC_P));
-
-    set_keymap({mod_tap_key});
-
-    EXPECT_NO_REPORT(driver);
-    mod_tap_key.press();
-    idle_for(TAPPING_TERM - 1);
-    VERIFY_AND_CLEAR(driver);
-
-    EXPECT_REPORT(driver, (KC_P));
-    EXPECT_EMPTY_REPORT(driver);
-    mod_tap_key.release();
-    run_one_scan_loop();
-    VERIFY_AND_CLEAR(driver);
-}
-
-TEST_F(ChordalHold, hold_mod_tap_key) {
-    TestDriver driver;
-    InSequence s;
-    auto mod_tap_key = KeymapKey(0, 1, 0, SFT_T(KC_P));
-
-    set_keymap({mod_tap_key});
-
-    EXPECT_REPORT(driver, (KC_LEFT_SHIFT));
-    mod_tap_key.press();
-    idle_for(TAPPING_TERM + 1);
-    VERIFY_AND_CLEAR(driver);
-
-    EXPECT_EMPTY_REPORT(driver);
-    mod_tap_key.release();
-    run_one_scan_loop();
-    VERIFY_AND_CLEAR(driver);
-}
-
-TEST_F(ChordalHold, chordal_hold_ignores_multiple_mod_taps) {
-    TestDriver driver;
-    InSequence s;
-    auto       mod_tap_key1 = KeymapKey(0, 1, 0, SFT_T(KC_A));
-    auto       mod_tap_key2 = KeymapKey(0, 2, 0, RSFT_T(KC_B));
-
-    set_keymap({mod_tap_key1, mod_tap_key2});
-
-    /* Press mod-tap-hold key. */
-    EXPECT_NO_REPORT(driver);
-    mod_tap_key1.press();
-    run_one_scan_loop();
-    VERIFY_AND_CLEAR(driver);
-
-    /* Press second mod-tap key. */
-    EXPECT_REPORT(driver, (KC_LEFT_SHIFT));
-    EXPECT_REPORT(driver, (KC_LEFT_SHIFT, KC_RIGHT_SHIFT));
-    mod_tap_key2.press();
-    idle_for(TAPPING_TERM + 1);
-    VERIFY_AND_CLEAR(driver);
-
-    /* Release keys. */
-    EXPECT_REPORT(driver, (KC_RIGHT_SHIFT));
-    EXPECT_EMPTY_REPORT(driver);
-    mod_tap_key1.release();
-    run_one_scan_loop();
-    mod_tap_key2.release();
-    run_one_scan_loop();
-    VERIFY_AND_CLEAR(driver);
-}
-

tests/tap_hold_configurations/chordal_hold_and_hold_on_other_key_press/test_tap_hold.cpp

@@ -30,34 +30,97 @@ TEST_F(ChordalHoldAndHoldOnOtherKeypress, chord_with_mod_tap_settled_as_hold) {
     TestDriver driver;
     InSequence s;
     // Mod-tap key on the left hand.
-    auto       mod_tap_hold_key = KeymapKey(0, 1, 0, SFT_T(KC_P));
+    auto mod_tap_key = KeymapKey(0, 1, 0, SFT_T(KC_P));
     // Regular key on the right hand.
     auto regular_key = KeymapKey(0, MATRIX_COLS - 1, 0, KC_A);
 
-    set_keymap({mod_tap_hold_key, regular_key});
+    set_keymap({mod_tap_key, regular_key});
 
-    /* Press mod-tap-hold key. */
+    // Press mod-tap-hold key.
     EXPECT_NO_REPORT(driver);
-    mod_tap_hold_key.press();
+    mod_tap_key.press();
     run_one_scan_loop();
     VERIFY_AND_CLEAR(driver);
 
-    /* Press regular key. */
+    // Press regular key.
     EXPECT_REPORT(driver, (KC_LEFT_SHIFT));
     EXPECT_REPORT(driver, (KC_LEFT_SHIFT, KC_A));
     regular_key.press();
-    idle_for(TAPPING_TERM);
+    run_one_scan_loop();
     VERIFY_AND_CLEAR(driver);
 
-    /* Release regular key. */
+    // Release regular key.
     EXPECT_REPORT(driver, (KC_LEFT_SHIFT));
     regular_key.release();
     run_one_scan_loop();
     VERIFY_AND_CLEAR(driver);
 
-    /* Release mod-tap-hold key. */
+    // Release mod-tap-hold key.
     EXPECT_EMPTY_REPORT(driver);
-    mod_tap_hold_key.release();
+    mod_tap_key.release();
+    run_one_scan_loop();
+    VERIFY_AND_CLEAR(driver);
+}
+
+TEST_F(ChordalHoldAndHoldOnOtherKeypress, chord_nested_press_settled_as_hold) {
+    TestDriver driver;
+    InSequence s;
+    // Mod-tap key on the left hand.
+    auto mod_tap_key = KeymapKey(0, 1, 0, SFT_T(KC_P));
+    // Regular key on the right hand.
+    auto regular_key = KeymapKey(0, MATRIX_COLS - 1, 0, KC_A);
+
+    set_keymap({mod_tap_key, regular_key});
+
+    // Press mod-tap-hold key.
+    EXPECT_NO_REPORT(driver);
+    mod_tap_key.press();
+    run_one_scan_loop();
+    VERIFY_AND_CLEAR(driver);
+
+    // Tap regular key.
+    EXPECT_REPORT(driver, (KC_LEFT_SHIFT));
+    EXPECT_REPORT(driver, (KC_LEFT_SHIFT, KC_A));
+    EXPECT_REPORT(driver, (KC_LEFT_SHIFT));
+    tap_key(regular_key);
+    VERIFY_AND_CLEAR(driver);
+
+    // Release mod-tap-hold key.
+    EXPECT_EMPTY_REPORT(driver);
+    mod_tap_key.release();
+    run_one_scan_loop();
+    VERIFY_AND_CLEAR(driver);
+}
+
+TEST_F(ChordalHoldAndHoldOnOtherKeypress, chord_rolled_press_settled_as_hold) {
+    TestDriver driver;
+    InSequence s;
+    // Mod-tap key on the left hand.
+    auto mod_tap_key = KeymapKey(0, 1, 0, SFT_T(KC_P));
+    // Regular key on the right hand.
+    auto regular_key = KeymapKey(0, MATRIX_COLS - 1, 0, KC_A);
+
+    set_keymap({mod_tap_key, regular_key});
+
+    // Press mod-tap key.
+    EXPECT_NO_REPORT(driver);
+    mod_tap_key.press();
+    run_one_scan_loop();
+    VERIFY_AND_CLEAR(driver);
+
+    // Press regular key and release mod-tap key.
+    EXPECT_REPORT(driver, (KC_LEFT_SHIFT));
+    EXPECT_REPORT(driver, (KC_LEFT_SHIFT, KC_A));
+    EXPECT_REPORT(driver, (KC_A));
+    regular_key.press();
+    run_one_scan_loop();
+    mod_tap_key.release();
+    run_one_scan_loop();
+    VERIFY_AND_CLEAR(driver);
+
+    // Release regular key.
+    EXPECT_EMPTY_REPORT(driver);
+    regular_key.release();
     run_one_scan_loop();
     VERIFY_AND_CLEAR(driver);
 }
@@ -66,33 +129,33 @@ TEST_F(ChordalHoldAndHoldOnOtherKeypress, non_chord_with_mod_tap_settled_as_tap)
     TestDriver driver;
     InSequence s;
     // Mod-tap key and regular key both on the left hand.
-    auto       mod_tap_hold_key = KeymapKey(0, 1, 0, SFT_T(KC_P));
+    auto mod_tap_key = KeymapKey(0, 1, 0, SFT_T(KC_P));
     auto regular_key = KeymapKey(0, 2, 0, KC_A);
 
-    set_keymap({mod_tap_hold_key, regular_key});
+    set_keymap({mod_tap_key, regular_key});
 
-    /* Press mod-tap-hold key. */
+    // Press mod-tap-hold key.
     EXPECT_NO_REPORT(driver);
-    mod_tap_hold_key.press();
+    mod_tap_key.press();
     run_one_scan_loop();
     VERIFY_AND_CLEAR(driver);
 
-    /* Press regular key. */
+    // Press regular key.
     EXPECT_REPORT(driver, (KC_P));
     EXPECT_REPORT(driver, (KC_P, KC_A));
     regular_key.press();
     run_one_scan_loop();
     VERIFY_AND_CLEAR(driver);
 
-    /* Release regular key. */
+    // Release regular key.
     EXPECT_REPORT(driver, (KC_P));
     regular_key.release();
-    idle_for(TAPPING_TERM);
+    run_one_scan_loop();
     VERIFY_AND_CLEAR(driver);
 
-    /* Release mod-tap-hold key. */
+    // Release mod-tap-hold key.
     EXPECT_EMPTY_REPORT(driver);
-    mod_tap_hold_key.release();
+    mod_tap_key.release();
     run_one_scan_loop();
     VERIFY_AND_CLEAR(driver);
 }
@@ -142,20 +205,20 @@ TEST_F(ChordalHoldAndHoldOnOtherKeypress, chordal_hold_ignores_multiple_mod_taps
 
     set_keymap({mod_tap_key1, mod_tap_key2});
 
-    /* Press mod-tap-hold key. */
+    // Press mod-tap-hold key.
     EXPECT_NO_REPORT(driver);
     mod_tap_key1.press();
     run_one_scan_loop();
     VERIFY_AND_CLEAR(driver);
 
-    /* Press second mod-tap key. */
+    // Press second mod-tap key.
     EXPECT_REPORT(driver, (KC_LEFT_SHIFT));
     EXPECT_REPORT(driver, (KC_LEFT_SHIFT, KC_RIGHT_SHIFT));
     mod_tap_key2.press();
     idle_for(TAPPING_TERM + 1);
     VERIFY_AND_CLEAR(driver);
 
-    /* Release keys. */
+    // Release keys.
     EXPECT_REPORT(driver, (KC_RIGHT_SHIFT));
     EXPECT_EMPTY_REPORT(driver);
     mod_tap_key1.release();
@@ -164,4 +227,3 @@ TEST_F(ChordalHoldAndHoldOnOtherKeypress, chordal_hold_ignores_multiple_mod_taps
     run_one_scan_loop();
     VERIFY_AND_CLEAR(driver);
 }
-

tests/tap_hold_configurations/chordal_hold_and_permissive_hold/config.h

@@ -18,5 +18,6 @@
 
 #include "test_common.h"
 #define CHORDAL_HOLD
+#define CHORDAL_HOLD_LAYOUT
 #define PERMISSIVE_HOLD
 

tests/tap_hold_configurations/chordal_hold_and_permissive_hold/test.mk

@@ -13,6 +13,6 @@
 # You should have received a copy of the GNU General Public License
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
-# --------------------------------------------------------------------------------
+COMBO_ENABLE = yes
-# Keep this file, even if it is empty, as a marker that this folder contains tests
+
-# --------------------------------------------------------------------------------
+INTROSPECTION_KEYMAP_C = test_keymap.c

tests/tap_hold_configurations/chordal_hold_and_permissive_hold/test_keymap.c

@@ -0,0 +1,7 @@
+#include "quantum.h"
+
+const uint16_t ab_combo[] = {KC_A, KC_B, COMBO_END};
+
+combo_t key_combos[] = {
+    COMBO(ab_combo, KC_X),
+};

tests/tap_hold_configurations/chordal_hold_and_permissive_hold/test_tap_hold.cpp

@@ -24,40 +24,121 @@
 using testing::_;
 using testing::InSequence;
 
+extern "C" {
+const char chordal_hold_layout[MATRIX_ROWS][MATRIX_COLS] PROGMEM = {
+    {'L', 'L', 'L', 'L', 'L', 'R', 'R', 'R', 'R', 'R'},
+    {'L', 'L', 'L', 'L', 'L', 'R', 'R', 'R', 'R', 'R'},
+    {'*', 'L', 'L', 'L', 'L', 'R', 'R', 'R', 'R', 'R'},
+    {'L', 'L', 'L', 'L', 'L', 'R', 'R', 'R', 'R', 'R'},
+};
+} // extern "C"
+
 class ChordalHoldAndPermissiveHold : public TestFixture {};
 
-TEST_F(ChordalHoldAndPermissiveHold, chord_with_mod_tap_settled_as_hold) {
+TEST_F(ChordalHoldAndPermissiveHold, chordal_hold_handedness) {
+    EXPECT_EQ(chordal_hold_handedness_user({.col = 0, .row = 0}), 'L');
+    EXPECT_EQ(chordal_hold_handedness_user({.col = MATRIX_COLS - 1, .row = 0}), 'R');
+    EXPECT_EQ(chordal_hold_handedness_user({.col = 0, .row = 2}), '*');
+}
+
+TEST_F(ChordalHoldAndPermissiveHold, get_chordal_hold_default) {
+    auto make_record = [](uint8_t row, uint8_t col, keyevent_type_t type = KEY_EVENT, uint16_t keycode = KC_NO) {
+        return keyrecord_t{
+            .event =
+                {
+                    .key     = {.col = col, .row = row},
+                    .type    = type,
+                    .pressed = true,
+                },
+            .keycode = keycode,
+        };
+    };
+    // Create two records on the left hand.
+    keyrecord_t record_l0 = make_record(0, 0);
+    keyrecord_t record_l1 = make_record(1, 0);
+    // Create a record on the right hand.
+    keyrecord_t record_r = make_record(0, MATRIX_COLS - 1);
+
+    // Function should return true when records are on opposite hands.
+    EXPECT_TRUE(get_chordal_hold_default(&record_l0, &record_r));
+    EXPECT_TRUE(get_chordal_hold_default(&record_r, &record_l0));
+    // ... and false when on the same hand.
+    EXPECT_FALSE(get_chordal_hold_default(&record_l0, &record_l1));
+    EXPECT_FALSE(get_chordal_hold_default(&record_l1, &record_l0));
+    // But (2, 0) has handedness '*', for which true is returned for chords
+    // with either hand.
+    keyrecord_t record_l2 = make_record(2, 0);
+    EXPECT_TRUE(get_chordal_hold_default(&record_l2, &record_l0));
+    EXPECT_TRUE(get_chordal_hold_default(&record_l2, &record_r));
+
+    // Create a record resulting from a combo.
+    keyrecord_t record_combo = make_record(0, 0, COMBO_EVENT, KC_X);
+    // Function returns true in all cases.
+    EXPECT_TRUE(get_chordal_hold_default(&record_l0, &record_combo));
+    EXPECT_TRUE(get_chordal_hold_default(&record_r, &record_combo));
+    EXPECT_TRUE(get_chordal_hold_default(&record_combo, &record_l0));
+    EXPECT_TRUE(get_chordal_hold_default(&record_combo, &record_r));
+}
+
+TEST_F(ChordalHoldAndPermissiveHold, chord_nested_press_settled_as_hold) {
     TestDriver driver;
     InSequence s;
     // Mod-tap key on the left hand.
-    auto       mod_tap_hold_key = KeymapKey(0, 1, 0, SFT_T(KC_P));
+    auto mod_tap_key = KeymapKey(0, 1, 0, SFT_T(KC_P));
     // Regular key on the right hand.
     auto regular_key = KeymapKey(0, MATRIX_COLS - 1, 0, KC_A);
 
-    set_keymap({mod_tap_hold_key, regular_key});
+    set_keymap({mod_tap_key, regular_key});
 
-    /* Press mod-tap-hold key. */
+    // Press mod-tap key.
     EXPECT_NO_REPORT(driver);
-    mod_tap_hold_key.press();
+    mod_tap_key.press();
     run_one_scan_loop();
     VERIFY_AND_CLEAR(driver);
 
-    /* Press regular key. */
+    // Tap regular key.
     EXPECT_REPORT(driver, (KC_LEFT_SHIFT));
     EXPECT_REPORT(driver, (KC_LEFT_SHIFT, KC_A));
-    regular_key.press();
+    EXPECT_REPORT(driver, (KC_LEFT_SHIFT));
-    idle_for(TAPPING_TERM);
+    tap_key(regular_key);
     VERIFY_AND_CLEAR(driver);
 
-    /* Release regular key. */
+    // Release mod-tap key.
-    EXPECT_REPORT(driver, (KC_LEFT_SHIFT));
+    EXPECT_EMPTY_REPORT(driver);
-    regular_key.release();
+    mod_tap_key.release();
+    run_one_scan_loop();
+    VERIFY_AND_CLEAR(driver);
+}
+
+TEST_F(ChordalHoldAndPermissiveHold, chord_rolled_press_settled_as_tap) {
+    TestDriver driver;
+    InSequence s;
+    // Mod-tap key on the left hand.
+    auto mod_tap_key = KeymapKey(0, 1, 0, SFT_T(KC_P));
+    // Regular key on the right hand.
+    auto regular_key = KeymapKey(0, MATRIX_COLS - 1, 0, KC_A);
+
+    set_keymap({mod_tap_key, regular_key});
+
+    // Press mod-tap key.
+    EXPECT_NO_REPORT(driver);
+    mod_tap_key.press();
     run_one_scan_loop();
     VERIFY_AND_CLEAR(driver);
 
-    /* Release mod-tap-hold key. */
+    // Press regular key and release mod-tap key.
+    EXPECT_REPORT(driver, (KC_P));
+    EXPECT_REPORT(driver, (KC_P, KC_A));
+    EXPECT_REPORT(driver, (KC_A));
+    regular_key.press();
+    run_one_scan_loop();
+    mod_tap_key.release();
+    run_one_scan_loop();
+    VERIFY_AND_CLEAR(driver);
+
+    // Release regular key.
     EXPECT_EMPTY_REPORT(driver);
-    mod_tap_hold_key.release();
+    regular_key.release();
     run_one_scan_loop();
     VERIFY_AND_CLEAR(driver);
 }
@@ -66,33 +147,28 @@ TEST_F(ChordalHoldAndPermissiveHold, non_chord_with_mod_tap_settled_as_tap) {
     TestDriver driver;
     InSequence s;
     // Mod-tap key and regular key both on the left hand.
-    auto       mod_tap_hold_key = KeymapKey(0, 1, 0, SFT_T(KC_P));
+    auto mod_tap_key = KeymapKey(0, 1, 0, SFT_T(KC_P));
     auto regular_key = KeymapKey(0, 2, 0, KC_A);
 
-    set_keymap({mod_tap_hold_key, regular_key});
+    set_keymap({mod_tap_key, regular_key});
 
-    /* Press mod-tap-hold key. */
+    // Press mod-tap-hold key.
     EXPECT_NO_REPORT(driver);
-    mod_tap_hold_key.press();
+    mod_tap_key.press();
     run_one_scan_loop();
     VERIFY_AND_CLEAR(driver);
 
-    /* Press regular key. */
+    // Tap regular key.
     EXPECT_REPORT(driver, (KC_P));
     EXPECT_REPORT(driver, (KC_P, KC_A));
-    regular_key.press();
+    EXPECT_REPORT(driver, (KC_P));
+    tap_key(regular_key);
     run_one_scan_loop();
     VERIFY_AND_CLEAR(driver);
 
-    /* Release regular key. */
+    // Release mod-tap-hold key.
-    EXPECT_REPORT(driver, (KC_P));
-    regular_key.release();
-    idle_for(TAPPING_TERM);
-    VERIFY_AND_CLEAR(driver);
-
-    /* Release mod-tap-hold key. */
     EXPECT_EMPTY_REPORT(driver);
-    mod_tap_hold_key.release();
+    mod_tap_key.release();
     run_one_scan_loop();
     VERIFY_AND_CLEAR(driver);
 }
@@ -142,20 +218,20 @@ TEST_F(ChordalHoldAndPermissiveHold, chordal_hold_ignores_multiple_mod_taps) {
 
     set_keymap({mod_tap_key1, mod_tap_key2});
 
-    /* Press mod-tap-hold key. */
+    // Press mod-tap-hold key.
     EXPECT_NO_REPORT(driver);
     mod_tap_key1.press();
     run_one_scan_loop();
     VERIFY_AND_CLEAR(driver);
 
-    /* Press second mod-tap key. */
+    // Press second mod-tap key.
     EXPECT_REPORT(driver, (KC_LEFT_SHIFT));
     EXPECT_REPORT(driver, (KC_LEFT_SHIFT, KC_RIGHT_SHIFT));
     mod_tap_key2.press();
     idle_for(TAPPING_TERM + 1);
     VERIFY_AND_CLEAR(driver);
 
-    /* Release keys. */
+    // Release keys.
     EXPECT_REPORT(driver, (KC_RIGHT_SHIFT));
     EXPECT_EMPTY_REPORT(driver);
     mod_tap_key1.release();
@@ -164,4 +240,3 @@ TEST_F(ChordalHoldAndPermissiveHold, chordal_hold_ignores_multiple_mod_taps) {
     run_one_scan_loop();
     VERIFY_AND_CLEAR(driver);
 }
-