From 09cde67e511d9bc7f61254a1dba4f39cf9fa8207 Mon Sep 17 00:00:00 2001 From: Jeremiah K <17190268+jeremiah-k@users.noreply.github.com> Date: Thu, 25 Jun 2026 05:36:17 -0500 Subject: [PATCH] fix(usb): Surface permission denial as permanent disconnect (#5943) --- .../network/radio/SerialRadioTransport.kt | 24 +++++++++++--- .../core/network/radio/StreamTransport.kt | 22 +++++++++---- .../core/network/radio/ReplayFuzzTest.kt | 4 ++- .../network/radio/ReplayRadioTransportTest.kt | 4 ++- .../core/repository/RadioTransportCallback.kt | 8 +++-- .../repository/TransportDisconnectReason.kt | 23 ++++++++++++++ .../service/SharedRadioInterfaceService.kt | 14 +++++++-- ...SharedRadioInterfaceServiceLivenessTest.kt | 31 +++++++++++++++++++ .../core/testing/FakeRadioInterfaceService.kt | 3 +- .../org/meshtastic/desktop/stub/NoopStubs.kt | 3 +- 10 files changed, 116 insertions(+), 20 deletions(-) create mode 100644 core/repository/src/commonMain/kotlin/org/meshtastic/core/repository/TransportDisconnectReason.kt diff --git a/core/network/src/androidMain/kotlin/org/meshtastic/core/network/radio/SerialRadioTransport.kt b/core/network/src/androidMain/kotlin/org/meshtastic/core/network/radio/SerialRadioTransport.kt index 7e4ae81f8..b983165da 100644 --- a/core/network/src/androidMain/kotlin/org/meshtastic/core/network/radio/SerialRadioTransport.kt +++ b/core/network/src/androidMain/kotlin/org/meshtastic/core/network/radio/SerialRadioTransport.kt @@ -25,6 +25,7 @@ import org.meshtastic.core.network.repository.SerialConnectionListener import org.meshtastic.core.network.repository.UsbRepository import org.meshtastic.core.network.transport.HeartbeatSender import org.meshtastic.core.repository.RadioTransportCallback +import org.meshtastic.core.repository.TransportDisconnectReason import java.util.concurrent.atomic.AtomicBoolean import java.util.concurrent.atomic.AtomicReference @@ -64,9 +65,14 @@ class SerialRadioTransport( } } - override fun onDeviceDisconnect(waitForStopped: Boolean, isPermanent: Boolean) { + override fun onDeviceDisconnect( + waitForStopped: Boolean, + isPermanent: Boolean, + errorMessage: String?, + reason: TransportDisconnectReason?, + ) { if (closeConnection(waitForStopped)) { - super.onDeviceDisconnect(waitForStopped, isPermanent) + super.onDeviceDisconnect(waitForStopped, isPermanent, errorMessage, reason) } } @@ -104,6 +110,14 @@ class SerialRadioTransport( Logger.e { "[$address] Serial connection failed - missing USB permissions for device: $device" } + // Permission denial is terminal for this connection attempt: stop the reconnect loop + // and let the service/UI layer choose the user-facing copy for the structured reason. + onDeviceDisconnect( + waitForStopped = false, + isPermanent = true, + errorMessage = null, + reason = TransportDisconnectReason.UsbPermissionDenied, + ) } override fun onConnected() { @@ -129,6 +143,7 @@ class SerialRadioTransport( if (explicitCloseInProgress.get()) { return } + val uptime = if (connectionStartTime > 0) { nowMillis - connectionStartTime @@ -146,8 +161,9 @@ class SerialRadioTransport( "Packets RX: $packetsReceived ($bytesReceived bytes)" } // USB unplug / cable error is transient — the transport will reconnect when - // the device is replugged or the OS re-enumerates the port. Only an explicit - // close() (user disconnects) should signal a permanent disconnect. + // the device is replugged or the OS re-enumerates the port. Only close() + // (user disconnects) and missing-permission (see onMissingPermission) signal + // a permanent disconnect; cable unplug / I/O errors are transient. onDeviceDisconnect(waitForStopped = false, isPermanent = false) } }, diff --git a/core/network/src/commonMain/kotlin/org/meshtastic/core/network/radio/StreamTransport.kt b/core/network/src/commonMain/kotlin/org/meshtastic/core/network/radio/StreamTransport.kt index d972fd060..e8225b630 100644 --- a/core/network/src/commonMain/kotlin/org/meshtastic/core/network/radio/StreamTransport.kt +++ b/core/network/src/commonMain/kotlin/org/meshtastic/core/network/radio/StreamTransport.kt @@ -22,6 +22,7 @@ import org.meshtastic.core.common.util.handledLaunch import org.meshtastic.core.network.transport.StreamFrameCodec import org.meshtastic.core.repository.RadioTransport import org.meshtastic.core.repository.RadioTransportCallback +import org.meshtastic.core.repository.TransportDisconnectReason /** * An interface that assumes we are talking to a meshtastic device over some sort of stream connection (serial or TCP @@ -44,13 +45,22 @@ abstract class StreamTransport(protected val callback: RadioTransportCallback, p * * @param waitForStopped if true we should wait for the transport to finish - must be false if called from inside * transport callbacks - * @param isPermanent true only when the service layer is signaling a user-initiated terminal disconnect. USB - * unplug, I/O errors, and similar conditions are transient — the transport may recover when the device is - * replugged or the OS re-enumerates. Defaults to false so callbacks default to "may come back". The service layer - * owns explicit close notifications so automatic stop/start recovery can close transport resources silently. + * @param isPermanent true when the user explicitly disconnects (e.g. [close] was called), or when an authorization + * failure makes the current connection attempt unrecoverable. USB unplug, I/O errors, and similar conditions are + * transient — the transport may recover when the device is replugged or the OS re-enumerates. Defaults to false + * so callbacks default to "may come back". + * @param errorMessage optional user-facing reason for the disconnect; surfaced via + * [RadioTransportCallback.onDisconnect]. Prefer [reason] for newly-classified disconnect causes so transports do + * not own UI copy. + * @param reason optional structured cause for service/UI-specific handling. */ - protected open fun onDeviceDisconnect(waitForStopped: Boolean, isPermanent: Boolean = false) { - callback.onDisconnect(isPermanent = isPermanent) + protected open fun onDeviceDisconnect( + waitForStopped: Boolean, + isPermanent: Boolean = false, + errorMessage: String? = null, + reason: TransportDisconnectReason? = null, + ) { + callback.onDisconnect(isPermanent = isPermanent, errorMessage = errorMessage, reason = reason) } protected open fun connect() { diff --git a/core/network/src/commonTest/kotlin/org/meshtastic/core/network/radio/ReplayFuzzTest.kt b/core/network/src/commonTest/kotlin/org/meshtastic/core/network/radio/ReplayFuzzTest.kt index 6fff14edb..87d4ab5b3 100644 --- a/core/network/src/commonTest/kotlin/org/meshtastic/core/network/radio/ReplayFuzzTest.kt +++ b/core/network/src/commonTest/kotlin/org/meshtastic/core/network/radio/ReplayFuzzTest.kt @@ -19,6 +19,7 @@ package org.meshtastic.core.network.radio import kotlinx.coroutines.test.runTest import org.meshtastic.core.repository.HandshakeConstants import org.meshtastic.core.repository.RadioTransportCallback +import org.meshtastic.core.repository.TransportDisconnectReason import org.meshtastic.proto.FromRadio import org.meshtastic.proto.MeshPacket import org.meshtastic.proto.MyNodeInfo @@ -47,7 +48,8 @@ class ReplayFuzzTest { override fun onConnect() = Unit - override fun onDisconnect(isPermanent: Boolean, errorMessage: String?) = Unit + override fun onDisconnect(isPermanent: Boolean, errorMessage: String?, reason: TransportDisconnectReason?) = + Unit override fun handleFromRadio(bytes: ByteArray) { frames += bytes diff --git a/core/network/src/commonTest/kotlin/org/meshtastic/core/network/radio/ReplayRadioTransportTest.kt b/core/network/src/commonTest/kotlin/org/meshtastic/core/network/radio/ReplayRadioTransportTest.kt index 8094dffde..7871c81c8 100644 --- a/core/network/src/commonTest/kotlin/org/meshtastic/core/network/radio/ReplayRadioTransportTest.kt +++ b/core/network/src/commonTest/kotlin/org/meshtastic/core/network/radio/ReplayRadioTransportTest.kt @@ -20,6 +20,7 @@ import kotlinx.coroutines.test.runTest import okio.Buffer import org.meshtastic.core.repository.HandshakeConstants import org.meshtastic.core.repository.RadioTransportCallback +import org.meshtastic.core.repository.TransportDisconnectReason import org.meshtastic.proto.FromRadio import org.meshtastic.proto.MeshPacket import org.meshtastic.proto.MyNodeInfo @@ -45,7 +46,8 @@ class ReplayRadioTransportTest { connects++ } - override fun onDisconnect(isPermanent: Boolean, errorMessage: String?) = Unit + override fun onDisconnect(isPermanent: Boolean, errorMessage: String?, reason: TransportDisconnectReason?) = + Unit override fun handleFromRadio(bytes: ByteArray) { received.add(FromRadio.ADAPTER.decode(bytes)) diff --git a/core/repository/src/commonMain/kotlin/org/meshtastic/core/repository/RadioTransportCallback.kt b/core/repository/src/commonMain/kotlin/org/meshtastic/core/repository/RadioTransportCallback.kt index 9771062a5..6583e134c 100644 --- a/core/repository/src/commonMain/kotlin/org/meshtastic/core/repository/RadioTransportCallback.kt +++ b/core/repository/src/commonMain/kotlin/org/meshtastic/core/repository/RadioTransportCallback.kt @@ -30,11 +30,13 @@ interface RadioTransportCallback { /** * Called when the transport has disconnected. * - * @param isPermanent true if the device is definitely gone (e.g. USB unplugged, max retries exhausted), false if it - * may come back (e.g. BLE range, TCP transient). + * @param isPermanent true when the current connection attempt should stop retrying (e.g. user disconnect, + * permission denied, max retries exhausted), false when it may come back (e.g. BLE range, TCP transient). * @param errorMessage optional user-facing error message describing the disconnect reason. + * @param reason optional structured reason when the transport can classify the disconnect without owning + * user-facing text. */ - fun onDisconnect(isPermanent: Boolean, errorMessage: String? = null) + fun onDisconnect(isPermanent: Boolean, errorMessage: String? = null, reason: TransportDisconnectReason? = null) /** Called when the transport has received raw data from the radio. */ fun handleFromRadio(bytes: ByteArray) diff --git a/core/repository/src/commonMain/kotlin/org/meshtastic/core/repository/TransportDisconnectReason.kt b/core/repository/src/commonMain/kotlin/org/meshtastic/core/repository/TransportDisconnectReason.kt new file mode 100644 index 000000000..09a9a28c9 --- /dev/null +++ b/core/repository/src/commonMain/kotlin/org/meshtastic/core/repository/TransportDisconnectReason.kt @@ -0,0 +1,23 @@ +/* + * Copyright (c) 2026 Meshtastic LLC + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ +package org.meshtastic.core.repository + +/** Machine-readable transport disconnect causes that need service/UI-specific handling. */ +enum class TransportDisconnectReason { + /** Android denied USB device access for the current connection attempt. */ + UsbPermissionDenied, +} diff --git a/core/service/src/commonMain/kotlin/org/meshtastic/core/service/SharedRadioInterfaceService.kt b/core/service/src/commonMain/kotlin/org/meshtastic/core/service/SharedRadioInterfaceService.kt index 89cad3800..0550bf380 100644 --- a/core/service/src/commonMain/kotlin/org/meshtastic/core/service/SharedRadioInterfaceService.kt +++ b/core/service/src/commonMain/kotlin/org/meshtastic/core/service/SharedRadioInterfaceService.kt @@ -64,9 +64,12 @@ import org.meshtastic.core.repository.RadioInterfaceService import org.meshtastic.core.repository.RadioPrefs import org.meshtastic.core.repository.RadioTransport import org.meshtastic.core.repository.RadioTransportFactory +import org.meshtastic.core.repository.TransportDisconnectReason import org.meshtastic.proto.ToRadio import kotlin.concurrent.Volatile +private const val USB_PERMISSION_DENIED_ERROR = "USB permission denied. Reconnect the device to try again." + private data class SelectedSerialPresence(val key: String?, val present: Boolean) private data class UsbRecoverySnapshot(val presence: SelectedSerialPresence, val state: ConnectionState) @@ -116,6 +119,10 @@ private fun UsbRecoveryTriggerState.next(snapshot: UsbRecoverySnapshot): UsbReco } } +private fun TransportDisconnectReason.toConnectionErrorMessage(): String = when (this) { + TransportDisconnectReason.UsbPermissionDenied -> USB_PERMISSION_DENIED_ERROR +} + /** * Shared multiplatform connection orchestrator for Meshtastic radios. * @@ -725,9 +732,10 @@ class SharedRadioInterfaceService( } } - override fun onDisconnect(isPermanent: Boolean, errorMessage: String?) { - if (errorMessage != null) { - processLifecycle.coroutineScope.launch(dispatchers.default) { _connectionError.emit(errorMessage) } + override fun onDisconnect(isPermanent: Boolean, errorMessage: String?, reason: TransportDisconnectReason?) { + val resolvedErrorMessage = errorMessage ?: reason?.toConnectionErrorMessage() + if (resolvedErrorMessage != null) { + processLifecycle.coroutineScope.launch(dispatchers.default) { _connectionError.emit(resolvedErrorMessage) } } val newTargetState = if (isPermanent) ConnectionState.Disconnected else ConnectionState.DeviceSleep if (_connectionState.value != newTargetState) { diff --git a/core/service/src/commonTest/kotlin/org/meshtastic/core/service/SharedRadioInterfaceServiceLivenessTest.kt b/core/service/src/commonTest/kotlin/org/meshtastic/core/service/SharedRadioInterfaceServiceLivenessTest.kt index cb82a1f3e..2027f8132 100644 --- a/core/service/src/commonTest/kotlin/org/meshtastic/core/service/SharedRadioInterfaceServiceLivenessTest.kt +++ b/core/service/src/commonTest/kotlin/org/meshtastic/core/service/SharedRadioInterfaceServiceLivenessTest.kt @@ -47,6 +47,7 @@ import org.meshtastic.core.network.repository.SerialDevicePresence import org.meshtastic.core.repository.PlatformAnalytics import org.meshtastic.core.repository.RadioTransport import org.meshtastic.core.repository.RadioTransportFactory +import org.meshtastic.core.repository.TransportDisconnectReason import org.meshtastic.core.testing.FakeBluetoothRepository import org.meshtastic.core.testing.FakeRadioPrefs import org.meshtastic.core.testing.FakeRadioTransport @@ -552,6 +553,36 @@ class SharedRadioInterfaceServiceLivenessTest { } } + @Test + fun `USB permission denial emits error and permanent disconnected state`() = runTest(testDispatcher) { + clock = 0L + val service = createConnectedService("s/dev/bus/usb/001/002") + try { + val errors = mutableListOf() + val collectJob = backgroundScope.launch { service.connectionError.collect { errors.add(it) } } + + service.onDisconnect( + isPermanent = true, + errorMessage = null, + reason = TransportDisconnectReason.UsbPermissionDenied, + ) + testDispatcher.scheduler.runCurrent() + advanceTimeBy(1_000L) + + collectJob.cancel() + + assertEquals(ConnectionState.Disconnected, service.connectionState.value) + assertEquals( + listOf("USB permission denied. Reconnect the device to try again."), + errors, + "USB permission denial must surface a specific error message.", + ) + } finally { + service.disconnect() + advanceTimeBy(1_000L) + } + } + @Test fun `BLE liveness does not fire when connection state is not Connected`() = runTest(testDispatcher) { clock = 0L diff --git a/core/testing/src/commonMain/kotlin/org/meshtastic/core/testing/FakeRadioInterfaceService.kt b/core/testing/src/commonMain/kotlin/org/meshtastic/core/testing/FakeRadioInterfaceService.kt index d707ffb9c..21cf6c63f 100644 --- a/core/testing/src/commonMain/kotlin/org/meshtastic/core/testing/FakeRadioInterfaceService.kt +++ b/core/testing/src/commonMain/kotlin/org/meshtastic/core/testing/FakeRadioInterfaceService.kt @@ -30,6 +30,7 @@ import org.meshtastic.core.model.DeviceType import org.meshtastic.core.model.InterfaceId import org.meshtastic.core.model.MeshActivity import org.meshtastic.core.repository.RadioInterfaceService +import org.meshtastic.core.repository.TransportDisconnectReason /** * A test double for [RadioInterfaceService] that provides an in-memory implementation. @@ -98,7 +99,7 @@ class FakeRadioInterfaceService(override val serviceScope: CoroutineScope = Main _connectionState.value = ConnectionState.Connected } - override fun onDisconnect(isPermanent: Boolean, errorMessage: String?) { + override fun onDisconnect(isPermanent: Boolean, errorMessage: String?, reason: TransportDisconnectReason?) { _connectionState.value = ConnectionState.Disconnected } diff --git a/desktopApp/src/main/kotlin/org/meshtastic/desktop/stub/NoopStubs.kt b/desktopApp/src/main/kotlin/org/meshtastic/desktop/stub/NoopStubs.kt index 0b576d77e..898c6e292 100644 --- a/desktopApp/src/main/kotlin/org/meshtastic/desktop/stub/NoopStubs.kt +++ b/desktopApp/src/main/kotlin/org/meshtastic/desktop/stub/NoopStubs.kt @@ -40,6 +40,7 @@ import org.meshtastic.core.repository.MeshLocationManager import org.meshtastic.core.repository.MeshWorkerManager import org.meshtastic.core.repository.PlatformAnalytics import org.meshtastic.core.repository.RadioInterfaceService +import org.meshtastic.core.repository.TransportDisconnectReason import org.meshtastic.proto.MqttClientProxyMessage import org.meshtastic.mqtt.ConnectionState as MqttConnectionState import org.meshtastic.proto.Position as ProtoPosition @@ -102,7 +103,7 @@ class NoopRadioInterfaceService : RadioInterfaceService { override fun onConnect() {} - override fun onDisconnect(isPermanent: Boolean, errorMessage: String?) {} + override fun onDisconnect(isPermanent: Boolean, errorMessage: String?, reason: TransportDisconnectReason?) {} override fun handleFromRadio(bytes: ByteArray) {}