fix(usb): Surface permission denial as permanent disconnect (#5943)

2026-06-25 22:15:33 -04:00 · 2026-06-25 05:36:17 -05:00
parent ab07347e5a
commit 09cde67e51
10 changed files with 116 additions and 20 deletions
--- 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)
                        }
                    },
--- 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() {
--- 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
--- 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))
--- 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)
--- a/core/repository/src/commonMain/kotlin/org/meshtastic/core/repository/TransportDisconnectReason.kt
+++ 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 <https://www.gnu.org/licenses/>.
+ */
+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,
+}
--- 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) {
--- 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<String>()
+            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
--- 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
    }

--- 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) {}