Defer sending keyboard report to host until after matrix loop

This change defers calling "host_keyboard_send(keyboard_report)" until after the matrix loop.

This enables the ability to send multiple key changes in a single report, which also means that multiple key changes can be detected in a single polling cycle. This essentially allows us report more information in the same amount of time at no additional cost.

Before this change, only one key change (a press, or release) could be sent at a time. This is because "host_keyboard_send(..)" was created as a blocking method so that data cannot be changed mid-transmission to host, and this method was formerly called immediately after any change was made to the keyboard report. Calling this method at this point meant that the remainder of the polling interval would be used to send the report to host.

The impact of this can be very significant in gaming scenarios, when multiple keypresses need to be triggered simultaneously. See issue qmk#4904 for more details.

I tested the change manually on a Fox Leaf 60 and a Romac, and compared directly against the same firmware without the change. I've confirmed in manual testing that this allows multiple simultaneous keypresses, while not seeming to introduce any new issues.

Exact changes include:
- In "send_keyboard_report(..)" method, flag the last report's state as dirty instead of immediately sending report to host
- Create a new "send_keyboard_report_immediate(..)" method which is called after the matrix loop, which checks if the last report sent to host is dirty and sends an updated report if so, then sets it to clean

Gate deferred keyboard reports feature behind an optional define

Create a define to enable the experimental "deferred keyboard reports" feature (the feature is off by default).

- I manually tested a Fox Leaf 60 firmware with the define on and firmware with the define off.
- All unit tests pass when feature define is not specified.
- 9 unit tests fail when feature define is specified, keeping it in experimental status.

Refactor "defer send keyboard report" feature to support macros

This refactor makes the "defer send keyboard report" feature compatible with macros.

Rather than modify the functionality of all "send_keyboard_report(..)" calls - the method now sends the keyboard report immediately like before. A new function called "send_keyboard_report_deferred(..)" handles the deferred support if the define is set, and I've replaced former calls to "send_keyboard_report(..)" with the new "send_keyboard_report_deferred(..)" function along the code path for handling keys through the matrix scan (this will not affect other calls, such as how macros are handled).

When the feature define is included, it is now passing all but 5 unit tests.

Adapt for tapped and subsequent shifted key events

Tapped key events by default work only on **key release** by sending a key press
event and a subsequent key release event to the host. Now with deferring
the send event after the complete matrix scan these events are merged
and erase each other becoming effectively a nop. The idea is to buffer the
unregister event of the key release for all tapped keys (normal keys are
not buffered) and send these in a separate keyboard report just after
the key press events have been send.

Unregister events/key codes are stored in the unregister_keycodes
struct by unregister_code_buffered() which is called for all tapped
key codes and send after a complete scan is completed by
send_keyboard_report_buffered_unregister_keys().

Subsequent shifted key codes like pressing KC_EQL and KC_PLUS (KC_LSHIFT +
KC_EQL) while holding the former need an additional key release event
of KC_EQL before sending the combination again. This release event isn't
defeered anymore but send as a report immediatly.

All failing unit-tests have been fixed or rather adapted for this
feature. As it turns out most of the failing key press cases expect one
report per key event so they are false negatives.

DEFER_KEYBOARD_REPORT_ENABLE was renamed to REGISTER_MULTIPLE_KEYEVENTS_ENABLE
because it rather states a implementation detail but doesn't communicate purpose
very well in my opinion.

builddefs/show_options.mk

@@ -83,6 +83,7 @@ OTHER_OPTION_NAMES = \
   CAPS_WORD_ENABLE \
   AUTOCORRECT_ENABLE \
   TRI_LAYER_ENABLE \
+  REGISTER_MULTIPLE_KEYEVENTS_ENABLE \
   REPEAT_KEY_ENABLE
 
 define NAME_ECHO

docs/config_options.md

@@ -426,6 +426,8 @@ Use these to enable or disable building certain features. The more you have enab
   * Key combo feature
 * `NKRO_ENABLE`
   * USB N-Key Rollover - if this doesn't work, see here: https://github.com/tmk/tmk_keyboard/wiki/FAQ#nkro-doesnt-work
+* `REGISTER_MULTIPLE_KEYEVENTS_ENABLE`
+  * This allows for multiple key presses and or releases per USB hid report by combining all key state changes of a single matrix scan before reporting to host.
 * `RING_BUFFERED_6KRO_REPORT_ENABLE`
   * USB 6-Key Rollover - Instead of stopping any new input once 6 keys are pressed, the oldest key is released and the new key is pressed.
 * `AUDIO_ENABLE`

quantum/action.c

@@ -32,6 +32,12 @@ along with this program.  If not, see <http://www.gnu.org/licenses/>.
 #include "debug.h"
 #include "quantum.h"
 
+/* private functions */
+static void register_code_P(uint8_t code, void send_report_f(void));
+static void unregister_code_P(uint8_t code, void send_report_f(void));
+
+extern volatile unregister_keycodes_t unregister_keycodes;
+
 #ifdef BACKLIGHT_ENABLE
 #    include "backlight.h"
 #endif
@@ -401,18 +407,18 @@ void process_action(keyrecord_t *record, action_t action) {
                     } else {
                         add_weak_mods(mods);
                     }
-                    send_keyboard_report();
+                    send_keyboard_report_deferred();
                 }
-                register_code(action.key.code);
+                register_code_deferred(action.key.code);
             } else {
-                unregister_code(action.key.code);
+                unregister_code_deferred(action.key.code);
                 if (mods) {
                     if (IS_MODIFIER_KEYCODE(action.key.code) || action.key.code == KC_NO) {
                         del_mods(mods);
                     } else {
                         del_weak_mods(mods);
                     }
-                    send_keyboard_report();
+                    send_keyboard_report_deferred();
                 }
             }
         } break;
@@ -520,12 +526,11 @@ void process_action(keyrecord_t *record, action_t action) {
                     } else {
                         if (tap_count > 0) {
                             ac_dprintf("MODS_TAP: Tap: unregister_code\n");
+                            uint16_t delay = TAP_CODE_DELAY;
                             if (action.layer_tap.code == KC_CAPS_LOCK) {
-                                wait_ms(TAP_HOLD_CAPS_DELAY);
-                            } else {
-                                wait_ms(TAP_CODE_DELAY);
+                                delay = TAP_HOLD_CAPS_DELAY;
                             }
-                            unregister_code(action.key.code);
+                            unregister_code_buffered(action.key.code, delay);
                         } else {
                             ac_dprintf("MODS_TAP: No tap: add_mods\n");
 #    if defined(RETRO_TAPPING) && defined(DUMMY_MOD_NEUTRALIZER_KEYCODE)
@@ -700,12 +705,11 @@ void process_action(keyrecord_t *record, action_t action) {
                     } else {
                         if (tap_count > 0) {
                             ac_dprintf("KEYMAP_TAP_KEY: Tap: unregister_code\n");
+                            uint16_t delay = TAP_CODE_DELAY;
                             if (action.layer_tap.code == KC_CAPS_LOCK) {
-                                wait_ms(TAP_HOLD_CAPS_DELAY);
-                            } else {
-                                wait_ms(TAP_CODE_DELAY);
+                                delay = TAP_HOLD_CAPS_DELAY;
                             }
-                            unregister_code(action.layer_tap.code);
+                            unregister_code_buffered(action.layer_tap.code, delay);
                         } else {
                             ac_dprintf("KEYMAP_TAP_KEY: No tap: Off on release\n");
                             layer_off(action.layer_tap.val);
@@ -788,10 +792,9 @@ void process_action(keyrecord_t *record, action_t action) {
                             swap_held  = false;
                         }
                         if (event.pressed) {
-                            register_code(action.swap.code);
+                            register_code_deferred(action.swap.code);
                         } else {
-                            wait_ms(TAP_CODE_DELAY);
-                            unregister_code(action.swap.code);
+                            unregister_code_buffered(action.swap.code, TAP_CODE_DELAY);
                             *record = (keyrecord_t){}; // hack: reset tap mode
                         }
                     } else {
@@ -875,11 +878,21 @@ void process_action(keyrecord_t *record, action_t action) {
 #endif
 }
 
+__attribute__((weak)) void register_code_deferred(uint8_t code) {
+#if defined(REGISTER_MULTIPLE_KEYEVENTS_ENABLE)
+    register_code_P(code, &send_keyboard_report_deferred);
+#else
+    register_code_P(code, &send_keyboard_report);
+#endif
+}
+
+__attribute__((weak)) void register_code(uint8_t code) { register_code_P(code, &send_keyboard_report); }
+
 /** \brief Utilities for actions. (FIXME: Needs better description)
  *
  * FIXME: Needs documentation.
  */
-__attribute__((weak)) void register_code(uint8_t code) {
+void register_code_P(uint8_t code, void send_report_f(void)) {
     if (code == KC_NO) {
         return;
 
@@ -893,7 +906,7 @@ __attribute__((weak)) void register_code(uint8_t code) {
         send_keyboard_report();
         wait_ms(TAP_HOLD_CAPS_DELAY);
         del_key(KC_CAPS_LOCK);
-        send_keyboard_report();
+        send_report_f();
 
     } else if (KC_LOCKING_NUM_LOCK == code) {
 #    ifdef LOCKING_RESYNC_ENABLE
@@ -903,7 +916,7 @@ __attribute__((weak)) void register_code(uint8_t code) {
         send_keyboard_report();
         wait_ms(100);
         del_key(KC_NUM_LOCK);
-        send_keyboard_report();
+        send_report_f();
 
     } else if (KC_LOCKING_SCROLL_LOCK == code) {
 #    ifdef LOCKING_RESYNC_ENABLE
@@ -913,7 +926,7 @@ __attribute__((weak)) void register_code(uint8_t code) {
         send_keyboard_report();
         wait_ms(100);
         del_key(KC_SCROLL_LOCK);
-        send_keyboard_report();
+        send_report_f();
 #endif
 
     } else if (IS_BASIC_KEYCODE(code)) {
@@ -928,10 +941,10 @@ __attribute__((weak)) void register_code(uint8_t code) {
             send_keyboard_report();
         }
         add_key(code);
-        send_keyboard_report();
+        send_report_f();
     } else if (IS_MODIFIER_KEYCODE(code)) {
         add_mods(MOD_BIT(code));
-        send_keyboard_report();
+        send_report_f();
 
 #ifdef EXTRAKEY_ENABLE
     } else if (IS_SYSTEM_KEYCODE(code)) {
@@ -945,11 +958,42 @@ __attribute__((weak)) void register_code(uint8_t code) {
     }
 }
 
+__attribute__((weak)) void unregister_code_deferred(uint8_t code) {
+#if defined(REGISTER_MULTIPLE_KEYEVENTS_ENABLE)
+    unregister_code_P(code, &send_keyboard_report_deferred);
+#else
+    unregister_code_P(code, &send_keyboard_report);
+#endif
+}
+
+__attribute__((weak)) void unregister_code(uint8_t code) { unregister_code_P(code, &send_keyboard_report); }
+
+__attribute__((weak)) void unregister_code_buffered(uint8_t code, uint16_t delay) {
+#if defined(REGISTER_MULTIPLE_KEYEVENTS_ENABLE)
+    if (unregister_keycodes.len > UNREGISTER_KEYCODES_BUFFER_SIZE) {
+        dprintln("ERROR: couldn't add unregister keycode, buffer is full!");
+        return;
+    }
+    unregister_keycodes.buffer[unregister_keycodes.len] = code;
+    unregister_keycodes.len += 1;
+    if (unregister_keycodes.tap_delay < delay) {
+        unregister_keycodes.tap_delay = delay;
+    }
+#else
+
+    if (delay > 0) {
+        wait_ms(delay);
+    }
+
+    unregister_code_P(code, &send_keyboard_report);
+#endif
+}
+
 /** \brief Utilities for actions. (FIXME: Needs better description)
  *
  * FIXME: Needs documentation.
  */
-__attribute__((weak)) void unregister_code(uint8_t code) {
+void unregister_code_P(uint8_t code, void send_report_f(void)) {
     if (code == KC_NO) {
         return;
 
@@ -960,35 +1004,35 @@ __attribute__((weak)) void unregister_code(uint8_t code) {
         if (!host_keyboard_led_state().caps_lock) return;
 #    endif
         add_key(KC_CAPS_LOCK);
-        send_keyboard_report();
+        send_report_f();
         del_key(KC_CAPS_LOCK);
-        send_keyboard_report();
+        send_report_f();
 
     } else if (KC_LOCKING_NUM_LOCK == code) {
 #    ifdef LOCKING_RESYNC_ENABLE
         if (!host_keyboard_led_state().num_lock) return;
 #    endif
         add_key(KC_NUM_LOCK);
-        send_keyboard_report();
+        send_report_f();
         del_key(KC_NUM_LOCK);
-        send_keyboard_report();
+        send_report_f();
 
     } else if (KC_LOCKING_SCROLL_LOCK == code) {
 #    ifdef LOCKING_RESYNC_ENABLE
         if (!host_keyboard_led_state().scroll_lock) return;
 #    endif
         add_key(KC_SCROLL_LOCK);
-        send_keyboard_report();
+        send_report_f();
         del_key(KC_SCROLL_LOCK);
-        send_keyboard_report();
+        send_report_f();
 #endif
 
     } else if (IS_BASIC_KEYCODE(code)) {
         del_key(code);
-        send_keyboard_report();
+        send_report_f();
     } else if (IS_MODIFIER_KEYCODE(code)) {
         del_mods(MOD_BIT(code));
-        send_keyboard_report();
+        send_report_f();
 
 #ifdef EXTRAKEY_ENABLE
     } else if (IS_SYSTEM_KEYCODE(code)) {

quantum/action.h

@@ -112,7 +112,10 @@ void process_record(keyrecord_t *record);
 void process_record_handler(keyrecord_t *record);
 void post_process_record_quantum(keyrecord_t *record);
 void process_action(keyrecord_t *record, action_t action);
+void register_code_deferred(uint8_t code);
 void register_code(uint8_t code);
+void unregister_code_deferred(uint8_t code);
+void unregister_code_buffered(uint8_t code, uint16_t delay);
 void unregister_code(uint8_t code);
 void tap_code(uint8_t code);
 void tap_code_delay(uint8_t code, uint16_t delay);

quantum/action_util.c

@@ -17,6 +17,7 @@ along with this program.  If not, see <http://www.gnu.org/licenses/>.
 #include "host.h"
 #include "report.h"
 #include "debug.h"
+#include "wait.h"
 #include "action_util.h"
 #include "action_layer.h"
 #include "timer.h"
@@ -38,6 +39,8 @@ report_keyboard_t *keyboard_report = &(report_keyboard_t){};
 #ifdef NKRO_ENABLE
 report_nkro_t *nkro_report = &(report_nkro_t){};
 #endif
+bool keyboard_report_has_deferred_keycodes;
+volatile unregister_keycodes_t unregister_keycodes;
 
 extern inline void add_key(uint8_t key);
 extern inline void del_key(uint8_t key);
@@ -312,20 +315,65 @@ void send_nkro_report(void) {
 }
 #endif
 
+/** \brief Send keyboard report deferred
+ *
+ * Flags the keyboard report to be sent at the soonest availability
+ */
+void send_keyboard_report_deferred(void) {
+    keyboard_report_has_deferred_keycodes = true;
+#if !defined(REGISTER_MULTIPLE_KEYEVENTS_ENABLE)
+    send_keyboard_report_immediate();
+#endif
+}
+
 /** \brief Send keyboard report
  *
- * FIXME: needs doc
+ * Sends the latest keyboard report immediately
  */
 void send_keyboard_report(void) {
+    send_keyboard_report_deferred();
+    send_keyboard_report_immediate();
+}
+
+/** \brief Send keyboard report immediate
+ *
+ * Checks if the keyboard report is different from the last one sent, and if so, sends the updated one
+ */
+void send_keyboard_report_immediate(void) {
+    if (keyboard_report_has_deferred_keycodes) {
 #ifdef NKRO_ENABLE
-    if (keyboard_protocol && keymap_config.nkro) {
-        send_nkro_report();
-    } else {
-        send_6kro_report();
-    }
+        if (keyboard_protocol && keymap_config.nkro) {
+            send_nkro_report();
+        } else {
+            send_6kro_report();
+        }
 #else
-    send_6kro_report();
+        send_6kro_report();
 #endif
+        keyboard_report_has_deferred_keycodes = false;
+    }
+}
+
+/**
+ * @brief Send all unregister keycodes which have been recorded during keycode processing.
+ *
+ */
+void send_keyboard_report_buffered_unregister_keys(void) {
+    if (unregister_keycodes.len > 0) {
+        while (unregister_keycodes.len > 0) {
+            unregister_keycodes.len--;
+            unregister_code_deferred(unregister_keycodes.buffer[unregister_keycodes.len]);
+        }
+
+        if (unregister_keycodes.tap_delay != 0) {
+            wait_ms(unregister_keycodes.tap_delay);
+            unregister_keycodes.tap_delay = 0;
+        }
+
+        /* reset unregister_keycodes buffer. */
+        unregister_keycodes.len = 0;
+        send_keyboard_report_immediate();
+    }
 }
 
 /** \brief Get mods

quantum/action_util.h

@@ -18,19 +18,33 @@ along with this program.  If not, see <http://www.gnu.org/licenses/>.
 #pragma once
 
 #include <stdint.h>
+#include <stddef.h>
 #include "report.h"
 #include "modifiers.h"
 
 #ifdef __cplusplus
 extern "C" {
 #endif
 
+#define UNREGISTER_KEYCODES_BUFFER_SIZE 16  // Hopefully this is enough...
+
+typedef struct {
+    uint8_t  buffer[UNREGISTER_KEYCODES_BUFFER_SIZE];
+    size_t   len;
+    uint16_t tap_delay;
+} unregister_keycodes_t;
+
 extern report_keyboard_t *keyboard_report;
 #ifdef NKRO_ENABLE
 extern report_nkro_t *nkro_report;
 #endif
+extern bool keyboard_report_has_deferred_keycodes;
+extern volatile unregister_keycodes_t unregister_keycodes;
 
+void send_keyboard_report_deferred(void);
 void send_keyboard_report(void);
+void send_keyboard_report_immediate(void);
+void send_keyboard_report_buffered_unregister_keys(void);
 
 /* key */
 inline void add_key(uint8_t key) {