Nkey-rollover - conflict fixes and refactors.

Author: Karel Tuček <[email protected]>

Split the commit to only perform the refactoring, before the NKRO
changes are applied. Now the keyboard's protocol is internally
always faked to be the boot protocol so we can verify these changes
independently.

Signed-off-by: Benedek Kupper <[email protected]>

right/src/macro_recorder.c

@@ -15,8 +15,8 @@
  *     - 1 bit = left shift
  *     - 1 bit = right shift
  *     - both = full mod mask follow
- *   - 3 bits= number of scancodes
- * - further bytes mod mask and scancodes as specified by header
+ *   - 3 bits = number of scancodes, 7 = full size follows
+ * - further bytes: mod mask and scancodes as specified by header
  */
 
 bool RuntimeMacroPlaying = false;
@@ -154,15 +154,15 @@ static void playReport(usb_basic_keyboard_report_t *report)
         break;
     case BasicKeyboardSimple:
         memset(report, 0, sizeof *report);
-        report->scancodes[0] = readByte();
+        UsbBasicKeyboard_AddScancode(report, readByte(), NULL);
         break;
     case BasicKeyboard:
         memset(report, 0, sizeof *report);
         {
             uint8_t size = readByte();
             report->modifiers = readByte();
             for (int i = 0; i < size; i++) {
-                report->scancodes[i] = readByte();
+                UsbBasicKeyboard_AddScancode(report, readByte(), NULL);
             }
         }
         break;
@@ -208,38 +208,41 @@ static bool playRuntimeMacroContinue(usb_basic_keyboard_report_t* report)
     return RuntimeMacroPlaying;
 }
 
+void writeReportScancodes(usb_basic_keyboard_report_t *report)
+{
+    UsbBasicKeyboard_ForeachScancode(report, &writeByte);
+}
+
 void MacroRecorder_RecordBasicReport(usb_basic_keyboard_report_t *report)
 {
     if (!RuntimeMacroRecording) {
         return;
     }
+
+    uint8_t sc = UsbBasicKeyboard_ScancodeCount(report);
+
     if (
-            (reportBufferLength + REPORT_BUFFER_SAFETY_MARGIN >= REPORT_BUFFER_MAX_LENGTH) ||
-            (recordingHeader->length >= REPORT_BUFFER_MAX_MACRO_LENGTH)
+            (reportBufferLength + 3 + sc > REPORT_BUFFER_MAX_LENGTH) ||
+            (recordingHeader->length + 1 > REPORT_BUFFER_MAX_MACRO_LENGTH)
     ) {
         recordRuntimeMacroEnd();
         discardLastHeaderSlot();
         return;
     }
-    if (report->modifiers == 0 && report->scancodes[0] == 0) {
+    if ((report->modifiers == 0) && (sc == 0)) {
         writeByte(BasicKeyboardEmpty);
         return;
     }
-    if (report->modifiers == 0 && report->scancodes[1] == 0) {
+    if ((report->modifiers == 0) && (sc == 1)) {
         writeByte(BasicKeyboardSimple);
-        writeByte(report->scancodes[0]);
+        writeReportScancodes(report);
         return;
     }
+
     writeByte(BasicKeyboard);
-    uint8_t size = 0;
-    while( size < USB_BASIC_KEYBOARD_MAX_KEYS && report->scancodes[size] != 0) {
-        size++;
-    }
-    writeByte(size);
+    writeByte(sc);
     writeByte(report->modifiers);
-    for (int i = 0; i < size; i++) {
-        writeByte(report->scancodes[i]);
-    }
+    writeReportScancodes(report);
 }
 
 void MacroRecorder_RecordDelay(uint16_t delay)

right/src/macros.c

@@ -7,6 +7,7 @@
 #include "config_parser/parse_macro.h"
 #include "config_parser/config_globals.h"
 #include "timer.h"
+#include "usb_interfaces/usb_interface_basic_keyboard.h"
 #include "keymap.h"
 #include "usb_report_updater.h"
 #include "led_display.h"
@@ -110,16 +111,8 @@ static void addBasicScancode(uint8_t scancode)
     if (!scancode) {
         return;
     }
-    for (uint8_t i = 0; i < USB_BASIC_KEYBOARD_MAX_KEYS; i++) {
-        if (s->ms.macroBasicKeyboardReport.scancodes[i] == scancode) {
-            return;
-        }
-    }
-    for (uint8_t i = 0; i < USB_BASIC_KEYBOARD_MAX_KEYS; i++) {
-        if (!s->ms.macroBasicKeyboardReport.scancodes[i]) {
-            s->ms.macroBasicKeyboardReport.scancodes[i] = scancode;
-            break;
-        }
+    if (!UsbBasicKeyboard_ContainsScancode(&s->ms.macroBasicKeyboardReport, scancode)) {
+        UsbBasicKeyboard_AddScancode(&s->ms.macroBasicKeyboardReport, scancode, NULL);
     }
 }
 
@@ -128,12 +121,7 @@ static void deleteBasicScancode(uint8_t scancode)
     if (!scancode) {
         return;
     }
-    for (uint8_t i = 0; i < USB_BASIC_KEYBOARD_MAX_KEYS; i++) {
-        if (s->ms.macroBasicKeyboardReport.scancodes[i] == scancode) {
-            s->ms.macroBasicKeyboardReport.scancodes[i] = 0;
-            return;
-        }
-    }
+    UsbBasicKeyboard_RemoveScancode(&s->ms.macroBasicKeyboardReport, scancode);
 }
 
 static void addModifiers(uint8_t modifiers)
@@ -182,30 +170,15 @@ static void addSystemScancode(uint8_t scancode)
     if (!scancode) {
         return;
     }
-    for (uint8_t i = 0; i < USB_SYSTEM_KEYBOARD_MAX_KEYS; i++) {
-        if (s->ms.macroSystemKeyboardReport.scancodes[i] == scancode) {
-            return;
-        }
-    }
-    for (uint8_t i = 0; i < USB_SYSTEM_KEYBOARD_MAX_KEYS; i++) {
-        if (!s->ms.macroSystemKeyboardReport.scancodes[i]) {
-            s->ms.macroSystemKeyboardReport.scancodes[i] = scancode;
-            break;
-        }
-    }
+    UsbSystemKeyboard_AddScancode(&s->ms.macroSystemKeyboardReport, scancode, NULL);
 }
 
 static void deleteSystemScancode(uint8_t scancode)
 {
     if (!scancode) {
         return;
     }
-    for (uint8_t i = 0; i < USB_SYSTEM_KEYBOARD_MAX_KEYS; i++) {
-        if (s->ms.macroSystemKeyboardReport.scancodes[i] == scancode) {
-            s->ms.macroSystemKeyboardReport.scancodes[i] = 0;
-            return;
-        }
-    }
+    UsbSystemKeyboard_RemoveScancode(&s->ms.macroSystemKeyboardReport, scancode);
 }
 
 static void addScancode(uint16_t scancode, keystroke_type_t type)
@@ -597,11 +570,14 @@ static macro_result_t dispatchText(const char* text, uint16_t textLen)
     } else {
         dispatchMutex = s;
     }
-    uint8_t max_keys = USB_BASIC_KEYBOARD_MAX_KEYS/2;
+    uint8_t max_keys;
     char character = 0;
     uint8_t scancode = 0;
     uint8_t mods = 0;
 
+    if (UsbBasicKeyboardGetProtocol() == USB_HID_BOOT_PROTOCOL) {
+        max_keys = USB_BOOT_KEYBOARD_MAX_KEYS - 2;
+    }
     // Precompute modifiers and scancode.
     if (s->as.dispatchData.textIdx != textLen) {
         character = text[s->as.dispatchData.textIdx];
@@ -643,15 +619,13 @@ static macro_result_t dispatchText(const char* text, uint16_t textLen)
     // If current character is already contained in the report, we need to
     // release it first. We do so by artificially marking the report
     // full. Next call will do rest of the work for us.
-    for (uint8_t i = 0; i < s->as.dispatchData.reportIdx; i++) {
-        if (s->ms.macroBasicKeyboardReport.scancodes[i] == scancode) {
-            s->as.dispatchData.reportIdx = max_keys;
-            return MacroResult_Blocking;
-        }
+    if (UsbBasicKeyboard_ContainsScancode(&s->ms.macroBasicKeyboardReport, scancode)) {
+        s->as.dispatchData.reportIdx = max_keys;
+        return MacroResult_Blocking;
     }
 
     // Send the scancode.
-    s->ms.macroBasicKeyboardReport.scancodes[s->as.dispatchData.reportIdx++] = scancode;
+    UsbBasicKeyboard_AddScancode(&s->ms.macroBasicKeyboardReport, scancode, &s->as.dispatchData.reportIdx);
     ++s->as.dispatchData.textIdx;
     return MacroResult_Blocking;
 }

right/src/usb_interfaces/usb_interface_basic_keyboard.c

@@ -25,7 +25,7 @@ void UsbBasicKeyboardResetActiveReport(void)
 
 usb_hid_protocol_t UsbBasicKeyboardGetProtocol(void)
 {
-    return usbBasicKeyboardProtocol;
+    return USB_HID_BOOT_PROTOCOL;
 }
 
 usb_status_t UsbBasicKeyboardAction(void)
@@ -136,3 +136,95 @@ usb_status_t UsbBasicKeyboardCallback(class_handle_t handle, uint32_t event, voi
 
     return error;
 }
+
+static void setRolloverError(usb_basic_keyboard_report_t* report)
+{
+    if (report->scancodes[0] != HID_KEYBOARD_SC_ERROR_ROLLOVER) {
+        memset(report->scancodes, HID_KEYBOARD_SC_ERROR_ROLLOVER, ARRAY_SIZE(report->scancodes));
+    }
+}
+
+void UsbBasicKeyboard_AddScancode(usb_basic_keyboard_report_t* report, uint8_t scancode, uint8_t* idx)
+{
+    if (scancode == 0)
+        return;
+
+    if (UsbBasicKeyboardGetProtocol() == USB_HID_BOOT_PROTOCOL) {
+        if (idx == NULL) {
+            for (uint8_t i = 0; i < ARRAY_SIZE(report->scancodes); i++) {
+                if (report->scancodes[i] == 0) {
+                    report->scancodes[i] = scancode;
+                    return;
+                }
+            }
+        } else if (*idx < ARRAY_SIZE(report->scancodes)) {
+            report->scancodes[(*idx)++] = scancode;
+            return;
+        } else {
+            /* invalid index */
+        }
+
+        setRolloverError(report);
+    }
+}
+
+void UsbBasicKeyboard_RemoveScancode(usb_basic_keyboard_report_t* report, uint8_t scancode)
+{
+    if (UsbBasicKeyboardGetProtocol() == USB_HID_BOOT_PROTOCOL) {
+        for (uint8_t i = 0; i < ARRAY_SIZE(report->scancodes); i++) {
+            if (report->scancodes[i] == scancode) {
+                report->scancodes[i] = 0;
+                return;
+            }
+        }
+    }
+}
+
+bool UsbBasicKeyboard_ContainsScancode(const usb_basic_keyboard_report_t* report, uint8_t scancode)
+{
+    if (UsbBasicKeyboardGetProtocol() == USB_HID_BOOT_PROTOCOL) {
+        for (uint8_t i = 0; i < ARRAY_SIZE(report->scancodes); i++) {
+            if (report->scancodes[i] == scancode) {
+                return true;
+            }
+        }
+        return false;
+    } else {
+        return false;
+    }
+}
+
+size_t UsbBasicKeyboard_ScancodeCount(const usb_basic_keyboard_report_t* report)
+{
+    size_t size = 0;
+    if (UsbBasicKeyboardGetProtocol() == USB_HID_BOOT_PROTOCOL) {
+        while ((size < ARRAY_SIZE(report->scancodes)) && (report->scancodes[size] != 0)) {
+            size++;
+        }
+    }
+    return size;
+}
+
+void UsbBasicKeyboard_MergeReports(const usb_basic_keyboard_report_t* sourceReport, usb_basic_keyboard_report_t* targetReport, uint8_t* idx)
+{
+    if (UsbBasicKeyboardGetProtocol() == USB_HID_BOOT_PROTOCOL) {
+        targetReport->modifiers |= sourceReport->modifiers;
+        for (uint8_t i = 0; (i < ARRAY_SIZE(sourceReport->scancodes)) && (sourceReport->scancodes[i] != 0); i++) {
+            if (*idx < ARRAY_SIZE(sourceReport->scancodes)) {
+                targetReport->scancodes[(*idx)++] = sourceReport->scancodes[i];
+            } else {
+                setRolloverError(targetReport);
+                return;
+            }
+        }
+    }
+}
+
+void UsbBasicKeyboard_ForeachScancode(const usb_basic_keyboard_report_t* report, void(*action)(uint8_t))
+{
+    if (UsbBasicKeyboardGetProtocol() == USB_HID_BOOT_PROTOCOL) {
+        for (uint8_t i = 0; (i < ARRAY_SIZE(report->scancodes)) && (report->scancodes[i] != 0); i++) {
+            action(report->scancodes[i]);
+        }
+    }
+}

right/src/usb_interfaces/usb_interface_basic_keyboard.h

@@ -21,6 +21,9 @@
 
     #define USB_BASIC_KEYBOARD_REPORT_LENGTH 8
     #define USB_BASIC_KEYBOARD_OUT_REPORT_LENGTH 1
+
+    #define USB_BOOT_KEYBOARD_REPORT_LENGTH 8
+    #define USB_BOOT_KEYBOARD_MAX_KEYS 6
 // Typedefs:
 
     // Note: We support boot protocol mode in this interface, thus the keyboard
@@ -50,4 +53,11 @@
     usb_status_t UsbBasicKeyboardCheckIdleElapsed();
     usb_status_t UsbBasicKeyboardCheckReportReady();
 
+    void UsbBasicKeyboard_AddScancode(usb_basic_keyboard_report_t* report, uint8_t scancode, uint8_t* idx);
+    void UsbBasicKeyboard_RemoveScancode(usb_basic_keyboard_report_t* report, uint8_t scancode);
+    bool UsbBasicKeyboard_ContainsScancode(const usb_basic_keyboard_report_t* report, uint8_t scancode);
+    size_t UsbBasicKeyboard_ScancodeCount(const usb_basic_keyboard_report_t* report);
+    void UsbBasicKeyboard_MergeReports(const usb_basic_keyboard_report_t* sourceReport, usb_basic_keyboard_report_t* targetReport, uint8_t* idx);
+    void UsbBasicKeyboard_ForeachScancode(const usb_basic_keyboard_report_t* report, void(*action)(uint8_t));
+
 #endif

right/src/usb_interfaces/usb_interface_media_keyboard.c

@@ -89,3 +89,32 @@ usb_status_t UsbMediaKeyboardCallback(class_handle_t handle, uint32_t event, voi
 
     return error;
 }
+
+void UsbMediaKeyboard_AddScancode(usb_media_keyboard_report_t* report, uint8_t scancode, uint8_t* idx)
+{
+    if (scancode == 0)
+        return;
+
+    if (idx == NULL) {
+        for (uint8_t i = 0; i < ARRAY_SIZE(report->scancodes); i++) {
+            if (report->scancodes[i] == 0) {
+                report->scancodes[i] = scancode;
+                return;
+            }
+        }
+    } else if (*idx < ARRAY_SIZE(report->scancodes)) {
+        report->scancodes[(*idx)++] = scancode;
+        return;
+    } else {
+        /* invalid index */
+    }
+}
+
+void UsbMediaKeyboard_MergeReports(const usb_media_keyboard_report_t* sourceReport, usb_media_keyboard_report_t* targetReport, uint8_t* idx)
+{
+    for (uint8_t i = 0; (i < ARRAY_SIZE(sourceReport->scancodes)) && (sourceReport->scancodes[i] != 0); i++) {
+        if (*idx < ARRAY_SIZE(sourceReport->scancodes)) {
+            targetReport->scancodes[(*idx)++] = sourceReport->scancodes[i];
+        }
+    }
+}

right/src/usb_interfaces/usb_interface_media_keyboard.h

@@ -40,4 +40,7 @@
     usb_status_t UsbMediaKeyboardCheckIdleElapsed();
     usb_status_t UsbMediaKeyboardCheckReportReady();
 
+    void UsbMediaKeyboard_AddScancode(usb_media_keyboard_report_t* report, uint8_t scancode, uint8_t* idx);
+    void UsbMediaKeyboard_MergeReports(const usb_media_keyboard_report_t* sourceReport, usb_media_keyboard_report_t* targetReport, uint8_t* idx);
+
 #endif