From fa857db172a0120fc9bee7d0318d189abd76e4bf Mon Sep 17 00:00:00 2001 From: Pascal Getreuer Date: Fri, 1 Nov 2024 23:58:36 -0700 Subject: [PATCH 01/13] Chordal Hold: restrict what chords settle as hold --- quantum/action_tapping.c | 61 ++++++- quantum/action_tapping.h | 76 ++++++++ .../chordal_hold/config.h | 21 +++ .../chordal_hold/test.mk | 18 ++ .../chordal_hold/test_tap_hold.cpp | 167 ++++++++++++++++++ .../config.h | 22 +++ .../test.mk | 18 ++ .../test_tap_hold.cpp | 167 ++++++++++++++++++ .../chordal_hold_and_permissive_hold/config.h | 22 +++ .../chordal_hold_and_permissive_hold/test.mk | 18 ++ .../test_tap_hold.cpp | 167 ++++++++++++++++++ 11 files changed, 756 insertions(+), 1 deletion(-) create mode 100644 tests/tap_hold_configurations/chordal_hold/config.h create mode 100644 tests/tap_hold_configurations/chordal_hold/test.mk create mode 100644 tests/tap_hold_configurations/chordal_hold/test_tap_hold.cpp create mode 100644 tests/tap_hold_configurations/chordal_hold_and_hold_on_other_key_press/config.h create mode 100644 tests/tap_hold_configurations/chordal_hold_and_hold_on_other_key_press/test.mk create mode 100644 tests/tap_hold_configurations/chordal_hold_and_hold_on_other_key_press/test_tap_hold.cpp create mode 100644 tests/tap_hold_configurations/chordal_hold_and_permissive_hold/config.h create mode 100644 tests/tap_hold_configurations/chordal_hold_and_permissive_hold/test.mk create mode 100644 tests/tap_hold_configurations/chordal_hold_and_permissive_hold/test_tap_hold.cpp diff --git a/quantum/action_tapping.c b/quantum/action_tapping.c index 8f238490f2a..af7b8b5ce1c 100644 --- a/quantum/action_tapping.c +++ b/quantum/action_tapping.c @@ -49,6 +49,47 @@ __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) { + 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) { + return true; + } + uint8_t other_hand = chordal_hold_handedness_user(other_record->event.key); + return other_hand == 0 || tap_hold_hand != other_hand; +} + +__attribute__((weak)) uint8_t chordal_hold_handedness_kb(keypos_t key) { +# if defined(SPLIT_KEYBOARD) || ((MATRIX_ROWS) > (MATRIX_COLS)) +#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; +# else +#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; +# endif +} + +__attribute__((weak)) uint8_t 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]); +# else + return chordal_hold_handedness_kb(key); +# endif +} +# 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; @@ -188,7 +229,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 @@ -271,6 +312,22 @@ bool process_tapping(keyrecord_t *keyp) { // set interrupted flag when other key pressed during tapping 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"); + 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. + tapping_key.tap.interrupted = false; + process_record(&tapping_key); + } else +# endif if (TAP_GET_HOLD_ON_OTHER_KEY_PRESS # if defined(AUTO_SHIFT_ENABLE) && defined(RETRO_SHIFT) // Auto Shift cannot evaluate this early @@ -278,6 +335,8 @@ bool process_tapping(keyrecord_t *keyp) { && !(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}; diff --git a/quantum/action_tapping.h b/quantum/action_tapping.h index 6b518b82988..0e7721b5008 100644 --- a/quantum/action_tapping.h +++ b/quantum/action_tapping.h @@ -46,6 +46,82 @@ 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 is considered 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. + * + * Returning true indicates that the tap-hold key should be considered held, or + * false to consider it tapped. + * + * @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. + */ +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. + * + * 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" 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 is considered held; false if tapped. + */ +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. + * + * @param key A key matrix position. + * @return Handedness value. + */ +uint8_t chordal_hold_handedness_kb(keypos_t key); +/** User callback to determine handedness of a key. */ +uint8_t chordal_hold_handedness_user(keypos_t key); + +# ifdef CHORDAL_HOLD_LAYOUT +extern const uint8_t chordal_hold_layout[MATRIX_ROWS][MATRIX_COLS] PROGMEM; +# endif +#endif + #ifdef DYNAMIC_TAPPING_TERM_ENABLE extern uint16_t g_tapping_term; #endif diff --git a/tests/tap_hold_configurations/chordal_hold/config.h b/tests/tap_hold_configurations/chordal_hold/config.h new file mode 100644 index 00000000000..510b6a035f2 --- /dev/null +++ b/tests/tap_hold_configurations/chordal_hold/config.h @@ -0,0 +1,21 @@ +/* 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 new file mode 100644 index 00000000000..6b5968df16f --- /dev/null +++ b/tests/tap_hold_configurations/chordal_hold/test.mk @@ -0,0 +1,18 @@ +# 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 new file mode 100644 index 00000000000..cbe8e4e1c20 --- /dev/null +++ b/tests/tap_hold_configurations/chordal_hold/test_tap_hold.cpp @@ -0,0 +1,167 @@ +/* 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/config.h b/tests/tap_hold_configurations/chordal_hold_and_hold_on_other_key_press/config.h new file mode 100644 index 00000000000..b1d37b1e3f9 --- /dev/null +++ b/tests/tap_hold_configurations/chordal_hold_and_hold_on_other_key_press/config.h @@ -0,0 +1,22 @@ +/* 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 +#define HOLD_ON_OTHER_KEY_PRESS + diff --git a/tests/tap_hold_configurations/chordal_hold_and_hold_on_other_key_press/test.mk b/tests/tap_hold_configurations/chordal_hold_and_hold_on_other_key_press/test.mk new file mode 100644 index 00000000000..6b5968df16f --- /dev/null +++ b/tests/tap_hold_configurations/chordal_hold_and_hold_on_other_key_press/test.mk @@ -0,0 +1,18 @@ +# 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_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 new file mode 100644 index 00000000000..68ee607ac8f --- /dev/null +++ b/tests/tap_hold_configurations/chordal_hold_and_hold_on_other_key_press/test_tap_hold.cpp @@ -0,0 +1,167 @@ +/* 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 ChordalHoldAndHoldOnOtherKeypress : public TestFixture {}; + +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)); + // 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(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); + + 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(ChordalHoldAndHoldOnOtherKeypress, 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(ChordalHoldAndHoldOnOtherKeypress, 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(ChordalHoldAndHoldOnOtherKeypress, 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_permissive_hold/config.h b/tests/tap_hold_configurations/chordal_hold_and_permissive_hold/config.h new file mode 100644 index 00000000000..7f06b679a85 --- /dev/null +++ b/tests/tap_hold_configurations/chordal_hold_and_permissive_hold/config.h @@ -0,0 +1,22 @@ +/* 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 +#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 new file mode 100644 index 00000000000..6b5968df16f --- /dev/null +++ b/tests/tap_hold_configurations/chordal_hold_and_permissive_hold/test.mk @@ -0,0 +1,18 @@ +# 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_and_permissive_hold/test_tap_hold.cpp b/tests/tap_hold_configurations/chordal_hold_and_permissive_hold/test_tap_hold.cpp new file mode 100644 index 00000000000..fdb053a5b21 --- /dev/null +++ b/tests/tap_hold_configurations/chordal_hold_and_permissive_hold/test_tap_hold.cpp @@ -0,0 +1,167 @@ +/* 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 ChordalHoldAndPermissiveHold : public TestFixture {}; + +TEST_F(ChordalHoldAndPermissiveHold, 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(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); + + 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(ChordalHoldAndPermissiveHold, 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(ChordalHoldAndPermissiveHold, 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(ChordalHoldAndPermissiveHold, 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); +} + From e0f648d260ff683d84d75f493de12095eee6b842 Mon Sep 17 00:00:00 2001 From: Pascal Getreuer Date: Sat, 2 Nov 2024 22:58:34 -0700 Subject: [PATCH 02/13] 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); } - From 352f4fa0957ccd1f70fb62d5548dd54d29f57889 Mon Sep 17 00:00:00 2001 From: Pascal Getreuer Date: Sun, 3 Nov 2024 10:21:56 -0800 Subject: [PATCH 03/13] Fix formatting. --- .../chordal_hold_and_hold_on_other_key_press/config.h | 1 - .../chordal_hold_and_permissive_hold/config.h | 1 - 2 files changed, 2 deletions(-) diff --git a/tests/tap_hold_configurations/chordal_hold_and_hold_on_other_key_press/config.h b/tests/tap_hold_configurations/chordal_hold_and_hold_on_other_key_press/config.h index b1d37b1e3f9..723b0cbc0e4 100644 --- a/tests/tap_hold_configurations/chordal_hold_and_hold_on_other_key_press/config.h +++ b/tests/tap_hold_configurations/chordal_hold_and_hold_on_other_key_press/config.h @@ -19,4 +19,3 @@ #include "test_common.h" #define CHORDAL_HOLD #define HOLD_ON_OTHER_KEY_PRESS - 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 f1027806d35..910a753c270 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 @@ -20,4 +20,3 @@ #define CHORDAL_HOLD #define CHORDAL_HOLD_LAYOUT #define PERMISSIVE_HOLD - From ed53b7dadc4692422583fccdbbcee885b3c3b5a2 Mon Sep 17 00:00:00 2001 From: Pascal Getreuer Date: Sun, 3 Nov 2024 20:02:46 -0800 Subject: [PATCH 04/13] Doc rewording and minor edit. --- docs/tap_hold.md | 22 +++++++++++----------- quantum/action_tapping.c | 3 --- 2 files changed, 11 insertions(+), 14 deletions(-) diff --git a/docs/tap_hold.md b/docs/tap_hold.md index 25ba4e56fb3..a246ea97939 100644 --- a/docs/tap_hold.md +++ b/docs/tap_hold.md @@ -437,20 +437,20 @@ On Other Key Press. Chordal Hold is enabled by adding to your `config.h`: 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. +keys are on the same hand. + +Otherwise, if the keys are on opposite hands, Chordal Hold introduces no new +behavior. Hold On Other Key Press or Permissive Hold may be used together with +Chordal Hold to configure the behavior in the opposite hands case. 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 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 diff --git a/quantum/action_tapping.c b/quantum/action_tapping.c index 14a9faacc73..4f232d34e2b 100644 --- a/quantum/action_tapping.c +++ b/quantum/action_tapping.c @@ -69,12 +69,10 @@ bool get_chordal_hold_default(keyrecord_t *tap_hold_record, keyrecord_t *other_r __attribute__((weak)) char chordal_hold_handedness_kb(keypos_t key) { # if defined(SPLIT_KEYBOARD) || ((MATRIX_ROWS) > (MATRIX_COLS)) -# 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*/ 'L' : /*right*/ 'R'; # else -# 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*/ 'L' : /*right*/ 'R'; # endif @@ -82,7 +80,6 @@ __attribute__((weak)) char chordal_hold_handedness_kb(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 (char)pgm_read_byte(&chordal_hold_layout[key.row][key.col]); # else From 3fdbb079cbc28b2c4e8e85861c306f3669617dfa Mon Sep 17 00:00:00 2001 From: Pascal Getreuer Date: Thu, 7 Nov 2024 19:14:30 -0800 Subject: [PATCH 05/13] Support Chordal Hold of multiple tap-hold keys. --- docs/tap_hold.md | 4 - quantum/action_tapping.c | 211 +++++-- .../hold_on_other_key_press}/config.h | 0 .../hold_on_other_key_press}/test.mk | 0 .../hold_on_other_key_press/test_tap_hold.cpp | 499 +++++++++++++++ .../permissive_hold}/config.h | 0 .../permissive_hold}/test.mk | 2 - .../permissive_hold/test_keymap.c | 8 + .../permissive_hold/test_one_shot_keys.cpp | 148 +++++ .../permissive_hold/test_tap_hold.cpp | 576 ++++++++++++++++++ .../retro_shift_permissive_hold/config.h | 27 + .../retro_shift_permissive_hold/test.mk | 16 + .../test_retro_shift.cpp | 419 +++++++++++++ .../test_tap_hold.cpp | 229 ------- .../test_keymap.c | 7 - .../test_tap_hold.cpp | 242 -------- 16 files changed, 1852 insertions(+), 536 deletions(-) rename tests/tap_hold_configurations/{chordal_hold_and_hold_on_other_key_press => chordal_hold/hold_on_other_key_press}/config.h (100%) rename tests/tap_hold_configurations/{chordal_hold_and_hold_on_other_key_press => chordal_hold/hold_on_other_key_press}/test.mk (100%) create mode 100644 tests/tap_hold_configurations/chordal_hold/hold_on_other_key_press/test_tap_hold.cpp rename tests/tap_hold_configurations/{chordal_hold_and_permissive_hold => chordal_hold/permissive_hold}/config.h (100%) rename tests/tap_hold_configurations/{chordal_hold_and_permissive_hold => chordal_hold/permissive_hold}/test.mk (97%) create mode 100644 tests/tap_hold_configurations/chordal_hold/permissive_hold/test_keymap.c create mode 100644 tests/tap_hold_configurations/chordal_hold/permissive_hold/test_one_shot_keys.cpp create mode 100644 tests/tap_hold_configurations/chordal_hold/permissive_hold/test_tap_hold.cpp create mode 100644 tests/tap_hold_configurations/chordal_hold/retro_shift_permissive_hold/config.h create mode 100644 tests/tap_hold_configurations/chordal_hold/retro_shift_permissive_hold/test.mk create mode 100644 tests/tap_hold_configurations/chordal_hold/retro_shift_permissive_hold/test_retro_shift.cpp delete mode 100644 tests/tap_hold_configurations/chordal_hold_and_hold_on_other_key_press/test_tap_hold.cpp delete mode 100644 tests/tap_hold_configurations/chordal_hold_and_permissive_hold/test_keymap.c delete mode 100644 tests/tap_hold_configurations/chordal_hold_and_permissive_hold/test_tap_hold.cpp diff --git a/docs/tap_hold.md b/docs/tap_hold.md index a246ea97939..84d0a17ab88 100644 --- a/docs/tap_hold.md +++ b/docs/tap_hold.md @@ -453,10 +453,6 @@ Notes: * 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. diff --git a/quantum/action_tapping.c b/quantum/action_tapping.c index 4f232d34e2b..22e0e23b2d0 100644 --- a/quantum/action_tapping.c +++ b/quantum/action_tapping.c @@ -50,42 +50,17 @@ __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) { - return get_chordal_hold_default(tap_hold_record, other_record); -} +// Tracks whether multiple tap-hold keys are simultaenously unsettled. +static bool multi_tap_sequence = false; -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; - } - char other_hand = chordal_hold_handedness_user(other_record->event.key); - return other_hand == '*' || tap_hold_hand != other_hand; -} - -__attribute__((weak)) char chordal_hold_handedness_kb(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 bool is_one_shot(uint16_t keycode) { + return IS_QK_ONE_SHOT_MOD(keycode) || IS_QK_ONE_SHOT_LAYER(keycode); } +static uint8_t waiting_buffer_find_chordal_hold_tap(void); +static void waiting_buffer_process_until(uint8_t new_tail); +static void waiting_buffer_process_regular(void); +# else +# define multi_tap_sequence false # endif // CHORDAL_HOLD # ifdef HOLD_ON_OTHER_KEY_PRESS_PER_KEY @@ -149,6 +124,11 @@ void action_tapping_process(keyrecord_t record) { if (IS_EVENT(record.event)) { ac_dprintf("\n"); } +# ifdef CHORDAL_HOLD + if (waiting_buffer_tail == waiting_buffer_head) { + multi_tap_sequence = false; + } +# endif } /* Some conditionally defined helper macros to keep process_tapping more @@ -219,6 +199,20 @@ bool process_tapping(keyrecord_t *keyp) { waiting_buffer_scan_tap(); debug_tapping_key(); } else { +# if defined(CHORDAL_HOLD) + // If keyp is a tap-hold key release, and the tail waiting buffer + // is the corresponding press, settle the key as tapped. + if (waiting_buffer_tail != waiting_buffer_head && is_tap_record(keyp)) { + keyrecord_t *tail = &waiting_buffer[waiting_buffer_tail]; + if (IS_EVENT(tail->event) && KEYEQ(tail->event.key, keyp->event.key) && tail->event.pressed) { + tail->tap.count = 1; + keyp->tap.count = 1; + process_record(tail); + // Pop tail from the queue. + waiting_buffer_tail = (waiting_buffer_tail + 1) % WAITING_BUFFER_SIZE; + } + } +# endif // the current key is just a regular key, pass it on for regular // processing process_record(keyp); @@ -238,7 +232,25 @@ bool process_tapping(keyrecord_t *keyp) { // early return for tick events return true; } - if (tapping_key.tap.count == 0) { +# ifdef CHORDAL_HOLD + if (!event.pressed && multi_tap_sequence && waiting_buffer_typed(event)) { + const uint8_t first_tap = waiting_buffer_find_chordal_hold_tap(); + + // Settle and process the tapping key and waiting events + // preceding first_tap as *held*. + if (first_tap < WAITING_BUFFER_SIZE) { + ac_dprintf("Tapping: End. No tap. Interfered by typing key\n"); + process_record(&tapping_key); + tapping_key = (keyrecord_t){0}; + debug_tapping_key(); + + waiting_buffer_process_until(first_tap); + } + // enqueue + return false; + } else +# endif + if (tapping_key.tap.count == 0) { if (IS_TAPPING_RECORD(keyp) && !event.pressed) { // first tap! ac_dprintf("Tapping: First tap(0->1).\n"); @@ -277,7 +289,7 @@ bool process_tapping(keyrecord_t *keyp) { * Without this unexpected repeating will occur with having fast repeating setting * https://github.com/tmk/tmk_keyboard/issues/60 */ - else if (!event.pressed && !waiting_buffer_typed(event)) { + else if (!multi_tap_sequence && !event.pressed && !waiting_buffer_typed(event)) { // Modifier/Layer should be retained till end of this tapping. action_t action = layer_switch_get_action(event.key); switch (action.kind.id) { @@ -312,17 +324,26 @@ bool process_tapping(keyrecord_t *keyp) { 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. Chord considered a tap\n"); - tapping_key.tap.count = 1; + if (!is_one_shot(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; - process_record(&tapping_key); - debug_tapping_key(); + + if (is_tap_record(keyp)) { + // Multiple tap-hold keys are unsettled. + multi_tap_sequence = true; + } else { + // 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; + process_record(&tapping_key); + debug_tapping_key(); + + // Process regular keys in the waiting buffer. + waiting_buffer_process_regular(); + } } else # endif if (TAP_GET_HOLD_ON_OTHER_KEY_PRESS @@ -574,26 +595,112 @@ void waiting_buffer_scan_tap(void) { } } -/** \brief Tapping key debug print +# 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_user(tap_hold_record->event.key); + if (tap_hold_hand == '*') { + return true; + } + char other_hand = chordal_hold_handedness_user(other_record->event.key); + return other_hand == '*' || tap_hold_hand != other_hand; +} + +__attribute__((weak)) char chordal_hold_handedness_kb(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 +} + +/** \brief Finds which queued events should be held according to Chordal Hold. * - * FIXME: Needs docs + * 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 + * lastest 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) { + 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_tap_record(prev) || is_one_shot(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; +} + +/** \brief Processes and pops buffered events preceding `new_tail`. */ +static void waiting_buffer_process_until(uint8_t new_tail) { + for (; waiting_buffer_tail != new_tail; waiting_buffer_tail = (waiting_buffer_tail + 1) % WAITING_BUFFER_SIZE) { + ac_dprintf("waiting_buffer_process_until: processing [%u]\n", waiting_buffer_tail); + process_record(&waiting_buffer[waiting_buffer_tail]); + } + + debug_waiting_buffer(); +} + +/** \brief Processes and pops buffered events until the first tap-hold event. */ +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/tests/tap_hold_configurations/chordal_hold_and_hold_on_other_key_press/config.h b/tests/tap_hold_configurations/chordal_hold/hold_on_other_key_press/config.h similarity index 100% rename from tests/tap_hold_configurations/chordal_hold_and_hold_on_other_key_press/config.h rename to tests/tap_hold_configurations/chordal_hold/hold_on_other_key_press/config.h diff --git a/tests/tap_hold_configurations/chordal_hold_and_hold_on_other_key_press/test.mk b/tests/tap_hold_configurations/chordal_hold/hold_on_other_key_press/test.mk similarity index 100% rename from tests/tap_hold_configurations/chordal_hold_and_hold_on_other_key_press/test.mk rename to tests/tap_hold_configurations/chordal_hold/hold_on_other_key_press/test.mk diff --git a/tests/tap_hold_configurations/chordal_hold/hold_on_other_key_press/test_tap_hold.cpp b/tests/tap_hold_configurations/chordal_hold/hold_on_other_key_press/test_tap_hold.cpp new file mode 100644 index 00000000000..1c9e6de2ec0 --- /dev/null +++ b/tests/tap_hold_configurations/chordal_hold/hold_on_other_key_press/test_tap_hold.cpp @@ -0,0 +1,499 @@ +// Copyright 2024 Google LLC +// +// 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 ChordalHoldHoldOnOtherKeyPress : public TestFixture {}; + +TEST_F(ChordalHoldHoldOnOtherKeyPress, chord_with_mod_tap_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); + + // Press regular key. + EXPECT_REPORT(driver, (KC_LEFT_SHIFT)); + EXPECT_REPORT(driver, (KC_LEFT_SHIFT, KC_A)); + regular_key.press(); + run_one_scan_loop(); + 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_key.release(); + run_one_scan_loop(); + VERIFY_AND_CLEAR(driver); +} + +TEST_F(ChordalHoldHoldOnOtherKeyPress, 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(ChordalHoldHoldOnOtherKeyPress, 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); +} + +TEST_F(ChordalHoldHoldOnOtherKeyPress, 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_key = KeymapKey(0, 1, 0, SFT_T(KC_P)); + auto regular_key = KeymapKey(0, 2, 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); + + // 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(); + run_one_scan_loop(); + 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(ChordalHoldHoldOnOtherKeyPress, 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(ChordalHoldHoldOnOtherKeyPress, 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(ChordalHoldHoldOnOtherKeyPress, two_mod_taps_nested_press_opposite_hands) { + TestDriver driver; + InSequence s; + auto mod_tap_key1 = KeymapKey(0, 1, 0, SFT_T(KC_A)); + auto mod_tap_key2 = KeymapKey(0, MATRIX_COLS - 1, 0, RSFT_T(KC_B)); + + set_keymap({mod_tap_key1, mod_tap_key2}); + + // Press first mod-tap key. + EXPECT_NO_REPORT(driver); + mod_tap_key1.press(); + run_one_scan_loop(); + VERIFY_AND_CLEAR(driver); + + // Tap second mod-tap key. + EXPECT_REPORT(driver, (KC_LEFT_SHIFT)); + EXPECT_REPORT(driver, (KC_LEFT_SHIFT, KC_B)); + EXPECT_REPORT(driver, (KC_LEFT_SHIFT)); + EXPECT_EMPTY_REPORT(driver); + mod_tap_key2.press(); + run_one_scan_loop(); + mod_tap_key2.release(); + run_one_scan_loop(); + // Release first mod-tap key. + mod_tap_key1.release(); + run_one_scan_loop(); + VERIFY_AND_CLEAR(driver); +} + +TEST_F(ChordalHoldHoldOnOtherKeyPress, two_mod_taps_nested_press_same_hand) { + 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 keys. + EXPECT_NO_REPORT(driver); + mod_tap_key1.press(); + run_one_scan_loop(); + VERIFY_AND_CLEAR(driver); + + // Release mod-tap keys. + EXPECT_REPORT(driver, (KC_A)); + EXPECT_REPORT(driver, (KC_A, KC_B)); + EXPECT_REPORT(driver, (KC_A)); + EXPECT_EMPTY_REPORT(driver); + mod_tap_key2.press(); + run_one_scan_loop(); + mod_tap_key2.release(); + run_one_scan_loop(); + mod_tap_key1.release(); + run_one_scan_loop(); + VERIFY_AND_CLEAR(driver); +} + +TEST_F(ChordalHoldHoldOnOtherKeyPress, three_mod_taps_same_hand_streak_roll) { + TestDriver driver; + InSequence s; + auto mod_tap_key1 = KeymapKey(0, 1, 0, SFT_T(KC_A)); + auto mod_tap_key2 = KeymapKey(0, 2, 0, CTL_T(KC_B)); + auto mod_tap_key3 = KeymapKey(0, 3, 0, RSFT_T(KC_C)); + + set_keymap({mod_tap_key1, mod_tap_key2, mod_tap_key3}); + + // Press mod-tap keys. + EXPECT_NO_REPORT(driver); + mod_tap_key1.press(); + run_one_scan_loop(); + mod_tap_key2.press(); + run_one_scan_loop(); + mod_tap_key3.press(); + run_one_scan_loop(); + VERIFY_AND_CLEAR(driver); + + // Release keys 1, 2, 3. + EXPECT_REPORT(driver, (KC_A)); + EXPECT_REPORT(driver, (KC_A, KC_B)); + EXPECT_REPORT(driver, (KC_A, KC_B, KC_C)); + EXPECT_REPORT(driver, (KC_B, KC_C)); + EXPECT_REPORT(driver, (KC_C)); + EXPECT_EMPTY_REPORT(driver); + mod_tap_key1.release(); + run_one_scan_loop(); + mod_tap_key2.release(); + run_one_scan_loop(); + mod_tap_key3.release(); + run_one_scan_loop(); + VERIFY_AND_CLEAR(driver); + + EXPECT_NO_REPORT(driver); + VERIFY_AND_CLEAR(driver); +} + +TEST_F(ChordalHoldHoldOnOtherKeyPress, three_mod_taps_same_hand_streak_orders) { + TestDriver driver; + InSequence s; + auto mod_tap_key1 = KeymapKey(0, 1, 0, SFT_T(KC_A)); + auto mod_tap_key2 = KeymapKey(0, 2, 0, CTL_T(KC_B)); + auto mod_tap_key3 = KeymapKey(0, 3, 0, RSFT_T(KC_C)); + + set_keymap({mod_tap_key1, mod_tap_key2, mod_tap_key3}); + + EXPECT_REPORT(driver, (KC_A)); + EXPECT_REPORT(driver, (KC_A, KC_B)); + EXPECT_REPORT(driver, (KC_A, KC_B, KC_C)); + EXPECT_REPORT(driver, (KC_A, KC_B)); + EXPECT_REPORT(driver, (KC_A)); + EXPECT_EMPTY_REPORT(driver); + // Press mod-tap keys. + mod_tap_key1.press(); + run_one_scan_loop(); + mod_tap_key2.press(); + run_one_scan_loop(); + mod_tap_key3.press(); + run_one_scan_loop(); + // Release keys 3, 2, 1. + mod_tap_key3.release(); + run_one_scan_loop(); + mod_tap_key2.release(); + run_one_scan_loop(); + mod_tap_key1.release(); + run_one_scan_loop(); + VERIFY_AND_CLEAR(driver); + + EXPECT_REPORT(driver, (KC_A)); + EXPECT_REPORT(driver, (KC_A, KC_B)); + EXPECT_REPORT(driver, (KC_A, KC_B, KC_C)); + EXPECT_REPORT(driver, (KC_A, KC_B)); + EXPECT_REPORT(driver, (KC_B)); + EXPECT_EMPTY_REPORT(driver); + idle_for(TAPPING_TERM); + // Press mod-tap keys. + mod_tap_key1.press(); + run_one_scan_loop(); + mod_tap_key2.press(); + run_one_scan_loop(); + mod_tap_key3.press(); + run_one_scan_loop(); + // Release keys 3, 1, 2. + mod_tap_key3.release(); + run_one_scan_loop(); + mod_tap_key1.release(); + run_one_scan_loop(); + mod_tap_key2.release(); + run_one_scan_loop(); + VERIFY_AND_CLEAR(driver); + + EXPECT_REPORT(driver, (KC_A)); + EXPECT_REPORT(driver, (KC_A, KC_B)); + EXPECT_REPORT(driver, (KC_A, KC_B, KC_C)); + EXPECT_REPORT(driver, (KC_A, KC_C)); + EXPECT_REPORT(driver, (KC_A)); + EXPECT_EMPTY_REPORT(driver); + idle_for(TAPPING_TERM); + // Press mod-tap keys. + mod_tap_key1.press(); + run_one_scan_loop(); + mod_tap_key2.press(); + run_one_scan_loop(); + mod_tap_key3.press(); + run_one_scan_loop(); + // Release keys 2, 3, 1. + mod_tap_key2.release(); + run_one_scan_loop(); + mod_tap_key3.release(); + run_one_scan_loop(); + mod_tap_key1.release(); + run_one_scan_loop(); + VERIFY_AND_CLEAR(driver); +} + +TEST_F(ChordalHoldHoldOnOtherKeyPress, three_mod_taps_two_held_one_tapped) { + TestDriver driver; + InSequence s; + auto mod_tap_key1 = KeymapKey(0, 1, 0, SFT_T(KC_A)); + auto mod_tap_key2 = KeymapKey(0, 2, 0, CTL_T(KC_B)); + auto mod_tap_key3 = KeymapKey(0, MATRIX_COLS - 1, 0, RSFT_T(KC_C)); + + set_keymap({mod_tap_key1, mod_tap_key2, mod_tap_key3}); + + // Press mod-tap keys. + EXPECT_REPORT(driver, (KC_LEFT_SHIFT)); + EXPECT_REPORT(driver, (KC_LEFT_SHIFT, KC_LEFT_CTRL)); + mod_tap_key1.press(); + run_one_scan_loop(); + mod_tap_key2.press(); + run_one_scan_loop(); + mod_tap_key3.press(); + run_one_scan_loop(); + VERIFY_AND_CLEAR(driver); + + // Release key 3. + EXPECT_REPORT(driver, (KC_LEFT_SHIFT, KC_LEFT_CTRL, KC_C)); + EXPECT_REPORT(driver, (KC_LEFT_SHIFT, KC_LEFT_CTRL)); + mod_tap_key3.release(); + run_one_scan_loop(); + VERIFY_AND_CLEAR(driver); + + // Release key 2, then key 1. + EXPECT_REPORT(driver, (KC_LEFT_SHIFT)); + EXPECT_EMPTY_REPORT(driver); + mod_tap_key2.release(); + run_one_scan_loop(); + mod_tap_key1.release(); + run_one_scan_loop(); + VERIFY_AND_CLEAR(driver); + + // Press mod-tap keys. + EXPECT_REPORT(driver, (KC_LEFT_SHIFT)); + EXPECT_REPORT(driver, (KC_LEFT_SHIFT, KC_LEFT_CTRL)); + idle_for(TAPPING_TERM); + mod_tap_key1.press(); + run_one_scan_loop(); + mod_tap_key2.press(); + run_one_scan_loop(); + mod_tap_key3.press(); + run_one_scan_loop(); + VERIFY_AND_CLEAR(driver); + + // Release key 3. + EXPECT_REPORT(driver, (KC_LEFT_SHIFT, KC_LEFT_CTRL, KC_C)); + EXPECT_REPORT(driver, (KC_LEFT_SHIFT, KC_LEFT_CTRL)); + mod_tap_key3.release(); + run_one_scan_loop(); + VERIFY_AND_CLEAR(driver); + + // Release key 1, then key 2. + EXPECT_REPORT(driver, (KC_LEFT_CTRL)); + EXPECT_EMPTY_REPORT(driver); + mod_tap_key1.release(); + run_one_scan_loop(); + mod_tap_key2.release(); + run_one_scan_loop(); + VERIFY_AND_CLEAR(driver); +} + +TEST_F(ChordalHoldHoldOnOtherKeyPress, three_mod_taps_one_held_two_tapped) { + TestDriver driver; + InSequence s; + auto mod_tap_key1 = KeymapKey(0, 1, 0, SFT_T(KC_A)); + auto mod_tap_key2 = KeymapKey(0, MATRIX_COLS - 2, 0, CTL_T(KC_B)); + auto mod_tap_key3 = KeymapKey(0, MATRIX_COLS - 1, 0, RSFT_T(KC_C)); + + set_keymap({mod_tap_key1, mod_tap_key2, mod_tap_key3}); + + // Press mod-tap keys. + EXPECT_NO_REPORT(driver); + mod_tap_key1.press(); + run_one_scan_loop(); + VERIFY_AND_CLEAR(driver); + + EXPECT_REPORT(driver, (KC_LEFT_SHIFT)); + mod_tap_key2.press(); + run_one_scan_loop(); + VERIFY_AND_CLEAR(driver); + + // Release keys 3, 2, 1. + EXPECT_REPORT(driver, (KC_LEFT_SHIFT, KC_B)); + EXPECT_REPORT(driver, (KC_LEFT_SHIFT, KC_B, KC_C)); + EXPECT_REPORT(driver, (KC_LEFT_SHIFT, KC_B)); + EXPECT_REPORT(driver, (KC_LEFT_SHIFT)); + EXPECT_EMPTY_REPORT(driver); + mod_tap_key3.press(); + run_one_scan_loop(); + mod_tap_key3.release(); + run_one_scan_loop(); + mod_tap_key2.release(); + run_one_scan_loop(); + mod_tap_key1.release(); + run_one_scan_loop(); + VERIFY_AND_CLEAR(driver); + + // Press mod-tap keys. + EXPECT_NO_REPORT(driver); + idle_for(TAPPING_TERM); + mod_tap_key1.press(); + run_one_scan_loop(); + + EXPECT_REPORT(driver, (KC_LEFT_SHIFT)); + mod_tap_key2.press(); + run_one_scan_loop(); + VERIFY_AND_CLEAR(driver); + + // Release keys 3, 1, 2. + EXPECT_REPORT(driver, (KC_LEFT_SHIFT, KC_B)); + EXPECT_REPORT(driver, (KC_LEFT_SHIFT, KC_B, KC_C)); + EXPECT_REPORT(driver, (KC_LEFT_SHIFT, KC_B)); + EXPECT_REPORT(driver, (KC_B)); + EXPECT_EMPTY_REPORT(driver); + mod_tap_key3.press(); + run_one_scan_loop(); + mod_tap_key3.release(); + run_one_scan_loop(); + 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_permissive_hold/config.h b/tests/tap_hold_configurations/chordal_hold/permissive_hold/config.h similarity index 100% rename from tests/tap_hold_configurations/chordal_hold_and_permissive_hold/config.h rename to tests/tap_hold_configurations/chordal_hold/permissive_hold/config.h diff --git a/tests/tap_hold_configurations/chordal_hold_and_permissive_hold/test.mk b/tests/tap_hold_configurations/chordal_hold/permissive_hold/test.mk similarity index 97% rename from tests/tap_hold_configurations/chordal_hold_and_permissive_hold/test.mk rename to tests/tap_hold_configurations/chordal_hold/permissive_hold/test.mk index 81ba8da66d4..86e45bc5bce 100644 --- a/tests/tap_hold_configurations/chordal_hold_and_permissive_hold/test.mk +++ b/tests/tap_hold_configurations/chordal_hold/permissive_hold/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 . -COMBO_ENABLE = yes - INTROSPECTION_KEYMAP_C = test_keymap.c diff --git a/tests/tap_hold_configurations/chordal_hold/permissive_hold/test_keymap.c b/tests/tap_hold_configurations/chordal_hold/permissive_hold/test_keymap.c new file mode 100644 index 00000000000..ffc914b645e --- /dev/null +++ b/tests/tap_hold_configurations/chordal_hold/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'}, +}; diff --git a/tests/tap_hold_configurations/chordal_hold/permissive_hold/test_one_shot_keys.cpp b/tests/tap_hold_configurations/chordal_hold/permissive_hold/test_one_shot_keys.cpp new file mode 100644 index 00000000000..e9436919f5d --- /dev/null +++ b/tests/tap_hold_configurations/chordal_hold/permissive_hold/test_one_shot_keys.cpp @@ -0,0 +1,148 @@ +/* Copyright 2021 Stefan Kerkmann + * + * 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 "action_util.h" +#include "keyboard_report_util.hpp" +#include "test_common.hpp" + +using testing::_; +using testing::InSequence; + +class OneShot : public TestFixture {}; +class OneShotParametrizedTestFixture : public ::testing::WithParamInterface>, public OneShot {}; + +TEST_P(OneShotParametrizedTestFixture, OSMAsRegularModifierWithAdditionalKeypress) { + TestDriver driver; + KeymapKey osm_key = GetParam().first; + KeymapKey regular_key = GetParam().second; + + set_keymap({osm_key, regular_key}); + + // Press OSM. + EXPECT_NO_REPORT(driver); + osm_key.press(); + run_one_scan_loop(); + VERIFY_AND_CLEAR(driver); + + // Press regular key. + EXPECT_NO_REPORT(driver); + regular_key.press(); + run_one_scan_loop(); + VERIFY_AND_CLEAR(driver); + + // Release regular key. + EXPECT_REPORT(driver, (osm_key.report_code)).Times(2); + EXPECT_REPORT(driver, (regular_key.report_code, osm_key.report_code)); + regular_key.release(); + run_one_scan_loop(); + VERIFY_AND_CLEAR(driver); + + // Release OSM. + EXPECT_EMPTY_REPORT(driver); + osm_key.release(); + run_one_scan_loop(); + VERIFY_AND_CLEAR(driver); +} + +// clang-format off + +INSTANTIATE_TEST_CASE_P( + OneShotModifierTests, + OneShotParametrizedTestFixture, + ::testing::Values( + // First is osm key, second is regular key. + std::make_pair(KeymapKey{0, 0, 0, OSM(MOD_LSFT), KC_LSFT}, KeymapKey{0, 1, 1, KC_A}), + std::make_pair(KeymapKey{0, 0, 0, OSM(MOD_LCTL), KC_LCTL}, KeymapKey{0, 1, 1, KC_A}), + std::make_pair(KeymapKey{0, 0, 0, OSM(MOD_LALT), KC_LALT}, KeymapKey{0, 1, 1, KC_A}), + std::make_pair(KeymapKey{0, 0, 0, OSM(MOD_LGUI), KC_LGUI}, KeymapKey{0, 1, 1, KC_A}), + std::make_pair(KeymapKey{0, 0, 0, OSM(MOD_RCTL), KC_RCTL}, KeymapKey{0, 1, 1, KC_A}), + std::make_pair(KeymapKey{0, 0, 0, OSM(MOD_RSFT), KC_RSFT}, KeymapKey{0, 1, 1, KC_A}), + std::make_pair(KeymapKey{0, 0, 0, OSM(MOD_RALT), KC_RALT}, KeymapKey{0, 1, 1, KC_A}), + std::make_pair(KeymapKey{0, 0, 0, OSM(MOD_RGUI), KC_RGUI}, KeymapKey{0, 1, 1, KC_A}) + )); +// clang-format on + +TEST_F(OneShot, OSLWithAdditionalKeypress) { + TestDriver driver; + InSequence s; + KeymapKey osl_key = KeymapKey{0, 0, 0, OSL(1)}; + KeymapKey osl_key1 = KeymapKey{1, 0, 0, KC_X}; + KeymapKey regular_key0 = KeymapKey{0, 1, 0, KC_Y}; + KeymapKey regular_key1 = KeymapKey{1, 1, 0, KC_A}; + + set_keymap({osl_key, osl_key1, regular_key0, regular_key1}); + + // Press OSL key. + EXPECT_NO_REPORT(driver); + osl_key.press(); + run_one_scan_loop(); + VERIFY_AND_CLEAR(driver); + + // Release OSL key. + EXPECT_NO_REPORT(driver); + osl_key.release(); + run_one_scan_loop(); + VERIFY_AND_CLEAR(driver); + + // Press regular key. + EXPECT_REPORT(driver, (regular_key1.report_code)); + EXPECT_EMPTY_REPORT(driver); + regular_key1.press(); + run_one_scan_loop(); + VERIFY_AND_CLEAR(driver); + + // Release regular key. + EXPECT_NO_REPORT(driver); + regular_key1.release(); + run_one_scan_loop(); + VERIFY_AND_CLEAR(driver); +} + +TEST_F(OneShot, OSLWithOsmAndAdditionalKeypress) { + TestDriver driver; + InSequence s; + KeymapKey osl_key = KeymapKey{0, 0, 0, OSL(1)}; + KeymapKey osm_key = KeymapKey{1, 1, 0, OSM(MOD_LSFT), KC_LSFT}; + KeymapKey regular_key = KeymapKey{1, 1, 1, KC_A}; + KeymapKey blank_key = KeymapKey{1, 0, 0, KC_NO}; + + set_keymap({osl_key, osm_key, regular_key, blank_key}); + + // Press OSL key. + EXPECT_NO_REPORT(driver); + osl_key.press(); + run_one_scan_loop(); + VERIFY_AND_CLEAR(driver); + + // Release OSL key. + EXPECT_NO_REPORT(driver); + osl_key.release(); + run_one_scan_loop(); + EXPECT_TRUE(layer_state_is(1)); + VERIFY_AND_CLEAR(driver); + + // Press and release OSM. + EXPECT_NO_REPORT(driver); + tap_key(osm_key); + EXPECT_TRUE(layer_state_is(1)); + VERIFY_AND_CLEAR(driver); + + // Tap regular key. + EXPECT_REPORT(driver, (osm_key.report_code, regular_key.report_code)); + EXPECT_EMPTY_REPORT(driver); + tap_key(regular_key); + VERIFY_AND_CLEAR(driver); +} diff --git a/tests/tap_hold_configurations/chordal_hold/permissive_hold/test_tap_hold.cpp b/tests/tap_hold_configurations/chordal_hold/permissive_hold/test_tap_hold.cpp new file mode 100644 index 00000000000..c600573be42 --- /dev/null +++ b/tests/tap_hold_configurations/chordal_hold/permissive_hold/test_tap_hold.cpp @@ -0,0 +1,576 @@ +// Copyright 2024 Google LLC +// +// 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 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_user({.col = MATRIX_COLS - 1, .row = 0}), 'R'); + EXPECT_EQ(chordal_hold_handedness_user({.col = 0, .row = 2}), '*'); +} + +TEST_F(ChordalHoldPermissiveHold, get_chordal_hold_default) { + auto make_record = [](uint8_t row, uint8_t col, keyevent_type_t type = KEY_EVENT) { + return keyrecord_t{ + .event = + { + .key = {.col = col, .row = row}, + .type = type, + .pressed = true, + }, + }; + }; + // 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); + // 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(ChordalHoldPermissiveHold, 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 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 key. + EXPECT_EMPTY_REPORT(driver); + mod_tap_key.release(); + run_one_scan_loop(); + VERIFY_AND_CLEAR(driver); +} + +TEST_F(ChordalHoldPermissiveHold, 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); + + // 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); + regular_key.release(); + run_one_scan_loop(); + VERIFY_AND_CLEAR(driver); +} + +TEST_F(ChordalHoldPermissiveHold, 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_key = KeymapKey(0, 1, 0, SFT_T(KC_P)); + auto regular_key = KeymapKey(0, 2, 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_P)); + EXPECT_REPORT(driver, (KC_P, KC_A)); + EXPECT_REPORT(driver, (KC_P)); + tap_key(regular_key); + run_one_scan_loop(); + 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(ChordalHoldPermissiveHold, 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(ChordalHoldPermissiveHold, 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(ChordalHoldPermissiveHold, two_mod_taps_nested_press_opposite_hands) { + TestDriver driver; + InSequence s; + auto mod_tap_key1 = KeymapKey(0, 1, 0, SFT_T(KC_A)); + auto mod_tap_key2 = KeymapKey(0, MATRIX_COLS - 1, 0, RSFT_T(KC_B)); + + set_keymap({mod_tap_key1, mod_tap_key2}); + + // Press mod-tap keys. + EXPECT_NO_REPORT(driver); + mod_tap_key1.press(); + run_one_scan_loop(); + mod_tap_key2.press(); + run_one_scan_loop(); + VERIFY_AND_CLEAR(driver); + + // Release mod-tap keys. + EXPECT_REPORT(driver, (KC_LEFT_SHIFT)); + EXPECT_REPORT(driver, (KC_LEFT_SHIFT, KC_B)); + EXPECT_REPORT(driver, (KC_LEFT_SHIFT)); + mod_tap_key2.release(); + run_one_scan_loop(); + VERIFY_AND_CLEAR(driver); + + EXPECT_EMPTY_REPORT(driver); + mod_tap_key1.release(); + run_one_scan_loop(); + VERIFY_AND_CLEAR(driver); +} + +TEST_F(ChordalHoldPermissiveHold, two_mod_taps_nested_press_same_hand) { + 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 keys. + EXPECT_NO_REPORT(driver); + mod_tap_key1.press(); + run_one_scan_loop(); + mod_tap_key2.press(); + run_one_scan_loop(); + VERIFY_AND_CLEAR(driver); + + // Release mod-tap keys. + EXPECT_REPORT(driver, (KC_A)); + EXPECT_REPORT(driver, (KC_A, KC_B)); + EXPECT_REPORT(driver, (KC_A)); + mod_tap_key2.release(); + run_one_scan_loop(); + + EXPECT_EMPTY_REPORT(driver); + mod_tap_key1.release(); + run_one_scan_loop(); + VERIFY_AND_CLEAR(driver); +} + +TEST_F(ChordalHoldPermissiveHold, three_mod_taps_same_hand_streak_roll) { + TestDriver driver; + InSequence s; + auto mod_tap_key1 = KeymapKey(0, 1, 0, SFT_T(KC_A)); + auto mod_tap_key2 = KeymapKey(0, 2, 0, CTL_T(KC_B)); + auto mod_tap_key3 = KeymapKey(0, 3, 0, RSFT_T(KC_C)); + + set_keymap({mod_tap_key1, mod_tap_key2, mod_tap_key3}); + + // Press mod-tap keys. + EXPECT_NO_REPORT(driver); + mod_tap_key1.press(); + run_one_scan_loop(); + mod_tap_key2.press(); + run_one_scan_loop(); + mod_tap_key3.press(); + run_one_scan_loop(); + VERIFY_AND_CLEAR(driver); + + // Release keys 1, 2, 3. + EXPECT_REPORT(driver, (KC_A)); + EXPECT_REPORT(driver, (KC_A, KC_B)); + EXPECT_REPORT(driver, (KC_A, KC_B, KC_C)); + EXPECT_REPORT(driver, (KC_B, KC_C)); + EXPECT_REPORT(driver, (KC_C)); + EXPECT_EMPTY_REPORT(driver); + mod_tap_key1.release(); + run_one_scan_loop(); + mod_tap_key2.release(); + run_one_scan_loop(); + mod_tap_key3.release(); + run_one_scan_loop(); + VERIFY_AND_CLEAR(driver); + + EXPECT_NO_REPORT(driver); + VERIFY_AND_CLEAR(driver); +} + +TEST_F(ChordalHoldPermissiveHold, three_mod_taps_same_hand_streak_orders) { + TestDriver driver; + InSequence s; + auto mod_tap_key1 = KeymapKey(0, 1, 0, SFT_T(KC_A)); + auto mod_tap_key2 = KeymapKey(0, 2, 0, CTL_T(KC_B)); + auto mod_tap_key3 = KeymapKey(0, 3, 0, RSFT_T(KC_C)); + + set_keymap({mod_tap_key1, mod_tap_key2, mod_tap_key3}); + + // Press mod-tap keys. + EXPECT_NO_REPORT(driver); + mod_tap_key1.press(); + run_one_scan_loop(); + mod_tap_key2.press(); + run_one_scan_loop(); + mod_tap_key3.press(); + run_one_scan_loop(); + VERIFY_AND_CLEAR(driver); + + // Release keys 3, 2, 1. + EXPECT_REPORT(driver, (KC_A)); + EXPECT_REPORT(driver, (KC_A, KC_B)); + EXPECT_REPORT(driver, (KC_A, KC_B, KC_C)); + EXPECT_REPORT(driver, (KC_A, KC_B)); + EXPECT_REPORT(driver, (KC_A)); + EXPECT_EMPTY_REPORT(driver); + mod_tap_key3.release(); + run_one_scan_loop(); + mod_tap_key2.release(); + run_one_scan_loop(); + mod_tap_key1.release(); + run_one_scan_loop(); + VERIFY_AND_CLEAR(driver); + + // Press mod-tap keys. + EXPECT_NO_REPORT(driver); + idle_for(TAPPING_TERM); + mod_tap_key1.press(); + run_one_scan_loop(); + mod_tap_key2.press(); + run_one_scan_loop(); + mod_tap_key3.press(); + run_one_scan_loop(); + VERIFY_AND_CLEAR(driver); + + // Release keys 3, 1, 2. + EXPECT_REPORT(driver, (KC_A)); + EXPECT_REPORT(driver, (KC_A, KC_B)); + EXPECT_REPORT(driver, (KC_A, KC_B, KC_C)); + EXPECT_REPORT(driver, (KC_A, KC_B)); + EXPECT_REPORT(driver, (KC_B)); + EXPECT_EMPTY_REPORT(driver); + mod_tap_key3.release(); + run_one_scan_loop(); + mod_tap_key1.release(); + run_one_scan_loop(); + mod_tap_key2.release(); + run_one_scan_loop(); + VERIFY_AND_CLEAR(driver); + + // Press mod-tap keys. + EXPECT_NO_REPORT(driver); + idle_for(TAPPING_TERM); + mod_tap_key1.press(); + run_one_scan_loop(); + mod_tap_key2.press(); + run_one_scan_loop(); + mod_tap_key3.press(); + run_one_scan_loop(); + VERIFY_AND_CLEAR(driver); + + // Release keys 2, 3, 1. + EXPECT_REPORT(driver, (KC_A)); + EXPECT_REPORT(driver, (KC_A, KC_B)); + EXPECT_REPORT(driver, (KC_A, KC_B, KC_C)); + EXPECT_REPORT(driver, (KC_A, KC_C)); + EXPECT_REPORT(driver, (KC_A)); + EXPECT_EMPTY_REPORT(driver); + mod_tap_key2.release(); + run_one_scan_loop(); + mod_tap_key3.release(); + run_one_scan_loop(); + mod_tap_key1.release(); + run_one_scan_loop(); + VERIFY_AND_CLEAR(driver); + + EXPECT_NO_REPORT(driver); + VERIFY_AND_CLEAR(driver); +} + +TEST_F(ChordalHoldPermissiveHold, three_mod_taps_two_held_one_tapped) { + TestDriver driver; + InSequence s; + auto mod_tap_key1 = KeymapKey(0, 1, 0, SFT_T(KC_A)); + auto mod_tap_key2 = KeymapKey(0, 2, 0, CTL_T(KC_B)); + auto mod_tap_key3 = KeymapKey(0, MATRIX_COLS - 1, 0, RSFT_T(KC_C)); + + set_keymap({mod_tap_key1, mod_tap_key2, mod_tap_key3}); + + // Press mod-tap keys. + EXPECT_NO_REPORT(driver); + mod_tap_key1.press(); + run_one_scan_loop(); + mod_tap_key2.press(); + run_one_scan_loop(); + mod_tap_key3.press(); + run_one_scan_loop(); + VERIFY_AND_CLEAR(driver); + + // Release key 3. + EXPECT_REPORT(driver, (KC_LEFT_SHIFT)); + EXPECT_REPORT(driver, (KC_LEFT_SHIFT, KC_LEFT_CTRL)); + EXPECT_REPORT(driver, (KC_LEFT_SHIFT, KC_LEFT_CTRL, KC_C)); + EXPECT_REPORT(driver, (KC_LEFT_SHIFT, KC_LEFT_CTRL)); + mod_tap_key3.release(); + run_one_scan_loop(); + VERIFY_AND_CLEAR(driver); + + // Release key 2, then key 1. + EXPECT_REPORT(driver, (KC_LEFT_SHIFT)); + EXPECT_EMPTY_REPORT(driver); + mod_tap_key2.release(); + run_one_scan_loop(); + mod_tap_key1.release(); + run_one_scan_loop(); + VERIFY_AND_CLEAR(driver); + + // Press mod-tap keys. + EXPECT_NO_REPORT(driver); + idle_for(TAPPING_TERM); + mod_tap_key1.press(); + run_one_scan_loop(); + mod_tap_key2.press(); + run_one_scan_loop(); + mod_tap_key3.press(); + run_one_scan_loop(); + VERIFY_AND_CLEAR(driver); + + // Release key 3. + EXPECT_REPORT(driver, (KC_LEFT_SHIFT)); + EXPECT_REPORT(driver, (KC_LEFT_SHIFT, KC_LEFT_CTRL)); + EXPECT_REPORT(driver, (KC_LEFT_SHIFT, KC_LEFT_CTRL, KC_C)); + EXPECT_REPORT(driver, (KC_LEFT_SHIFT, KC_LEFT_CTRL)); + mod_tap_key3.release(); + run_one_scan_loop(); + VERIFY_AND_CLEAR(driver); + + // Release key 1, then key 2. + EXPECT_REPORT(driver, (KC_LEFT_CTRL)); + EXPECT_EMPTY_REPORT(driver); + mod_tap_key1.release(); + run_one_scan_loop(); + mod_tap_key2.release(); + run_one_scan_loop(); + VERIFY_AND_CLEAR(driver); +} + +TEST_F(ChordalHoldPermissiveHold, three_mod_taps_one_held_two_tapped) { + TestDriver driver; + InSequence s; + auto mod_tap_key1 = KeymapKey(0, 1, 0, SFT_T(KC_A)); + auto mod_tap_key2 = KeymapKey(0, MATRIX_COLS - 2, 0, CTL_T(KC_B)); + auto mod_tap_key3 = KeymapKey(0, MATRIX_COLS - 1, 0, RSFT_T(KC_C)); + + set_keymap({mod_tap_key1, mod_tap_key2, mod_tap_key3}); + + // Press mod-tap keys. + EXPECT_NO_REPORT(driver); + mod_tap_key1.press(); + run_one_scan_loop(); + mod_tap_key2.press(); + run_one_scan_loop(); + mod_tap_key3.press(); + run_one_scan_loop(); + VERIFY_AND_CLEAR(driver); + + // Release keys 3, 2, 1. + EXPECT_REPORT(driver, (KC_LEFT_SHIFT)); + EXPECT_REPORT(driver, (KC_LEFT_SHIFT, KC_B)); + EXPECT_REPORT(driver, (KC_LEFT_SHIFT, KC_B, KC_C)); + EXPECT_REPORT(driver, (KC_LEFT_SHIFT, KC_B)); + EXPECT_REPORT(driver, (KC_LEFT_SHIFT)); + EXPECT_EMPTY_REPORT(driver); + mod_tap_key3.release(); + run_one_scan_loop(); + mod_tap_key2.release(); + run_one_scan_loop(); + mod_tap_key1.release(); + run_one_scan_loop(); + VERIFY_AND_CLEAR(driver); + + // Press mod-tap keys. + EXPECT_NO_REPORT(driver); + idle_for(TAPPING_TERM); + mod_tap_key1.press(); + run_one_scan_loop(); + mod_tap_key2.press(); + run_one_scan_loop(); + mod_tap_key3.press(); + run_one_scan_loop(); + VERIFY_AND_CLEAR(driver); + + // Release keys 3, 1, 2. + EXPECT_REPORT(driver, (KC_LEFT_SHIFT)); + EXPECT_REPORT(driver, (KC_LEFT_SHIFT, KC_B)); + EXPECT_REPORT(driver, (KC_LEFT_SHIFT, KC_B, KC_C)); + EXPECT_REPORT(driver, (KC_LEFT_SHIFT, KC_B)); + EXPECT_REPORT(driver, (KC_B)); + EXPECT_EMPTY_REPORT(driver); + mod_tap_key3.release(); + run_one_scan_loop(); + mod_tap_key1.release(); + run_one_scan_loop(); + mod_tap_key2.release(); + run_one_scan_loop(); + VERIFY_AND_CLEAR(driver); +} + +TEST_F(ChordalHoldPermissiveHold, two_mod_taps_one_regular_key) { + TestDriver driver; + InSequence s; + auto mod_tap_key1 = KeymapKey(0, 1, 0, SFT_T(KC_A)); + auto mod_tap_key2 = KeymapKey(0, MATRIX_COLS - 2, 0, CTL_T(KC_B)); + auto regular_key = KeymapKey(0, MATRIX_COLS - 1, 0, KC_C); + + set_keymap({mod_tap_key1, mod_tap_key2, regular_key}); + + // Press mod-tap keys. + EXPECT_NO_REPORT(driver); + mod_tap_key1.press(); + run_one_scan_loop(); + mod_tap_key2.press(); + run_one_scan_loop(); + regular_key.press(); + run_one_scan_loop(); + VERIFY_AND_CLEAR(driver); + + // Release key 3. + EXPECT_REPORT(driver, (KC_LEFT_SHIFT)); + EXPECT_REPORT(driver, (KC_LEFT_SHIFT, KC_B)); + EXPECT_REPORT(driver, (KC_LEFT_SHIFT, KC_B, KC_C)); + EXPECT_REPORT(driver, (KC_LEFT_SHIFT, KC_C)); + EXPECT_REPORT(driver, (KC_LEFT_SHIFT)); + mod_tap_key2.release(); + run_one_scan_loop(); + regular_key.release(); + run_one_scan_loop(); + VERIFY_AND_CLEAR(driver); + + // Release key 2, then key 1. + EXPECT_EMPTY_REPORT(driver); + mod_tap_key1.release(); + run_one_scan_loop(); + VERIFY_AND_CLEAR(driver); + + // Press mod-tap keys. + EXPECT_REPORT(driver, (KC_LEFT_SHIFT)); + EXPECT_REPORT(driver, (KC_LEFT_SHIFT, KC_B)); + EXPECT_REPORT(driver, (KC_LEFT_SHIFT, KC_B, KC_C)); + EXPECT_REPORT(driver, (KC_LEFT_SHIFT, KC_B)); + EXPECT_REPORT(driver, (KC_B)); + EXPECT_EMPTY_REPORT(driver); + idle_for(TAPPING_TERM); + mod_tap_key1.press(); + run_one_scan_loop(); + mod_tap_key2.press(); + run_one_scan_loop(); + regular_key.press(); + run_one_scan_loop(); + // Release keys. + regular_key.release(); + run_one_scan_loop(); + 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/retro_shift_permissive_hold/config.h b/tests/tap_hold_configurations/chordal_hold/retro_shift_permissive_hold/config.h new file mode 100644 index 00000000000..8a4e8d07a07 --- /dev/null +++ b/tests/tap_hold_configurations/chordal_hold/retro_shift_permissive_hold/config.h @@ -0,0 +1,27 @@ +/* Copyright 2022 Isaac Elenbaas + * + * 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 +#define PERMISSIVE_HOLD + +#define RETRO_SHIFT 2 * TAPPING_TERM +// releases between AUTO_SHIFT_TIMEOUT and TAPPING_TERM are not tested +#define AUTO_SHIFT_TIMEOUT TAPPING_TERM +#define AUTO_SHIFT_MODIFIERS diff --git a/tests/tap_hold_configurations/chordal_hold/retro_shift_permissive_hold/test.mk b/tests/tap_hold_configurations/chordal_hold/retro_shift_permissive_hold/test.mk new file mode 100644 index 00000000000..0660eb3e6b8 --- /dev/null +++ b/tests/tap_hold_configurations/chordal_hold/retro_shift_permissive_hold/test.mk @@ -0,0 +1,16 @@ +# Copyright 2022 Isaac Elenbaas +# +# 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 . + +AUTO_SHIFT_ENABLE = yes diff --git a/tests/tap_hold_configurations/chordal_hold/retro_shift_permissive_hold/test_retro_shift.cpp b/tests/tap_hold_configurations/chordal_hold/retro_shift_permissive_hold/test_retro_shift.cpp new file mode 100644 index 00000000000..8d56fcc266e --- /dev/null +++ b/tests/tap_hold_configurations/chordal_hold/retro_shift_permissive_hold/test_retro_shift.cpp @@ -0,0 +1,419 @@ +/* Copyright 2022 Isaac Elenbaas + * + * 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" + +bool get_auto_shifted_key(uint16_t keycode, keyrecord_t *record) { + return true; +} + +using testing::_; +using testing::AnyNumber; +using testing::AnyOf; +using testing::InSequence; + +class RetroShiftPermissiveHold : public TestFixture {}; + +TEST_F(RetroShiftPermissiveHold, tap_regular_key_while_mod_tap_key_is_held_under_tapping_term) { + TestDriver driver; + InSequence s; + auto mod_tap_hold_key = KeymapKey(0, 0, 0, CTL_T(KC_P)); + 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(); + testing::Mock::VerifyAndClearExpectations(&driver); + + /* Press regular key. */ + EXPECT_NO_REPORT(driver); + regular_key.press(); + run_one_scan_loop(); + testing::Mock::VerifyAndClearExpectations(&driver); + + /* Release regular key. */ + EXPECT_CALL(driver, send_keyboard_mock(KeyboardReport(KC_LCTL))).Times(AnyNumber()); + EXPECT_REPORT(driver, (KC_LCTL, KC_A)); + EXPECT_REPORT(driver, (KC_LCTL)); + regular_key.release(); + run_one_scan_loop(); + testing::Mock::VerifyAndClearExpectations(&driver); + + /* Release mod-tap-hold key. */ + EXPECT_EMPTY_REPORT(driver); + mod_tap_hold_key.release(); + run_one_scan_loop(); + testing::Mock::VerifyAndClearExpectations(&driver); +} + +TEST_F(RetroShiftPermissiveHold, tap_mod_tap_key_while_mod_tap_key_is_held_under_tapping_term) { + TestDriver driver; + InSequence s; + auto mod_tap_hold_key = KeymapKey(0, 0, 0, CTL_T(KC_P)); + auto mod_tap_regular_key = KeymapKey(0, MATRIX_COLS - 1, 0, ALT_T(KC_A)); + + set_keymap({mod_tap_hold_key, mod_tap_regular_key}); + + /* Press mod-tap-hold key. */ + EXPECT_NO_REPORT(driver); + mod_tap_hold_key.press(); + run_one_scan_loop(); + testing::Mock::VerifyAndClearExpectations(&driver); + + /* Press mod-tap-regular key. */ + EXPECT_NO_REPORT(driver); + mod_tap_regular_key.press(); + run_one_scan_loop(); + testing::Mock::VerifyAndClearExpectations(&driver); + + /* Release mod-tap-regular key. */ + EXPECT_CALL(driver, send_keyboard_mock(KeyboardReport(KC_LCTL))).Times(AnyNumber()); + EXPECT_REPORT(driver, (KC_LCTL, KC_A)); + EXPECT_REPORT(driver, (KC_LCTL)); + mod_tap_regular_key.release(); + run_one_scan_loop(); + testing::Mock::VerifyAndClearExpectations(&driver); + + /* Release mod-tap-hold key. */ + EXPECT_EMPTY_REPORT(driver); + mod_tap_hold_key.release(); + run_one_scan_loop(); + testing::Mock::VerifyAndClearExpectations(&driver); +} + +TEST_F(RetroShiftPermissiveHold, tap_regular_key_while_mod_tap_key_is_held_over_tapping_term) { + TestDriver driver; + InSequence s; + auto mod_tap_hold_key = KeymapKey(0, 0, 0, CTL_T(KC_P)); + 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(); + testing::Mock::VerifyAndClearExpectations(&driver); + + /* Press regular key. */ + EXPECT_NO_REPORT(driver); + regular_key.press(); + run_one_scan_loop(); + testing::Mock::VerifyAndClearExpectations(&driver); + + /* Release regular key. */ + EXPECT_CALL(driver, send_keyboard_mock(KeyboardReport(KC_LCTL))).Times(AnyNumber()); + EXPECT_REPORT(driver, (KC_LCTL, KC_A)); + EXPECT_REPORT(driver, (KC_LCTL)); + regular_key.release(); + run_one_scan_loop(); + idle_for(TAPPING_TERM); + testing::Mock::VerifyAndClearExpectations(&driver); + + /* Release mod-tap-hold key. */ + EXPECT_EMPTY_REPORT(driver); + mod_tap_hold_key.release(); + run_one_scan_loop(); + testing::Mock::VerifyAndClearExpectations(&driver); +} + +TEST_F(RetroShiftPermissiveHold, tap_mod_tap_key_while_mod_tap_key_is_held_over_tapping_term) { + TestDriver driver; + InSequence s; + auto mod_tap_hold_key = KeymapKey(0, 0, 0, CTL_T(KC_P)); + auto mod_tap_regular_key = KeymapKey(0, MATRIX_COLS - 1, 0, ALT_T(KC_A)); + + set_keymap({mod_tap_hold_key, mod_tap_regular_key}); + + /* Press mod-tap-hold key. */ + EXPECT_NO_REPORT(driver); + mod_tap_hold_key.press(); + run_one_scan_loop(); + testing::Mock::VerifyAndClearExpectations(&driver); + + /* Press mod-tap-regular key. */ + EXPECT_NO_REPORT(driver); + mod_tap_regular_key.press(); + run_one_scan_loop(); + testing::Mock::VerifyAndClearExpectations(&driver); + + /* Release mod-tap-regular key. */ + EXPECT_CALL(driver, send_keyboard_mock(KeyboardReport(KC_LCTL))).Times(AnyNumber()); + EXPECT_REPORT(driver, (KC_LCTL, KC_A)); + EXPECT_REPORT(driver, (KC_LCTL)); + mod_tap_regular_key.release(); + run_one_scan_loop(); + idle_for(TAPPING_TERM); + testing::Mock::VerifyAndClearExpectations(&driver); + + /* Release mod-tap-hold key. */ + EXPECT_EMPTY_REPORT(driver); + mod_tap_hold_key.release(); + run_one_scan_loop(); + testing::Mock::VerifyAndClearExpectations(&driver); +} + +TEST_F(RetroShiftPermissiveHold, hold_regular_key_while_mod_tap_key_is_held_over_tapping_term) { + TestDriver driver; + InSequence s; + auto mod_tap_hold_key = KeymapKey(0, 0, 0, CTL_T(KC_P)); + 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(); + testing::Mock::VerifyAndClearExpectations(&driver); + + /* Press regular key. */ + EXPECT_NO_REPORT(driver); + regular_key.press(); + run_one_scan_loop(); + idle_for(AUTO_SHIFT_TIMEOUT); + testing::Mock::VerifyAndClearExpectations(&driver); + + /* Release regular key. */ + // clang-format off + EXPECT_CALL(driver, send_keyboard_mock(AnyOf( + KeyboardReport(KC_LCTL, KC_LSFT), + KeyboardReport(KC_LSFT), + KeyboardReport(KC_LCTL)))) + .Times(AnyNumber()); + // clang-format on + EXPECT_REPORT(driver, (KC_LCTL, KC_LSFT, KC_A)); + // clang-format off + EXPECT_CALL(driver, send_keyboard_mock(AnyOf( + KeyboardReport(KC_LCTL, KC_LSFT), + KeyboardReport(KC_LSFT)))) + .Times(AnyNumber()); + // clang-format on + EXPECT_REPORT(driver, (KC_LCTL)); + regular_key.release(); + run_one_scan_loop(); + testing::Mock::VerifyAndClearExpectations(&driver); + + /* Release mod-tap-hold key. */ + EXPECT_EMPTY_REPORT(driver); + mod_tap_hold_key.release(); + run_one_scan_loop(); + testing::Mock::VerifyAndClearExpectations(&driver); +} + +TEST_F(RetroShiftPermissiveHold, hold_mod_tap_key_while_mod_tap_key_is_held_over_tapping_term) { + TestDriver driver; + InSequence s; + auto mod_tap_hold_key = KeymapKey(0, 0, 0, CTL_T(KC_P)); + auto mod_tap_regular_key = KeymapKey(0, MATRIX_COLS - 1, 0, ALT_T(KC_A)); + + set_keymap({mod_tap_hold_key, mod_tap_regular_key}); + + /* Press mod-tap-hold key. */ + EXPECT_NO_REPORT(driver); + mod_tap_hold_key.press(); + run_one_scan_loop(); + testing::Mock::VerifyAndClearExpectations(&driver); + + /* Press mod-tap-regular key. */ + EXPECT_NO_REPORT(driver); + mod_tap_regular_key.press(); + run_one_scan_loop(); + idle_for(AUTO_SHIFT_TIMEOUT); + testing::Mock::VerifyAndClearExpectations(&driver); + + /* Release mod-tap-regular key. */ + // clang-format off + EXPECT_CALL(driver, send_keyboard_mock(AnyOf( + KeyboardReport(KC_LCTL, KC_LSFT), + KeyboardReport(KC_LSFT), + KeyboardReport(KC_LCTL)))) + .Times(AnyNumber()); + // clang-format on + EXPECT_REPORT(driver, (KC_LCTL, KC_LSFT, KC_A)); + // clang-format off + EXPECT_CALL(driver, send_keyboard_mock(AnyOf( + KeyboardReport(KC_LCTL, KC_LSFT), + KeyboardReport(KC_LSFT)))) + .Times(AnyNumber()); + // clang-format on + EXPECT_REPORT(driver, (KC_LCTL)); + mod_tap_regular_key.release(); + run_one_scan_loop(); + idle_for(TAPPING_TERM); + testing::Mock::VerifyAndClearExpectations(&driver); + + /* Release mod-tap-hold key. */ + EXPECT_EMPTY_REPORT(driver); + mod_tap_hold_key.release(); + run_one_scan_loop(); + testing::Mock::VerifyAndClearExpectations(&driver); +} + +TEST_F(RetroShiftPermissiveHold, roll_tap_regular_key_while_mod_tap_key_is_held_under_tapping_term) { + TestDriver driver; + InSequence s; + auto mod_tap_hold_key = KeymapKey(0, 0, 0, CTL_T(KC_P)); + 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(); + testing::Mock::VerifyAndClearExpectations(&driver); + + /* Press regular key. */ + EXPECT_NO_REPORT(driver); + regular_key.press(); + run_one_scan_loop(); + testing::Mock::VerifyAndClearExpectations(&driver); + + /* Release mod-tap-hold key. */ + EXPECT_REPORT(driver, (KC_P)); + EXPECT_EMPTY_REPORT(driver); + mod_tap_hold_key.release(); + run_one_scan_loop(); + testing::Mock::VerifyAndClearExpectations(&driver); + + /* Release regular key. */ + EXPECT_REPORT(driver, (KC_A)); + EXPECT_EMPTY_REPORT(driver); + regular_key.release(); + run_one_scan_loop(); + testing::Mock::VerifyAndClearExpectations(&driver); +} + +TEST_F(RetroShiftPermissiveHold, roll_tap_mod_tap_key_while_mod_tap_key_is_held_under_tapping_term) { + TestDriver driver; + InSequence s; + auto mod_tap_hold_key = KeymapKey(0, 0, 0, CTL_T(KC_P)); + auto mod_tap_regular_key = KeymapKey(0, MATRIX_COLS - 1, 0, ALT_T(KC_A)); + + set_keymap({mod_tap_hold_key, mod_tap_regular_key}); + + /* Press mod-tap-hold key. */ + EXPECT_NO_REPORT(driver); + mod_tap_hold_key.press(); + run_one_scan_loop(); + testing::Mock::VerifyAndClearExpectations(&driver); + + /* Press mod-tap-regular key. */ + EXPECT_NO_REPORT(driver); + mod_tap_regular_key.press(); + run_one_scan_loop(); + testing::Mock::VerifyAndClearExpectations(&driver); + + /* Release mod-tap-hold key. */ + EXPECT_REPORT(driver, (KC_P)); + EXPECT_EMPTY_REPORT(driver); + mod_tap_hold_key.release(); + run_one_scan_loop(); + testing::Mock::VerifyAndClearExpectations(&driver); + + /* Release mod-tap-regular key. */ + EXPECT_REPORT(driver, (KC_A)); + EXPECT_EMPTY_REPORT(driver); + mod_tap_regular_key.release(); + run_one_scan_loop(); + testing::Mock::VerifyAndClearExpectations(&driver); +} + +TEST_F(RetroShiftPermissiveHold, roll_hold_regular_key_while_mod_tap_key_is_held_under_tapping_term) { + TestDriver driver; + InSequence s; + auto mod_tap_hold_key = KeymapKey(0, 0, 0, CTL_T(KC_P)); + 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(); + testing::Mock::VerifyAndClearExpectations(&driver); + + /* Press regular key. */ + EXPECT_NO_REPORT(driver); + regular_key.press(); + run_one_scan_loop(); + testing::Mock::VerifyAndClearExpectations(&driver); + + /* Release mod-tap-hold key. */ + EXPECT_REPORT(driver, (KC_P)); + EXPECT_EMPTY_REPORT(driver); + EXPECT_CALL(driver, send_keyboard_mock(KeyboardReport(KC_LSFT))).Times(AnyNumber()); + EXPECT_REPORT(driver, (KC_LSFT, KC_A)); + EXPECT_CALL(driver, send_keyboard_mock(KeyboardReport(KC_LSFT))).Times(AnyNumber()); + EXPECT_EMPTY_REPORT(driver); + mod_tap_hold_key.release(); + run_one_scan_loop(); + idle_for(AUTO_SHIFT_TIMEOUT); + testing::Mock::VerifyAndClearExpectations(&driver); + + /* Release regular key. */ + EXPECT_NO_REPORT(driver); + regular_key.release(); + run_one_scan_loop(); + testing::Mock::VerifyAndClearExpectations(&driver); +} + +TEST_F(RetroShiftPermissiveHold, roll_hold_mod_tap_key_while_mod_tap_key_is_held_under_tapping_term) { + TestDriver driver; + InSequence s; + auto mod_tap_hold_key = KeymapKey(0, 0, 0, CTL_T(KC_P)); + auto mod_tap_regular_key = KeymapKey(0, MATRIX_COLS - 1, 0, ALT_T(KC_A)); + + set_keymap({mod_tap_hold_key, mod_tap_regular_key}); + + /* Press mod-tap-hold key. */ + EXPECT_NO_REPORT(driver); + mod_tap_hold_key.press(); + run_one_scan_loop(); + testing::Mock::VerifyAndClearExpectations(&driver); + + /* Press mod-tap-regular key. */ + EXPECT_NO_REPORT(driver); + mod_tap_regular_key.press(); + run_one_scan_loop(); + testing::Mock::VerifyAndClearExpectations(&driver); + + /* Release mod-tap-hold key. */ + EXPECT_REPORT(driver, (KC_P)); + EXPECT_EMPTY_REPORT(driver); + mod_tap_hold_key.release(); + run_one_scan_loop(); + testing::Mock::VerifyAndClearExpectations(&driver); + + /* Release mod-tap-regular key. */ + EXPECT_CALL(driver, send_keyboard_mock(KeyboardReport(KC_LSFT))).Times(AnyNumber()); + EXPECT_REPORT(driver, (KC_LSFT, KC_A)); + EXPECT_CALL(driver, send_keyboard_mock(KeyboardReport(KC_LSFT))).Times(AnyNumber()); + EXPECT_EMPTY_REPORT(driver); + idle_for(AUTO_SHIFT_TIMEOUT); + mod_tap_regular_key.release(); + run_one_scan_loop(); + testing::Mock::VerifyAndClearExpectations(&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 deleted file mode 100644 index c18d568798a..00000000000 --- a/tests/tap_hold_configurations/chordal_hold_and_hold_on_other_key_press/test_tap_hold.cpp +++ /dev/null @@ -1,229 +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 ChordalHoldAndHoldOnOtherKeypress : public TestFixture {}; - -TEST_F(ChordalHoldAndHoldOnOtherKeypress, chord_with_mod_tap_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); - - // Press regular key. - EXPECT_REPORT(driver, (KC_LEFT_SHIFT)); - EXPECT_REPORT(driver, (KC_LEFT_SHIFT, KC_A)); - regular_key.press(); - run_one_scan_loop(); - 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_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); -} - -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_key = KeymapKey(0, 1, 0, SFT_T(KC_P)); - auto regular_key = KeymapKey(0, 2, 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); - - // 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(); - run_one_scan_loop(); - 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, 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(ChordalHoldAndHoldOnOtherKeypress, 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(ChordalHoldAndHoldOnOtherKeypress, 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_permissive_hold/test_keymap.c b/tests/tap_hold_configurations/chordal_hold_and_permissive_hold/test_keymap.c deleted file mode 100644 index 195f970a799..00000000000 --- a/tests/tap_hold_configurations/chordal_hold_and_permissive_hold/test_keymap.c +++ /dev/null @@ -1,7 +0,0 @@ -#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 deleted file mode 100644 index df506b9c099..00000000000 --- a/tests/tap_hold_configurations/chordal_hold_and_permissive_hold/test_tap_hold.cpp +++ /dev/null @@ -1,242 +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; - -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, 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_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); - - // 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 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); - - // 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); - regular_key.release(); - run_one_scan_loop(); - VERIFY_AND_CLEAR(driver); -} - -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_key = KeymapKey(0, 1, 0, SFT_T(KC_P)); - auto regular_key = KeymapKey(0, 2, 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_P)); - EXPECT_REPORT(driver, (KC_P, KC_A)); - EXPECT_REPORT(driver, (KC_P)); - tap_key(regular_key); - run_one_scan_loop(); - 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(ChordalHoldAndPermissiveHold, 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(ChordalHoldAndPermissiveHold, 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(ChordalHoldAndPermissiveHold, 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); -} From 99d49aca58f9c44da87ed81695d0404009d8d092 Mon Sep 17 00:00:00 2001 From: Pascal Getreuer Date: Thu, 7 Nov 2024 20:47:29 -0800 Subject: [PATCH 06/13] Fix formatting. --- quantum/action_tapping.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/quantum/action_tapping.c b/quantum/action_tapping.c index 22e0e23b2d0..367a5a5b898 100644 --- a/quantum/action_tapping.c +++ b/quantum/action_tapping.c @@ -648,7 +648,7 @@ static uint8_t waiting_buffer_find_chordal_hold_tap(void) { 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]; + keyrecord_t * cur = &waiting_buffer[i]; const uint16_t cur_keycode = get_record_keycode(cur, false); if (!cur->event.pressed || !is_tap_record(prev) || is_one_shot(prev_keycode)) { From da8ccf0d187330b8fcede09b147548c74d09573b Mon Sep 17 00:00:00 2001 From: Pascal Getreuer Date: Fri, 8 Nov 2024 13:32:16 -0800 Subject: [PATCH 07/13] Simplification and additional test. --- quantum/action_tapping.c | 117 +++++++++--------- .../hold_on_other_key_press/test_tap_hold.cpp | 16 ++- .../permissive_hold/test_tap_hold.cpp | 77 ++++++++++-- 3 files changed, 139 insertions(+), 71 deletions(-) diff --git a/quantum/action_tapping.c b/quantum/action_tapping.c index 367a5a5b898..6d90fd458c0 100644 --- a/quantum/action_tapping.c +++ b/quantum/action_tapping.c @@ -50,17 +50,12 @@ __attribute__((weak)) bool get_permissive_hold(uint16_t keycode, keyrecord_t *re # endif # ifdef CHORDAL_HOLD -// Tracks whether multiple tap-hold keys are simultaenously unsettled. -static bool multi_tap_sequence = false; - static bool is_one_shot(uint16_t keycode) { return IS_QK_ONE_SHOT_MOD(keycode) || IS_QK_ONE_SHOT_LAYER(keycode); } -static uint8_t waiting_buffer_find_chordal_hold_tap(void); +static uint8_t waiting_buffer_find_chordal_hold(void); static void waiting_buffer_process_until(uint8_t new_tail); static void waiting_buffer_process_regular(void); -# else -# define multi_tap_sequence false # endif // CHORDAL_HOLD # ifdef HOLD_ON_OTHER_KEY_PRESS_PER_KEY @@ -124,11 +119,6 @@ void action_tapping_process(keyrecord_t record) { if (IS_EVENT(record.event)) { ac_dprintf("\n"); } -# ifdef CHORDAL_HOLD - if (waiting_buffer_tail == waiting_buffer_head) { - multi_tap_sequence = false; - } -# endif } /* Some conditionally defined helper macros to keep process_tapping more @@ -199,20 +189,6 @@ bool process_tapping(keyrecord_t *keyp) { waiting_buffer_scan_tap(); debug_tapping_key(); } else { -# if defined(CHORDAL_HOLD) - // If keyp is a tap-hold key release, and the tail waiting buffer - // is the corresponding press, settle the key as tapped. - if (waiting_buffer_tail != waiting_buffer_head && is_tap_record(keyp)) { - keyrecord_t *tail = &waiting_buffer[waiting_buffer_tail]; - if (IS_EVENT(tail->event) && KEYEQ(tail->event.key, keyp->event.key) && tail->event.pressed) { - tail->tap.count = 1; - keyp->tap.count = 1; - process_record(tail); - // Pop tail from the queue. - waiting_buffer_tail = (waiting_buffer_tail + 1) % WAITING_BUFFER_SIZE; - } - } -# endif // the current key is just a regular key, pass it on for regular // processing process_record(keyp); @@ -232,25 +208,8 @@ bool process_tapping(keyrecord_t *keyp) { // early return for tick events return true; } -# ifdef CHORDAL_HOLD - if (!event.pressed && multi_tap_sequence && waiting_buffer_typed(event)) { - const uint8_t first_tap = waiting_buffer_find_chordal_hold_tap(); - // Settle and process the tapping key and waiting events - // preceding first_tap as *held*. - if (first_tap < WAITING_BUFFER_SIZE) { - ac_dprintf("Tapping: End. No tap. Interfered by typing key\n"); - process_record(&tapping_key); - tapping_key = (keyrecord_t){0}; - debug_tapping_key(); - - waiting_buffer_process_until(first_tap); - } - // enqueue - return false; - } else -# endif - if (tapping_key.tap.count == 0) { + if (tapping_key.tap.count == 0) { if (IS_TAPPING_RECORD(keyp) && !event.pressed) { // first tap! ac_dprintf("Tapping: First tap(0->1).\n"); @@ -263,6 +222,28 @@ bool process_tapping(keyrecord_t *keyp) { // enqueue return false; } +# ifdef CHORDAL_HOLD + else if (!TAP_GET_RETRO_TAPPING(keyp) && waiting_buffer_tail != waiting_buffer_head && is_tap_record(&waiting_buffer[waiting_buffer_tail]) && !event.pressed && waiting_buffer_typed(event)) { + /* (!event.pressed || !is_tap_record(keyp)) && !is_one_shot(get_record_keycode(keyp, false))) { */ + const uint8_t new_tail = waiting_buffer_find_chordal_hold(); + ac_dprintf("Chordal hold: new tail = %u\n", new_tail); + + // Settle and process the tapping key and waiting events + // preceding first_tap as *held*. + if (new_tail < WAITING_BUFFER_SIZE) { + ac_dprintf("Tapping: End. No tap. Chord considered held\n"); + process_record(&tapping_key); + tapping_key = (keyrecord_t){0}; + debug_tapping_key(); + + waiting_buffer_process_until(new_tail); + waiting_buffer_process_regular(); + } + + // enqueue + return false; + } +# endif /* 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. @@ -288,8 +269,21 @@ 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 (!multi_tap_sequence && !event.pressed && !waiting_buffer_typed(event)) { + else if (!event.pressed && !waiting_buffer_typed(event)) { // Modifier/Layer should be retained till end of this tapping. action_t action = layer_switch_get_action(event.key); switch (action.kind.id) { @@ -330,10 +324,7 @@ bool process_tapping(keyrecord_t *keyp) { // this needs to be set false. tapping_key.tap.interrupted = false; - if (is_tap_record(keyp)) { - // Multiple tap-hold keys are unsettled. - multi_tap_sequence = true; - } else { + if (!is_tap_record(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"); @@ -357,7 +348,17 @@ bool process_tapping(keyrecord_t *keyp) { // 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}; + +# ifdef 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; + } else +# endif + { + tapping_key = (keyrecord_t){0}; + } debug_tapping_key(); } } @@ -636,32 +637,28 @@ __attribute__((weak)) char chordal_hold_handedness_user(keypos_t key) { /** \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 - * lastest event in the queue that settles as held according to + * for a successive pair of keys that settles as held according to * get_chordal_hold(). * - * \return Index of the first tap, or equivalently, one past the latest hold. + * \return Buffer index of the hold, or WAITING_BUFFER_SIZE if none is found. */ -static uint8_t waiting_buffer_find_chordal_hold_tap(void) { +static uint8_t waiting_buffer_find_chordal_hold(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]; + keyrecord_t *cur = &waiting_buffer[i]; const uint16_t cur_keycode = get_record_keycode(cur, false); - if (!cur->event.pressed || !is_tap_record(prev) || is_one_shot(prev_keycode)) { - break; - } else if (get_chordal_hold(prev_keycode, prev, cur_keycode, cur)) { - first_tap = i; // Track one index past the latest hold. + if (get_chordal_hold(prev_keycode, prev, cur_keycode, cur)) { + return i; } prev = cur; prev_keycode = cur_keycode; } - return first_tap; + return WAITING_BUFFER_SIZE; } /** \brief Processes and pops buffered events preceding `new_tail`. */ diff --git a/tests/tap_hold_configurations/chordal_hold/hold_on_other_key_press/test_tap_hold.cpp b/tests/tap_hold_configurations/chordal_hold/hold_on_other_key_press/test_tap_hold.cpp index 1c9e6de2ec0..4c6ebfec7f4 100644 --- a/tests/tap_hold_configurations/chordal_hold/hold_on_other_key_press/test_tap_hold.cpp +++ b/tests/tap_hold_configurations/chordal_hold/hold_on_other_key_press/test_tap_hold.cpp @@ -273,10 +273,22 @@ TEST_F(ChordalHoldHoldOnOtherKeyPress, three_mod_taps_same_hand_streak_roll) { VERIFY_AND_CLEAR(driver); // Release keys 1, 2, 3. + // + // NOTE: The correct order of events should be + // EXPECT_REPORT(driver, (KC_A)); + // EXPECT_REPORT(driver, (KC_A, KC_B)); + // EXPECT_REPORT(driver, (KC_A, KC_B, KC_C)); + // EXPECT_REPORT(driver, (KC_B, KC_C)); + // EXPECT_REPORT(driver, (KC_C)); + // EXPECT_EMPTY_REPORT(driver); + // + // However, due to a workaround for https://github.com/tmk/tmk_keyboard/issues/60, + // the events are processed out of order, with the first two keys released + // before pressing KC_C. EXPECT_REPORT(driver, (KC_A)); EXPECT_REPORT(driver, (KC_A, KC_B)); - EXPECT_REPORT(driver, (KC_A, KC_B, KC_C)); - EXPECT_REPORT(driver, (KC_B, KC_C)); + EXPECT_REPORT(driver, (KC_B)); + EXPECT_EMPTY_REPORT(driver); EXPECT_REPORT(driver, (KC_C)); EXPECT_EMPTY_REPORT(driver); mod_tap_key1.release(); diff --git a/tests/tap_hold_configurations/chordal_hold/permissive_hold/test_tap_hold.cpp b/tests/tap_hold_configurations/chordal_hold/permissive_hold/test_tap_hold.cpp index c600573be42..b13b17286c9 100644 --- a/tests/tap_hold_configurations/chordal_hold/permissive_hold/test_tap_hold.cpp +++ b/tests/tap_hold_configurations/chordal_hold/permissive_hold/test_tap_hold.cpp @@ -278,10 +278,22 @@ TEST_F(ChordalHoldPermissiveHold, three_mod_taps_same_hand_streak_roll) { VERIFY_AND_CLEAR(driver); // Release keys 1, 2, 3. + // + // NOTE: The correct order of events should be + // EXPECT_REPORT(driver, (KC_A)); + // EXPECT_REPORT(driver, (KC_A, KC_B)); + // EXPECT_REPORT(driver, (KC_A, KC_B, KC_C)); + // EXPECT_REPORT(driver, (KC_B, KC_C)); + // EXPECT_REPORT(driver, (KC_C)); + // EXPECT_EMPTY_REPORT(driver); + // + // However, due to a workaround for https://github.com/tmk/tmk_keyboard/issues/60, + // the events are processed out of order, with the first two keys released + // before pressing KC_C. EXPECT_REPORT(driver, (KC_A)); EXPECT_REPORT(driver, (KC_A, KC_B)); - EXPECT_REPORT(driver, (KC_A, KC_B, KC_C)); - EXPECT_REPORT(driver, (KC_B, KC_C)); + EXPECT_REPORT(driver, (KC_B)); + EXPECT_EMPTY_REPORT(driver); EXPECT_REPORT(driver, (KC_C)); EXPECT_EMPTY_REPORT(driver); mod_tap_key1.release(); @@ -291,9 +303,6 @@ TEST_F(ChordalHoldPermissiveHold, three_mod_taps_same_hand_streak_roll) { mod_tap_key3.release(); run_one_scan_loop(); VERIFY_AND_CLEAR(driver); - - EXPECT_NO_REPORT(driver); - VERIFY_AND_CLEAR(driver); } TEST_F(ChordalHoldPermissiveHold, three_mod_taps_same_hand_streak_orders) { @@ -386,6 +395,55 @@ TEST_F(ChordalHoldPermissiveHold, three_mod_taps_same_hand_streak_orders) { VERIFY_AND_CLEAR(driver); } +TEST_F(ChordalHoldPermissiveHold, three_mod_taps_opposite_hands_roll) { + TestDriver driver; + InSequence s; + auto mod_tap_key1 = KeymapKey(0, 1, 0, SFT_T(KC_A)); + auto mod_tap_key2 = KeymapKey(0, 2, 0, CTL_T(KC_B)); + auto mod_tap_key3 = KeymapKey(0, MATRIX_COLS - 1, 0, RSFT_T(KC_C)); + + set_keymap({mod_tap_key1, mod_tap_key2, mod_tap_key3}); + + // Press mod-tap keys. + EXPECT_NO_REPORT(driver); + mod_tap_key1.press(); + run_one_scan_loop(); + mod_tap_key2.press(); + run_one_scan_loop(); + mod_tap_key3.press(); + run_one_scan_loop(); + VERIFY_AND_CLEAR(driver); + + // Release keys 1, 2, 3. + // + // NOTE: The correct order of events should be + // EXPECT_REPORT(driver, (KC_A)); + // EXPECT_REPORT(driver, (KC_A, KC_B)); + // EXPECT_REPORT(driver, (KC_A, KC_B, KC_C)); + // EXPECT_REPORT(driver, (KC_B, KC_C)); + // EXPECT_REPORT(driver, (KC_C)); + // EXPECT_EMPTY_REPORT(driver); + // + // However, due to a workaround for https://github.com/tmk/tmk_keyboard/issues/60, + // the events are processed out of order, with the first two keys released + // before pressing KC_C. + + // Release keys 1, 2, 3. + EXPECT_REPORT(driver, (KC_A)); + EXPECT_REPORT(driver, (KC_A, KC_B)); + EXPECT_REPORT(driver, (KC_B)); + EXPECT_EMPTY_REPORT(driver); + EXPECT_REPORT(driver, (KC_C)); + EXPECT_EMPTY_REPORT(driver); + mod_tap_key1.release(); + run_one_scan_loop(); + mod_tap_key2.release(); + run_one_scan_loop(); + mod_tap_key3.release(); + run_one_scan_loop(); + VERIFY_AND_CLEAR(driver); +} + TEST_F(ChordalHoldPermissiveHold, three_mod_taps_two_held_one_tapped) { TestDriver driver; InSequence s; @@ -523,7 +581,7 @@ TEST_F(ChordalHoldPermissiveHold, two_mod_taps_one_regular_key) { set_keymap({mod_tap_key1, mod_tap_key2, regular_key}); - // Press mod-tap keys. + // Press keys. EXPECT_NO_REPORT(driver); mod_tap_key1.press(); run_one_scan_loop(); @@ -533,19 +591,20 @@ TEST_F(ChordalHoldPermissiveHold, two_mod_taps_one_regular_key) { run_one_scan_loop(); VERIFY_AND_CLEAR(driver); - // Release key 3. + // Release keys. EXPECT_REPORT(driver, (KC_LEFT_SHIFT)); EXPECT_REPORT(driver, (KC_LEFT_SHIFT, KC_B)); EXPECT_REPORT(driver, (KC_LEFT_SHIFT, KC_B, KC_C)); EXPECT_REPORT(driver, (KC_LEFT_SHIFT, KC_C)); - EXPECT_REPORT(driver, (KC_LEFT_SHIFT)); mod_tap_key2.release(); run_one_scan_loop(); + VERIFY_AND_CLEAR(driver); + + EXPECT_REPORT(driver, (KC_LEFT_SHIFT)); regular_key.release(); run_one_scan_loop(); VERIFY_AND_CLEAR(driver); - // Release key 2, then key 1. EXPECT_EMPTY_REPORT(driver); mod_tap_key1.release(); run_one_scan_loop(); From 1606d67f51e8d7cc8d77e9983bb1ecb93e974bac Mon Sep 17 00:00:00 2001 From: Pascal Getreuer Date: Fri, 8 Nov 2024 13:36:12 -0800 Subject: [PATCH 08/13] Fix formatting. --- quantum/action_tapping.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/quantum/action_tapping.c b/quantum/action_tapping.c index 6d90fd458c0..49ac47107dc 100644 --- a/quantum/action_tapping.c +++ b/quantum/action_tapping.c @@ -647,7 +647,7 @@ static uint8_t waiting_buffer_find_chordal_hold(void) { uint16_t prev_keycode = get_record_keycode(&tapping_key, false); for (uint8_t i = waiting_buffer_tail; i != waiting_buffer_head; i = (i + 1) % WAITING_BUFFER_SIZE) { - keyrecord_t *cur = &waiting_buffer[i]; + keyrecord_t * cur = &waiting_buffer[i]; const uint16_t cur_keycode = get_record_keycode(cur, false); if (get_chordal_hold(prev_keycode, prev, cur_keycode, cur)) { From bd7e54a31dbf54cf4e3e183173cb2b29ba403483 Mon Sep 17 00:00:00 2001 From: Pascal Getreuer Date: Sun, 10 Nov 2024 21:01:06 -0800 Subject: [PATCH 09/13] Tighten tests. --- .../hold_on_other_key_press/test_tap_hold.cpp | 24 ++++++++----- .../permissive_hold/test_tap_hold.cpp | 36 ++++++++++++------- 2 files changed, 40 insertions(+), 20 deletions(-) diff --git a/tests/tap_hold_configurations/chordal_hold/hold_on_other_key_press/test_tap_hold.cpp b/tests/tap_hold_configurations/chordal_hold/hold_on_other_key_press/test_tap_hold.cpp index 4c6ebfec7f4..a6281ad32b7 100644 --- a/tests/tap_hold_configurations/chordal_hold/hold_on_other_key_press/test_tap_hold.cpp +++ b/tests/tap_hold_configurations/chordal_hold/hold_on_other_key_press/test_tap_hold.cpp @@ -107,12 +107,15 @@ TEST_F(ChordalHoldHoldOnOtherKeyPress, chord_rolled_press_settled_as_hold) { run_one_scan_loop(); VERIFY_AND_CLEAR(driver); - // Press regular key and release mod-tap key. + // Press regular 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(); + VERIFY_AND_CLEAR(driver); + + // Release mod-tap key. + EXPECT_REPORT(driver, (KC_A)); mod_tap_key.release(); run_one_scan_loop(); VERIFY_AND_CLEAR(driver); @@ -210,16 +213,21 @@ TEST_F(ChordalHoldHoldOnOtherKeyPress, two_mod_taps_nested_press_opposite_hands) run_one_scan_loop(); VERIFY_AND_CLEAR(driver); - // Tap second mod-tap key. + // Press second mod-tap key. EXPECT_REPORT(driver, (KC_LEFT_SHIFT)); - EXPECT_REPORT(driver, (KC_LEFT_SHIFT, KC_B)); - EXPECT_REPORT(driver, (KC_LEFT_SHIFT)); - EXPECT_EMPTY_REPORT(driver); mod_tap_key2.press(); run_one_scan_loop(); + VERIFY_AND_CLEAR(driver); + + // Release second mod-tap key. + EXPECT_REPORT(driver, (KC_LEFT_SHIFT, KC_B)); + EXPECT_REPORT(driver, (KC_LEFT_SHIFT)); mod_tap_key2.release(); run_one_scan_loop(); + VERIFY_AND_CLEAR(driver); + // Release first mod-tap key. + EXPECT_EMPTY_REPORT(driver); mod_tap_key1.release(); run_one_scan_loop(); VERIFY_AND_CLEAR(driver); @@ -237,6 +245,8 @@ TEST_F(ChordalHoldHoldOnOtherKeyPress, two_mod_taps_nested_press_same_hand) { EXPECT_NO_REPORT(driver); mod_tap_key1.press(); run_one_scan_loop(); + mod_tap_key2.press(); + run_one_scan_loop(); VERIFY_AND_CLEAR(driver); // Release mod-tap keys. @@ -244,8 +254,6 @@ TEST_F(ChordalHoldHoldOnOtherKeyPress, two_mod_taps_nested_press_same_hand) { EXPECT_REPORT(driver, (KC_A, KC_B)); EXPECT_REPORT(driver, (KC_A)); EXPECT_EMPTY_REPORT(driver); - mod_tap_key2.press(); - run_one_scan_loop(); mod_tap_key2.release(); run_one_scan_loop(); mod_tap_key1.release(); diff --git a/tests/tap_hold_configurations/chordal_hold/permissive_hold/test_tap_hold.cpp b/tests/tap_hold_configurations/chordal_hold/permissive_hold/test_tap_hold.cpp index b13b17286c9..efa5aeab9d1 100644 --- a/tests/tap_hold_configurations/chordal_hold/permissive_hold/test_tap_hold.cpp +++ b/tests/tap_hold_configurations/chordal_hold/permissive_hold/test_tap_hold.cpp @@ -109,18 +109,18 @@ TEST_F(ChordalHoldPermissiveHold, chord_rolled_press_settled_as_tap) { set_keymap({mod_tap_key, regular_key}); - // Press mod-tap key. + // Press mod-tap key and regular key. EXPECT_NO_REPORT(driver); mod_tap_key.press(); run_one_scan_loop(); + regular_key.press(); + run_one_scan_loop(); VERIFY_AND_CLEAR(driver); - // Press regular key and release mod-tap key. + // 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); @@ -147,11 +147,16 @@ TEST_F(ChordalHoldPermissiveHold, non_chord_with_mod_tap_settled_as_tap) { run_one_scan_loop(); VERIFY_AND_CLEAR(driver); - // Tap 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. EXPECT_REPORT(driver, (KC_P)); - tap_key(regular_key); + regular_key.release(); run_one_scan_loop(); VERIFY_AND_CLEAR(driver); @@ -611,12 +616,7 @@ TEST_F(ChordalHoldPermissiveHold, two_mod_taps_one_regular_key) { VERIFY_AND_CLEAR(driver); // Press mod-tap keys. - EXPECT_REPORT(driver, (KC_LEFT_SHIFT)); - EXPECT_REPORT(driver, (KC_LEFT_SHIFT, KC_B)); - EXPECT_REPORT(driver, (KC_LEFT_SHIFT, KC_B, KC_C)); - EXPECT_REPORT(driver, (KC_LEFT_SHIFT, KC_B)); - EXPECT_REPORT(driver, (KC_B)); - EXPECT_EMPTY_REPORT(driver); + EXPECT_NO_REPORT(driver); idle_for(TAPPING_TERM); mod_tap_key1.press(); run_one_scan_loop(); @@ -624,11 +624,23 @@ TEST_F(ChordalHoldPermissiveHold, two_mod_taps_one_regular_key) { run_one_scan_loop(); regular_key.press(); run_one_scan_loop(); + VERIFY_AND_CLEAR(driver); + // Release keys. + EXPECT_REPORT(driver, (KC_LEFT_SHIFT)); + EXPECT_REPORT(driver, (KC_LEFT_SHIFT, KC_B)); + EXPECT_REPORT(driver, (KC_LEFT_SHIFT, KC_B, KC_C)); + EXPECT_REPORT(driver, (KC_LEFT_SHIFT, KC_B)); regular_key.release(); run_one_scan_loop(); + VERIFY_AND_CLEAR(driver); + + EXPECT_REPORT(driver, (KC_B)); mod_tap_key1.release(); run_one_scan_loop(); + VERIFY_AND_CLEAR(driver); + + EXPECT_EMPTY_REPORT(driver); mod_tap_key2.release(); run_one_scan_loop(); VERIFY_AND_CLEAR(driver); From 6b558247db5bb07551f76e99a9a2d42806dd8f4b Mon Sep 17 00:00:00 2001 From: Pascal Getreuer Date: Wed, 13 Nov 2024 22:47:28 -0800 Subject: [PATCH 10/13] Add test two_mod_taps_same_hand_hold_til_timeout. --- .../hold_on_other_key_press/test_tap_hold.cpp | 34 +++++++++++++++++++ .../permissive_hold/test_tap_hold.cpp | 34 +++++++++++++++++++ 2 files changed, 68 insertions(+) diff --git a/tests/tap_hold_configurations/chordal_hold/hold_on_other_key_press/test_tap_hold.cpp b/tests/tap_hold_configurations/chordal_hold/hold_on_other_key_press/test_tap_hold.cpp index a6281ad32b7..e551a75dee3 100644 --- a/tests/tap_hold_configurations/chordal_hold/hold_on_other_key_press/test_tap_hold.cpp +++ b/tests/tap_hold_configurations/chordal_hold/hold_on_other_key_press/test_tap_hold.cpp @@ -199,6 +199,40 @@ TEST_F(ChordalHoldHoldOnOtherKeyPress, hold_mod_tap_key) { VERIFY_AND_CLEAR(driver); } +TEST_F(ChordalHoldHoldOnOtherKeyPress, two_mod_taps_same_hand_hold_til_timeout) { + TestDriver driver; + InSequence s; + auto mod_tap_key1 = KeymapKey(0, MATRIX_COLS - 2, 0, RCTL_T(KC_A)); + auto mod_tap_key2 = KeymapKey(0, MATRIX_COLS - 1, 0, RSFT_T(KC_B)); + + set_keymap({mod_tap_key1, mod_tap_key2}); + + // Press mod-tap keys. + EXPECT_NO_REPORT(driver); + mod_tap_key1.press(); + run_one_scan_loop(); + mod_tap_key2.press(); + run_one_scan_loop(); + VERIFY_AND_CLEAR(driver); + + // Continue holding til the tapping term. + EXPECT_REPORT(driver, (KC_RIGHT_CTRL)); + EXPECT_REPORT(driver, (KC_RIGHT_CTRL, KC_RIGHT_SHIFT)); + idle_for(TAPPING_TERM); + VERIFY_AND_CLEAR(driver); + + // Release mod-tap keys. + EXPECT_REPORT(driver, (KC_RIGHT_SHIFT)); + mod_tap_key1.release(); + run_one_scan_loop(); + VERIFY_AND_CLEAR(driver); + + EXPECT_EMPTY_REPORT(driver); + mod_tap_key2.release(); + run_one_scan_loop(); + VERIFY_AND_CLEAR(driver); +} + TEST_F(ChordalHoldHoldOnOtherKeyPress, two_mod_taps_nested_press_opposite_hands) { TestDriver driver; InSequence s; diff --git a/tests/tap_hold_configurations/chordal_hold/permissive_hold/test_tap_hold.cpp b/tests/tap_hold_configurations/chordal_hold/permissive_hold/test_tap_hold.cpp index efa5aeab9d1..bcc4a6f91dd 100644 --- a/tests/tap_hold_configurations/chordal_hold/permissive_hold/test_tap_hold.cpp +++ b/tests/tap_hold_configurations/chordal_hold/permissive_hold/test_tap_hold.cpp @@ -204,6 +204,40 @@ TEST_F(ChordalHoldPermissiveHold, hold_mod_tap_key) { VERIFY_AND_CLEAR(driver); } +TEST_F(ChordalHoldPermissiveHold, two_mod_taps_same_hand_hold_til_timeout) { + TestDriver driver; + InSequence s; + auto mod_tap_key1 = KeymapKey(0, MATRIX_COLS - 2, 0, RCTL_T(KC_A)); + auto mod_tap_key2 = KeymapKey(0, MATRIX_COLS - 1, 0, RSFT_T(KC_B)); + + set_keymap({mod_tap_key1, mod_tap_key2}); + + // Press mod-tap keys. + EXPECT_NO_REPORT(driver); + mod_tap_key1.press(); + run_one_scan_loop(); + mod_tap_key2.press(); + run_one_scan_loop(); + VERIFY_AND_CLEAR(driver); + + // Continue holding til the tapping term. + EXPECT_REPORT(driver, (KC_RIGHT_CTRL)); + EXPECT_REPORT(driver, (KC_RIGHT_CTRL, KC_RIGHT_SHIFT)); + idle_for(TAPPING_TERM); + VERIFY_AND_CLEAR(driver); + + // Release mod-tap keys. + EXPECT_REPORT(driver, (KC_RIGHT_SHIFT)); + mod_tap_key1.release(); + run_one_scan_loop(); + VERIFY_AND_CLEAR(driver); + + EXPECT_EMPTY_REPORT(driver); + mod_tap_key2.release(); + run_one_scan_loop(); + VERIFY_AND_CLEAR(driver); +} + TEST_F(ChordalHoldPermissiveHold, two_mod_taps_nested_press_opposite_hands) { TestDriver driver; InSequence s; From e924a0cd36b7f377fd194a112635c38c034f4d2b Mon Sep 17 00:00:00 2001 From: Pascal Getreuer Date: Fri, 15 Nov 2024 20:08:52 -0800 Subject: [PATCH 11/13] Revise handing of pairs of tap-hold keys. --- docs/tap_hold.md | 106 ++++++++++++-- quantum/action_tapping.c | 128 ++++++++--------- .../hold_on_other_key_press/test_tap_hold.cpp | 62 ++++++--- .../permissive_hold/test_tap_hold.cpp | 129 +++++++++++++----- 4 files changed, 300 insertions(+), 125 deletions(-) diff --git a/docs/tap_hold.md b/docs/tap_hold.md index 84d0a17ab88..0ade6a86737 100644 --- a/docs/tap_hold.md +++ b/docs/tap_hold.md @@ -560,26 +560,106 @@ 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 LCTL_T(KC_Z): + 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; + case RCTL_T(KC_SLSH): + 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. +As shown in the last line above, you may use +`get_chordal_hold_default(tap_hold_record, other_record)` to get the default tap +vs. hold decision according to the opposite hands rule. + +If you use home row mods, you may want to produce a hotkey like Ctrl+Shift+V by +holding Ctrl and Shift mod-taps on one hand while tapping `KC_V` with the other +hand, say: + +- `RCTL_T(KC_K)` Down +- `RSFT_T(KC_L)` Down (on the same hand as `RCTL_T(KC_K)`) +- `KC_V` Down +- `KC_V` Up +- `RCTL_T(KC_K)` Up +- `RSFT_T(KC_L)` Up + +However, supposing `RCTL_T(KC_K)` and `RSFT_T(KC_L)` are on the same hand, +Chordal Hold by default considers `RCTL_T(KC_K)` tapped, producing "`kV`" +instead of the desired Ctrl+Shift+V. + +To address this, `get_chordal_hold()` may be defined to allow chords between any +pair of mod-tap keys with + +```c +bool get_chordal_hold(uint16_t tap_hold_keycode, keyrecord_t* tap_hold_record, + uint16_t other_keycode, keyrecord_t* other_record) { + // Allow hold between any pair of mod-tap keys. + if (IS_QK_MOD_TAP(tap_hold_keycode) && IS_QK_MOD_TAP(other_keycode)) { + return true; + } + + // Otherwise defer to the opposite hands rule. + return get_chordal_hold_default(tap_hold_record, other_record); +} +``` + +Or to allow one-handed chords of specific mod-taps but not others, use: + +```c +bool get_chordal_hold(uint16_t tap_hold_keycode, keyrecord_t* tap_hold_record, + uint16_t other_keycode, keyrecord_t* other_record) { + switch (tap_hold_keycode) { + case RCTL_T(KC_K): + if (other_keycode == RSFT_T(KC_L)) { + // Allow hold in "RCTL_T(KC_K) down, RSFT_T(KC_L) down". + return true; + } + break; + + case RSFT_T(KC_L): + if (other_keycode == RCTL_T(KC_K)) { + // Allow hold in "RSFT_T(KC_L) down, RCTL_T(KC_K) down". + return true; + } + break; + } + // Otherwise defer to the opposite hands rule. + return get_chordal_hold_default(tap_hold_record, other_record); +} +``` + +Above, two exceptions are defined, one where `RCTL_T(KC_K)` is pressed first and +another where `RSFT_T(KC_L)` is held first, such that Ctrl+Shift+V could be done +by holding the mod-taps in either order. For yet finer control, you could choose +to define an exception for one order but not the other: + +```c +bool get_chordal_hold(uint16_t tap_hold_keycode, keyrecord_t* tap_hold_record, + uint16_t other_keycode, keyrecord_t* other_record) { + switch (tap_hold_keycode) { + case RCTL_T(KC_K): + if (other_keycode == RSFT_T(KC_L)) { + // Allow hold in "RCTL_T(KC_K) down, RSFT_T(KC_L), down". + return true; + } + break; + + // ... but RSFT_T(KC_L) is considered tapped in + // "RSFT_T(KC_L) down, RCTL_T(KC_K) down". + } + // Otherwise defer to the opposite hands rule. + return get_chordal_hold_default(tap_hold_record, other_record); +} +``` ## Retro Tapping diff --git a/quantum/action_tapping.c b/quantum/action_tapping.c index 49ac47107dc..53a3413c385 100644 --- a/quantum/action_tapping.c +++ b/quantum/action_tapping.c @@ -50,12 +50,25 @@ __attribute__((weak)) bool get_permissive_hold(uint16_t keycode, keyrecord_t *re # endif # ifdef CHORDAL_HOLD +# 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); +/** Processes and pops buffered events until the first tap-hold event. */ +static void waiting_buffer_process_regular(void); + static bool is_one_shot(uint16_t keycode) { return IS_QK_ONE_SHOT_MOD(keycode) || IS_QK_ONE_SHOT_LAYER(keycode); } -static uint8_t waiting_buffer_find_chordal_hold(void); -static void waiting_buffer_process_until(uint8_t new_tail); -static void waiting_buffer_process_regular(void); # endif // CHORDAL_HOLD # ifdef HOLD_ON_OTHER_KEY_PRESS_PER_KEY @@ -175,6 +188,20 @@ void action_tapping_process(keyrecord_t record) { bool process_tapping(keyrecord_t *keyp) { const keyevent_t event = keyp->event; +# ifdef 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("CHORDAL_HOLD: 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)) { @@ -222,28 +249,6 @@ bool process_tapping(keyrecord_t *keyp) { // enqueue return false; } -# ifdef CHORDAL_HOLD - else if (!TAP_GET_RETRO_TAPPING(keyp) && waiting_buffer_tail != waiting_buffer_head && is_tap_record(&waiting_buffer[waiting_buffer_tail]) && !event.pressed && waiting_buffer_typed(event)) { - /* (!event.pressed || !is_tap_record(keyp)) && !is_one_shot(get_record_keycode(keyp, false))) { */ - const uint8_t new_tail = waiting_buffer_find_chordal_hold(); - ac_dprintf("Chordal hold: new tail = %u\n", new_tail); - - // Settle and process the tapping key and waiting events - // preceding first_tap as *held*. - if (new_tail < WAITING_BUFFER_SIZE) { - ac_dprintf("Tapping: End. No tap. Chord considered held\n"); - process_record(&tapping_key); - tapping_key = (keyrecord_t){0}; - debug_tapping_key(); - - waiting_buffer_process_until(new_tail); - waiting_buffer_process_regular(); - } - - // enqueue - return false; - } -# endif /* 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. @@ -324,17 +329,15 @@ bool process_tapping(keyrecord_t *keyp) { // this needs to be set false. tapping_key.tap.interrupted = false; - if (!is_tap_record(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; - process_record(&tapping_key); - debug_tapping_key(); + 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}; - // Process regular keys in the waiting buffer. - waiting_buffer_process_regular(); - } + // Process regular keys in the waiting buffer. + waiting_buffer_process_regular(); } else # endif if (TAP_GET_HOLD_ON_OTHER_KEY_PRESS @@ -634,44 +637,43 @@ __attribute__((weak)) char chordal_hold_handedness_user(keypos_t key) { # endif } -/** \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 - * for a successive pair of keys that settles as held according to - * get_chordal_hold(). - * - * \return Buffer index of the hold, or WAITING_BUFFER_SIZE if none is found. - */ -static uint8_t waiting_buffer_find_chordal_hold(void) { - keyrecord_t *prev = &tapping_key; - uint16_t prev_keycode = get_record_keycode(&tapping_key, false); +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; + } - 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); + registered_taps[num_registered_taps] = key; + ++num_registered_taps; +} - if (get_chordal_hold(prev_keycode, prev, cur_keycode, cur)) { +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; } - - prev = cur; - prev_keycode = cur_keycode; } - - return WAITING_BUFFER_SIZE; + return -1; } -/** \brief Processes and pops buffered events preceding `new_tail`. */ -static void waiting_buffer_process_until(uint8_t new_tail) { - for (; waiting_buffer_tail != new_tail; waiting_buffer_tail = (waiting_buffer_tail + 1) % WAITING_BUFFER_SIZE) { - ac_dprintf("waiting_buffer_process_until: processing [%u]\n", waiting_buffer_tail); - process_record(&waiting_buffer[waiting_buffer_tail]); +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]; + } } - - debug_waiting_buffer(); } -/** \brief Processes and pops buffered events until the first tap-hold event. */ +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 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])) { diff --git a/tests/tap_hold_configurations/chordal_hold/hold_on_other_key_press/test_tap_hold.cpp b/tests/tap_hold_configurations/chordal_hold/hold_on_other_key_press/test_tap_hold.cpp index e551a75dee3..cb08e5c3fbf 100644 --- a/tests/tap_hold_configurations/chordal_hold/hold_on_other_key_press/test_tap_hold.cpp +++ b/tests/tap_hold_configurations/chordal_hold/hold_on_other_key_press/test_tap_hold.cpp @@ -211,13 +211,15 @@ TEST_F(ChordalHoldHoldOnOtherKeyPress, two_mod_taps_same_hand_hold_til_timeout) EXPECT_NO_REPORT(driver); mod_tap_key1.press(); run_one_scan_loop(); + VERIFY_AND_CLEAR(driver); + + EXPECT_REPORT(driver, (KC_A)); mod_tap_key2.press(); run_one_scan_loop(); VERIFY_AND_CLEAR(driver); // Continue holding til the tapping term. - EXPECT_REPORT(driver, (KC_RIGHT_CTRL)); - EXPECT_REPORT(driver, (KC_RIGHT_CTRL, KC_RIGHT_SHIFT)); + EXPECT_REPORT(driver, (KC_A, KC_RIGHT_SHIFT)); idle_for(TAPPING_TERM); VERIFY_AND_CLEAR(driver); @@ -279,12 +281,14 @@ TEST_F(ChordalHoldHoldOnOtherKeyPress, two_mod_taps_nested_press_same_hand) { EXPECT_NO_REPORT(driver); mod_tap_key1.press(); run_one_scan_loop(); + VERIFY_AND_CLEAR(driver); + + EXPECT_REPORT(driver, (KC_A)); mod_tap_key2.press(); run_one_scan_loop(); VERIFY_AND_CLEAR(driver); // Release mod-tap keys. - EXPECT_REPORT(driver, (KC_A)); EXPECT_REPORT(driver, (KC_A, KC_B)); EXPECT_REPORT(driver, (KC_A)); EXPECT_EMPTY_REPORT(driver); @@ -308,8 +312,14 @@ TEST_F(ChordalHoldHoldOnOtherKeyPress, three_mod_taps_same_hand_streak_roll) { EXPECT_NO_REPORT(driver); mod_tap_key1.press(); run_one_scan_loop(); + VERIFY_AND_CLEAR(driver); + + EXPECT_REPORT(driver, (KC_A)); mod_tap_key2.press(); run_one_scan_loop(); + VERIFY_AND_CLEAR(driver); + + EXPECT_REPORT(driver, (KC_A, KC_B)); mod_tap_key3.press(); run_one_scan_loop(); VERIFY_AND_CLEAR(driver); @@ -327,8 +337,6 @@ TEST_F(ChordalHoldHoldOnOtherKeyPress, three_mod_taps_same_hand_streak_roll) { // However, due to a workaround for https://github.com/tmk/tmk_keyboard/issues/60, // the events are processed out of order, with the first two keys released // before pressing KC_C. - EXPECT_REPORT(driver, (KC_A)); - EXPECT_REPORT(driver, (KC_A, KC_B)); EXPECT_REPORT(driver, (KC_B)); EXPECT_EMPTY_REPORT(driver); EXPECT_REPORT(driver, (KC_C)); @@ -399,9 +407,20 @@ TEST_F(ChordalHoldHoldOnOtherKeyPress, three_mod_taps_same_hand_streak_orders) { run_one_scan_loop(); VERIFY_AND_CLEAR(driver); + // NOTE: The correct order of events should be + // EXPECT_REPORT(driver, (KC_A)); + // EXPECT_REPORT(driver, (KC_A, KC_B)); + // EXPECT_REPORT(driver, (KC_A, KC_B, KC_C)); + // EXPECT_REPORT(driver, (KC_A, KC_C)); + // EXPECT_REPORT(driver, (KC_A)); + // EXPECT_EMPTY_REPORT(driver); + // + // However, due to a workaround for https://github.com/tmk/tmk_keyboard/issues/60, + // the events are processed out of order, with the first two keys released + // before pressing KC_C. EXPECT_REPORT(driver, (KC_A)); EXPECT_REPORT(driver, (KC_A, KC_B)); - EXPECT_REPORT(driver, (KC_A, KC_B, KC_C)); + EXPECT_REPORT(driver, (KC_A)); EXPECT_REPORT(driver, (KC_A, KC_C)); EXPECT_REPORT(driver, (KC_A)); EXPECT_EMPTY_REPORT(driver); @@ -423,7 +442,7 @@ TEST_F(ChordalHoldHoldOnOtherKeyPress, three_mod_taps_same_hand_streak_orders) { VERIFY_AND_CLEAR(driver); } -TEST_F(ChordalHoldHoldOnOtherKeyPress, three_mod_taps_two_held_one_tapped) { +TEST_F(ChordalHoldHoldOnOtherKeyPress, three_mod_taps_two_left_one_right) { TestDriver driver; InSequence s; auto mod_tap_key1 = KeymapKey(0, 1, 0, SFT_T(KC_A)); @@ -433,25 +452,30 @@ TEST_F(ChordalHoldHoldOnOtherKeyPress, three_mod_taps_two_held_one_tapped) { set_keymap({mod_tap_key1, mod_tap_key2, mod_tap_key3}); // Press mod-tap keys. - EXPECT_REPORT(driver, (KC_LEFT_SHIFT)); - EXPECT_REPORT(driver, (KC_LEFT_SHIFT, KC_LEFT_CTRL)); + EXPECT_NO_REPORT(driver); mod_tap_key1.press(); run_one_scan_loop(); + VERIFY_AND_CLEAR(driver); + + EXPECT_REPORT(driver, (KC_A)); mod_tap_key2.press(); run_one_scan_loop(); + VERIFY_AND_CLEAR(driver); + + EXPECT_REPORT(driver, (KC_A, KC_LEFT_CTRL)); mod_tap_key3.press(); run_one_scan_loop(); VERIFY_AND_CLEAR(driver); // Release key 3. - EXPECT_REPORT(driver, (KC_LEFT_SHIFT, KC_LEFT_CTRL, KC_C)); - EXPECT_REPORT(driver, (KC_LEFT_SHIFT, KC_LEFT_CTRL)); + EXPECT_REPORT(driver, (KC_A, KC_LEFT_CTRL, KC_C)); + EXPECT_REPORT(driver, (KC_A, KC_LEFT_CTRL)); mod_tap_key3.release(); run_one_scan_loop(); VERIFY_AND_CLEAR(driver); // Release key 2, then key 1. - EXPECT_REPORT(driver, (KC_LEFT_SHIFT)); + EXPECT_REPORT(driver, (KC_A)); EXPECT_EMPTY_REPORT(driver); mod_tap_key2.release(); run_one_scan_loop(); @@ -460,20 +484,24 @@ TEST_F(ChordalHoldHoldOnOtherKeyPress, three_mod_taps_two_held_one_tapped) { VERIFY_AND_CLEAR(driver); // Press mod-tap keys. - EXPECT_REPORT(driver, (KC_LEFT_SHIFT)); - EXPECT_REPORT(driver, (KC_LEFT_SHIFT, KC_LEFT_CTRL)); - idle_for(TAPPING_TERM); + EXPECT_NO_REPORT(driver); mod_tap_key1.press(); run_one_scan_loop(); + VERIFY_AND_CLEAR(driver); + + EXPECT_REPORT(driver, (KC_A)); mod_tap_key2.press(); run_one_scan_loop(); + VERIFY_AND_CLEAR(driver); + + EXPECT_REPORT(driver, (KC_A, KC_LEFT_CTRL)); mod_tap_key3.press(); run_one_scan_loop(); VERIFY_AND_CLEAR(driver); // Release key 3. - EXPECT_REPORT(driver, (KC_LEFT_SHIFT, KC_LEFT_CTRL, KC_C)); - EXPECT_REPORT(driver, (KC_LEFT_SHIFT, KC_LEFT_CTRL)); + EXPECT_REPORT(driver, (KC_A, KC_LEFT_CTRL, KC_C)); + EXPECT_REPORT(driver, (KC_A, KC_LEFT_CTRL)); mod_tap_key3.release(); run_one_scan_loop(); VERIFY_AND_CLEAR(driver); diff --git a/tests/tap_hold_configurations/chordal_hold/permissive_hold/test_tap_hold.cpp b/tests/tap_hold_configurations/chordal_hold/permissive_hold/test_tap_hold.cpp index bcc4a6f91dd..3a79dbf35aa 100644 --- a/tests/tap_hold_configurations/chordal_hold/permissive_hold/test_tap_hold.cpp +++ b/tests/tap_hold_configurations/chordal_hold/permissive_hold/test_tap_hold.cpp @@ -216,13 +216,15 @@ TEST_F(ChordalHoldPermissiveHold, two_mod_taps_same_hand_hold_til_timeout) { EXPECT_NO_REPORT(driver); mod_tap_key1.press(); run_one_scan_loop(); + VERIFY_AND_CLEAR(driver); + + EXPECT_REPORT(driver, (KC_A)); mod_tap_key2.press(); run_one_scan_loop(); VERIFY_AND_CLEAR(driver); // Continue holding til the tapping term. - EXPECT_REPORT(driver, (KC_RIGHT_CTRL)); - EXPECT_REPORT(driver, (KC_RIGHT_CTRL, KC_RIGHT_SHIFT)); + EXPECT_REPORT(driver, (KC_A, KC_RIGHT_SHIFT)); idle_for(TAPPING_TERM); VERIFY_AND_CLEAR(driver); @@ -280,12 +282,14 @@ TEST_F(ChordalHoldPermissiveHold, two_mod_taps_nested_press_same_hand) { EXPECT_NO_REPORT(driver); mod_tap_key1.press(); run_one_scan_loop(); + VERIFY_AND_CLEAR(driver); + + EXPECT_REPORT(driver, (KC_A)); mod_tap_key2.press(); run_one_scan_loop(); VERIFY_AND_CLEAR(driver); // Release mod-tap keys. - EXPECT_REPORT(driver, (KC_A)); EXPECT_REPORT(driver, (KC_A, KC_B)); EXPECT_REPORT(driver, (KC_A)); mod_tap_key2.release(); @@ -310,8 +314,14 @@ TEST_F(ChordalHoldPermissiveHold, three_mod_taps_same_hand_streak_roll) { EXPECT_NO_REPORT(driver); mod_tap_key1.press(); run_one_scan_loop(); + VERIFY_AND_CLEAR(driver); + + EXPECT_REPORT(driver, (KC_A)); mod_tap_key2.press(); run_one_scan_loop(); + VERIFY_AND_CLEAR(driver); + + EXPECT_REPORT(driver, (KC_A, KC_B)); mod_tap_key3.press(); run_one_scan_loop(); VERIFY_AND_CLEAR(driver); @@ -319,8 +329,6 @@ TEST_F(ChordalHoldPermissiveHold, three_mod_taps_same_hand_streak_roll) { // Release keys 1, 2, 3. // // NOTE: The correct order of events should be - // EXPECT_REPORT(driver, (KC_A)); - // EXPECT_REPORT(driver, (KC_A, KC_B)); // EXPECT_REPORT(driver, (KC_A, KC_B, KC_C)); // EXPECT_REPORT(driver, (KC_B, KC_C)); // EXPECT_REPORT(driver, (KC_C)); @@ -329,8 +337,6 @@ TEST_F(ChordalHoldPermissiveHold, three_mod_taps_same_hand_streak_roll) { // However, due to a workaround for https://github.com/tmk/tmk_keyboard/issues/60, // the events are processed out of order, with the first two keys released // before pressing KC_C. - EXPECT_REPORT(driver, (KC_A)); - EXPECT_REPORT(driver, (KC_A, KC_B)); EXPECT_REPORT(driver, (KC_B)); EXPECT_EMPTY_REPORT(driver); EXPECT_REPORT(driver, (KC_C)); @@ -357,15 +363,19 @@ TEST_F(ChordalHoldPermissiveHold, three_mod_taps_same_hand_streak_orders) { EXPECT_NO_REPORT(driver); mod_tap_key1.press(); run_one_scan_loop(); + VERIFY_AND_CLEAR(driver); + + EXPECT_REPORT(driver, (KC_A)); mod_tap_key2.press(); run_one_scan_loop(); + VERIFY_AND_CLEAR(driver); + + EXPECT_REPORT(driver, (KC_A, KC_B)); mod_tap_key3.press(); run_one_scan_loop(); VERIFY_AND_CLEAR(driver); // Release keys 3, 2, 1. - EXPECT_REPORT(driver, (KC_A)); - EXPECT_REPORT(driver, (KC_A, KC_B)); EXPECT_REPORT(driver, (KC_A, KC_B, KC_C)); EXPECT_REPORT(driver, (KC_A, KC_B)); EXPECT_REPORT(driver, (KC_A)); @@ -383,15 +393,19 @@ TEST_F(ChordalHoldPermissiveHold, three_mod_taps_same_hand_streak_orders) { idle_for(TAPPING_TERM); mod_tap_key1.press(); run_one_scan_loop(); + VERIFY_AND_CLEAR(driver); + + EXPECT_REPORT(driver, (KC_A)); mod_tap_key2.press(); run_one_scan_loop(); + VERIFY_AND_CLEAR(driver); + + EXPECT_REPORT(driver, (KC_A, KC_B)); mod_tap_key3.press(); run_one_scan_loop(); VERIFY_AND_CLEAR(driver); // Release keys 3, 1, 2. - EXPECT_REPORT(driver, (KC_A)); - EXPECT_REPORT(driver, (KC_A, KC_B)); EXPECT_REPORT(driver, (KC_A, KC_B, KC_C)); EXPECT_REPORT(driver, (KC_A, KC_B)); EXPECT_REPORT(driver, (KC_B)); @@ -409,16 +423,29 @@ TEST_F(ChordalHoldPermissiveHold, three_mod_taps_same_hand_streak_orders) { idle_for(TAPPING_TERM); mod_tap_key1.press(); run_one_scan_loop(); + VERIFY_AND_CLEAR(driver); + + EXPECT_REPORT(driver, (KC_A)); mod_tap_key2.press(); run_one_scan_loop(); + VERIFY_AND_CLEAR(driver); + + EXPECT_REPORT(driver, (KC_A, KC_B)); mod_tap_key3.press(); run_one_scan_loop(); VERIFY_AND_CLEAR(driver); // Release keys 2, 3, 1. + // + // NOTE: The correct order of events should be + // EXPECT_REPORT(driver, (KC_A, KC_B, KC_C)); + // EXPECT_REPORT(driver, (KC_A, KC_C)); + // EXPECT_REPORT(driver, (KC_A)); + // EXPECT_EMPTY_REPORT(driver); + // + // However, due to a workaround for https://github.com/tmk/tmk_keyboard/issues/60, + // the events are processed out of order. EXPECT_REPORT(driver, (KC_A)); - EXPECT_REPORT(driver, (KC_A, KC_B)); - EXPECT_REPORT(driver, (KC_A, KC_B, KC_C)); EXPECT_REPORT(driver, (KC_A, KC_C)); EXPECT_REPORT(driver, (KC_A)); EXPECT_EMPTY_REPORT(driver); @@ -447,8 +474,13 @@ TEST_F(ChordalHoldPermissiveHold, three_mod_taps_opposite_hands_roll) { EXPECT_NO_REPORT(driver); mod_tap_key1.press(); run_one_scan_loop(); + VERIFY_AND_CLEAR(driver); + + EXPECT_REPORT(driver, (KC_A)); mod_tap_key2.press(); run_one_scan_loop(); + VERIFY_AND_CLEAR(driver); + mod_tap_key3.press(); run_one_scan_loop(); VERIFY_AND_CLEAR(driver); @@ -456,7 +488,6 @@ TEST_F(ChordalHoldPermissiveHold, three_mod_taps_opposite_hands_roll) { // Release keys 1, 2, 3. // // NOTE: The correct order of events should be - // EXPECT_REPORT(driver, (KC_A)); // EXPECT_REPORT(driver, (KC_A, KC_B)); // EXPECT_REPORT(driver, (KC_A, KC_B, KC_C)); // EXPECT_REPORT(driver, (KC_B, KC_C)); @@ -468,8 +499,7 @@ TEST_F(ChordalHoldPermissiveHold, three_mod_taps_opposite_hands_roll) { // before pressing KC_C. // Release keys 1, 2, 3. - EXPECT_REPORT(driver, (KC_A)); - EXPECT_REPORT(driver, (KC_A, KC_B)); + EXPECT_EMPTY_REPORT(driver); EXPECT_REPORT(driver, (KC_B)); EXPECT_EMPTY_REPORT(driver); EXPECT_REPORT(driver, (KC_C)); @@ -483,7 +513,7 @@ TEST_F(ChordalHoldPermissiveHold, three_mod_taps_opposite_hands_roll) { VERIFY_AND_CLEAR(driver); } -TEST_F(ChordalHoldPermissiveHold, three_mod_taps_two_held_one_tapped) { +TEST_F(ChordalHoldPermissiveHold, three_mod_taps_two_left_one_right) { TestDriver driver; InSequence s; auto mod_tap_key1 = KeymapKey(0, 1, 0, SFT_T(KC_A)); @@ -496,26 +526,33 @@ TEST_F(ChordalHoldPermissiveHold, three_mod_taps_two_held_one_tapped) { EXPECT_NO_REPORT(driver); mod_tap_key1.press(); run_one_scan_loop(); + VERIFY_AND_CLEAR(driver); + + EXPECT_REPORT(driver, (KC_A)); mod_tap_key2.press(); run_one_scan_loop(); + VERIFY_AND_CLEAR(driver); + + EXPECT_NO_REPORT(driver); mod_tap_key3.press(); run_one_scan_loop(); VERIFY_AND_CLEAR(driver); // Release key 3. - EXPECT_REPORT(driver, (KC_LEFT_SHIFT)); - EXPECT_REPORT(driver, (KC_LEFT_SHIFT, KC_LEFT_CTRL)); - EXPECT_REPORT(driver, (KC_LEFT_SHIFT, KC_LEFT_CTRL, KC_C)); - EXPECT_REPORT(driver, (KC_LEFT_SHIFT, KC_LEFT_CTRL)); + EXPECT_REPORT(driver, (KC_A, KC_LEFT_CTRL)); + EXPECT_REPORT(driver, (KC_A, KC_LEFT_CTRL, KC_C)); + EXPECT_REPORT(driver, (KC_A, KC_LEFT_CTRL)); mod_tap_key3.release(); run_one_scan_loop(); VERIFY_AND_CLEAR(driver); // Release key 2, then key 1. - EXPECT_REPORT(driver, (KC_LEFT_SHIFT)); - EXPECT_EMPTY_REPORT(driver); + EXPECT_REPORT(driver, (KC_A)); mod_tap_key2.release(); run_one_scan_loop(); + VERIFY_AND_CLEAR(driver); + + EXPECT_EMPTY_REPORT(driver); mod_tap_key1.release(); run_one_scan_loop(); VERIFY_AND_CLEAR(driver); @@ -525,26 +562,32 @@ TEST_F(ChordalHoldPermissiveHold, three_mod_taps_two_held_one_tapped) { idle_for(TAPPING_TERM); mod_tap_key1.press(); run_one_scan_loop(); + VERIFY_AND_CLEAR(driver); + + EXPECT_REPORT(driver, (KC_A)); mod_tap_key2.press(); run_one_scan_loop(); + + EXPECT_NO_REPORT(driver); mod_tap_key3.press(); run_one_scan_loop(); VERIFY_AND_CLEAR(driver); // Release key 3. - EXPECT_REPORT(driver, (KC_LEFT_SHIFT)); - EXPECT_REPORT(driver, (KC_LEFT_SHIFT, KC_LEFT_CTRL)); - EXPECT_REPORT(driver, (KC_LEFT_SHIFT, KC_LEFT_CTRL, KC_C)); - EXPECT_REPORT(driver, (KC_LEFT_SHIFT, KC_LEFT_CTRL)); + EXPECT_REPORT(driver, (KC_A, KC_LEFT_CTRL)); + EXPECT_REPORT(driver, (KC_A, KC_LEFT_CTRL, KC_C)); + EXPECT_REPORT(driver, (KC_A, KC_LEFT_CTRL)); mod_tap_key3.release(); run_one_scan_loop(); VERIFY_AND_CLEAR(driver); // Release key 1, then key 2. EXPECT_REPORT(driver, (KC_LEFT_CTRL)); - EXPECT_EMPTY_REPORT(driver); mod_tap_key1.release(); run_one_scan_loop(); + VERIFY_AND_CLEAR(driver); + + EXPECT_EMPTY_REPORT(driver); mod_tap_key2.release(); run_one_scan_loop(); VERIFY_AND_CLEAR(driver); @@ -570,10 +613,21 @@ TEST_F(ChordalHoldPermissiveHold, three_mod_taps_one_held_two_tapped) { VERIFY_AND_CLEAR(driver); // Release keys 3, 2, 1. + // + // NOTE: The correct order of events should be + // EXPECT_REPORT(driver, (KC_LEFT_SHIFT)); + // EXPECT_REPORT(driver, (KC_LEFT_SHIFT, KC_B)); + // EXPECT_REPORT(driver, (KC_LEFT_SHIFT, KC_B, KC_C)); + // EXPECT_REPORT(driver, (KC_LEFT_SHIFT, KC_B)); + // EXPECT_REPORT(driver, (KC_LEFT_SHIFT)); + // EXPECT_EMPTY_REPORT(driver); + // + // However, due to a workaround for https://github.com/tmk/tmk_keyboard/issues/60, + // the events are processed out of order. EXPECT_REPORT(driver, (KC_LEFT_SHIFT)); EXPECT_REPORT(driver, (KC_LEFT_SHIFT, KC_B)); - EXPECT_REPORT(driver, (KC_LEFT_SHIFT, KC_B, KC_C)); - EXPECT_REPORT(driver, (KC_LEFT_SHIFT, KC_B)); + EXPECT_REPORT(driver, (KC_LEFT_SHIFT)); + EXPECT_REPORT(driver, (KC_LEFT_SHIFT, KC_C)); EXPECT_REPORT(driver, (KC_LEFT_SHIFT)); EXPECT_EMPTY_REPORT(driver); mod_tap_key3.release(); @@ -596,10 +650,21 @@ TEST_F(ChordalHoldPermissiveHold, three_mod_taps_one_held_two_tapped) { VERIFY_AND_CLEAR(driver); // Release keys 3, 1, 2. + // + // NOTE: The correct order of events should be + // EXPECT_REPORT(driver, (KC_LEFT_SHIFT)); + // EXPECT_REPORT(driver, (KC_LEFT_SHIFT, KC_B)); + // EXPECT_REPORT(driver, (KC_LEFT_SHIFT, KC_B, KC_C)); + // EXPECT_REPORT(driver, (KC_LEFT_SHIFT, KC_B)); + // EXPECT_REPORT(driver, (KC_B)); + // EXPECT_EMPTY_REPORT(driver); + // + // However, due to a workaround for https://github.com/tmk/tmk_keyboard/issues/60, + // the events are processed out of order. EXPECT_REPORT(driver, (KC_LEFT_SHIFT)); EXPECT_REPORT(driver, (KC_LEFT_SHIFT, KC_B)); - EXPECT_REPORT(driver, (KC_LEFT_SHIFT, KC_B, KC_C)); - EXPECT_REPORT(driver, (KC_LEFT_SHIFT, KC_B)); + EXPECT_REPORT(driver, (KC_B)); + EXPECT_REPORT(driver, (KC_B, KC_C)); EXPECT_REPORT(driver, (KC_B)); EXPECT_EMPTY_REPORT(driver); mod_tap_key3.release(); From fb6c2d8c6a1160c1aea3526cde100daa22b5807e Mon Sep 17 00:00:00 2001 From: Pascal Getreuer Date: Sat, 16 Nov 2024 23:56:17 -0800 Subject: [PATCH 12/13] Generate a default chordal_hold_layout. --- data/schemas/keyboard.jsonschema | 6 +- docs/tap_hold.md | 51 +++++----------- lib/python/qmk/cli/generate/keyboard_c.py | 60 +++++++++++++++++-- quantum/action_tapping.c | 24 ++------ quantum/action_tapping.h | 14 +---- .../hold_on_other_key_press/test.mk | 4 +- .../hold_on_other_key_press/test_keymap.c | 8 +++ .../chordal_hold/permissive_hold/config.h | 1 - .../permissive_hold/test_tap_hold.cpp | 6 +- .../retro_shift_permissive_hold/test.mk | 1 + .../retro_shift_permissive_hold/test_keymap.c | 8 +++ 11 files changed, 104 insertions(+), 79 deletions(-) create mode 100644 tests/tap_hold_configurations/chordal_hold/hold_on_other_key_press/test_keymap.c create mode 100644 tests/tap_hold_configurations/chordal_hold/retro_shift_permissive_hold/test_keymap.c diff --git a/data/schemas/keyboard.jsonschema b/data/schemas/keyboard.jsonschema index b699f862770..f2543939655 100644 --- a/data/schemas/keyboard.jsonschema +++ b/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*]", + } } } } diff --git a/docs/tap_hold.md b/docs/tap_hold.md index 0ade6a86737..49f798157bd 100644 --- a/docs/tap_hold.md +++ b/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 -inaccurate, handedness may be specified in several ways. +handedness is guessed based on keyboard geometry. -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: +Handedness may be specified with `chordal_hold_layout`. 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 -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. +a character indicating the handedness of one key, 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: +Keyboard makers may specify handedness in keyboard.json. Under `"layouts"`, +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 -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'; -} +```json +{"matrix": [5, 6], "x": 0, "y": 5.5, "w": 1.25, "hand", "*"}, ``` -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 diff --git a/lib/python/qmk/cli/generate/keyboard_c.py b/lib/python/qmk/cli/generate/keyboard_c.py index 5a6c9674860..5eb210b77fa 100755 --- a/lib/python/qmk/cli/generate/keyboard_c.py +++ b/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_h_lines.extend(_gen_matrix_mask(kb_info_json)) + keyboard_c_lines.extend(_gen_led_configs(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) diff --git a/quantum/action_tapping.c b/quantum/action_tapping.c index 53a3413c385..984bf098a8b 100644 --- a/quantum/action_tapping.c +++ b/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) { -# 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. +__attribute__((weak)) char chordal_hold_handedness(keypos_t key) { 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) { diff --git a/quantum/action_tapping.h b/quantum/action_tapping.h index 6227a394d6d..77c45f504fa 100644 --- a/quantum/action_tapping.h +++ b/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); -/** User callback to determine handedness of a key. */ -char chordal_hold_handedness_user(keypos_t key); +char chordal_hold_handedness(keypos_t key); # ifdef CHORDAL_HOLD_LAYOUT extern const char chordal_hold_layout[MATRIX_ROWS][MATRIX_COLS] PROGMEM; diff --git a/tests/tap_hold_configurations/chordal_hold/hold_on_other_key_press/test.mk b/tests/tap_hold_configurations/chordal_hold/hold_on_other_key_press/test.mk index 6b5968df16f..86e45bc5bce 100644 --- a/tests/tap_hold_configurations/chordal_hold/hold_on_other_key_press/test.mk +++ b/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 . -# -------------------------------------------------------------------------------- -# Keep this file, even if it is empty, as a marker that this folder contains tests -# -------------------------------------------------------------------------------- +INTROSPECTION_KEYMAP_C = test_keymap.c diff --git a/tests/tap_hold_configurations/chordal_hold/hold_on_other_key_press/test_keymap.c b/tests/tap_hold_configurations/chordal_hold/hold_on_other_key_press/test_keymap.c new file mode 100644 index 00000000000..ffc914b645e --- /dev/null +++ b/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'}, +}; diff --git a/tests/tap_hold_configurations/chordal_hold/permissive_hold/config.h b/tests/tap_hold_configurations/chordal_hold/permissive_hold/config.h index 910a753c270..f8787f08338 100644 --- a/tests/tap_hold_configurations/chordal_hold/permissive_hold/config.h +++ b/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 diff --git a/tests/tap_hold_configurations/chordal_hold/permissive_hold/test_tap_hold.cpp b/tests/tap_hold_configurations/chordal_hold/permissive_hold/test_tap_hold.cpp index 3a79dbf35aa..61cab76b8c3 100644 --- a/tests/tap_hold_configurations/chordal_hold/permissive_hold/test_tap_hold.cpp +++ b/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_user({.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 = 0}), 'L'); + EXPECT_EQ(chordal_hold_handedness({.col = MATRIX_COLS - 1, .row = 0}), 'R'); + EXPECT_EQ(chordal_hold_handedness({.col = 0, .row = 2}), '*'); } TEST_F(ChordalHoldPermissiveHold, get_chordal_hold_default) { diff --git a/tests/tap_hold_configurations/chordal_hold/retro_shift_permissive_hold/test.mk b/tests/tap_hold_configurations/chordal_hold/retro_shift_permissive_hold/test.mk index 0660eb3e6b8..d59be407c5c 100644 --- a/tests/tap_hold_configurations/chordal_hold/retro_shift_permissive_hold/test.mk +++ b/tests/tap_hold_configurations/chordal_hold/retro_shift_permissive_hold/test.mk @@ -14,3 +14,4 @@ # along with this program. If not, see . AUTO_SHIFT_ENABLE = yes +INTROSPECTION_KEYMAP_C = test_keymap.c diff --git a/tests/tap_hold_configurations/chordal_hold/retro_shift_permissive_hold/test_keymap.c b/tests/tap_hold_configurations/chordal_hold/retro_shift_permissive_hold/test_keymap.c new file mode 100644 index 00000000000..ffc914b645e --- /dev/null +++ b/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'}, +}; From 5b5ff41d796c2c65b0a2e56527875faf95c48bab Mon Sep 17 00:00:00 2001 From: Pascal Getreuer Date: Tue, 19 Nov 2024 20:17:44 -0800 Subject: [PATCH 13/13] Document chordal_hold_handedness(). --- docs/tap_hold.md | 34 +++++++++++++++++++++++++++++++--- 1 file changed, 31 insertions(+), 3 deletions(-) diff --git a/docs/tap_hold.md b/docs/tap_hold.md index 49f798157bd..9573923af89 100644 --- a/docs/tap_hold.md +++ b/docs/tap_hold.md @@ -508,9 +508,9 @@ 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 indicating the handedness of one key, 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 +for right, or `'*'` to exempt keys from the "opposite hands rule." A key with +`'*'` handedness may settle as held in chords with any other key. This could be +used perhaps on thumb keys or other places where you want to allow same-hand chords. Keyboard makers may specify handedness in keyboard.json. Under `"layouts"`, @@ -522,6 +522,34 @@ layouts, only the first one is read. For example: {"matrix": [5, 6], "x": 0, "y": 5.5, "w": 1.25, "hand", "*"}, ``` +Alternatively, handedness may be defined functionally with +`chordal_hold_handedness()`. For example, in keymap.c define: + +```c +char chordal_hold_handedness(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. + +::: 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. +::: + +::: tip If you define both `chordal_hold_layout[MATRIX_ROWS][MATRIX_COLS]` and +`chordal_hold_handedness(keypos_t key)` for handedness, the latter takes +precedence. + ### Per-chord customization