fix(usb): Add serial presence recovery for USB replug#5923
Merged
jamesarich merged 8 commits intoJun 24, 2026
Conversation
jamesarich
requested changes
Jun 23, 2026
jamesarich
left a comment
Collaborator
There was a problem hiding this comment.
Really nice work — the race analysis is sound and the test matrix covers both callback orderings, the flap/dedup case, and stale-arm-across-reconnect. New tests pass locally (:core:service:jvmTest, :core:network:jvmTest). One cross-platform gap to fix before merge, plus two small inline suggestions.
iOS DI gap (please fix)
SharedRadioInterfaceService is @Single in commonMain and now takes a hard SerialDevicePresence dependency, but the only @Single impls are AndroidSerialDevicePresence (androidMain) and JvmSerialDevicePresence (jvmMain). core:network/core:service both target iOS (compileKotlinIosArm64 / iosArm64MainKlibrary exist), so the iOS Koin graph has no binding for SerialDevicePresence. It compiles (the interface is in commonMain) — which is why CI is green — but Koin resolution of SharedRadioInterfaceService would throw NoDefinitionFoundException at runtime on iOS. Not a shipping bug today (iOS is compile-only), but it's a regression to the common graph's iOS-resolvability and the Migration Notes silently omit iOS.
Laziest correct fix — a 4-line core/network/src/iosMain/kotlin/org/meshtastic/core/network/repository/IosSerialDevicePresence.kt mirroring the Jvm one (expect/actual or a commonMain default would collide with Android's @Single for the same type, so two trivial copies is the path of least resistance):
@Single
class IosSerialDevicePresence : SerialDevicePresence {
override val deviceKeys: StateFlow<Set<String>> = MutableStateFlow<Set<String>>(emptySet()).asStateFlow()
}
jamesarich
requested changes
Jun 23, 2026
jamesarich
left a comment
Collaborator
There was a problem hiding this comment.
some small changes needed, otherwise appreciate this QOL fix for usb serial
SharedRadioInterfaceService.initStateListeners observed bluetooth and network state recovery but not USB. When a USB device was unplugged and replugged, the transport stayed disconnected and the user had to manually re-select. Add a SerialDevicePresence abstraction wired into a USB recovery observer that restarts the SERIAL transport when the selected device transitions back into the presence set. The recovery trigger combines the device address, the serial device-key set, and the transport connection state, firing only when the selected device is present AND the transport is in zombie DeviceSleep. Including connection state in the trigger closes the unplug/replug race where presence arrives before the I/O-death callback has flipped state: the late Connected → DeviceSleep transition re-emits the flow so the zombie transport is recovered. Both stop and start inside the recovery branch run under ignoreExceptionSuspend so a transient USB error cannot escape the onEach and terminate the recovery flow for the rest of the session. The recovery observer is extracted into observeUsbRecoveryTriggers() so initStateListeners stays under detekt's LongMethod cap. SerialRadioTransport.close() now sets an explicitCloseInProgress guard so the reader thread's synchronous onDisconnected callback during port.close() does not emit a transient DeviceSleep on top of the service-layer notification that owns post-close state transitions. AndroidSerialDevicePresence publishes an immutable Set snapshot per emission instead of a Map.keys view, and JvmSerialDevicePresence exposes its perpetually-empty flow as a plain StateFlow. Scoped to the selected device only; distinctUntilChanged coalesces same-value re-emissions so a device that flaps N times produces at most N restarts, never an unbounded loop. disconnect() immediately disarms the observer via the connectionRequested gate.
Require the selected serial device key to be observed absent before a later present emission can trigger zombie transport recovery. This prevents a normal unplug sequence from restarting immediately when the transport disconnect callback reaches DeviceSleep before UsbRepository has removed the still-present key. Keep recovery teardown and fresh start in separate ignoreExceptionSuspend blocks so a close failure cannot skip the restart attempt. Add regression coverage for the inverse callback ordering: selected key already present, onDisconnect(false) first, then detach and reattach. The restart now happens only on the absent-to-present replug edge.
SerialConnection.close() may synchronously invoke SerialConnectionListener.onDisconnected through the reader error path. During a USB unplug, that can reenter onDeviceDisconnect while the outer call is still closing the same connection. Return whether closeConnection actually cleared connRef, and only forward the transient disconnect callback from the invocation that owns that transition. This keeps the service from seeing duplicate DeviceSleep emissions for one unplug.
Prevent the serial replug observer from arming recovery when UsbRepository starts with its initial empty device-key snapshot and then refreshes to a present selected device while the transport is still healthy. Model the real present-to-absent-to-present race in the liveness test and add a regression for initial empty-to-present refreshes.
USB attach intents still refresh UsbRepository so the serial device list stays current, but selected-device replug no longer sends a deep-link PendingIntent back to MainActivity. Only route to the Connections screen in-process when no device is selected, preserving the first-attach flow without recreating the app during automatic USB recovery.
Reset the USB replug recovery reducer whenever the service is in the permanent Disconnected state so armed-by-absence state from a prior session cannot leak into a later reconnect. Add a regression covering detach while connected, explicit disconnect, later reconnect, and a normal unplug callback with the selected key still present.
Address review feedback by adding an iOS no-op SerialDevicePresence binding so SharedRadioInterfaceService remains resolvable across current KMP targets. Normalize selected serial key extraction through selectedSerialPresence and reuse it for the under-lock presence recheck, avoiding empty keys for bare serial addresses. Convert the USB recovery flow rationale into concise KDoc so the race and gating contract remains discoverable.
e3af281 to
61b4b3c
Compare
jamesarich
approved these changes
Jun 23, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Overview
This pull request improves USB serial resilience by adding a platform abstraction for tracking currently connected serial devices and using it to recover the selected SERIAL transport after an unplug/replug cycle.
Previously, unplugging a selected USB serial device could leave the transport in a zombie
ConnectionState.DeviceSleepstate, and replugging the same device did not reliably restart the selected SERIAL transport. USB attach handling could also route through an Activity/deep-link reload path even when a device was already selected, causing unnecessary UI/ViewModel churn.This change fixes the full selected-device replug flow by:
Key Changes
Features
Added
SerialDevicePresence, a common interface exposingdeviceKeys: StateFlow<Set<String>>for currently connected SERIAL device keys.Implemented
SerialDevicePresenceper platform:AndroidSerialDevicePresencemapsUsbRepository.serialDevicesto a hotStateFlowof serial device keys.JvmSerialDevicePresenceemits an always-empty device-key set because desktop serial hot-plug observation is not currently provided.Updated
SharedRadioInterfaceServiceto observe:_connectionState.Added guarded USB replug recovery for the selected SERIAL device:
ConnectionState.DeviceSleep,transportMutexbefore restart.Restart is performed as silent recovery by stopping the zombie transport without permanent-disconnect notification and immediately starting the selected SERIAL transport again.
USB attach behavior
MainActivityUSB attach handling so attach/replug no longer always routes through Connections-page navigation.MainActivityand ViewModel churn during selected-device replug.Fixes
ConnectedorConnecting.service.disconnect().Disconnected, preventing an old detach observation from leaking into a later reconnect session.DeviceSleepbefore USB presence updates does not immediately restart against stale presence data.DeviceSleep; the later state transition can still complete recovery.Shutdown / teardown changes
SerialRadioTransport.explicitCloseInProgressso reader-thread disconnect callbacks do not forward transient disconnects during explicitclose().closeConnection(waitForStopped)teardown helper for SERIAL connections.close()andonDeviceDisconnect(...).StreamTransport.close()so explicit close no longer invokesonDeviceDisconnect(...); explicit disconnect signaling is owned by the service layer.Refactors
SerialRadioTransport.observeUsbRecoveryTriggers()to keep service initialization focused.Testing
Extended
SharedRadioInterfaceServiceLivenessTestwith deterministic USB replug recovery scenarios covering:DeviceSleepordering,Added
StreamTransportTestcoverage confirmingStreamTransport.close()does not emit a disconnect callback.Breaking Changes / Migration Notes
StreamTransport.close()no longer callsonDeviceDisconnect(...)directly. Explicit close/disconnect notification is now owned by the service layer so transport recovery can close and recreate transports without emitting duplicate user-facing disconnect state.SharedRadioInterfaceServicenow requires aSerialDevicePresenceimplementation in the DI graph. Android providesAndroidSerialDevicePresence; JVM/Desktop providesJvmSerialDevicePresence.