Semaphores: Revive the old UsbReportUpdateSemaphore, and hook it to retries and logs.

Author: kareltucek

device/src/messenger.c

@@ -241,7 +241,7 @@ static void processSyncablePropertyDongle(device_id_t src, const uint8_t* data,
 
 #if DEVICE_IS_UHK_DONGLE
     uint8_t retryCounter = 0;
-    while (ShouldResendReport(ret == 0, &retryCounter)) {
+    while (ShouldResendReport(ret, &retryCounter)) {
         uint16_t delay = GetResendThrottleDelay(retryCounter);
         k_sleep(K_MSEC(delay));
         k_sem_take(&dongleUsbSem, K_MSEC(128));

right/src/debug.h

@@ -16,6 +16,7 @@
 #define DEBUG_MODE false
 #define DEBUG_STRESS_UART false
 #define DEBUG_STRESS_GATT false
+#define DEBUG_STRESS_REPORTS false
 #define DEBUG_TEST_RTT false
 #define DEBUG_LOG_UART true
 #define DEBUG_LOG_MESSAGES false
@@ -150,7 +151,7 @@
 #define WATCH_SEMAPHORE_TAKE(SEM, FILENAME, N) if(CurrentWatch == N) { WatchSemaforeTake(SEM, FILENAME, N); } else { k_sem_take(SEM, K_FOREVER); }
 #define SEM_TAKE(SEM) WATCH_SEMAPHORE_TAKE(SEM, __FILE__, 0)
 #else
-#define SEM_TAKE(SEM) if (k_sem_take(SEM, K_MSEC(256)) != 0) { uint8_t tgt = Cfg.DevMode ? LogTarget_Uart | LogTarget_ErrorBuffer : LogTarget_Uart; LogTo(DEVICE_ID, tgt, "Failed to take semaphore " #SEM " in file %s. This shouldn't have happened. Please report it!\n", __FILE__); Trace_Print(tgt, "Failed semaphore");  }
+#define SEM_TAKE(SEM) if (k_sem_take(SEM, K_MSEC(256)) != 0) { uint8_t tgt = Cfg.DevMode ? LogTarget_Uart | LogTarget_ErrorBuffer : LogTarget_Uart; LogTo(DEVICE_ID, tgt, "Failed to take semaphore " #SEM " in file %s.!\n", __FILE__); Trace_Print(tgt, "Failed semaphore");  }
 #define WATCH_SEMAPHORE_TAKE(SEM, LABEL, N) k_sem_take(SEM, K_FOREVER);
 #endif
 

right/src/event_scheduler.c

@@ -10,6 +10,7 @@
 #include "slave_drivers/uhk_module_driver.h"
 #include "peripherals/merge_sensor.h"
 #include "power_mode.h"
+#include "usb_report_updater.h"
 #include "oneshot.h"
 #include "trace.h"
 
@@ -251,6 +252,11 @@ static void processEvt(event_scheduler_event_t evt)
         case EventSchedulerEvent_SendUsbReports:
             EventVector_Set(EventVector_SendUsbReports);
             break;
+        case EventSchedulerEvent_UpdateUsbSemaphore:
+            // Drives the semaphore timeout so a stuck (unconfirmed) report gets resent
+            // even if nothing else wakes the event loop.
+            UsbReadyForTransfers();
+            break;
         case EventSchedulerEvent_CheckLeftBleVsUart:
 #if DEVICE_IS_UHK80_LEFT
             BtManager_CheckLeftBleVsUart();

right/src/event_scheduler.h

@@ -50,6 +50,7 @@
         EventSchedulerEvent_OneShotTimeout,
         EventSchedulerEvent_KickHid,
         EventSchedulerEvent_SendUsbReports,
+        EventSchedulerEvent_UpdateUsbSemaphore,
         EventSchedulerEvent_CheckLeftBleVsUart,
         EventSchedulerEvent_ConnectionsUpdateState,
         EventSchedulerEvent_Count

right/src/hid/command_app.cpp

@@ -65,9 +65,19 @@ void command_app::get_report(hid::report::selector select, const std::span<uint8
 
 void command_app::in_report_sent(const std::span<const uint8_t> &data)
 {
+#if DEVICE_IS_UHK80_RIGHT
+    // On BLE all apps share one merged HOGP interface, so in_report_sent is broadcast
+    // to every app; act only on our own (command) report ID. The USB handle is a
+    // standalone interface (whose report may carry no report-ID byte), so the filter
+    // must not be applied there.
+    if ((this == &ble_handle()) && (data.front() != report_ids::IN_COMMAND)) {
+        return;
+    }
+#else
     if (data.front() != report_ids::IN_COMMAND) {
         return;
     }
+#endif
     auto buf_idx = in_buffer_.indexof(data.data());
     in_buffer_.compare_swap(buf_idx);
 }

right/src/hid/controls_app.cpp

@@ -33,6 +33,14 @@ int controls_app::send_report(const hid_controls_report_t &report)
 
 void controls_app::in_report_sent(const std::span<const uint8_t> &data)
 {
+#if DEVICE_IS_UHK80_RIGHT
+    // On BLE all apps share one merged HOGP interface, so in_report_sent is broadcast
+    // to every app for every sent report. Act only on our own (controls) report ID.
+    // The USB handle is a standalone interface, so the filter must not be applied there.
+    if ((this == &ble_handle()) && (data.front() != report_ids::IN_CONTROLS)) {
+        return;
+    }
+#endif
     Hid_ControlsReportSentCallback((this == &usb_handle()) ? HID_TRANSPORT_USB : HID_TRANSPORT_BLE);
 }
 

right/src/hid/keyboard_app.cpp

@@ -180,8 +180,8 @@ void keyboard_app::set_report(hid::report::type type, const std::span<const uint
 void keyboard_app::in_report_sent(const std::span<const uint8_t> &data)
 {
 #if DEVICE_IS_UHK80_RIGHT
-    if ((prot_ == hid::protocol::REPORT) && (data.front() != KEYS_NKRO_REPORT_ID) &&
-        (data.front() != KEYS_6KRO_REPORT_ID)) {
+    if ((this == &ble_handle()) && (prot_ == hid::protocol::REPORT) &&
+        (data.front() != KEYS_NKRO_REPORT_ID) && (data.front() != KEYS_6KRO_REPORT_ID)) {
         return;
     }
 #endif

right/src/hid/mouse_app.cpp

@@ -34,6 +34,15 @@ int mouse_app::send_report(const hid_mouse_report_t &report)
 
 void mouse_app::in_report_sent(const std::span<const uint8_t> &data)
 {
+#if DEVICE_IS_UHK80_RIGHT
+    // On BLE all apps share one merged HOGP interface, so in_report_sent is broadcast
+    // to every app for every sent report. Act only on our own (mouse) report ID. The
+    // USB handle is a standalone interface that only ever receives mouse completions,
+    // so the filter must not be applied there.
+    if ((this == &ble_handle()) && (data.front() != report_ids::IN_MOUSE)) {
+        return;
+    }
+#endif
     Hid_MouseReportSentCallback((this == &usb_handle()) ? HID_TRANSPORT_USB : HID_TRANSPORT_BLE);
 }
 

right/src/hid/transport.cpp

@@ -19,6 +19,7 @@ extern "C" {
 #include "led_display.h"
 #include "jitter_test.h"
 #include "usb_state.h"
+#include "utils.h"
 }
 #include "command_app.hpp"
 #include "controls_app.hpp"
@@ -173,6 +174,9 @@ extern "C" errno_t Hid_SendKeyboardReport(const hid_keyboard_report_t *report)
     noteReportDispatched(sink);
     Trace_Printf("z11,%d", sink);
     errno_t err;
+    if (DEBUG_STRESS_REPORTS && Utils_Random() % 16 == 0) {
+        return 1;
+    }
     switch (sink) {
     case ReportSink_Usb:
         err = keyboard_app::usb_handle().send_report(*report);
@@ -187,6 +191,8 @@ extern "C" errno_t Hid_SendKeyboardReport(const hid_keyboard_report_t *report)
         err = Messenger_Send2(DeviceId_Uhk_Dongle, MessageId_SyncableProperty, SyncablePropertyId_KeyboardReport, (const uint8_t *)report, sizeof(*report));
         if (err != 0) {
             printk("Failed to send keyboard report to dongle: %d\n", err);
+        } else {
+            UsbReportUpdater_ConfirmKeyboardReportSent();
         }
         break;
 #endif
@@ -203,7 +209,11 @@ extern "C" errno_t Hid_SendKeyboardReport(const hid_keyboard_report_t *report)
 
 extern "C" void Hid_KeyboardReportSentCallback(hid_transport_t transport)
 {
-    UsbReportUpdateSemaphore &= ~UsbReportUpdate_Keyboard;
+    if (DEBUG_STRESS_REPORTS && Utils_Random() % 16 == 0) {
+        // 666
+        return;
+    }
+    UsbReportUpdater_ConfirmKeyboardReportSent();
     noteReportSent(transport == HID_TRANSPORT_USB ? ReportSink_Usb : ReportSink_BleHid);
 #if DEVICE_IS_UHK_DONGLE
     Dongle_SignalUsbReportSent();
@@ -230,6 +240,8 @@ extern "C" errno_t Hid_SendMouseReport(const hid_mouse_report_t *report)
         err = Messenger_Send2(DeviceId_Uhk_Dongle, MessageId_SyncableProperty, SyncablePropertyId_MouseReport, (const uint8_t *)report, sizeof(*report));
         if (err != 0) {
             printk("Failed to send mouse report to dongle: %d\n", err);
+        } else {
+            UsbReportUpdater_ConfirmMouseReportSent();
         }
         break;
 #endif
@@ -249,7 +261,7 @@ extern "C" errno_t Hid_SendMouseReport(const hid_mouse_report_t *report)
 
 extern "C" void Hid_MouseReportSentCallback(hid_transport_t transport)
 {
-    UsbReportUpdateSemaphore &= ~UsbReportUpdate_Mouse;
+    UsbReportUpdater_ConfirmMouseReportSent();
     noteReportSent( transport == HID_TRANSPORT_USB ? ReportSink_Usb : ReportSink_BleHid);
 #if DEVICE_IS_UHK_DONGLE
     Dongle_SignalUsbReportSent();
@@ -276,6 +288,8 @@ extern "C" errno_t Hid_SendControlsReport(const hid_controls_report_t *report)
         err = Messenger_Send2(DeviceId_Uhk_Dongle, MessageId_SyncableProperty, SyncablePropertyId_ControlsReport, (const uint8_t *)report, sizeof(*report));
         if (err != 0) {
             printk("Failed to send controls report to dongle: %d\n", err);
+        } else {
+            UsbReportUpdater_ConfirmControlsReportSent();
         }
         break;
 #endif
@@ -292,7 +306,7 @@ extern "C" errno_t Hid_SendControlsReport(const hid_controls_report_t *report)
 
 extern "C" void Hid_ControlsReportSentCallback(hid_transport_t transport)
 {
-    UsbReportUpdateSemaphore &= ~UsbReportUpdate_Controls;
+    UsbReportUpdater_ConfirmControlsReportSent();
     noteReportSent( transport == HID_TRANSPORT_USB ? ReportSink_Usb : ReportSink_BleHid);
 #if DEVICE_IS_UHK_DONGLE
     Dongle_SignalUsbReportSent();

right/src/main.c

@@ -123,17 +123,6 @@ void CopyRightKeystateMatrix(void)
     }
 }
 
-bool UsbReadyForTransfers(void) {
-    if (UsbReportUpdateSemaphore && CurrentPowerMode > PowerMode_LastAwake) {
-        if (Timer_GetElapsedTime(&UpdateUsbReports_LastUpdateTime) < USB_SEMAPHORE_TIMEOUT) {
-            return false;
-        } else {
-            UsbReportUpdateSemaphore = 0;
-        }
-    }
-    return true;
-}
-
 static void initUsb() {
     while (!IsHardwareConfigInitialized) {
         __WFI();
@@ -258,7 +247,7 @@ int main(void)
                 checkSleepMode();
             }
 
-            if (UsbReadyForTransfers() && EventScheduler_Vector & EventVector_UserLogicUpdateMask) {
+            if (EventScheduler_Vector & EventVector_UserLogicUpdateMask) {
                 Trace('(');
                 RunUserLogic();
                 Trace(')');