From f886472d414c8fad5f682d5f794bfa738207b240 Mon Sep 17 00:00:00 2001 From: Taylor Yu Date: Wed, 14 Feb 2024 20:32:07 -0600 Subject: [PATCH] tweak timeout for resume --- src/kaleidoscope/driver/hid/tinyusb/HIDD.h | 35 ++++++++++++++++------ 1 file changed, 26 insertions(+), 9 deletions(-) diff --git a/src/kaleidoscope/driver/hid/tinyusb/HIDD.h b/src/kaleidoscope/driver/hid/tinyusb/HIDD.h index 2961c99efe..2c9925c988 100644 --- a/src/kaleidoscope/driver/hid/tinyusb/HIDD.h +++ b/src/kaleidoscope/driver/hid/tinyusb/HIDD.h @@ -47,18 +47,35 @@ class HIDD : public Adafruit_USBD_HID { bool sendReport(uint8_t report_id, void const *report, uint8_t len) { if (TinyUSBDevice.suspended()) { TinyUSBDevice.remoteWakeup(); - /* Block (with timeout) to avoid dropping events on resume */ - for (int timeout = 250; timeout--;) { - if (ready()) { - break; - } - delay(1); - } + /* + * Wait for 30ms, which is the minimum required by spec for a resume. + * (USB 2.0 ยง7.1.7.7) + * + * 20ms of host echoing the resume downstream, and 10ms of recovery time + * once the bus has resumed. + */ + delay(30); } /* - * Might need an additional timeout here, in case bus is resumed but host - * isn't polling, and the FIFO or peripheral is full + * Block for up to 250ms for endpoint to become ready. If the host is polling, + * this should take about 1ms (the requested polling interval, but the + * host is allowed to poll less frequently). On resume, especially remote + * wakeup on macOS, not only can the resume take longer much than the + * minimum (over 2 seconds, instead of 20ms), the recovery period can be + * over 100ms (instead of 10ms). + * + * This will mitigate, but not necessarily eliminate, stuck key situations + * during resume. One scenario is a key down report being queued while in + * the recovery period, where the bus has resumed, but the host isn't + * polling yet. The endpoint will be ready, but the transmission won't + * happen until the host resumes polling. The transmission of the key up + * event might time out, leading to a stuck key situation. + * + * Unfortunately, we usually can't abort a packet transmission once queued. */ + for (int timeout = 250; timeout-- && !ready();) { + delay(1); + } if (!ready()) { return false; }