diff --git a/docs/config_options.md b/docs/config_options.md index b2a2117693d..5fa6fbc9180 100644 --- a/docs/config_options.md +++ b/docs/config_options.md @@ -190,6 +190,8 @@ If you define these options you will enable the associated feature, which may in * how long for the Combo keys to be detected. Defaults to `TAPPING_TERM` if not defined. * `#define COMBO_MUST_HOLD_MODS` * Flag for enabling extending timeout on Combos containing modifiers +* `#define COMBO_HOLD_STRICT` + * Flag for requiring a combo's term to elapse without any other keys being pressed. * `#define COMBO_MOD_TERM 200` * Allows for extending COMBO_TERM for mod keys while mid-combo. * `#define COMBO_MUST_HOLD_PER_COMBO` diff --git a/docs/features/combo.md b/docs/features/combo.md index afe202ad54a..b09506a24ae 100644 --- a/docs/features/combo.md +++ b/docs/features/combo.md @@ -140,6 +140,8 @@ Processing combos has two buffers, one for the key presses, another for the comb ### Modifier Combos If a combo resolves to a Modifier, the window for processing the combo can be extended independently from normal combos. By default, this is disabled but can be enabled with `#define COMBO_MUST_HOLD_MODS`, and the time window can be configured with `#define COMBO_HOLD_TERM 150` (default: `TAPPING_TERM`). With `COMBO_MUST_HOLD_MODS`, you cannot tap the combo any more which makes the combo less prone to misfires. +By default, `COMBO_MUST_HOLD_MODS` ignores the `COMBO_HOLD_TERM` and fires the combo if another key is pressed before the term elapses. You can alter this behavior with `#define COMBO_HOLD_STRICT`. With `COMBO_HOLD_STRICT`, the combo will be discarded if another key is pressed before the term elapses. (For example, imagine you have a combo that makes `a`+`b` trigger `ctrl`. You press `a` and `b` together and then press `c` before `COMBO_HOLD_TERM` has elapsed. Without `COMBO_HOLD_STRICT`, you would get `ctrl`+`c`. With `COMBO_HOLD_STRICT`, you would get `abc`.) + ### Strict key press order By defining `COMBO_MUST_PRESS_IN_ORDER` combos only activate when the keys are pressed in the same order as they are defined in the key array. diff --git a/quantum/process_keycode/process_combo.c b/quantum/process_keycode/process_combo.c index c99a66a74b0..aa512b03561 100644 --- a/quantum/process_keycode/process_combo.c +++ b/quantum/process_keycode/process_combo.c @@ -357,7 +357,7 @@ void apply_combo(uint16_t combo_index, combo_t *combo) { drop_combo_from_buffer(combo_index); } -static inline void apply_combos(void) { +static inline void apply_combos(bool triggered_by_timer) { // Apply all buffered normal combos. for (uint8_t i = combo_buffer_read; i != combo_buffer_write; INCREMENT_MOD(i)) { queued_combo_t *buffered_combo = &combo_buffer[i]; @@ -369,6 +369,13 @@ static inline void apply_combos(void) { drop_combo_from_buffer(buffered_combo->combo_index); continue; } +#endif +#ifdef COMBO_HOLD_STRICT + if (!triggered_by_timer && _get_combo_must_hold(buffered_combo->combo_index, combo)) { + // When strict, hold-only combos are applied only if triggered by timer + drop_combo_from_buffer(buffered_combo->combo_index); + continue; + } #endif apply_combo(buffered_combo->combo_index, combo); } @@ -515,7 +522,7 @@ static combo_key_action_t process_single_combo(combo_t *combo, uint16_t keycode, else if (get_combo_must_tap(combo_index, combo)) { // immediately apply tap-only combo apply_combo(combo_index, combo); - apply_combos(); // also apply other prepared combos and dump key buffer + apply_combos(false); // also apply other prepared combos and dump key buffer # ifdef COMBO_PROCESS_KEY_RELEASE if (process_combo_key_release(combo_index, combo, key_index, keycode)) { release_combo(combo_index, combo); @@ -614,7 +621,7 @@ bool process_combo(uint16_t keycode, keyrecord_t *record) { } else { if (combo_buffer_read != combo_buffer_write) { // some combo is prepared - apply_combos(); + apply_combos(false); } else { // reset state if there are no combo keys pressed at all dump_key_buffer(); @@ -635,7 +642,7 @@ void combo_task(void) { #ifndef COMBO_NO_TIMER if (timer && timer_elapsed(timer) > longest_term) { if (combo_buffer_read != combo_buffer_write) { - apply_combos(); + apply_combos(true); longest_term = 0; timer = 0; } else { diff --git a/tests/combo/combo_hold_strict/config.h b/tests/combo/combo_hold_strict/config.h new file mode 100644 index 00000000000..272667cd6d8 --- /dev/null +++ b/tests/combo/combo_hold_strict/config.h @@ -0,0 +1,10 @@ +// Copyright 2024 @Filios92 +// SPDX-License-Identifier: GPL-2.0-or-later + +#pragma once + +#include "test_common.h" + +#define COMBO_MUST_HOLD_MODS +#define COMBO_HOLD_TERM 150 +#define COMBO_HOLD_STRICT diff --git a/tests/combo/combo_hold_strict/test.mk b/tests/combo/combo_hold_strict/test.mk new file mode 100644 index 00000000000..ca727f29e8b --- /dev/null +++ b/tests/combo/combo_hold_strict/test.mk @@ -0,0 +1,6 @@ +# Copyright 2024 @Filios92 +# SPDX-License-Identifier: GPL-2.0-or-later + +COMBO_ENABLE = yes + +INTROSPECTION_KEYMAP_C = test_combo_hold_strict.c diff --git a/tests/combo/combo_hold_strict/test_combo.cpp b/tests/combo/combo_hold_strict/test_combo.cpp new file mode 100644 index 00000000000..4435cdd202e --- /dev/null +++ b/tests/combo/combo_hold_strict/test_combo.cpp @@ -0,0 +1,73 @@ +// Copyright 2024 @Filios92 +// SPDX-License-Identifier: GPL-2.0-or-later + +#include "gmock/gmock.h" +#include "keyboard_report_util.hpp" +#include "keycode.h" +#include "test_common.h" +#include "test_common.hpp" +#include "test_driver.hpp" +#include "test_fixture.hpp" +#include "test_keymap_key.hpp" + +using testing::_; +using testing::InSequence; + +class ComboHoldStrict : public TestFixture {}; + +TEST_F(ComboHoldStrict, combo_hold_strict_held_full_term) { + TestDriver driver; + KeymapKey key_h(0, 0, 0, KC_H); + KeymapKey key_j(0, 0, 1, KC_J); + set_keymap({key_h, key_j}); + + EXPECT_REPORT(driver, (KC_LCTL)); + key_h.press(); + run_one_scan_loop(); + key_j.press(); + run_one_scan_loop(); + idle_for(COMBO_HOLD_TERM); + combo_task(); + VERIFY_AND_CLEAR(driver); +} + +TEST_F(ComboHoldStrict, combo_hold_strict_tap_other_key_before_term) { + TestDriver driver; + KeymapKey key_h(0, 0, 0, KC_H); + KeymapKey key_j(0, 0, 1, KC_J); + KeymapKey key_f(0, 0, 2, KC_F); + set_keymap({key_h, key_j, key_f}); + + EXPECT_REPORT(driver, (KC_H)); + EXPECT_REPORT(driver, (KC_H, KC_J)); + EXPECT_REPORT(driver, (KC_H, KC_J, KC_F)); + key_h.press(); + run_one_scan_loop(); + key_j.press(); + run_one_scan_loop(); + idle_for(COMBO_HOLD_TERM / 2); + combo_task(); + key_f.press(); + run_one_scan_loop(); + VERIFY_AND_CLEAR(driver); +} + +TEST_F(ComboHoldStrict, combo_hold_strict_tap_other_key_after_term) { + TestDriver driver; + KeymapKey key_h(0, 0, 0, KC_H); + KeymapKey key_j(0, 0, 1, KC_J); + KeymapKey key_f(0, 0, 2, KC_F); + set_keymap({key_h, key_j, key_f}); + + EXPECT_REPORT(driver, (KC_LCTL)); + EXPECT_REPORT(driver, (KC_LCTL, KC_F)); + key_h.press(); + run_one_scan_loop(); + key_j.press(); + run_one_scan_loop(); + idle_for(COMBO_HOLD_TERM); + combo_task(); + key_f.press(); + run_one_scan_loop(); + VERIFY_AND_CLEAR(driver); +} diff --git a/tests/combo/combo_hold_strict/test_combo_hold_strict.c b/tests/combo/combo_hold_strict/test_combo_hold_strict.c new file mode 100644 index 00000000000..e3c883aee30 --- /dev/null +++ b/tests/combo/combo_hold_strict/test_combo_hold_strict.c @@ -0,0 +1,11 @@ +// Copyright 2024 @Filios92 +// SPDX-License-Identifier: GPL-2.0-or-later +#include "quantum.h" + +enum combos { ctrl }; + +uint16_t const ctrl_combo[] = {KC_H, KC_J, COMBO_END}; + +combo_t key_combos[] = { + [ctrl] = COMBO(ctrl_combo, KC_LCTL) +};