Merge pull request #1587 from UltimateHackingKeyboard/semaphore_rework

Semaphore refactor.

Author: kareltucek

device/src/messenger.c

@@ -34,6 +34,7 @@
 #if DEVICE_IS_UHK_DONGLE
 #include <zephyr/kernel.h>
 #include "usb_report_updater.h"
+#include "usb_report_sender.h"
 
 static K_SEM_DEFINE(dongleUsbSem, 0, 1);
 
@@ -229,8 +230,8 @@ 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)) {
-        uint16_t delay = GetResendThrottleDelay(retryCounter);
+    while (!UsbReportSender_ShouldGiveUp(ret, &retryCounter)) {
+        uint16_t delay = UsbReportSender_ComputeResendDelay(retryCounter);
         k_sleep(K_MSEC(delay));
         k_sem_take(&dongleUsbSem, K_MSEC(128));
         ret = sendDongleReport(propertyId, message);

right/src/CMakeLists.txt

@@ -68,6 +68,9 @@ target_sources(${PROJECT_NAME} PRIVATE
     $<$<BOOL:${IS_MCUX_SDK}>:${CMAKE_CURRENT_SOURCE_DIR}/trace_reasons.c>
     usb_log_buffer.c
     usb_protocol_handler.c
+    usb_report_sender.c
+    usb_scheduler.c
+    usb_semaphore.c
     usb_report_updater.c
     usb_state.c
     user_logic.c

right/src/debug.h

@@ -150,7 +150,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"
 

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

@@ -16,9 +16,13 @@ extern "C" {
 #include "timer.h"
 #include "trace.h"
 #include "usb_report_updater.h"
+#include "usb_semaphore.h"
+#include "usb_scheduler.h"
 #include "led_display.h"
 #include "jitter_test.h"
 #include "usb_state.h"
+#include "utils.h"
+#include "test_suite/test_hooks.h"
 }
 #include "command_app.hpp"
 #include "controls_app.hpp"
@@ -30,87 +34,37 @@ extern "C" {
     #error "Either CONFIG_DEBUG or NDEBUG must be defined"
 #endif
 
-typedef enum {
-    ReportSink_Invalid,
-    ReportSink_Usb,
-    ReportSink_BleHid,
-    ReportSink_Dongle,
-} report_sink_t;
+#ifdef __ZEPHYR__
+#include <zephyr/logging/log.h>
+LOG_MODULE_REGISTER(Transport, LOG_LEVEL_INF);
+#endif
+// On mcux, logger.h provides the LOG_WRN / LOG_ERR / ... redirects.
 
 // Exponential moving average (alpha=1/8) of the measured delay between a
 // BLE HID report being handed to the stack and the corresponding sent callback
-// firing. Populated when DEBUG_BLE_LATENCY_STATS is enabled; useful for
-// observing whether the send pipeline is saturated.
+// firing. Populated when DEBUG_BLE_LATENCY_STATS is enabled (in usb_report_sender.c);
+// useful for observing whether the send pipeline is saturated.
 extern "C" {
 float HidReportBleLatencyAvgMs = 0;
 }
-static uint32_t dispatchTimeMs = 0;
 
-// Approximate transport window intervals (ms) used by the report-construction
-// throttle. After dispatch we estimate the next free window at "now + 2 *
-// interval" (worst case: we just missed a window). The send-completion
-// callback then reduces the estimate to "now + interval".
-static constexpr uint32_t USB_REPORT_INTERVAL_MS = 1;
-
-static uint32_t reportIntervalForSink(report_sink_t sink)
-{
-    switch (sink) {
-    case ReportSink_Usb:
-        return USB_REPORT_INTERVAL_MS;
-    case ReportSink_BleHid:
-    case ReportSink_Dongle:
-#if DEVICE_IS_UHK80_RIGHT
-        return BtConn_GetReportIntervalMs(ActiveHostConnectionId);
-#else
-        return 11;
-#endif
-    default:
-        return 0;
-    }
-}
-
-static void noteReportDispatched(report_sink_t sink)
-{
-    if (DEBUG_BLE_LATENCY_STATS) {
-        if (dispatchTimeMs == 0) {
-            dispatchTimeMs = Timer_GetCurrentTime();
-        }
-    }
-    UsbReportWindowEstimate = UsbReportWindowEstimateLast + 2 * reportIntervalForSink(sink);
-}
-
-static void noteReportSent(report_sink_t transport)
-{
-    if (DEBUG_BLE_LATENCY_STATS) {
-        if (dispatchTimeMs != 0) {
-            uint32_t delta = Timer_GetCurrentTime() - dispatchTimeMs;
-            HidReportBleLatencyAvgMs = (HidReportBleLatencyAvgMs * 7 + delta) / 8;
-            dispatchTimeMs = 0;
-        }
-    }
-    uint32_t reportInterval = reportIntervalForSink(transport);
-    uint32_t currentTime = Timer_GetCurrentTime();
-    int16_t jitterGuess = (currentTime - UsbReportWindowEstimateLast - reportInterval + 1) / 2;
-    jitterGuess = MAX(0, jitterGuess);
-    UsbReportWindowEstimateLast = currentTime - jitterGuess;
-    UsbReportWindowEstimate = currentTime - jitterGuess + reportInterval;
-    uint32_t nextIn = UsbReportWindowEstimate - USB_REPORT_WINDOW_LOOKAHEAD_MS;
-
-    if (DEVICE_IS_UHK80_RIGHT) {
-        EventScheduler_Schedule(nextIn, EventSchedulerEvent_Postponer, "Report sent. Recalculate report throttles");
-    }
-}
+bool UnreliableTransportTestMode = false;
 
 extern "C" void HidTransport_NoteNusReportSent(void)
 {
-    noteReportSent(ReportSink_Dongle);
+    UsbScheduler_ReportDelivered(ReportSink_Dongle);
 }
 
 static report_sink_t determineSink()
 {
+    if (TestHooks_Active) {
+        return ReportSink_TestSuite;
+    }
+
 #if DEVICE_IS_UHK_DONGLE || DEVICE_IS_UHK60
     return ReportSink_Usb;
 #else
+
     connection_type_t connectionType = Connections_Type(ActiveHostConnectionId);
 
     if (!Connections_IsReady(ActiveHostConnectionId)) {
@@ -170,9 +124,12 @@ extern "C" void Hid_TransportStateChanged(
 extern "C" errno_t Hid_SendKeyboardReport(const hid_keyboard_report_t *report)
 {
     report_sink_t sink = determineSink();
-    noteReportDispatched(sink);
+    UsbScheduler_ReportAcceptedByTransport(sink);
     Trace_Printf("z11,%d", sink);
     errno_t err;
+    if (UnreliableTransportTestMode && Utils_Random() % 7 == 0) {
+        return -EAGAIN;
+    }
     switch (sink) {
     case ReportSink_Usb:
         err = keyboard_app::usb_handle().send_report(*report);
@@ -187,9 +144,16 @@ 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 {
+            UsbSemaphore_Release(&UsbSemaphore.keyboard);
         }
         break;
 #endif
+    case ReportSink_TestSuite:
+        err = 0;
+        TestHooks_CaptureReport(report);
+        Hid_KeyboardReportSentCallback(HID_TRANSPORT_USB);
+        break;
     default:
 #ifdef __ZEPHYR__
         printk("Unhandled and unexpected switch state!\n");
@@ -203,8 +167,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;
-    noteReportSent(transport == HID_TRANSPORT_USB ? ReportSink_Usb : ReportSink_BleHid);
+    if (UnreliableTransportTestMode && Utils_Random() % 7 == 0) {
+        return;
+    }
+    UsbSemaphore_Release(&UsbSemaphore.keyboard);
+    UsbScheduler_ReportDelivered(transport == HID_TRANSPORT_USB ? ReportSink_Usb : ReportSink_BleHid);
 #if DEVICE_IS_UHK_DONGLE
     Dongle_SignalUsbReportSent();
 #endif
@@ -213,7 +180,7 @@ extern "C" void Hid_KeyboardReportSentCallback(hid_transport_t transport)
 extern "C" errno_t Hid_SendMouseReport(const hid_mouse_report_t *report)
 {
     report_sink_t sink = determineSink();
-    noteReportDispatched(sink);
+    UsbScheduler_ReportAcceptedByTransport(sink);
     Trace_Printf("z21,%d", sink);
     errno_t err;
     switch (sink) {
@@ -230,6 +197,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 {
+            UsbSemaphore_Release(&UsbSemaphore.mouse);
         }
         break;
 #endif
@@ -249,8 +218,8 @@ extern "C" errno_t Hid_SendMouseReport(const hid_mouse_report_t *report)
 
 extern "C" void Hid_MouseReportSentCallback(hid_transport_t transport)
 {
-    UsbReportUpdateSemaphore &= ~UsbReportUpdate_Mouse;
-    noteReportSent( transport == HID_TRANSPORT_USB ? ReportSink_Usb : ReportSink_BleHid);
+    UsbSemaphore_Release(&UsbSemaphore.mouse);
+    UsbScheduler_ReportDelivered( transport == HID_TRANSPORT_USB ? ReportSink_Usb : ReportSink_BleHid);
 #if DEVICE_IS_UHK_DONGLE
     Dongle_SignalUsbReportSent();
 #endif
@@ -259,7 +228,7 @@ extern "C" void Hid_MouseReportSentCallback(hid_transport_t transport)
 extern "C" errno_t Hid_SendControlsReport(const hid_controls_report_t *report)
 {
     report_sink_t sink = determineSink();
-    noteReportDispatched(sink);
+    UsbScheduler_ReportAcceptedByTransport(sink);
     Trace_Printf("z31,%d", sink);
     errno_t err;
     switch (sink) {
@@ -276,6 +245,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 {
+            UsbSemaphore_Release(&UsbSemaphore.controls);
         }
         break;
 #endif
@@ -292,8 +263,8 @@ extern "C" errno_t Hid_SendControlsReport(const hid_controls_report_t *report)
 
 extern "C" void Hid_ControlsReportSentCallback(hid_transport_t transport)
 {
-    UsbReportUpdateSemaphore &= ~UsbReportUpdate_Controls;
-    noteReportSent( transport == HID_TRANSPORT_USB ? ReportSink_Usb : ReportSink_BleHid);
+    UsbSemaphore_Release(&UsbSemaphore.controls);
+    UsbScheduler_ReportDelivered( transport == HID_TRANSPORT_USB ? ReportSink_Usb : ReportSink_BleHid);
 #if DEVICE_IS_UHK_DONGLE
     Dongle_SignalUsbReportSent();
 #endif

right/src/hid/transport.h

@@ -18,6 +18,16 @@ typedef enum  {
     HID_TRANSPORT_BLE,
 } hid_transport_t;
 
+// Which physical sink a report is dispatched to. Determined by transport.c's
+// determineSink(); shared so the report-sender module can size transport windows per sink.
+typedef enum {
+    ReportSink_Invalid,
+    ReportSink_Usb,
+    ReportSink_BleHid,
+    ReportSink_Dongle,
+    ReportSink_TestSuite,
+} report_sink_t;
+
 typedef enum
 {
     ROLLOVER_N_KEY = 0,
@@ -26,6 +36,7 @@ typedef enum
 
 
 extern float HidReportBleLatencyAvgMs;
+extern bool UnreliableTransportTestMode;
 
 void Hid_TransportStateChanged(hid_transport_t transport, bool enabled);