Add combo key repress feature (qmk#22858)

Co-authored-by: jack <[email protected]>

docs/features/combo.md

@@ -307,6 +307,50 @@ bool process_combo_key_release(uint16_t combo_index, combo_t *combo, uint8_t key
     return false;
 }
 ```
+
+### Customizable key repress
+By defining `COMBO_PROCESS_KEY_REPRESS` and implementing `bool process_combo_key_repress(uint16_t combo_index, combo_t *combo, uint8_t key_index, uint16_t keycode)` you can run your custom code when you repress just released key of a combo. By combining it with custom `process_combo_event` we can for example make special handling for Alt+Tab to switch windows, which, on combo F+G activation, registers Alt and presses Tab - then we can switch windows forward by releasing G and pressing it again, or backwards with F key. Here's the full example:
+
+```c
+enum combos {
+    CMB_ALTTAB
+};
+
+const uint16_t PROGMEM combo_alttab[] = {KC_F, KC_G, COMBO_END};
+
+combo_t key_combos[COMBO_LENGTH] = {
+    [CMB_ALTTAB] = COMBO(combo_alttab, KC_NO), // KC_NO to leave processing for process_combo_event
+};
+
+void process_combo_event(uint16_t combo_index, bool pressed) {
+    switch (combo_index) {
+        case CMB_ALTTAB:
+            if (pressed) {
+                register_mods(MOD_LALT);
+                tap_code(KC_TAB);
+            } else {
+                unregister_mods(MOD_LALT);
+            }
+            break;
+    }
+}
+
+bool process_combo_key_repress(uint16_t combo_index, combo_t *combo, uint8_t key_index, uint16_t keycode) {
+    switch (combo_index) {
+        case CMB_ALTTAB:
+            switch (keycode) {
+                case KC_F:
+                    tap_code16(S(KC_TAB));
+                    return true;
+                case KC_G:
+                    tap_code(KC_TAB);
+                    return true;
+            }
+    }
+    return false;
+}
+```
+
 ### Layer independent combos
 
 If you, for example, use multiple base layers for different key layouts, one for QWERTY, and another one for Colemak, you might want your combos to work from the same key positions on all layers. Defining the same combos again for another layout is redundant and takes more memory. The solution is to just check the keycodes from one layer.

quantum/process_keycode/process_combo.c

@@ -65,12 +65,20 @@ __attribute__((weak)) bool process_combo_key_release(uint16_t combo_index, combo
 }
 #endif
 
+#ifdef COMBO_PROCESS_KEY_REPRESS
+__attribute__((weak)) bool process_combo_key_repress(uint16_t combo_index, combo_t *combo, uint8_t key_index, uint16_t keycode) {
+    return false;
+}
+#endif
+
 #ifdef COMBO_SHOULD_TRIGGER
 __attribute__((weak)) bool combo_should_trigger(uint16_t combo_index, combo_t *combo, uint16_t keycode, keyrecord_t *record) {
     return true;
 }
 #endif
 
+typedef enum { COMBO_KEY_NOT_PRESSED, COMBO_KEY_PRESSED, COMBO_KEY_REPRESSED } combo_key_action_t;
+
 #ifndef COMBO_NO_TIMER
 static uint16_t timer = 0;
 #endif
@@ -414,14 +422,14 @@ static bool keys_pressed_in_order(uint16_t combo_index, combo_t *combo, uint16_t
 }
 #endif
 
-static bool process_single_combo(combo_t *combo, uint16_t keycode, keyrecord_t *record, uint16_t combo_index) {
+static combo_key_action_t process_single_combo(combo_t *combo, uint16_t keycode, keyrecord_t *record, uint16_t combo_index) {
     uint8_t  key_count = 0;
     uint16_t key_index = -1;
     _find_key_index_and_count(combo->keys, keycode, &key_index, &key_count);
 
     /* Continue processing if key isn't part of current combo. */
     if (-1 == (int16_t)key_index) {
-        return false;
+        return COMBO_KEY_NOT_PRESSED;
     }
 
     bool key_is_part_of_combo = (!COMBO_DISABLED(combo) && is_combo_enabled()
@@ -449,7 +457,7 @@ static bool process_single_combo(combo_t *combo, uint16_t keycode, keyrecord_t *
             /* Don't buffer this combo if its combo term has passed. */
             if (timer && timer_elapsed(timer) > time) {
                 DISABLE_COMBO(combo);
-                return true;
+                return COMBO_KEY_PRESSED;
             } else
 #endif
             {
@@ -485,6 +493,15 @@ static bool process_single_combo(combo_t *combo, uint16_t keycode, keyrecord_t *
                 }
             } // if timer elapsed end
         }
+#ifdef COMBO_PROCESS_KEY_REPRESS
+    } else if (record->event.pressed) {
+        if (COMBO_ACTIVE(combo)) {
+            if (process_combo_key_repress(combo_index, combo, key_index, keycode)) {
+                KEY_STATE_DOWN(combo->state, key_index);
+                return COMBO_KEY_REPRESSED;
+            }
+        }
+#endif
     } else {
         // chord releases
         if (!COMBO_ACTIVE(combo) && ALL_COMBO_KEYS_ARE_DOWN(COMBO_STATE(combo), key_count)) {
@@ -531,12 +548,12 @@ static bool process_single_combo(combo_t *combo, uint16_t keycode, keyrecord_t *
         KEY_STATE_UP(combo->state, key_index);
     }
 
-    return key_is_part_of_combo;
+    return key_is_part_of_combo ? COMBO_KEY_PRESSED : COMBO_KEY_NOT_PRESSED;
 }
 
 bool process_combo(uint16_t keycode, keyrecord_t *record) {
-    bool is_combo_key          = false;
-    bool no_combo_keys_pressed = true;
+    uint8_t is_combo_key          = COMBO_KEY_NOT_PRESSED;
+    bool    no_combo_keys_pressed = true;
 
     if (keycode == QK_COMBO_ON && record->event.pressed) {
         combo_enable();
@@ -582,12 +599,17 @@ bool process_combo(uint16_t keycode, keyrecord_t *record) {
 #    endif
 #endif
 
-        if (key_buffer_size < COMBO_KEY_BUFFER_LENGTH) {
-            key_buffer[key_buffer_size++] = (queued_record_t){
-                .record      = *record,
-                .keycode     = keycode,
-                .combo_index = -1, // this will be set when applying combos
-            };
+#ifdef COMBO_PROCESS_KEY_REPRESS
+        if (is_combo_key == COMBO_KEY_PRESSED)
+#endif
+        {
+            if (key_buffer_size < COMBO_KEY_BUFFER_LENGTH) {
+                key_buffer[key_buffer_size++] = (queued_record_t){
+                    .record      = *record,
+                    .keycode     = keycode,
+                    .combo_index = -1, // this will be set when applying combos
+                };
+            }
         }
     } else {
         if (combo_buffer_read != combo_buffer_write) {

tests/combo/combo_repress/config.h

@@ -0,0 +1,10 @@
+// Copyright 2024 @Filios92
+// SPDX-License-Identifier: GPL-2.0-or-later
+
+#pragma once
+
+#include "test_common.h"
+
+#define TAPPING_TERM 200
+
+#define COMBO_PROCESS_KEY_REPRESS

tests/combo/combo_repress/test.mk

@@ -0,0 +1,6 @@
+# Copyright 2024 @Filios92
+# SPDX-License-Identifier: GPL-2.0-or-later
+
+COMBO_ENABLE = yes
+
+INTROSPECTION_KEYMAP_C = test_combos_repress.c

tests/combo/combo_repress/test_combo.cpp

@@ -0,0 +1,158 @@
+// Copyright 2024 @Filios92
+// SPDX-License-Identifier: GPL-2.0-or-later
+
+#include "keyboard_report_util.hpp"
+#include "quantum.h"
+#include "keycode.h"
+#include "test_common.h"
+#include "test_driver.hpp"
+#include "test_fixture.hpp"
+#include "test_keymap_key.hpp"
+
+using testing::_;
+using testing::InSequence;
+
+class ComboRepress : public TestFixture {};
+
+TEST_F(ComboRepress, combo_repress_tapped) {
+    TestDriver driver;
+    KeymapKey  key_f(0, 0, 0, KC_F);
+    KeymapKey  key_g(0, 0, 1, KC_G);
+    set_keymap({key_f, key_g});
+
+    EXPECT_REPORT(driver, (KC_LEFT_ALT)).Times(2);
+    EXPECT_REPORT(driver, (KC_TAB, KC_LEFT_ALT));
+    EXPECT_EMPTY_REPORT(driver);
+    tap_combo({key_f, key_g}, 20);
+    VERIFY_AND_CLEAR(driver);
+}
+
+TEST_F(ComboRepress, combo_repress_held_released_one_key_and_repressed) {
+    TestDriver driver;
+    KeymapKey  key_f(0, 0, 0, KC_F);
+    KeymapKey  key_g(0, 0, 1, KC_G);
+    KeymapKey  key_h(0, 0, 2, KC_H);
+    KeymapKey  key_j(0, 0, 3, KC_J);
+    set_keymap({key_f, key_g, key_h, key_j});
+
+    /* Press combo F+G */
+    EXPECT_REPORT(driver, (KC_LEFT_ALT)).Times(2);
+    EXPECT_REPORT(driver, (KC_TAB, KC_LEFT_ALT));
+    key_f.press();
+    run_one_scan_loop();
+    key_g.press();
+    run_one_scan_loop();
+    idle_for(COMBO_TERM + 1);
+    VERIFY_AND_CLEAR(driver);
+
+    /* Release G */
+    EXPECT_NO_REPORT(driver);
+    key_g.release();
+    idle_for(80);
+    VERIFY_AND_CLEAR(driver);
+
+    /* Tap G */
+    EXPECT_REPORT(driver, (KC_TAB, KC_LEFT_ALT));
+    EXPECT_REPORT(driver, (KC_LEFT_ALT));
+    tap_key(key_g, TAPPING_TERM + 1);
+    VERIFY_AND_CLEAR(driver);
+
+    /* Tap G, but hold for longer */
+    EXPECT_REPORT(driver, (KC_TAB, KC_LEFT_ALT));
+    EXPECT_REPORT(driver, (KC_LEFT_ALT));
+    tap_key(key_g, TAPPING_TERM * 2);
+    VERIFY_AND_CLEAR(driver);
+
+    idle_for(500);
+
+    /* Tap other combo while holding F */
+    EXPECT_REPORT(driver, (KC_ESCAPE, KC_LEFT_ALT));
+    EXPECT_REPORT(driver, (KC_LEFT_ALT));
+    tap_combo({key_h, key_j}, TAPPING_TERM + 1);
+    VERIFY_AND_CLEAR(driver);
+
+    /* G press and hold */
+    EXPECT_REPORT(driver, (KC_TAB, KC_LEFT_ALT));
+    EXPECT_REPORT(driver, (KC_LEFT_ALT));
+    key_g.press();
+    run_one_scan_loop();
+    VERIFY_AND_CLEAR(driver);
+
+    /* F release and tap */
+    EXPECT_REPORT(driver, (KC_LEFT_ALT, KC_LEFT_SHIFT)).Times(2);
+    EXPECT_REPORT(driver, (KC_TAB, KC_LEFT_ALT, KC_LEFT_SHIFT));
+    EXPECT_REPORT(driver, (KC_LEFT_ALT));
+    key_f.release();
+    run_one_scan_loop();
+    tap_key(key_f);
+    VERIFY_AND_CLEAR(driver);
+
+    /* Release G */
+    EXPECT_EMPTY_REPORT(driver);
+    key_g.release();
+    run_one_scan_loop();
+    VERIFY_AND_CLEAR(driver);
+}
+
+TEST_F(ComboRepress, combo_repress_normal_combo) {
+    TestDriver driver;
+    KeymapKey  key_f(0, 0, 0, KC_F);
+    KeymapKey  key_g(0, 0, 1, KC_G);
+    KeymapKey  key_h(0, 0, 2, KC_H);
+    KeymapKey  key_j(0, 0, 3, KC_J);
+    set_keymap({key_f, key_g, key_h, key_j});
+
+    /* Press combo H+J */
+    EXPECT_REPORT(driver, (KC_ESCAPE));
+    key_h.press();
+    run_one_scan_loop();
+    key_j.press();
+    run_one_scan_loop();
+    idle_for(COMBO_TERM + 10);
+    VERIFY_AND_CLEAR(driver);
+
+    /* Release H */
+    EXPECT_NO_REPORT(driver);
+    key_h.release();
+    idle_for(80);
+    VERIFY_AND_CLEAR(driver);
+
+    /* Tap H */
+    EXPECT_REPORT(driver, (KC_H, KC_ESCAPE));
+    EXPECT_REPORT(driver, (KC_ESCAPE));
+    tap_key(key_h);
+    VERIFY_AND_CLEAR(driver);
+
+    /* Tap H, but hold for longer */
+    EXPECT_REPORT(driver, (KC_H, KC_ESCAPE));
+    EXPECT_REPORT(driver, (KC_ESCAPE));
+    tap_key(key_h, TAPPING_TERM + 1);
+    VERIFY_AND_CLEAR(driver);
+
+    idle_for(500);
+
+    /* Tap other combo while holding K */
+    EXPECT_REPORT(driver, (KC_ESCAPE, KC_LEFT_ALT)).Times(2);
+    EXPECT_REPORT(driver, (KC_ESCAPE, KC_TAB, KC_LEFT_ALT));
+    EXPECT_REPORT(driver, (KC_ESCAPE));
+    tap_combo({key_f, key_g}, TAPPING_TERM + 1);
+    VERIFY_AND_CLEAR(driver);
+
+    /* H press and hold */
+    EXPECT_REPORT(driver, (KC_H, KC_ESCAPE));
+    key_h.press();
+    run_one_scan_loop();
+    VERIFY_AND_CLEAR(driver);
+
+    /* J release and tap */
+    EXPECT_REPORT(driver, (KC_H));
+    key_j.release();
+    run_one_scan_loop();
+    VERIFY_AND_CLEAR(driver);
+
+    /* Release G */
+    EXPECT_EMPTY_REPORT(driver);
+    key_h.release();
+    run_one_scan_loop();
+    VERIFY_AND_CLEAR(driver);
+}

tests/combo/combo_repress/test_combos_repress.c

@@ -0,0 +1,43 @@
+// Copyright 2024 @Filios92
+// SPDX-License-Identifier: GPL-2.0-or-later
+#include "quantum.h"
+
+enum combos { alttab, esc };
+
+uint16_t const alttab_combo[] = {KC_F, KC_G, COMBO_END};
+uint16_t const esc_combo[]    = {KC_H, KC_J, COMBO_END};
+
+// clang-format off
+combo_t key_combos[] = {
+    [alttab]  = COMBO(alttab_combo, KC_NO),
+    [esc]     = COMBO(esc_combo, KC_ESC)
+};
+// clang-format on
+
+void process_combo_event(uint16_t combo_index, bool pressed) {
+    switch (combo_index) {
+        case alttab:
+            if (pressed) {
+                register_mods(MOD_LALT);
+                tap_code(KC_TAB);
+            } else {
+                unregister_mods(MOD_LALT);
+            }
+            break;
+    }
+}
+
+bool process_combo_key_repress(uint16_t combo_index, combo_t *combo, uint8_t key_index, uint16_t keycode) {
+    switch (combo_index) {
+        case alttab:
+            switch (keycode) {
+                case KC_F:
+                    tap_code16(S(KC_TAB));
+                    return true;
+                case KC_G:
+                    tap_code(KC_TAB);
+                    return true;
+            }
+    }
+    return false;
+}