Process combos earlier & overlapping combos (#8591)

* Combo processing improvements.

Now it is possible to use ModTap and LayerTap keys as part of combos.
Overlapping combos also don't trigger all the combos, just exactly the
one that you press.

New settings:
- COMBO_MUST_HOLD_MODS
- COMBO_MOD_TERM
- COMBO_TERM_PER_COMBO
- COMBO_MUST_HOLD_PER_COMBO
- COMBO_STRICT_TIMER
- COMBO_NO_TIMER

* Remove the size flags from combo_t struct boolean members.

This in the end actually saves space as the members are accessed so many
times. The amount of operations needed to access the bits uses more
memory than setting the size saves.

* Fix `process_combo_key_release` not called correctly with tap-only combos

* Fix not passing a pointer when NO_ACTION_TAPPING is defined.

* Docs for `COMBO_ONLY_FROM_LAYER`

* Update docs/feature_combo.md

Co-authored-by: precondition <57645186+precondition@users.noreply.github.com>

* Update quantum/process_keycode/process_combo.c

Co-authored-by: precondition <57645186+precondition@users.noreply.github.com>

* Add `EXTRA_SHORT_COMBOS` option.

Stuff combo's `disabled` and `active` flags into `state`. Possibly can
save some space.

* Add more examples and clarify things with dict management system.

- Simple examples now has a combo that has modifiers included.
- The slightly more advanced examples now are actually more advanced
  instead of just `tap_code16(<modded-keycode>)`.
- Added a note that `COMBO_ACTION`s are not needed anymore as you can
  just use custom keycodes.
- Added a note that the `g/keymap_combo.h` macros use the
  `process_combo_event` function and that it is not usable in one's
  keymap afterwards.

* Update docs/feature_combo.md

Co-authored-by: precondition <57645186+precondition@users.noreply.github.com>

* Update docs/feature_combo.md

Co-authored-by: precondition <57645186+precondition@users.noreply.github.com>

* Update docs/feature_combo.md

Co-authored-by: precondition <57645186+precondition@users.noreply.github.com>

* Update docs/feature_combo.md

Co-authored-by: precondition <57645186+precondition@users.noreply.github.com>

* Update docs/feature_combo.md

Co-authored-by: precondition <57645186+precondition@users.noreply.github.com>

* Change "the" combo action example to "email" example.

* Update docs/feature_combo.md

Co-authored-by: precondition <57645186+precondition@users.noreply.github.com>

* Fix sneaky infinite loop with `combo_disable()`

No need to call `dump_key_buffer` when disabling combos because the
buffer is either being dumped if a combo-key was pressed, or the buffer is empty
if a non-combo-key is pressed.

* Update docs/feature_combo.md

Co-authored-by: precondition <57645186+precondition@users.noreply.github.com>

* Update docs/feature_combo.md

Co-authored-by: precondition <57645186+precondition@users.noreply.github.com>

Co-authored-by: precondition <57645186+precondition@users.noreply.github.com>
Co-authored-by: Drashna Jaelre <drashna@live.com>

docs/config_options.md

@@ -188,9 +188,21 @@ If you define these options you will enable the associated feature, which may in
     few ms of delay from this. But if you're doing chording on something with 3-4ms
     scan times? You probably want this.
 * `#define COMBO_COUNT 2`
-  * Set this to the number of combos that you're using in the [Combo](feature_combo.md) feature.
+  * Set this to the number of combos that you're using in the [Combo](feature_combo.md) feature. Or leave it undefined and programmatically set the count.
 * `#define COMBO_TERM 200`
   * 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 modifers
+* `#define COMBO_MOD_TERM 200`
+  * Allows for extending COMBO_TERM for mod keys while mid-combo. 
+* `#define COMBO_MUST_HOLD_PER_COMBO`
+  * Flag to enable per-combo COMBO_TERM extension and `get_combo_must_hold()` function
+* `#define COMBO_TERM_PER_COMBO`
+  * Flag to enable per-combo COMBO_TERM extension and `get_combo_term()` function
+* `#define COMBO_STRICT_TIMER`
+  * Only start the combo timer on the first key press instead of on all key presses.
+* `#define COMBO_NO_TIMER`
+  * Disable the combo timer completely for relaxed combos.
 * `#define TAP_CODE_DELAY 100`
   * Sets the delay between `register_code` and `unregister_code`, if you're having issues with it registering properly (common on VUSB boards). The value is in milliseconds.
 * `#define TAP_HOLD_CAPS_DELAY 80`

keyboards/gboards/g/keymap_combo.h

@@ -28,7 +28,10 @@
 #define TOGG A_ENUM
 enum combos {
 #include "combos.def"
+    COMBO_LENGTH
 };
+// Export length to combo module
+uint16_t COMBO_LEN = COMBO_LENGTH;
 
 // Bake combos into mem
 #undef COMB
@@ -53,9 +56,6 @@ combo_t key_combos[] = {
 #undef SUBS
 #undef TOGG
 
-// Export length to combo module
-int COMBO_LEN = sizeof(key_combos) / sizeof(key_combos[0]);
-
 // Fill QMK hook
 #define COMB BLANK
 #define SUBS A_ACTI

quantum/keymap_common.c

@@ -40,7 +40,10 @@ extern keymap_config_t keymap_config;
 action_t action_for_key(uint8_t layer, keypos_t key) {
     // 16bit keycodes - important
     uint16_t keycode = keymap_key_to_keycode(layer, key);
+    return action_for_keycode(keycode);
+};
 
+action_t action_for_keycode(uint16_t keycode) {
     // keycode remapping
     keycode = keycode_config(keycode);
 

quantum/process_keycode/process_combo.h

@@ -20,23 +20,38 @@
 #include "quantum.h"
 #include <stdint.h>
 
-#ifdef EXTRA_EXTRA_LONG_COMBOS
+#ifdef EXTRA_SHORT_COMBOS
+#    define MAX_COMBO_LENGTH 6
+#elif defined(EXTRA_EXTRA_LONG_COMBOS)
 #    define MAX_COMBO_LENGTH 32
-#elif EXTRA_LONG_COMBOS
+#elif defined(EXTRA_LONG_COMBOS)
 #    define MAX_COMBO_LENGTH 16
 #else
 #    define MAX_COMBO_LENGTH 8
 #endif
 
+#ifndef COMBO_KEY_BUFFER_LENGTH
+#    define COMBO_KEY_BUFFER_LENGTH MAX_COMBO_LENGTH
+#endif
+#ifndef COMBO_BUFFER_LENGTH
+#    define COMBO_BUFFER_LENGTH 4
+#endif
+
 typedef struct {
     const uint16_t *keys;
     uint16_t        keycode;
-#ifdef EXTRA_EXTRA_LONG_COMBOS
-    uint32_t state;
-#elif EXTRA_LONG_COMBOS
-    uint16_t state;
-#else
+#ifdef EXTRA_SHORT_COMBOS
     uint8_t state;
+#else
+    bool disabled;
+    bool active;
+#    if defined(EXTRA_EXTRA_LONG_COMBOS)
+    uint32_t state;
+#    elif defined(EXTRA_LONG_COMBOS)
+    uint16_t state;
+#    else
+    uint8_t state;
+#    endif
 #endif
 } combo_t;
 
@@ -46,12 +61,15 @@ typedef struct {
     { .keys = &(ck)[0] }
 
 #define COMBO_END 0
-#ifndef COMBO_COUNT
-#    define COMBO_COUNT 0
-#endif
 #ifndef COMBO_TERM
-#    define COMBO_TERM TAPPING_TERM
+#    define COMBO_TERM 50
 #endif
+#ifndef COMBO_HOLD_TERM
+#    define COMBO_HOLD_TERM TAPPING_TERM
+#endif
+
+/* check if keycode is only modifiers */
+#define KEYCODE_IS_MOD(code) (IS_MOD(code) || (code >= QK_MODS && code <= QK_MODS_MAX && !(code & QK_BASIC_MAX)))
 
 bool process_combo(uint16_t keycode, keyrecord_t *record);
 void combo_task(void);

quantum/quantum.c

@@ -143,7 +143,13 @@ void reset_keyboard(void) {
 }
 
 /* Convert record into usable keycode via the contained event. */
-uint16_t get_record_keycode(keyrecord_t *record, bool update_layer_cache) { return get_event_keycode(record->event, update_layer_cache); }
+uint16_t get_record_keycode(keyrecord_t *record, bool update_layer_cache) {
+#ifdef COMBO_ENABLE
+    if (record->keycode) { return record->keycode; }
+#endif
+    return get_event_keycode(record->event, update_layer_cache);
+}
+
 
 /* Convert event into usable keycode. Checks the layer cache to ensure that it
  * retains the correct keycode after a layer change, if the key is still pressed.
@@ -169,6 +175,18 @@ uint16_t get_event_keycode(keyevent_t event, bool update_layer_cache) {
         return keymap_key_to_keycode(layer_switch_get_layer(event.key), event.key);
 }
 
+/* Get keycode, and then process pre tapping functionality */
+bool pre_process_record_quantum(keyrecord_t *record) {
+    if (!(
+#ifdef COMBO_ENABLE
+        process_combo(get_record_keycode(record, true), record) &&
+#endif
+        true)) {
+        return false;
+    }
+    return true; // continue processing
+}
+
 /* Get keycode, and then call keyboard function */
 void post_process_record_quantum(keyrecord_t *record) {
     uint16_t keycode = get_record_keycode(record, false);
@@ -254,9 +272,6 @@ bool process_record_quantum(keyrecord_t *record) {
 #ifdef LEADER_ENABLE
             process_leader(keycode, record) &&
 #endif
-#ifdef COMBO_ENABLE
-            process_combo(keycode, record) &&
-#endif
 #ifdef PRINTING_ENABLE
             process_printer(keycode, record) &&
 #endif

tmk_core/common/action.c

@@ -55,6 +55,8 @@ __attribute__((weak)) bool get_ignore_mod_tap_interrupt(uint16_t keycode, keyrec
 __attribute__((weak)) bool get_retro_tapping(uint16_t keycode, keyrecord_t *record) { return false; }
 #endif
 
+__attribute__((weak)) bool pre_process_record_quantum(keyrecord_t *record) { return true; }
+
 #ifndef TAP_CODE_DELAY
 #    define TAP_CODE_DELAY 0
 #endif
@@ -106,9 +108,13 @@ void action_exec(keyevent_t event) {
 #endif
 
 #ifndef NO_ACTION_TAPPING
-    action_tapping_process(record);
+    if (IS_NOEVENT(record.event) || pre_process_record_quantum(&record)) {
+        action_tapping_process(record);
+    }
 #else
-    process_record(&record);
+    if (IS_NOEVENT(record.event) || pre_process_record_quantum(&record)) {
+        process_record(&record);
+    }
     if (!IS_NOEVENT(record.event)) {
         dprint("processed: ");
         debug_record(record);
@@ -206,7 +212,16 @@ void process_record(keyrecord_t *record) {
 }
 
 void process_record_handler(keyrecord_t *record) {
+#ifdef COMBO_ENABLE
+    action_t action;
+    if (record->keycode) {
+        action = action_for_keycode(record->keycode);
+    } else {
+        action = store_or_get_action(record->event.pressed, record->event.key);
+    }
+#else
     action_t action = store_or_get_action(record->event.pressed, record->event.key);
+#endif
     dprint("ACTION: ");
     debug_action(action);
 #ifndef NO_ACTION_LAYER
@@ -990,6 +1005,24 @@ bool is_tap_key(keypos_t key) {
     return is_tap_action(action);
 }
 
+/** \brief Utilities for actions. (FIXME: Needs better description)
+ *
+ * FIXME: Needs documentation.
+ */
+bool is_tap_record(keyrecord_t *record) {
+#ifdef COMBO_ENABLE
+    action_t action;
+    if (record->keycode) {
+        action = action_for_keycode(record->keycode);
+    } else {
+        action = layer_switch_get_action(record->event.key);
+    }
+#else
+    action_t action = layer_switch_get_action(record->event.key);
+#endif
+    return is_tap_action(action);
+}
+
 /** \brief Utilities for actions. (FIXME: Needs better description)
  *
  * FIXME: Needs documentation.

tmk_core/common/action.h

@@ -53,6 +53,9 @@ typedef struct {
 #ifndef NO_ACTION_TAPPING
     tap_t tap;
 #endif
+#ifdef COMBO_ENABLE
+    uint16_t keycode;
+#endif
 } keyrecord_t;
 
 /* Execute action per keyevent */
@@ -60,6 +63,7 @@ void action_exec(keyevent_t event);
 
 /* action for key */
 action_t action_for_key(uint8_t layer, keypos_t key);
+action_t action_for_keycode(uint16_t keycode);
 
 /* macro */
 const macro_t *action_get_macro(keyrecord_t *record, uint8_t id, uint8_t opt);
@@ -111,6 +115,7 @@ void clear_keyboard_but_mods(void);
 void clear_keyboard_but_mods_and_keys(void);
 void layer_switch(uint8_t new_layer);
 bool is_tap_key(keypos_t key);
+bool is_tap_record(keyrecord_t *record);
 bool is_tap_action(action_t action);
 
 #ifndef NO_ACTION_TAPPING

tmk_core/common/action_tapping.c

@@ -18,11 +18,16 @@
 #    define IS_TAPPING_PRESSED() (IS_TAPPING() && tapping_key.event.pressed)
 #    define IS_TAPPING_RELEASED() (IS_TAPPING() && !tapping_key.event.pressed)
 #    define IS_TAPPING_KEY(k) (IS_TAPPING() && KEYEQ(tapping_key.event.key, (k)))
+#ifndef COMBO_ENABLE
+#    define IS_TAPPING_RECORD(r) (IS_TAPPING() && KEYEQ(tapping_key.event.key, (r->event.key)))
+#else
+#    define IS_TAPPING_RECORD(r) (IS_TAPPING() && KEYEQ(tapping_key.event.key, (r->event.key)) && tapping_key.keycode == r->keycode)
+#endif
 
 __attribute__((weak)) uint16_t get_tapping_term(uint16_t keycode, keyrecord_t *record) { return TAPPING_TERM; }
 
 #    ifdef TAPPING_TERM_PER_KEY
-#        define WITHIN_TAPPING_TERM(e) (TIMER_DIFF_16(e.time, tapping_key.event.time) < get_tapping_term(get_event_keycode(tapping_key.event, false), &tapping_key))
+#        define WITHIN_TAPPING_TERM(e) (TIMER_DIFF_16(e.time, tapping_key.event.time) < get_tapping_term(get_record_keycode(&tapping_key, false), &tapping_key))
 #    else
 #        define WITHIN_TAPPING_TERM(e) (TIMER_DIFF_16(e.time, tapping_key.event.time) < TAPPING_TERM)
 #    endif
@@ -103,7 +108,7 @@ bool process_tapping(keyrecord_t *keyp) {
     if (IS_TAPPING_PRESSED()) {
         if (WITHIN_TAPPING_TERM(event)) {
             if (tapping_key.tap.count == 0) {
-                if (IS_TAPPING_KEY(event.key) && !event.pressed) {
+                if (IS_TAPPING_RECORD(keyp) && !event.pressed) {
                     // first tap!
                     debug("Tapping: First tap(0->1).\n");
                     tapping_key.tap.count = 1;
@@ -122,14 +127,14 @@ bool process_tapping(keyrecord_t *keyp) {
 #    if defined(TAPPING_TERM_PER_KEY) || (TAPPING_TERM >= 500) || defined(PERMISSIVE_HOLD) || defined(PERMISSIVE_HOLD_PER_KEY)
                 else if (((
 #        ifdef TAPPING_TERM_PER_KEY
-                              get_tapping_term(get_event_keycode(tapping_key.event, false), keyp)
+                              get_tapping_term(get_record_keycode(&tapping_key, false), keyp)
 #        else
                               TAPPING_TERM
 #        endif
                               >= 500)
 
 #        ifdef PERMISSIVE_HOLD_PER_KEY
-                          || get_permissive_hold(get_event_keycode(tapping_key.event, false), keyp)
+                          || get_permissive_hold(get_record_keycode(&tapping_key, false), keyp)
 #        elif defined(PERMISSIVE_HOLD)
                           || true
 #        endif
@@ -177,7 +182,7 @@ bool process_tapping(keyrecord_t *keyp) {
             }
             // tap_count > 0
             else {
-                if (IS_TAPPING_KEY(event.key) && !event.pressed) {
+                if (IS_TAPPING_RECORD(keyp) && !event.pressed) {
                     debug("Tapping: Tap release(");
                     debug_dec(tapping_key.tap.count);
                     debug(")\n");
@@ -186,11 +191,15 @@ bool process_tapping(keyrecord_t *keyp) {
                     tapping_key = *keyp;
                     debug_tapping_key();
                     return true;
-                } else if (is_tap_key(event.key) && event.pressed) {
+                } else if (is_tap_record(keyp) && event.pressed) {
                     if (tapping_key.tap.count > 1) {
                         debug("Tapping: Start new tap with releasing last tap(>1).\n");
                         // unregister key
-                        process_record(&(keyrecord_t){.tap = tapping_key.tap, .event.key = tapping_key.event.key, .event.time = event.time, .event.pressed = false});
+                        process_record(&(keyrecord_t){.tap = tapping_key.tap, .event.key = tapping_key.event.key, .event.time = event.time, .event.pressed = false,
+#ifdef COMBO_ENABLE
+                                .keycode = tapping_key.keycode,
+#endif
+                                });
                     } else {
                         debug("Tapping: Start while last tap(1).\n");
                     }
@@ -218,17 +227,21 @@ bool process_tapping(keyrecord_t *keyp) {
                 debug_tapping_key();
                 return false;
             } else {
-                if (IS_TAPPING_KEY(event.key) && !event.pressed) {
+                if (IS_TAPPING_RECORD(keyp) && !event.pressed) {
                     debug("Tapping: End. last timeout tap release(>0).");
                     keyp->tap = tapping_key.tap;
                     process_record(keyp);
                     tapping_key = (keyrecord_t){};
                     return true;
-                } else if (is_tap_key(event.key) && event.pressed) {
+                } else if (is_tap_record(keyp) && event.pressed) {
                     if (tapping_key.tap.count > 1) {
                         debug("Tapping: Start new tap with releasing last timeout tap(>1).\n");
                         // unregister key
-                        process_record(&(keyrecord_t){.tap = tapping_key.tap, .event.key = tapping_key.event.key, .event.time = event.time, .event.pressed = false});
+                        process_record(&(keyrecord_t){.tap = tapping_key.tap, .event.key = tapping_key.event.key, .event.time = event.time, .event.pressed = false,
+#ifdef COMBO_ENABLE
+                                .keycode = tapping_key.keycode,
+#endif
+                                });
                     } else {
                         debug("Tapping: Start while last timeout tap(1).\n");
                     }
@@ -248,12 +261,12 @@ bool process_tapping(keyrecord_t *keyp) {
     } else if (IS_TAPPING_RELEASED()) {
         if (WITHIN_TAPPING_TERM(event)) {
             if (event.pressed) {
-                if (IS_TAPPING_KEY(event.key)) {
+                if (IS_TAPPING_RECORD(keyp)) {
 //#    ifndef TAPPING_FORCE_HOLD
 #    if !defined(TAPPING_FORCE_HOLD) || defined(TAPPING_FORCE_HOLD_PER_KEY)
                     if (
 #        ifdef TAPPING_FORCE_HOLD_PER_KEY
-                        !get_tapping_force_hold(get_event_keycode(tapping_key.event, false), keyp) &&
+                        !get_tapping_force_hold(get_record_keycode(&tapping_key, false), keyp) &&
 #        endif
                         !tapping_key.tap.interrupted && tapping_key.tap.count > 0) {
                         // sequential tap.
@@ -271,7 +284,7 @@ bool process_tapping(keyrecord_t *keyp) {
                     // FIX: start new tap again
                     tapping_key = *keyp;
                     return true;
-                } else if (is_tap_key(event.key)) {
+                } else if (is_tap_record(keyp)) {
                     // Sequential tap can be interfered with other tap key.
                     debug("Tapping: Start with interfering other tap.\n");
                     tapping_key = *keyp;
@@ -303,7 +316,7 @@ bool process_tapping(keyrecord_t *keyp) {
     }
     // not tapping state
     else {
-        if (event.pressed && is_tap_key(event.key)) {
+        if (event.pressed && is_tap_record(keyp)) {
             debug("Tapping: Start(Press tap key).\n");
             tapping_key = *keyp;
             process_record_tap_hint(&tapping_key);

tmk_core/common/action_tapping.h

@@ -30,6 +30,7 @@ along with this program.  If not, see <http://www.gnu.org/licenses/>.
 #define WAITING_BUFFER_SIZE 8
 
 #ifndef NO_ACTION_TAPPING
+uint16_t get_record_keycode(keyrecord_t *record, bool update_layer_cache);
 uint16_t get_event_keycode(keyevent_t event, bool update_layer_cache);
 void     action_tapping_process(keyrecord_t record);