From e0f648d260ff683d84d75f493de12095eee6b842 Mon Sep 17 00:00:00 2001 From: Pascal Getreuer Date: Sat, 2 Nov 2024 22:58:34 -0700 Subject: [PATCH] Chordal Hold: docs and further improvements --- docs/tap_hold.md | 161 +++++++++++++++++ quantum/action_tapping.c | 68 ++++--- quantum/action_tapping.h | 41 +++-- .../chordal_hold/config.h | 21 --- .../chordal_hold/test.mk | 18 -- .../chordal_hold/test_tap_hold.cpp | 167 ------------------ .../test_tap_hold.cpp | 114 +++++++++--- .../chordal_hold_and_permissive_hold/config.h | 1 + .../chordal_hold_and_permissive_hold/test.mk | 6 +- .../test_keymap.c | 7 + .../test_tap_hold.cpp | 145 +++++++++++---- 11 files changed, 423 insertions(+), 326 deletions(-) delete mode 100644 tests/tap_hold_configurations/chordal_hold/config.h delete mode 100644 tests/tap_hold_configurations/chordal_hold/test.mk delete mode 100644 tests/tap_hold_configurations/chordal_hold/test_tap_hold.cpp create mode 100644 tests/tap_hold_configurations/chordal_hold_and_permissive_hold/test_keymap.c diff --git a/docs/tap_hold.md b/docs/tap_hold.md index 9b7f6552cbf..25ba4e56fb3 100644 --- a/docs/tap_hold.md +++ b/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`: diff --git a/quantum/action_tapping.c b/quantum/action_tapping.c index af7b8b5ce1c..14a9faacc73 100644 --- a/quantum/action_tapping.c +++ b/quantum/action_tapping.c @@ -50,45 +50,46 @@ __attribute__((weak)) bool get_permissive_hold(uint16_t keycode, keyrecord_t *re # endif # 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) { +__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) { - uint8_t tap_hold_hand = chordal_hold_handedness_user(tap_hold_record->event.key); - if (tap_hold_hand == 0) { +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_user(tap_hold_record->event.key); + if (tap_hold_hand == '*') { return true; } - uint8_t other_hand = chordal_hold_handedness_user(other_record->event.key); - return other_hand == 0 || tap_hold_hand != other_hand; + char other_hand = chordal_hold_handedness_user(other_record->event.key); + 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" +# 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 } -# endif // CHORDAL_HOLD +# 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) { @@ -313,36 +314,33 @@ bool process_tapping(keyrecord_t *keyp) { if (event.pressed) { tapping_key.tap.interrupted = true; -# if defined(CHORDAL_HOLD) - if (!is_tap_record(keyp) && - !get_chordal_hold(tapping_keycode, &tapping_key, - get_record_keycode(keyp, true), keyp)) { - // Settle the tapping key as *tapped*, since it is - // not considered a held chord with keyp. - ac_dprintf("Tapping: End. Tap in non-chord\n"); +# if defined(CHORDAL_HOLD) + if (!is_tap_record(keyp) && !get_chordal_hold(tapping_keycode, &tapping_key, get_record_keycode(keyp, true), keyp)) { + // Settle the tapping key as *tapped*, since it + // is not considered a held chord with keyp. + ac_dprintf("Tapping: End. Chord considered a tap\n"); tapping_key.tap.count = 1; - // In process_action(), HOLD_ON_OTHER_KEY_PRESS will - // revert interrupted events to holds, so this needs - // to be set false. + // 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; process_record(&tapping_key); + debug_tapping_key(); } else # endif - if (TAP_GET_HOLD_ON_OTHER_KEY_PRESS + 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}; debug_tapping_key(); - // enqueue - return false; } } // enqueue diff --git a/quantum/action_tapping.h b/quantum/action_tapping.h index 0e7721b5008..6227a394d6d 100644 --- a/quantum/action_tapping.h +++ b/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 - * false to consider it tapped. + * 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 is considered held; false if tapped. + * @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); +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 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 - * returns true, indicating a hold. Otherwise, it returns false, indicating that - * the tap-hold key should settle as tapped. + * 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. * * "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( - keyrecord_t* tap_hold_record, keyrecord_t* other_record); +bool get_chordal_hold_default(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, - * 2 for keys on the right hand, - * 0 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. + * '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. */ -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 diff --git a/tests/tap_hold_configurations/chordal_hold/config.h b/tests/tap_hold_configurations/chordal_hold/config.h deleted file mode 100644 index 510b6a035f2..00000000000 --- a/tests/tap_hold_configurations/chordal_hold/config.h +++ /dev/null @@ -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 . - */ - -#pragma once - -#include "test_common.h" -#define CHORDAL_HOLD - diff --git a/tests/tap_hold_configurations/chordal_hold/test.mk b/tests/tap_hold_configurations/chordal_hold/test.mk deleted file mode 100644 index 6b5968df16f..00000000000 --- a/tests/tap_hold_configurations/chordal_hold/test.mk +++ /dev/null @@ -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 . - -# -------------------------------------------------------------------------------- -# Keep this file, even if it is empty, as a marker that this folder contains tests -# -------------------------------------------------------------------------------- diff --git a/tests/tap_hold_configurations/chordal_hold/test_tap_hold.cpp b/tests/tap_hold_configurations/chordal_hold/test_tap_hold.cpp deleted file mode 100644 index cbe8e4e1c20..00000000000 --- a/tests/tap_hold_configurations/chordal_hold/test_tap_hold.cpp +++ /dev/null @@ -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 . - */ - -#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); -} - diff --git a/tests/tap_hold_configurations/chordal_hold_and_hold_on_other_key_press/test_tap_hold.cpp b/tests/tap_hold_configurations/chordal_hold_and_hold_on_other_key_press/test_tap_hold.cpp index 68ee607ac8f..c18d568798a 100644 --- a/tests/tap_hold_configurations/chordal_hold_and_hold_on_other_key_press/test_tap_hold.cpp +++ b/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); + 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 regular_key = KeymapKey(0, 2, 0, KC_A); + 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); } @@ -100,7 +163,7 @@ TEST_F(ChordalHoldAndHoldOnOtherKeypress, non_chord_with_mod_tap_settled_as_tap) TEST_F(ChordalHoldAndHoldOnOtherKeypress, tap_mod_tap_key) { TestDriver driver; InSequence s; - auto mod_tap_key = KeymapKey(0, 1, 0, SFT_T(KC_P)); + auto mod_tap_key = KeymapKey(0, 1, 0, SFT_T(KC_P)); set_keymap({mod_tap_key}); @@ -119,7 +182,7 @@ TEST_F(ChordalHoldAndHoldOnOtherKeypress, tap_mod_tap_key) { TEST_F(ChordalHoldAndHoldOnOtherKeypress, hold_mod_tap_key) { TestDriver driver; InSequence s; - auto mod_tap_key = KeymapKey(0, 1, 0, SFT_T(KC_P)); + auto mod_tap_key = KeymapKey(0, 1, 0, SFT_T(KC_P)); set_keymap({mod_tap_key}); @@ -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); } - diff --git a/tests/tap_hold_configurations/chordal_hold_and_permissive_hold/config.h b/tests/tap_hold_configurations/chordal_hold_and_permissive_hold/config.h index 7f06b679a85..f1027806d35 100644 --- a/tests/tap_hold_configurations/chordal_hold_and_permissive_hold/config.h +++ b/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 diff --git a/tests/tap_hold_configurations/chordal_hold_and_permissive_hold/test.mk b/tests/tap_hold_configurations/chordal_hold_and_permissive_hold/test.mk index 6b5968df16f..81ba8da66d4 100644 --- a/tests/tap_hold_configurations/chordal_hold_and_permissive_hold/test.mk +++ b/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 . -# -------------------------------------------------------------------------------- -# Keep this file, even if it is empty, as a marker that this folder contains tests -# -------------------------------------------------------------------------------- +COMBO_ENABLE = yes + +INTROSPECTION_KEYMAP_C = test_keymap.c diff --git a/tests/tap_hold_configurations/chordal_hold_and_permissive_hold/test_keymap.c b/tests/tap_hold_configurations/chordal_hold_and_permissive_hold/test_keymap.c new file mode 100644 index 00000000000..195f970a799 --- /dev/null +++ b/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), +}; diff --git a/tests/tap_hold_configurations/chordal_hold_and_permissive_hold/test_tap_hold.cpp b/tests/tap_hold_configurations/chordal_hold_and_permissive_hold/test_tap_hold.cpp index fdb053a5b21..df506b9c099 100644 --- a/tests/tap_hold_configurations/chordal_hold_and_permissive_hold/test_tap_hold.cpp +++ b/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); + 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(); - idle_for(TAPPING_TERM); + EXPECT_REPORT(driver, (KC_LEFT_SHIFT)); + tap_key(regular_key); VERIFY_AND_CLEAR(driver); - /* Release regular key. */ - EXPECT_REPORT(driver, (KC_LEFT_SHIFT)); - regular_key.release(); + // Release mod-tap key. + EXPECT_EMPTY_REPORT(driver); + 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 regular_key = KeymapKey(0, 2, 0, KC_A); + 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. */ - EXPECT_REPORT(driver, (KC_P)); - regular_key.release(); - idle_for(TAPPING_TERM); - 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); } @@ -100,7 +176,7 @@ TEST_F(ChordalHoldAndPermissiveHold, non_chord_with_mod_tap_settled_as_tap) { TEST_F(ChordalHoldAndPermissiveHold, tap_mod_tap_key) { TestDriver driver; InSequence s; - auto mod_tap_key = KeymapKey(0, 1, 0, SFT_T(KC_P)); + auto mod_tap_key = KeymapKey(0, 1, 0, SFT_T(KC_P)); set_keymap({mod_tap_key}); @@ -119,7 +195,7 @@ TEST_F(ChordalHoldAndPermissiveHold, tap_mod_tap_key) { TEST_F(ChordalHoldAndPermissiveHold, hold_mod_tap_key) { TestDriver driver; InSequence s; - auto mod_tap_key = KeymapKey(0, 1, 0, SFT_T(KC_P)); + auto mod_tap_key = KeymapKey(0, 1, 0, SFT_T(KC_P)); set_keymap({mod_tap_key}); @@ -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); } -