From 23c79941d237cb132ebd1c14d5ee7f3362769093 Mon Sep 17 00:00:00 2001 From: Adam Shiervani Date: Wed, 17 Sep 2025 16:52:25 +0200 Subject: [PATCH] refactor: enhance keep-alive handling and jitter compensation in HID RPC - Implemented staleness guard to ignore outdated keep-alive packets. - Added jitter compensation logic to adjust timer extensions based on packet arrival times. - Introduced new methods for managing keep-alive state and reset functionality in the Session struct. - Updated auto-release delay mechanism to use dynamic durations based on keep-alive timing. - Adjusted keep-alive interval in the UI to improve responsiveness. --- hidrpc.go | 58 +++++++++++++++++++++++++++++- internal/usbgadget/hid_keyboard.go | 25 ++++++++----- internal/usbgadget/usbgadget.go | 1 + ui/src/hooks/useKeyboard.ts | 5 +-- usb.go | 10 ++++++ webrtc.go | 18 ++++++++-- 6 files changed, 101 insertions(+), 16 deletions(-) diff --git a/hidrpc.go b/hidrpc.go index ffcfd241..62eb8491 100644 --- a/hidrpc.go +++ b/hidrpc.go @@ -39,7 +39,63 @@ func handleHidRPCMessage(message hidrpc.Message, session *Session) { rpcCancelKeyboardMacro() return case hidrpc.TypeKeypressKeepAliveReport: - gadget.DelayAutoRelease() + session.keepAliveJitterLock.Lock() + defer session.keepAliveJitterLock.Unlock() + + now := time.Now() + + // Tunables + // Keep in mind + // macOS default: 15 * 15 = 225ms https://discussions.apple.com/thread/1316947?sortBy=rank + // Linux default: 250ms https://man.archlinux.org/man/kbdrate.8.en + // Windows default: 1s `HKEY_CURRENT_USER\Control Panel\Accessibility\Keyboard Response\AutoRepeatDelay` + + const expectedRate = 50 * time.Millisecond // expected keepalive interval + const maxLateness = 50 * time.Millisecond // max jitter we'll tolerate OR jitter budget + const baseExtension = expectedRate + maxLateness // 100ms extension on perfect tick + + const maxStaleness = 225 * time.Millisecond // discard ancient packets outright + + // 1) Staleness guard: ensures packets that arrive far beyond the life of a valid key hold + // (e.g. after a network stall, retransmit burst, or machine sleep) are ignored outright. + // This prevents “zombie” keepalives from reviving a key that should already be released. + if !session.lastTimerResetTime.IsZero() && now.Sub(session.lastTimerResetTime) > maxStaleness { + return + } + + validTick := true + timerExtension := baseExtension + + if !session.lastKeepAliveArrivalTime.IsZero() { + timeSinceLastTick := now.Sub(session.lastKeepAliveArrivalTime) + lateness := timeSinceLastTick - expectedRate + + if lateness > 0 { + if lateness <= maxLateness { + // --- Small lateness (within jitterBudget) --- + // This is normal jitter (e.g., Wi-Fi contention). + // We still accept the tick, but *reduce the extension* + // so that the total hold time stays aligned with REAL client side intent. + timerExtension -= lateness + } else { + // --- Large lateness (beyond jitterBudget) --- + // This is likely a retransmit stall or ordering delay. + // We reject the tick entirely and DO NOT extend, + // so the auto-release still fires on time. + validTick = false + } + } + } + + if validTick { + // Only valid ticks update our state and extend the timer. + session.lastKeepAliveArrivalTime = now + session.lastTimerResetTime = now + if ug := getUsbGadget(); ug != nil { + ug.DelayAutoReleaseWithDuration(timerExtension) + } + } + // On a miss: do not advance any state — keeps baseline stable. case hidrpc.TypePointerReport: pointerReport, err := message.PointerReport() if err != nil { diff --git a/internal/usbgadget/hid_keyboard.go b/internal/usbgadget/hid_keyboard.go index 94eefbde..37502190 100644 --- a/internal/usbgadget/hid_keyboard.go +++ b/internal/usbgadget/hid_keyboard.go @@ -26,11 +26,6 @@ var keyboardConfig = gadgetConfigItem{ reportDesc: keyboardReportDesc, } -// macOS default: 15 * 15 = 225ms https://discussions.apple.com/thread/1316947?sortBy=rank -// Linux default: 250ms https://man.archlinux.org/man/kbdrate.8.en -// Windows default: 1s `HKEY_CURRENT_USER\Control Panel\Accessibility\Keyboard Response\AutoRepeatDelay` -const autoReleaseKeyboardInterval = time.Millisecond * 225 - // Source: https://www.kernel.org/doc/Documentation/usb/gadget_hid.txt var keyboardReportDesc = []byte{ 0x05, 0x01, /* USAGE_PAGE (Generic Desktop) */ @@ -158,6 +153,10 @@ func (u *UsbGadget) SetOnKeysDownChange(f func(state KeysDownState)) { u.onKeysDownChange = &f } +func (u *UsbGadget) SetOnKeepAliveReset(f func()) { + u.onKeepAliveReset = &f +} + func (u *UsbGadget) scheduleAutoRelease(key byte) { u.kbdAutoReleaseLock.Lock() defer unlockWithLog(&u.kbdAutoReleaseLock, u.log, "autoRelease scheduled") @@ -166,7 +165,10 @@ func (u *UsbGadget) scheduleAutoRelease(key byte) { u.kbdAutoReleaseTimers[key].Stop() } - u.kbdAutoReleaseTimers[key] = time.AfterFunc(autoReleaseKeyboardInterval, func() { + // TODO: This shouldn't use the global autoReleaseKeyboardStartInterval + // but rather the baseExtension from the keepalive jitter compensation logic. + // Make them global as they will in the future likely be variable. + u.kbdAutoReleaseTimers[key] = time.AfterFunc(100*time.Millisecond, func() { u.performAutoRelease(key) }) } @@ -179,10 +181,15 @@ func (u *UsbGadget) cancelAutoRelease(key byte) { timer.Stop() u.kbdAutoReleaseTimers[key] = nil delete(u.kbdAutoReleaseTimers, key) + + // Reset keep-alive timing when key is released + if u.onKeepAliveReset != nil { + (*u.onKeepAliveReset)() + } } } -func (u *UsbGadget) DelayAutoRelease() { +func (u *UsbGadget) DelayAutoReleaseWithDuration(resetDuration time.Duration) { u.kbdAutoReleaseLock.Lock() defer unlockWithLog(&u.kbdAutoReleaseLock, u.log, "autoRelease delayed") @@ -190,9 +197,11 @@ func (u *UsbGadget) DelayAutoRelease() { return } + u.log.Debug().Dur("reset_duration", resetDuration).Msg("delaying auto-release with dynamic duration") + for _, timer := range u.kbdAutoReleaseTimers { if timer != nil { - timer.Reset(autoReleaseKeyboardInterval) + timer.Reset(resetDuration) } } } diff --git a/internal/usbgadget/usbgadget.go b/internal/usbgadget/usbgadget.go index 3db872cc..17abd9eb 100644 --- a/internal/usbgadget/usbgadget.go +++ b/internal/usbgadget/usbgadget.go @@ -88,6 +88,7 @@ type UsbGadget struct { onKeyboardStateChange *func(state KeyboardState) onKeysDownChange *func(state KeysDownState) + onKeepAliveReset *func() log *zerolog.Logger diff --git a/ui/src/hooks/useKeyboard.ts b/ui/src/hooks/useKeyboard.ts index 0ad463b1..c2ce9ab3 100644 --- a/ui/src/hooks/useKeyboard.ts +++ b/ui/src/hooks/useKeyboard.ts @@ -262,7 +262,7 @@ export default function useKeyboard() { }, [rpcHidReady, cancelOngoingKeyboardMacroHidRpc, abortController]); }; - const KEEPALIVE_INTERVAL = 75; // TODO: use an adaptive interval based on RTT later + const KEEPALIVE_INTERVAL = 50; const cancelKeepAlive = useCallback(() => { if (keepAliveTimerRef.current) { @@ -277,9 +277,6 @@ export default function useKeyboard() { clearInterval(keepAliveTimerRef.current); } - sendKeypressKeepAliveHidRpc(); - - // Create new interval timer keepAliveTimerRef.current = setInterval(() => { sendKeypressKeepAliveHidRpc(); }, KEEPALIVE_INTERVAL); diff --git a/usb.go b/usb.go index f63a6aab..d7ddf605 100644 --- a/usb.go +++ b/usb.go @@ -8,6 +8,10 @@ import ( var gadget *usbgadget.UsbGadget +func getUsbGadget() *usbgadget.UsbGadget { + return gadget +} + // initUsbGadget initializes the USB gadget. // call it only after the config is loaded. func initUsbGadget() { @@ -37,6 +41,12 @@ func initUsbGadget() { } }) + gadget.SetOnKeepAliveReset(func() { + if currentSession != nil { + currentSession.resetKeepAliveTime() + } + }) + // open the keyboard hid file to listen for keyboard events if err := gadget.OpenKeyboardHidFile(); err != nil { usbLogger.Error().Err(err).Msg("failed to open keyboard hid file") diff --git a/webrtc.go b/webrtc.go index 9d5c49d4..27d8e454 100644 --- a/webrtc.go +++ b/webrtc.go @@ -7,6 +7,7 @@ import ( "net" "strings" "sync" + "time" "github.com/coder/websocket" "github.com/coder/websocket/wsjson" @@ -28,13 +29,23 @@ type Session struct { rpcQueue chan webrtc.DataChannelMessage - hidRPCAvailable bool - hidQueueLock sync.Mutex - hidQueue []chan hidQueueMessage + hidRPCAvailable bool + lastKeepAliveArrivalTime time.Time // Track when last keep-alive packet arrived + lastTimerResetTime time.Time // Track when auto-release timer was last reset + keepAliveJitterLock sync.Mutex // Protect jitter compensation timing state + hidQueueLock sync.Mutex + hidQueue []chan hidQueueMessage keysDownStateQueue chan usbgadget.KeysDownState } +func (s *Session) resetKeepAliveTime() { + s.keepAliveJitterLock.Lock() + defer s.keepAliveJitterLock.Unlock() + s.lastKeepAliveArrivalTime = time.Time{} // Reset keep-alive timing tracking + s.lastTimerResetTime = time.Time{} // Reset auto-release timer tracking +} + type hidQueueMessage struct { webrtc.DataChannelMessage channel string @@ -148,6 +159,7 @@ func getOnHidMessageHandler(session *Session, scopedLogger *zerolog.Logger, chan l.Trace().Msg("received data in HID RPC message handler") + // Enqueue to ensure ordered processing queueIndex := hidrpc.GetQueueIndex(hidrpc.MessageType(msg.Data[0])) if queueIndex >= len(session.hidQueue) || queueIndex < 0 {