Separate 6KRO and NKRO report structs (#22267)

keyboards/annepro2/annepro2_ble.c

@@ -31,7 +31,7 @@ static void ap2_ble_swtich_ble_driver(void);
 
 /* -------------------- Static Local Variables ------------------------------ */
 static host_driver_t ap2_ble_driver = {
-    ap2_ble_leds, ap2_ble_keyboard, ap2_ble_mouse, ap2_ble_extra
+    ap2_ble_leds, ap2_ble_keyboard, NULL, ap2_ble_mouse, ap2_ble_extra
 };
 
 static uint8_t ble_mcu_wakeup[11] = {0x7b, 0x12, 0x53, 0x00, 0x03, 0x00, 0x01, 0x7d, 0x02, 0x01, 0x02};

keyboards/horrortroll/lemon40/info.json

@@ -19,8 +19,7 @@
             "knight": true,
             "christmas": true,
             "static_gradient": true,
-            "alternating": true,
-            "twinkle": true
+            "alternating": true
         }
     },
     "ws2812": {

keyboards/kbdfans/kbd67/rev1/info.json

@@ -38,7 +38,6 @@
       "knight": true,
       "christmas": true,
       "static_gradient": true,
-      "rgb_test": true,
       "alternating": true,
       "twinkle": true
     }

quantum/action.c

@@ -925,7 +925,7 @@ __attribute__((weak)) void register_code(uint8_t code) {
         // Force a new key press if the key is already pressed
         // without this, keys with the same keycode, but different
         // modifiers will be reported incorrectly, see issue #1708
-        if (is_key_pressed(keyboard_report, code)) {
+        if (is_key_pressed(code)) {
             del_key(code);
             send_keyboard_report();
         }

quantum/action_util.c

@@ -35,6 +35,9 @@ static uint8_t suppressed_mods    = 0;
 // TODO: pointer variable is not needed
 // report_keyboard_t keyboard_report = {};
 report_keyboard_t *keyboard_report = &(report_keyboard_t){};
+#ifdef NKRO_ENABLE
+report_nkro_t *nkro_report = &(report_nkro_t){};
+#endif
 
 extern inline void add_key(uint8_t key);
 extern inline void del_key(uint8_t key);
@@ -252,13 +255,8 @@ bool is_oneshot_enabled(void) {
 
 #endif
 
-/** \brief Send keyboard report
- *
- * FIXME: needs doc
- */
-void send_keyboard_report(void) {
-    keyboard_report->mods = real_mods;
-    keyboard_report->mods |= weak_mods;
+static uint8_t get_mods_for_report(void) {
+    uint8_t mods = real_mods | weak_mods;
 
 #ifndef NO_ACTION_ONESHOT
     if (oneshot_mods) {
@@ -268,20 +266,25 @@ void send_keyboard_report(void) {
             clear_oneshot_mods();
         }
 #    endif
-        keyboard_report->mods |= oneshot_mods;
-        if (has_anykey(keyboard_report)) {
+        mods |= oneshot_mods;
+        if (has_anykey()) {
             clear_oneshot_mods();
         }
     }
-
 #endif
 
 #ifdef KEY_OVERRIDE_ENABLE
     // These need to be last to be able to properly control key overrides
-    keyboard_report->mods &= ~suppressed_mods;
-    keyboard_report->mods |= weak_override_mods;
+    mods &= ~suppressed_mods;
+    mods |= weak_override_mods;
 #endif
 
+    return mods;
+}
+
+void send_6kro_report(void) {
+    keyboard_report->mods = get_mods_for_report();
+
 #ifdef PROTOCOL_VUSB
     host_keyboard_send(keyboard_report);
 #else
@@ -295,6 +298,36 @@ void send_keyboard_report(void) {
 #endif
 }
 
+#ifdef NKRO_ENABLE
+void send_nkro_report(void) {
+    nkro_report->mods = get_mods_for_report();
+
+    static report_nkro_t last_report;
+
+    /* Only send the report if there are changes to propagate to the host. */
+    if (memcmp(nkro_report, &last_report, sizeof(report_nkro_t)) != 0) {
+        memcpy(&last_report, nkro_report, sizeof(report_nkro_t));
+        host_nkro_send(nkro_report);
+    }
+}
+#endif
+
+/** \brief Send keyboard report
+ *
+ * FIXME: needs doc
+ */
+void send_keyboard_report(void) {
+#ifdef NKRO_ENABLE
+    if (keyboard_protocol && keymap_config.nkro) {
+        send_nkro_report();
+    } else {
+        send_6kro_report();
+    }
+#else
+    send_6kro_report();
+#endif
+}
+
 /** \brief Get mods
  *
  * FIXME: needs doc

quantum/action_util.h

@@ -26,20 +26,23 @@ extern "C" {
 #endif
 
 extern report_keyboard_t *keyboard_report;
+#ifdef NKRO_ENABLE
+extern report_nkro_t *nkro_report;
+#endif
 
 void send_keyboard_report(void);
 
 /* key */
 inline void add_key(uint8_t key) {
-    add_key_to_report(keyboard_report, key);
+    add_key_to_report(key);
 }
 
 inline void del_key(uint8_t key) {
-    del_key_from_report(keyboard_report, key);
+    del_key_from_report(key);
 }
 
 inline void clear_keys(void) {
-    clear_keys_from_report(keyboard_report);
+    clear_keys_from_report();
 }
 
 /* modifier */

tests/test_common/keyboard_report_util.cpp

@@ -95,12 +95,12 @@ std::ostream& operator<<(std::ostream& os, const report_keyboard_t& report) {
 }
 
 KeyboardReportMatcher::KeyboardReportMatcher(const std::vector<uint8_t>& keys) {
-    memset(m_report.raw, 0, sizeof(m_report.raw));
+    memset(&m_report, 0, sizeof(report_keyboard_t));
     for (auto k : keys) {
         if (IS_MODIFIER_KEYCODE(k)) {
             m_report.mods |= MOD_BIT(k);
         } else {
-            add_key_to_report(&m_report, k);
+            add_key_byte(&m_report, k);
         }
     }
 }

tests/test_common/test_driver.cpp

@@ -31,7 +31,7 @@ uint8_t hex_digit_to_keycode(uint8_t digit) {
 }
 } // namespace
 
-TestDriver::TestDriver() : m_driver{&TestDriver::keyboard_leds, &TestDriver::send_keyboard, &TestDriver::send_mouse, &TestDriver::send_extra} {
+TestDriver::TestDriver() : m_driver{&TestDriver::keyboard_leds, &TestDriver::send_keyboard, &TestDriver::send_nkro, &TestDriver::send_mouse, &TestDriver::send_extra} {
     host_set_driver(&m_driver);
     m_this = this;
 }
@@ -49,6 +49,10 @@ void TestDriver::send_keyboard(report_keyboard_t* report) {
     m_this->send_keyboard_mock(*report);
 }
 
+void TestDriver::send_nkro(report_nkro_t* report) {
+    m_this->send_nkro_mock(*report);
+}
+
 void TestDriver::send_mouse(report_mouse_t* report) {
     m_this->send_mouse_mock(*report);
 }

tests/test_common/test_driver.hpp

@@ -32,12 +32,14 @@ class TestDriver {
     }
 
     MOCK_METHOD1(send_keyboard_mock, void(report_keyboard_t&));
+    MOCK_METHOD1(send_nkro_mock, void(report_nkro_t&));
     MOCK_METHOD1(send_mouse_mock, void(report_mouse_t&));
     MOCK_METHOD1(send_extra_mock, void(report_extra_t&));
 
    private:
     static uint8_t     keyboard_leds(void);
     static void        send_keyboard(report_keyboard_t* report);
+    static void        send_nkro(report_nkro_t* report);
     static void        send_mouse(report_mouse_t* report);
     static void        send_extra(report_extra_t* report);
     host_driver_t      m_driver;

tmk_core/protocol.mk

@@ -46,9 +46,7 @@ else
 endif
 
 ifeq ($(strip $(NKRO_ENABLE)), yes)
-    ifeq ($(PROTOCOL), VUSB)
-        $(info NKRO is not currently supported on V-USB, and has been disabled.)
-    else ifeq ($(strip $(BLUETOOTH_ENABLE)), yes)
+    ifeq ($(strip $(BLUETOOTH_ENABLE)), yes)
         $(info NKRO is not currently supported with Bluetooth, and has been disabled.)
     else
         OPT_DEFS += -DNKRO_ENABLE

tmk_core/protocol/arm_atsam/main_arm_atsam.c

@@ -36,14 +36,15 @@ uint8_t g_usb_state = USB_FSMSTATUS_FSMSTATE_OFF_Val; // Saved USB state from ha
 void    main_subtasks(void);
 uint8_t keyboard_leds(void);
 void    send_keyboard(report_keyboard_t *report);
+void    send_nkro(report_nkro_t *report);
 void    send_mouse(report_mouse_t *report);
 void    send_extra(report_extra_t *report);
 
 #ifdef DEFERRED_EXEC_ENABLE
 void deferred_exec_task(void);
 #endif // DEFERRED_EXEC_ENABLE
 
-host_driver_t arm_atsam_driver = {keyboard_leds, send_keyboard, send_mouse, send_extra};
+host_driver_t arm_atsam_driver = {keyboard_leds, send_keyboard, send_nkro, send_mouse, send_extra};
 
 uint8_t led_states;
 
@@ -59,41 +60,41 @@ uint8_t keyboard_leds(void) {
 void send_keyboard(report_keyboard_t *report) {
     uint32_t irqflags;
 
-#ifdef NKRO_ENABLE
-    if (!keymap_config.nkro) {
-#endif // NKRO_ENABLE
-        while (udi_hid_kbd_b_report_trans_ongoing) {
-            main_subtasks();
-        } // Run other tasks while waiting for USB to be free
+    while (udi_hid_kbd_b_report_trans_ongoing) {
+        main_subtasks();
+    } // Run other tasks while waiting for USB to be free
 
-        irqflags = __get_PRIMASK();
-        __disable_irq();
-        __DMB();
+    irqflags = __get_PRIMASK();
+    __disable_irq();
+    __DMB();
 
-        memcpy(udi_hid_kbd_report, report->raw, UDI_HID_KBD_REPORT_SIZE);
-        udi_hid_kbd_b_report_valid = 1;
-        udi_hid_kbd_send_report();
+    memcpy(udi_hid_kbd_report, report, UDI_HID_KBD_REPORT_SIZE);
+    udi_hid_kbd_b_report_valid = 1;
+    udi_hid_kbd_send_report();
 
-        __DMB();
-        __set_PRIMASK(irqflags);
+    __DMB();
+    __set_PRIMASK(irqflags);
+}
+
+void send_nkro(report_nkro_t *report) {
 #ifdef NKRO_ENABLE
-    } else {
-        while (udi_hid_nkro_b_report_trans_ongoing) {
-            main_subtasks();
-        } // Run other tasks while waiting for USB to be free
+    uint32_t irqflags;
 
-        irqflags = __get_PRIMASK();
-        __disable_irq();
-        __DMB();
+    while (udi_hid_nkro_b_report_trans_ongoing) {
+        main_subtasks();
+    } // Run other tasks while waiting for USB to be free
 
-        memcpy(udi_hid_nkro_report, report->raw, UDI_HID_NKRO_REPORT_SIZE);
-        udi_hid_nkro_b_report_valid = 1;
-        udi_hid_nkro_send_report();
+    irqflags = __get_PRIMASK();
+    __disable_irq();
+    __DMB();
 
-        __DMB();
-        __set_PRIMASK(irqflags);
-    }
-#endif // NKRO_ENABLE
+    memcpy(udi_hid_nkro_report, report, UDI_HID_NKRO_REPORT_SIZE);
+    udi_hid_nkro_b_report_valid = 1;
+    udi_hid_nkro_send_report();
+
+    __DMB();
+    __set_PRIMASK(irqflags);
+#endif
 }
 
 void send_mouse(report_mouse_t *report) {

tmk_core/protocol/chibios/chibios.c

@@ -59,11 +59,12 @@
 /* declarations */
 uint8_t keyboard_leds(void);
 void    send_keyboard(report_keyboard_t *report);
+void    send_nkro(report_nkro_t *report);
 void    send_mouse(report_mouse_t *report);
 void    send_extra(report_extra_t *report);
 
 /* host struct */
-host_driver_t chibios_driver = {keyboard_leds, send_keyboard, send_mouse, send_extra};
+host_driver_t chibios_driver = {keyboard_leds, send_keyboard, send_nkro, send_mouse, send_extra};
 
 #ifdef VIRTSER_ENABLE
 void virtser_task(void);

tmk_core/protocol/chibios/usb_main.c

@@ -71,7 +71,7 @@ static virtual_timer_t keyboard_idle_timer;
 
 static void keyboard_idle_timer_cb(struct ch_virtual_timer *, void *arg);
 
-report_keyboard_t keyboard_report_sent = {{0}};
+report_keyboard_t keyboard_report_sent = {0};
 report_mouse_t    mouse_report_sent    = {0};
 
 union {
@@ -883,26 +883,22 @@ void send_report(uint8_t endpoint, void *report, size_t size) {
 /* prepare and start sending a report IN
  * not callable from ISR or locked state */
 void send_keyboard(report_keyboard_t *report) {
-    uint8_t ep   = KEYBOARD_IN_EPNUM;
-    size_t  size = KEYBOARD_REPORT_SIZE;
-
     /* If we're in Boot Protocol, don't send any report ID or other funky fields */
     if (!keyboard_protocol) {
-        send_report(ep, &report->mods, 8);
+        send_report(KEYBOARD_IN_EPNUM, &report->mods, 8);
     } else {
-#ifdef NKRO_ENABLE
-        if (keymap_config.nkro) {
-            ep   = SHARED_IN_EPNUM;
-            size = sizeof(struct nkro_report);
-        }
-#endif
-
-        send_report(ep, report, size);
+        send_report(KEYBOARD_IN_EPNUM, report, KEYBOARD_REPORT_SIZE);
     }
 
     keyboard_report_sent = *report;
 }
 
+void send_nkro(report_nkro_t *report) {
+#ifdef NKRO_ENABLE
+    send_report(SHARED_IN_EPNUM, report, sizeof(report_nkro_t));
+#endif
+}
+
 /* ---------------------------------------------------------
  *                     Mouse functions
  * ---------------------------------------------------------