Semaphore: refactors - move sending and scheduling stuff around.

Author: kareltucek

device/src/messenger.c

@@ -35,6 +35,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);
 
@@ -241,8 +242,8 @@ static void processSyncablePropertyDongle(device_id_t src, const uint8_t* data,
 
 #if DEVICE_IS_UHK_DONGLE
     uint8_t retryCounter = 0;
-    while (ShouldResendReport(ret, &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/event_scheduler.c

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

right/src/hid/transport.cpp

@@ -16,6 +16,8 @@ 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"
@@ -32,83 +34,25 @@ extern "C" {
     #error "Either CONFIG_DEBUG or NDEBUG must be defined"
 #endif
 
-typedef enum {
-    ReportSink_Invalid,
-    ReportSink_Usb,
-    ReportSink_BleHid,
-    ReportSink_Dongle,
-    ReportSink_TestSuite,
-} 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;
 
 bool UnreliableTransportTestMode = false;
 
-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 1;
-    }
-}
-
-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");
-    }
-}
-
 extern "C" void HidTransport_NoteNusReportSent(void)
 {
-    noteReportSent(ReportSink_Dongle);
+    UsbScheduler_ReportDelivered(ReportSink_Dongle);
 }
 
 static report_sink_t determineSink()
@@ -180,7 +124,7 @@ 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) {
@@ -201,7 +145,7 @@ extern "C" errno_t Hid_SendKeyboardReport(const hid_keyboard_report_t *report)
         if (err != 0) {
             printk("Failed to send keyboard report to dongle: %d\n", err);
         } else {
-            UsbReportUpdater_ConfirmKeyboardReportSent();
+            UsbSemaphore_Release(&UsbSemaphore.keyboard);
         }
         break;
 #endif
@@ -226,8 +170,8 @@ extern "C" void Hid_KeyboardReportSentCallback(hid_transport_t transport)
     if (UnreliableTransportTestMode && Utils_Random() % 7 == 0) {
         return;
     }
-    UsbReportUpdater_ConfirmKeyboardReportSent();
-    noteReportSent(transport == HID_TRANSPORT_USB ? ReportSink_Usb : ReportSink_BleHid);
+    UsbSemaphore_Release(&UsbSemaphore.keyboard);
+    UsbScheduler_ReportDelivered(transport == HID_TRANSPORT_USB ? ReportSink_Usb : ReportSink_BleHid);
 #if DEVICE_IS_UHK_DONGLE
     Dongle_SignalUsbReportSent();
 #endif
@@ -236,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) {
@@ -254,7 +198,7 @@ extern "C" errno_t Hid_SendMouseReport(const hid_mouse_report_t *report)
         if (err != 0) {
             printk("Failed to send mouse report to dongle: %d\n", err);
         } else {
-            UsbReportUpdater_ConfirmMouseReportSent();
+            UsbSemaphore_Release(&UsbSemaphore.mouse);
         }
         break;
 #endif
@@ -274,8 +218,8 @@ extern "C" errno_t Hid_SendMouseReport(const hid_mouse_report_t *report)
 
 extern "C" void Hid_MouseReportSentCallback(hid_transport_t transport)
 {
-    UsbReportUpdater_ConfirmMouseReportSent();
-    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
@@ -284,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) {
@@ -302,7 +246,7 @@ extern "C" errno_t Hid_SendControlsReport(const hid_controls_report_t *report)
         if (err != 0) {
             printk("Failed to send controls report to dongle: %d\n", err);
         } else {
-            UsbReportUpdater_ConfirmControlsReportSent();
+            UsbSemaphore_Release(&UsbSemaphore.controls);
         }
         break;
 #endif
@@ -319,8 +263,8 @@ extern "C" errno_t Hid_SendControlsReport(const hid_controls_report_t *report)
 
 extern "C" void Hid_ControlsReportSentCallback(hid_transport_t transport)
 {
-    UsbReportUpdater_ConfirmControlsReportSent();
-    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,

right/src/hid/transport_usb.cpp

@@ -6,6 +6,7 @@ extern "C" {
 #include "usb_state.h"
 #include "timer.h"
 #include "usb_report_updater.h"
+#include "usb_semaphore.h"
 #include "user_logic.h"
 #include "logger.h"
 #ifdef __ZEPHYR__
@@ -131,7 +132,7 @@ struct usb_manager {
             }
             if ((ev & event::CONFIGURATION_CHANGE) != event::NONE) {
                 // reset the semaphore on USB configuration or reset
-                UsbReportUpdater_ClearSemaphore(0xFF);
+                UsbSemaphore_Clear();
             }
         });
     }

right/src/main.c

@@ -22,6 +22,7 @@
 #include "peripherals/reset_button.h"
 #include "config_parser/config_globals.h"
 #include "usb_report_updater.h"
+#include "usb_semaphore.h"
 #include "macro_events.h"
 #include "macros/shortcut_parser.h"
 #include "macros/keyid_parser.h"
@@ -86,14 +87,14 @@ static void sendFirstReport()
     errno_t ret = -1;
     // Wait until sending a report is successful, but don't block longer than 5 seconds.
     while (ret && Timer_GetCurrentTime() < 5000) {
-        UsbReportUpdater_SetSemaphore(UsbReportUpdate_Keyboard);
+        UsbSemaphore_Set(UsbReportUpdate_Keyboard);
         ret = Hid_SendKeyboardReport(&emptyReport);
         if (ret) {
-            UsbReportUpdater_ClearSemaphore(UsbReportUpdate_Keyboard);
+            UsbSemaphore_Clear();
             __WFI();
         }
     }
-    while (UsbReportUpdater_GetSemaphore()) {
+    while (UsbSemaphore_Get()) {
         __WFI();
     }
 }

right/src/usb_commands/usb_command_get_variable.c

@@ -3,6 +3,7 @@
 #include "key_matrix.h"
 #include "test_switches.h"
 #include "usb_report_updater.h"
+#include "usb_semaphore.h"
 #include "macros/core.h"
 #include "config_manager.h"
 
@@ -29,7 +30,7 @@ void UsbCommand_GetVariable(const uint8_t *GenericHidOutBuffer, uint8_t *Generic
             SetUsbTxBufferUint8(1, Cfg.DebounceTimeRelease);
             break;
         case UsbVariable_UsbReportSemaphore:
-            SetUsbTxBufferUint8(1, UsbReportUpdater_GetSemaphore());
+            SetUsbTxBufferUint8(1, UsbSemaphore_Get());
             break;
         case UsbVariable_StatusBuffer:
             for (uint8_t i = 1; i < USB_COMMAND_BUFFER_LENGTH; i++) {

right/src/usb_commands/usb_command_set_variable.c

@@ -5,6 +5,7 @@
 #include "key_matrix.h"
 #include "test_switches.h"
 #include "usb_report_updater.h"
+#include "usb_semaphore.h"
 #include "config_manager.h"
 #include "ledmap.h"
 
@@ -37,12 +38,10 @@ void UsbCommand_SetVariable(const uint8_t *GenericHidOutBuffer, uint8_t *Generic
         case UsbVariable_DebounceTimeRelease:
             Cfg.DebounceTimeRelease = GetUsbRxBufferUint8(2);
             break;
-        case UsbVariable_UsbReportSemaphore: {
-            uint8_t bits = GetUsbRxBufferUint8(2);
-            UsbReportUpdater_ClearSemaphore((uint8_t)~bits);
-            UsbReportUpdater_SetSemaphore(bits);
+        case UsbVariable_UsbReportSemaphore:
+            // Set overwrites all three in-flight flags from the bitfield.
+            UsbSemaphore_Set(GetUsbRxBufferUint8(2));
             break;
-        }
         case UsbVariable_StatusBuffer:
             break;
         case UsbVariable_LedAudioRegisters: