diff --git a/core/repository/src/commonMain/kotlin/org/meshtastic/core/repository/RadioInterfaceService.kt b/core/repository/src/commonMain/kotlin/org/meshtastic/core/repository/RadioInterfaceService.kt index cbaf8b3dc..5ef6e1515 100644 --- a/core/repository/src/commonMain/kotlin/org/meshtastic/core/repository/RadioInterfaceService.kt +++ b/core/repository/src/commonMain/kotlin/org/meshtastic/core/repository/RadioInterfaceService.kt @@ -95,6 +95,13 @@ interface RadioInterfaceService : RadioTransportCallback { /** Initiates the connection to the radio. */ fun connect() + /** + * Explicitly tears down the active transport, sending a polite `ToRadio(disconnect = true)` goodbye frame first + * when a transport is live. Safe to call when nothing is connected — implementations must no-op in that case. + * Suspends until the teardown completes. + */ + suspend fun disconnect() + /** Returns the current device address. */ fun getDeviceAddress(): String? diff --git a/core/service/src/commonMain/kotlin/org/meshtastic/core/service/MeshServiceOrchestrator.kt b/core/service/src/commonMain/kotlin/org/meshtastic/core/service/MeshServiceOrchestrator.kt index ebac9f71b..66622c727 100644 --- a/core/service/src/commonMain/kotlin/org/meshtastic/core/service/MeshServiceOrchestrator.kt +++ b/core/service/src/commonMain/kotlin/org/meshtastic/core/service/MeshServiceOrchestrator.kt @@ -24,6 +24,7 @@ import kotlinx.coroutines.cancel import kotlinx.coroutines.flow.launchIn import kotlinx.coroutines.flow.onEach import kotlinx.coroutines.isActive +import kotlinx.coroutines.launch import org.koin.core.annotation.Single import org.meshtastic.core.common.database.DatabaseManager import org.meshtastic.core.common.util.handledLaunch @@ -149,6 +150,14 @@ class MeshServiceOrchestrator( if (takServerManager.isRunning.value) { takMeshIntegration.stop() } + // Best-effort polite goodbye on service teardown (onDestroy / process shutdown). We launch + // on a fresh detached scope — not the orchestrator's per-start scope — so the subsequent + // scope.cancel() below doesn't interrupt the short drain delay inside disconnect(). The + // coroutine is fire-and-forget; typical runtime is ~100-150ms which comfortably fits + // inside Android's onDestroy() grace window. + CoroutineScope(SupervisorJob() + dispatchers.default).launch { + runCatching { radioInterfaceService.disconnect() } + } scope?.cancel() scope = null } 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 1bb63971c..67db67d07 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 @@ -59,6 +59,7 @@ 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.proto.ToRadio import kotlin.concurrent.Volatile /** @@ -121,6 +122,13 @@ class SharedRadioInterfaceService( private var runningTransportId: InterfaceId? = null private var isStarted = false + /** + * Set while [stopTransportLocked] is draining the polite disconnect frame. [sendToRadio] checks this so any late + * traffic submitted after we've announced disconnection is dropped rather than racing in front of the firmware-side + * link teardown. + */ + @Volatile private var isStopping = false + private val listenersInitialized = atomic(false) private var heartbeatJob: Job? = null private var lastHeartbeatMillis = 0L @@ -135,6 +143,13 @@ class SharedRadioInterfaceService( // zombie (BLE stack didn't report disconnect). Two missed heartbeat intervals gives // the firmware a reasonable window to respond or send telemetry. private const val LIVENESS_TIMEOUT_MILLIS = HEARTBEAT_INTERVAL_MILLIS * 2 + + /** + * Upper bound on how long we wait for the polite `ToRadio(disconnect = true)` frame to flush before tearing the + * transport down. 500ms gives BLE's write-retry path (`BleRetry` backs off 500ms) room for one attempt on a + * flaky GATT connection. Serial and TCP typically flush well under this window. + */ + private const val POLITE_DISCONNECT_DRAIN_MS = 500L } private val initLock = Mutex() @@ -193,6 +208,10 @@ class SharedRadioInterfaceService( initStateListeners() } + override suspend fun disconnect() { + transportMutex.withLock { ignoreExceptionSuspend { stopTransportLocked() } } + } + override fun isMockTransport(): Boolean = transportFactory.isMockTransport() override fun toInterfaceAddress(interfaceId: InterfaceId, rest: String): String = @@ -257,9 +276,26 @@ class SharedRadioInterfaceService( private suspend fun stopTransportLocked() { val currentTransport = radioTransport Logger.i { "Stopping transport $currentTransport" } + // Best-effort polite goodbye: tell the firmware we're disconnecting on purpose so it can + // tear down its side of the link cleanly instead of relying on timeouts / hardware events. + // Flip isStopping before sending so any concurrent sendToRadio() drops incoming traffic — + // we don't want normal packets racing behind the disconnect frame. Skip only when already + // Disconnected; firmware can still consume the goodbye while handshaking or sleeping, so + // it's worth sending in every other state. The send is fire-and-forget through the + // transport's own scope; the drain delay gives async transports a window to flush before + // close() cancels their write scope. BLE's retry path backs off 500ms, so this window + // also covers one retry on flaky GATT links. + if (currentTransport != null && _connectionState.value != ConnectionState.Disconnected) { + isStopping = true + ignoreExceptionSuspend { + currentTransport.handleSendToRadio(ToRadio(disconnect = true).encode()) + delay(POLITE_DISCONNECT_DRAIN_MS) + } + } isStarted = false radioTransport = null runningTransportId = null + isStopping = false currentTransport?.close() _serviceScope.cancel("stopping transport") @@ -310,6 +346,10 @@ class SharedRadioInterfaceService( } override fun sendToRadio(bytes: ByteArray) { + if (isStopping) { + Logger.d { "sendToRadio: transport stopping, dropping ${bytes.size} bytes" } + return + } // Snapshot the transport to avoid calling handleSendToRadio on a null reference. // There is still a benign race: stopTransportLocked() may cancel _serviceScope // between the null-check and the launch, causing the coroutine to be silently 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 d3f8dc71e..cd31a2e97 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 @@ -75,6 +75,10 @@ class FakeRadioInterfaceService(override val serviceScope: CoroutineScope = Main connectCalled = true } + override suspend fun disconnect() { + connectCalled = false + } + override fun getDeviceAddress(): String? = _currentDeviceAddressFlow.value override fun setDeviceAddress(deviceAddr: String?): Boolean { diff --git a/desktop/src/main/kotlin/org/meshtastic/desktop/stub/NoopStubs.kt b/desktop/src/main/kotlin/org/meshtastic/desktop/stub/NoopStubs.kt index 707dfaf03..36cd28507 100644 --- a/desktop/src/main/kotlin/org/meshtastic/desktop/stub/NoopStubs.kt +++ b/desktop/src/main/kotlin/org/meshtastic/desktop/stub/NoopStubs.kt @@ -89,6 +89,10 @@ class NoopRadioInterfaceService : RadioInterfaceService { logWarn("NoopRadioInterfaceService.connect()") } + override suspend fun disconnect() { + logWarn("NoopRadioInterfaceService.disconnect()") + } + override fun getDeviceAddress(): String? = null override fun setDeviceAddress(deviceAddr: String?): Boolean = false