From 8b36cd323f28fbee757d4deba76e415439009b70 Mon Sep 17 00:00:00 2001 From: semnil <1051877+semnil@users.noreply.github.com> Date: Tue, 5 May 2026 06:41:41 +0900 Subject: [PATCH 1/4] Move logger.h include outside the Zephyr-only block in debug.c. Debug_RecordBleSendResult() references LogU() inside a DEBUG_BLE_LATENCY_STATS=false dead-code branch. The declaration of LogU() lives in logger.h, which until now was only included in the __ZEPHYR__ branch of debug.c. GCC <= 14 emitted -Wimplicit-function- declaration as a warning when this branch was compiled for UHK60, so the build silently passed; GCC 15 promotes that warning to an error by default and breaks the UHK60 build. logger.h itself is platform agnostic (it only pulls in device.h and stdint.h, and is already included unconditionally by main.c, mouse_controller.c, trace.c, usb_report_updater.c, and others), so the simplest fix is to move the include out of the Zephyr-only block. No behavior change: the affected code is dead under the current DEBUG_BLE_LATENCY_STATS setting. --- right/src/debug.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/right/src/debug.c b/right/src/debug.c index 70d36c643..b76675779 100644 --- a/right/src/debug.c +++ b/right/src/debug.c @@ -1,8 +1,8 @@ #include #include "debug.h" +#include "logger.h" #ifdef __ZEPHYR__ -#include "logger.h" #include "keyboard/oled/screens/screen_manager.h" #include #else From 9616cb4d7df7f7f39dd704f1896bcbfd0760077c Mon Sep 17 00:00:00 2001 From: semnil <1051877+semnil@users.noreply.github.com> Date: Tue, 5 May 2026 06:42:05 +0900 Subject: [PATCH 2/4] Preserve report state on USB send failure (issue #1090). When Hid_SendKeyboardReport / Hid_SendControlsReport / Hid_SendMouseReport returns a non-zero status (transient USB hub stall, busy endpoint, etc.) and the retry helper ShouldResendReport() decides to give up after its ~128 ms window, the previous code took the same path as a successful send: it cleared *NeedsResending and called switchActive*Report(). Switching the active buffer on a failed send is destructive. After the switch, the buffer that held the unsent report becomes the "previously sent" buffer in the double-buffer scheme, so subsequent CheckReportReady comparisons are made against state that was never delivered to the host. The most visible consequence is that release reports get dropped on a flaky link: the host keeps the key in its pressed set and auto-repeats forever, until the next press happens to produce a buffer difference that finally clears the stuck key. Short presses can disappear entirely, and apparent input-order swaps can occur for the same reason on cross-hand bigrams. The retry loop also has no rate limiting between attempts, so during a hub stall the firmware re-attempts on every main-loop iteration. This hammers the upstream side of any shared USB hub heavily enough that *other* HID devices on the same hub start losing reports too, which matches the user-visible symptom of a different keyboard, plugged into the same hub, also exhibiting infinite repeat while the UHK is struggling. Fix: - Branch on `ret != 0` rather than the result of ShouldResendReport(). - On any failure, keep *NeedsResending = true and re-arm EventVector_ResendUsbReports. Crucially, do not switch the active buffer. The next mergeReports() rebuilds the active buffer from the latest cached state and CheckReportReady will retry the send once the link recovers. - Still call ShouldResendReport() so its retry counter stays in sync; use its return value to decide between reportRetry() (transient, log nothing) and handleFail() (long stall, log and on Zephyr try a reconnect). The "give up" path no longer drops the report. - On success, call ShouldResendReport(true, ...) so the retry counter is reset for the next failure window. Applied identically to the keyboard, controls, and mouse send paths. Tested on UHK60 v1 over a USB hub that previously triggered the stuck release behavior with stock v17.0.0; the regression no longer reproduces and other devices on the same hub also stop misbehaving while the UHK is in heavy use. --- right/src/usb_report_updater.c | 46 ++++++++++++++++++++++------------ 1 file changed, 30 insertions(+), 16 deletions(-) diff --git a/right/src/usb_report_updater.c b/right/src/usb_report_updater.c index be8060889..0646ec42a 100644 --- a/right/src/usb_report_updater.c +++ b/right/src/usb_report_updater.c @@ -974,17 +974,25 @@ static void sendActiveReports(bool resending) { //The semaphore has to be set before the call. Assume what happens if a bus reset happens asynchronously here. (Deadlock.) UsbReportUpdateSemaphore |= UsbReportUpdate_Keyboard; ret = Hid_SendKeyboardReport(ActiveKeyboardReport); - if (ShouldResendReport(ret == 0, &keyboardRetries)) { - reportRetry(ret); + if (ret != 0) { + // Send failed (likely a transient USB hub stall or busy endpoint). + // Do NOT switch the buffer: that would lose this report's state and + // make it impossible to recover (e.g. release reports get dropped, + // causing the OS to keep auto-repeating until the next key press). + // The next merge cycle will rebuild the active buffer from the + // latest cached state, and CheckReportReady will retry the send. + if (ShouldResendReport(false, &keyboardRetries)) { + reportRetry(ret); + } else { + handleFail(ret); + } //This is *not* asynchronously safe as long as multiple reports of different type can be sent at the same time. //TODO: consider making it atomic, or lowering semaphore reset delay keyboardNeedsResending = true; UsbReportUpdateSemaphore &= ~UsbReportUpdate_Keyboard; EventVector_Set(EventVector_ResendUsbReports); } else { - if (ret != 0) { - handleFail(ret); - } + ShouldResendReport(true, &keyboardRetries); // reset retry counter keyboardNeedsResending = false; switchActiveKeyboardReport(); } @@ -999,15 +1007,18 @@ static void sendActiveReports(bool resending) { if (ControlsReport_HasChanges(controlsReports) && (!resending || controlsNeedsResending)) { UsbReportUpdateSemaphore |= UsbReportUpdate_Controls; ret = Hid_SendControlsReport(ActiveControlsReport); - if (ShouldResendReport(ret == 0, &controlsRetries)) { - reportRetry(ret); + if (ret != 0) { + // See keyboard send path comment. + if (ShouldResendReport(false, &controlsRetries)) { + reportRetry(ret); + } else { + handleFail(ret); + } controlsNeedsResending = true; UsbReportUpdateSemaphore &= ~UsbReportUpdate_Controls; EventVector_Set(EventVector_ResendUsbReports); } else { - if (ret != 0) { - handleFail(ret); - } + ShouldResendReport(true, &controlsRetries); controlsNeedsResending = false; switchActiveControlsReport(); } @@ -1023,16 +1034,19 @@ static void sendActiveReports(bool resending) { UsbReportUpdateSemaphore |= UsbReportUpdate_Mouse; ret = Hid_SendMouseReport(ActiveMouseReport); - if (ShouldResendReport(ret == 0, &mouseRetries)) { - reportRetry(ret); + if (ret != 0) { + // See keyboard send path comment. + if (ShouldResendReport(false, &mouseRetries)) { + reportRetry(ret); + } else { + handleFail(ret); + clearMouseMovement(); // Don't make cursor jump if we have connection issues. + } mouseNeedsResending = true; UsbReportUpdateSemaphore &= ~UsbReportUpdate_Mouse; EventVector_Set(EventVector_ResendUsbReports); } else { - if (ret != 0) { - handleFail(ret); - clearMouseMovement(); // Don't make cursor jump if we have connection issues. - } + ShouldResendReport(true, &mouseRetries); mouseNeedsResending = false; switchActiveMouseReport(); } From a8aa0d4eaa5be27c31caa358d48b91d7df781d3e Mon Sep 17 00:00:00 2001 From: semnil <1051877+semnil@users.noreply.github.com> Date: Tue, 5 May 2026 08:34:07 +0900 Subject: [PATCH 3/4] Throttle USB resend retries via EventScheduler. After a Hid_Send*Report() failure, the previous code (including the prior commit's state-preservation fix) re-armed EventVector_ResendUsbReports synchronously. EventVector_ResendUsbReports is part of EventVector_ReportUpdateMask, so RunUserLogic() calls UpdateUsbReports() again on the very next main-loop iteration. An idle main loop iteration is short (sub-100us on UHK60), so during a hub stall the firmware re-attempts the send at roughly the main-loop rate. This saturates the USB hub upstream IN-token queue and disturbs the periodic interrupt transfers of every other HID device that shares the hub. Replace the immediate flag set with a 4 ms delayed schedule: EventScheduler_Schedule(now + USB_RESEND_DELAY_MS, EventSchedulerEvent_UsbResend, "usb-resend"); A new EventSchedulerEvent_UsbResend processes by setting EventVector_ResendUsbReports, so the existing UpdateUsbReports() entry point is unchanged. EventScheduler keeps only the earliest scheduled time for a given event id, so the three send paths (keyboard, controls, mouse) collapse into a single scheduled wakeup when they fail in the same cycle. USB_RESEND_DELAY_MS is set to 4 ms. USB Full Speed SOF is 1 ms, so this gives the hub four frames of headroom between attempts. At a typical 30-50 ms keystroke duration this latency is well below any human perception threshold and is paid only when the link has already failed once; the initial send remains immediate. Applied identically to the keyboard, controls, and mouse send paths. The main thread now sleeps via __WFI() between failed attempts instead of spinning, which also lowers CPU activity during a hub stall. --- right/src/event_scheduler.c | 3 +++ right/src/event_scheduler.h | 1 + right/src/usb_report_updater.c | 22 +++++++++++++++++++--- 3 files changed, 23 insertions(+), 3 deletions(-) diff --git a/right/src/event_scheduler.c b/right/src/event_scheduler.c index 88071ec4c..66f86b8bb 100644 --- a/right/src/event_scheduler.c +++ b/right/src/event_scheduler.c @@ -246,6 +246,9 @@ static void processEvt(event_scheduler_event_t evt) BtConn_KickHid(); #endif break; + case EventSchedulerEvent_UsbResend: + EventVector_Set(EventVector_ResendUsbReports); + break; default: return; } diff --git a/right/src/event_scheduler.h b/right/src/event_scheduler.h index ca18febb2..47663a665 100644 --- a/right/src/event_scheduler.h +++ b/right/src/event_scheduler.h @@ -49,6 +49,7 @@ EventSchedulerEvent_UnselectHostConnection, EventSchedulerEvent_OneShotTimeout, EventSchedulerEvent_KickHid, + EventSchedulerEvent_UsbResend, EventSchedulerEvent_Count } event_scheduler_event_t; diff --git a/right/src/usb_report_updater.c b/right/src/usb_report_updater.c index 0646ec42a..d8ca67778 100644 --- a/right/src/usb_report_updater.c +++ b/right/src/usb_report_updater.c @@ -886,6 +886,16 @@ static bool controlsNeedsResending = false; static uint8_t mouseRetries = 0; static bool mouseNeedsResending = false; +// Delay between consecutive resend attempts when a Hid_Send*Report() call +// fails (e.g. because of a transient USB hub stall). Without this throttle +// the resend would re-fire on every main-loop iteration (sub-100us cadence), +// which saturates the hub upstream and disturbs other HID devices on the +// same hub. 4 ms is short enough to feel responsive (a single key event +// missing the 1 ms USB SOF four times in a row is still well under the +// keystroke-perception threshold) and long enough to free the bus between +// attempts. +#define USB_RESEND_DELAY_MS 4 + // Try resending a report for 512ms. Give up if it doesn't succeed by then. bool ShouldResendReport(bool statusOk, uint8_t* counter) { @@ -990,7 +1000,11 @@ static void sendActiveReports(bool resending) { //TODO: consider making it atomic, or lowering semaphore reset delay keyboardNeedsResending = true; UsbReportUpdateSemaphore &= ~UsbReportUpdate_Keyboard; - EventVector_Set(EventVector_ResendUsbReports); + // Throttle the retry: schedule it instead of re-arming the flag + // immediately. Re-arming immediately would call sendActiveReports() + // every main-loop iteration and saturate the hub upstream. + EventScheduler_Schedule(Timer_GetCurrentTime() + USB_RESEND_DELAY_MS, + EventSchedulerEvent_UsbResend, "usb-resend"); } else { ShouldResendReport(true, &keyboardRetries); // reset retry counter keyboardNeedsResending = false; @@ -1016,7 +1030,8 @@ static void sendActiveReports(bool resending) { } controlsNeedsResending = true; UsbReportUpdateSemaphore &= ~UsbReportUpdate_Controls; - EventVector_Set(EventVector_ResendUsbReports); + EventScheduler_Schedule(Timer_GetCurrentTime() + USB_RESEND_DELAY_MS, + EventSchedulerEvent_UsbResend, "usb-resend"); } else { ShouldResendReport(true, &controlsRetries); controlsNeedsResending = false; @@ -1044,7 +1059,8 @@ static void sendActiveReports(bool resending) { } mouseNeedsResending = true; UsbReportUpdateSemaphore &= ~UsbReportUpdate_Mouse; - EventVector_Set(EventVector_ResendUsbReports); + EventScheduler_Schedule(Timer_GetCurrentTime() + USB_RESEND_DELAY_MS, + EventSchedulerEvent_UsbResend, "usb-resend"); } else { ShouldResendReport(true, &mouseRetries); mouseNeedsResending = false; From 373cf4f2bb7a0559a5ac8a623f27ca4c04cb4da0 Mon Sep 17 00:00:00 2001 From: semnil <1051877+semnil@users.noreply.github.com> Date: Tue, 5 May 2026 09:36:34 +0900 Subject: [PATCH 4/4] Rename ShouldResendReport to IsWithinResendWindow. The helper only manages a per-counter retry window now. It is consumed as a log-routing predicate in usb_report_updater.c (transient retry vs fatal) and as the resend-loop condition in messenger.c. The previous name implied the helper drove sending, which is no longer accurate after the state-preservation fix moved that responsibility elsewhere. Also corrects the stale "Try resending a report for 512ms" header comment; the actual window is 128 ms. --- device/src/messenger.c | 2 +- right/src/usb_report_updater.c | 18 ++++++++++-------- right/src/usb_report_updater.h | 2 +- 3 files changed, 12 insertions(+), 10 deletions(-) diff --git a/device/src/messenger.c b/device/src/messenger.c index 497821897..4ba7b70e6 100644 --- a/device/src/messenger.c +++ b/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 (IsWithinResendWindow(ret == 0, &retryCounter)) { k_sem_take(&dongleUsbSem, K_MSEC(128)); ret = sendDongleReport(propertyId, message); } diff --git a/right/src/usb_report_updater.c b/right/src/usb_report_updater.c index d8ca67778..a7c72a4ca 100644 --- a/right/src/usb_report_updater.c +++ b/right/src/usb_report_updater.c @@ -896,8 +896,10 @@ static bool mouseNeedsResending = false; // attempts. #define USB_RESEND_DELAY_MS 4 -// Try resending a report for 512ms. Give up if it doesn't succeed by then. -bool ShouldResendReport(bool statusOk, uint8_t* counter) { +// Tracks a 128ms retry window per counter. Returns true while a failed call +// is still within the window; returns false (and resets the counter) on +// success or after the window expires. +bool IsWithinResendWindow(bool statusOk, uint8_t* counter) { if (statusOk) { *counter = 0; @@ -991,7 +993,7 @@ static void sendActiveReports(bool resending) { // causing the OS to keep auto-repeating until the next key press). // The next merge cycle will rebuild the active buffer from the // latest cached state, and CheckReportReady will retry the send. - if (ShouldResendReport(false, &keyboardRetries)) { + if (IsWithinResendWindow(false, &keyboardRetries)) { reportRetry(ret); } else { handleFail(ret); @@ -1006,7 +1008,7 @@ static void sendActiveReports(bool resending) { EventScheduler_Schedule(Timer_GetCurrentTime() + USB_RESEND_DELAY_MS, EventSchedulerEvent_UsbResend, "usb-resend"); } else { - ShouldResendReport(true, &keyboardRetries); // reset retry counter + IsWithinResendWindow(true, &keyboardRetries); // reset retry counter keyboardNeedsResending = false; switchActiveKeyboardReport(); } @@ -1023,7 +1025,7 @@ static void sendActiveReports(bool resending) { ret = Hid_SendControlsReport(ActiveControlsReport); if (ret != 0) { // See keyboard send path comment. - if (ShouldResendReport(false, &controlsRetries)) { + if (IsWithinResendWindow(false, &controlsRetries)) { reportRetry(ret); } else { handleFail(ret); @@ -1033,7 +1035,7 @@ static void sendActiveReports(bool resending) { EventScheduler_Schedule(Timer_GetCurrentTime() + USB_RESEND_DELAY_MS, EventSchedulerEvent_UsbResend, "usb-resend"); } else { - ShouldResendReport(true, &controlsRetries); + IsWithinResendWindow(true, &controlsRetries); controlsNeedsResending = false; switchActiveControlsReport(); } @@ -1051,7 +1053,7 @@ static void sendActiveReports(bool resending) { ret = Hid_SendMouseReport(ActiveMouseReport); if (ret != 0) { // See keyboard send path comment. - if (ShouldResendReport(false, &mouseRetries)) { + if (IsWithinResendWindow(false, &mouseRetries)) { reportRetry(ret); } else { handleFail(ret); @@ -1062,7 +1064,7 @@ static void sendActiveReports(bool resending) { EventScheduler_Schedule(Timer_GetCurrentTime() + USB_RESEND_DELAY_MS, EventSchedulerEvent_UsbResend, "usb-resend"); } else { - ShouldResendReport(true, &mouseRetries); + IsWithinResendWindow(true, &mouseRetries); mouseNeedsResending = false; switchActiveMouseReport(); } diff --git a/right/src/usb_report_updater.h b/right/src/usb_report_updater.h index 3aeee8b1e..608760961 100644 --- a/right/src/usb_report_updater.h +++ b/right/src/usb_report_updater.h @@ -69,6 +69,6 @@ void RecordKeyTiming_ReportKeystroke(key_state_t *keyState, bool active, uint32_t pressTime, uint32_t activationTime); hid_keyboard_report_t* GetInactiveKeyboardReport(void); - bool ShouldResendReport(bool statusOk, uint8_t* counter); + bool IsWithinResendWindow(bool statusOk, uint8_t* counter); #endif