Generate a default chordal_hold_layout.

data/schemas/keyboard.jsonschema

@@ -384,7 +384,11 @@
                                 "h": {"$ref": "qmk.definitions.v1#/key_unit"},
                                 "w": {"$ref": "qmk.definitions.v1#/key_unit"},
                                 "x": {"$ref": "qmk.definitions.v1#/key_unit"},
-                                "y": {"$ref": "qmk.definitions.v1#/key_unit"}
+                                "y": {"$ref": "qmk.definitions.v1#/key_unit"},
+                                "hand": {
+                                    "type": "string",
+                                    "pattern": "[LR*]",
+                                }
                             }
                         }
                     }

docs/tap_hold.md

@@ -491,17 +491,10 @@ moment that `KC_C` is released.
 
 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
+handedness is guessed based on keyboard geometry.
-inaccurate, handedness may be specified in several ways.
 
-The easiest way to specify handedness is by `chordal_hold_layout`. Define in
+Handedness may be specified with `chordal_hold_layout`. In keymap.c, define
-config.h:
+`chordal_hold_layout` in the following form:
-
-```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 =
@@ -514,37 +507,21 @@ const char chordal_hold_layout[MATRIX_ROWS][MATRIX_COLS] PROGMEM =
 ```
 
 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
+a character indicating the handedness of one key, either `'L'` for left, `'R'`
-from the "opposite hands rule." When a key has `'*'` handedness, pressing it
+for right, or `'*'` to exempt keys from the "opposite hands rule." When a key
-with either hand results in a hold. This could be used perhaps on thumb keys or
+has `'*'` handedness, pressing it with either hand results in a hold. This could
-other places where you want to allow same-hand chords.
+be used perhaps on thumb keys or other places where you want to allow same-hand
+chords.
 
-Alternatively, handedness may be defined functionally with
+Keyboard makers may specify handedness in keyboard.json. Under `"layouts"`,
-`chordal_hold_handedness_user()`. For example, in keymap.c define:
+specify the handedness of a key by adding a `"hand"` field with a value of
+either `"L"`, `"R"`, or `"*"`. Note that if `"layouts"` contains multiple
+layouts, only the first one is read. For example:
 
-```c
+```json
-char chordal_hold_handedness_user(keypos_t key) {
+{"matrix": [5, 6], "x": 0, "y": 5.5, "w": 1.25, "hand", "*"},
-    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
 

lib/python/qmk/cli/generate/keyboard_c.py

@@ -1,5 +1,7 @@
 """Used by the make system to generate keyboard.c from info.json.
 """
+import statistics
+
 from milc import cli
 
 from qmk.info import info_json
@@ -87,6 +89,55 @@ def _gen_matrix_mask(info_data):
         lines.append(f'    0b{"".join(reversed(mask[i]))},')
     lines.append('};')
     lines.append('#endif')
+    lines.append('')
+
+    return lines
+
+
+def _gen_chordal_hold_layout(info_data):
+    keys_x = []
+    keys_hand = []
+
+    # Get x-coordinate for the center of each key.
+    # NOTE: If there are multiple layouts, only the first is read.
+    for layout_name, layout_data in info_data['layouts'].items():
+        for key_data in layout_data['layout']:
+            keys_x.append(key_data['x'] + key_data.get('w', 1.0) / 2)
+            keys_hand.append(key_data.get('hand', ''))
+        break
+
+    x_midline = statistics.median(keys_x)
+    x_prev = None
+
+    layout_handedness = [[]]
+
+    for x, hand in zip(keys_x, keys_hand):
+        if x_prev is not None and x < x_prev:
+            layout_handedness.append([])
+
+        if not hand:
+            # Where unspecified, assume handedness based on the key's location
+            # relative to the midline.
+            if abs(x - x_midline) > 0.25:
+                hand = 'L' if x < x_midline else 'R'
+            else:
+                hand = '*'
+
+        layout_handedness[-1].append(hand)
+        x_prev = x
+
+    lines = []
+    lines.append('#ifdef CHORDAL_HOLD')
+    lines.append('__attribute__((weak)) const char chordal_hold_layout[MATRIX_ROWS][MATRIX_COLS] PROGMEM = ' + layout_name + '(')
+
+    for i, row in enumerate(layout_handedness):
+        line = '  ' + ', '.join(f"'{c}'" for c in row)
+        if i < len(layout_handedness) - 1:
+            line += ','
+        lines.append(line)
+
+    lines.append(');')
+    lines.append('#endif')
 
     return lines
 
@@ -101,10 +152,11 @@ def generate_keyboard_c(cli):
     kb_info_json = info_json(cli.args.keyboard)
 
     # Build the layouts.h file.
-    keyboard_h_lines = [GPL2_HEADER_C_LIKE, GENERATED_HEADER_C_LIKE, '#include QMK_KEYBOARD_H', '']
+    keyboard_c_lines = [GPL2_HEADER_C_LIKE, GENERATED_HEADER_C_LIKE, '#include QMK_KEYBOARD_H', '']
 
-    keyboard_h_lines.extend(_gen_led_configs(kb_info_json))
+    keyboard_c_lines.extend(_gen_led_configs(kb_info_json))
-    keyboard_h_lines.extend(_gen_matrix_mask(kb_info_json))
+    keyboard_c_lines.extend(_gen_matrix_mask(kb_info_json))
+    keyboard_c_lines.extend(_gen_chordal_hold_layout(kb_info_json))
 
     # Show the results
-    dump_lines(cli.args.output, keyboard_h_lines, cli.args.quiet)
+    dump_lines(cli.args.output, keyboard_c_lines, cli.args.quiet)

quantum/action_tapping.c

@@ -50,6 +50,8 @@ __attribute__((weak)) bool get_permissive_hold(uint16_t keycode, keyrecord_t *re
 #    endif
 
 #    ifdef 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] = {};
@@ -609,32 +611,16 @@ bool get_chordal_hold_default(keyrecord_t *tap_hold_record, keyrecord_t *other_r
         return true; // Return true on combos or other non-key events.
     }
 
-    char tap_hold_hand = chordal_hold_handedness_user(tap_hold_record->event.key);
+    char tap_hold_hand = chordal_hold_handedness(tap_hold_record->event.key);
     if (tap_hold_hand == '*') {
         return true;
     }
-    char other_hand = chordal_hold_handedness_user(other_record->event.key);
+    char other_hand = chordal_hold_handedness(other_record->event.key);
     return other_hand == '*' || tap_hold_hand != other_hand;
 }
 
-__attribute__((weak)) char chordal_hold_handedness_kb(keypos_t key) {
+__attribute__((weak)) char chordal_hold_handedness(keypos_t key) {
-#        if defined(SPLIT_KEYBOARD) || ((MATRIX_ROWS) > (MATRIX_COLS))
-    // 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*/ 'L' : /*right*/ 'R';
-#        else
-    // Otherwise, assume the first half of the cols are left, others are right.
-    return (key.col < (MATRIX_COLS) / 2) ? /*left*/ 'L' : /*right*/ 'R';
-#        endif
-}
-
-__attribute__((weak)) char chordal_hold_handedness_user(keypos_t key) {
-#        if defined(CHORDAL_HOLD_LAYOUT)
-    // If given, read handedness from `chordal_hold_layout` array.
     return (char)pgm_read_byte(&chordal_hold_layout[key.row][key.col]);
-#        else
-    return chordal_hold_handedness_kb(key);
-#        endif
 }
 
 static void registered_taps_add(keypos_t key) {

quantum/action_tapping.h

@@ -88,12 +88,6 @@ bool get_chordal_hold(uint16_t tap_hold_keycode, keyrecord_t *tap_hold_record, u
  * returns true, indicating that it may be held. Otherwise, it returns false,
  * 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.
- * 2. `chordal_hold_layout`, if CHORDAL_HOLD_LAYOUT is defined.
- * 3. `chordal_hold_handedness_kb()`, if defined.
- * 4. fallback assumption based on keyboard matrix dimensions.
- *
  * @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.
@@ -101,9 +95,9 @@ bool get_chordal_hold(uint16_t tap_hold_keycode, keyrecord_t *tap_hold_record, u
 bool get_chordal_hold_default(keyrecord_t *tap_hold_record, keyrecord_t *other_record);
 
 /**
- * Keyboard-level callback to determine handedness of a key.
+ * Gets the handedness of a key.
  *
- * This function should return:
+ * 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
@@ -112,9 +106,7 @@ bool get_chordal_hold_default(keyrecord_t *tap_hold_record, keyrecord_t *other_r
  * @param key   A key matrix position.
  * @return Handedness value.
  */
-char chordal_hold_handedness_kb(keypos_t key);
+char chordal_hold_handedness(keypos_t key);
-/** User callback to determine handedness of a key. */
-char chordal_hold_handedness_user(keypos_t key);
 
 #    ifdef CHORDAL_HOLD_LAYOUT
 extern const char chordal_hold_layout[MATRIX_ROWS][MATRIX_COLS] PROGMEM;

tests/tap_hold_configurations/chordal_hold/hold_on_other_key_press/test.mk

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

tests/tap_hold_configurations/chordal_hold/hold_on_other_key_press/test_keymap.c

@@ -0,0 +1,8 @@
+#include "quantum.h"
+
+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'},
+};

tests/tap_hold_configurations/chordal_hold/permissive_hold/config.h

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

tests/tap_hold_configurations/chordal_hold/permissive_hold/test_tap_hold.cpp

@@ -26,9 +26,9 @@ using testing::InSequence;
 class ChordalHoldPermissiveHold : public TestFixture {};
 
 TEST_F(ChordalHoldPermissiveHold, chordal_hold_handedness) {
-    EXPECT_EQ(chordal_hold_handedness_user({.col = 0, .row = 0}), 'L');
+    EXPECT_EQ(chordal_hold_handedness({.col = 0, .row = 0}), 'L');
-    EXPECT_EQ(chordal_hold_handedness_user({.col = MATRIX_COLS - 1, .row = 0}), 'R');
+    EXPECT_EQ(chordal_hold_handedness({.col = MATRIX_COLS - 1, .row = 0}), 'R');
-    EXPECT_EQ(chordal_hold_handedness_user({.col = 0, .row = 2}), '*');
+    EXPECT_EQ(chordal_hold_handedness({.col = 0, .row = 2}), '*');
 }
 
 TEST_F(ChordalHoldPermissiveHold, get_chordal_hold_default) {

tests/tap_hold_configurations/chordal_hold/retro_shift_permissive_hold/test.mk

@@ -14,3 +14,4 @@
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
 AUTO_SHIFT_ENABLE = yes
+INTROSPECTION_KEYMAP_C = test_keymap.c

tests/tap_hold_configurations/chordal_hold/retro_shift_permissive_hold/test_keymap.c

@@ -0,0 +1,8 @@
+#include "quantum.h"
+
+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'},
+};