From 68b2b6d88ea2db38b95a1816cfa9aa6b848b93e0 Mon Sep 17 00:00:00 2001 From: James Rich <2199651+jamesarich@users.noreply.github.com> Date: Thu, 5 Mar 2026 12:58:34 -0600 Subject: [PATCH] refactor(ble): improve connection lifecycle and enhance OTA reliability (#4721) Signed-off-by: James Rich <2199651+jamesarich@users.noreply.github.com> --- .../radio/AndroidRadioInterfaceService.kt | 19 +- .../radio/MeshtasticRadioProfile.kt | 33 ++ .../radio/MeshtasticRadioServiceImpl.kt | 94 +++++ .../repository/radio/NordicBleInterface.kt | 327 ++++++------------ .../radio/NordicBleInterfaceRetryTest.kt | 5 +- .../radio/NordicBleInterfaceTest.kt | 8 +- core/ble/README.md | 33 +- .../org/meshtastic/core/ble/BleConnection.kt | 132 ++++--- .../org/meshtastic/core/ble/BleError.kt | 135 -------- .../org/meshtastic/core/ble/BleModule.kt | 5 +- .../org/meshtastic/core/ble/BleRetry.kt | 3 +- .../core/ble/BluetoothRepository.kt | 28 +- .../core/ble/MeshtasticBleConstants.kt | 11 + .../core/ble/BluetoothRepositoryTest.kt | 33 ++ .../core/repository/RadioInterfaceService.kt | 5 +- feature/firmware/README.md | 3 +- .../feature/firmware/ota/BleOtaTransport.kt | 193 +++++++---- .../BleOtaTransportServiceDiscoveryTest.kt | 209 +++++++++++ gradle/libs.versions.toml | 2 +- 19 files changed, 741 insertions(+), 537 deletions(-) create mode 100644 app/src/main/java/com/geeksville/mesh/repository/radio/MeshtasticRadioProfile.kt create mode 100644 app/src/main/java/com/geeksville/mesh/repository/radio/MeshtasticRadioServiceImpl.kt delete mode 100644 core/ble/src/main/kotlin/org/meshtastic/core/ble/BleError.kt create mode 100644 feature/firmware/src/test/kotlin/org/meshtastic/feature/firmware/ota/BleOtaTransportServiceDiscoveryTest.kt diff --git a/app/src/main/java/com/geeksville/mesh/repository/radio/AndroidRadioInterfaceService.kt b/app/src/main/java/com/geeksville/mesh/repository/radio/AndroidRadioInterfaceService.kt index cd190ad45..47230a08a 100644 --- a/app/src/main/java/com/geeksville/mesh/repository/radio/AndroidRadioInterfaceService.kt +++ b/app/src/main/java/com/geeksville/mesh/repository/radio/AndroidRadioInterfaceService.kt @@ -38,7 +38,6 @@ import kotlinx.coroutines.flow.launchIn import kotlinx.coroutines.flow.onEach import kotlinx.coroutines.launch import org.meshtastic.core.analytics.platform.PlatformAnalytics -import org.meshtastic.core.ble.BleError import org.meshtastic.core.ble.BluetoothRepository import org.meshtastic.core.common.util.BinaryLogFile import org.meshtastic.core.common.util.BuildUtils @@ -89,8 +88,8 @@ constructor( private val _receivedData = MutableSharedFlow(extraBufferCapacity = 64) override val receivedData: SharedFlow = _receivedData - private val _connectionError = MutableSharedFlow(extraBufferCapacity = 64) - val connectionError: SharedFlow = _connectionError.asSharedFlow() + private val _connectionError = MutableSharedFlow(extraBufferCapacity = 64) + val connectionError: SharedFlow = _connectionError.asSharedFlow() // Thread-safe StateFlow for tracking device address changes private val _currentDeviceAddressFlow = MutableStateFlow(radioPrefs.devAddr) @@ -259,22 +258,16 @@ constructor( } } - override fun onDisconnect(isPermanent: Boolean) { + override fun onDisconnect(isPermanent: Boolean, errorMessage: String?) { + if (errorMessage != null) { + processLifecycle.coroutineScope.launch(dispatchers.default) { _connectionError.emit(errorMessage) } + } val newTargetState = if (isPermanent) ConnectionState.Disconnected else ConnectionState.DeviceSleep if (_connectionState.value != newTargetState) { broadcastConnectionChanged(newTargetState) } } - override fun onDisconnect(error: Any) { - if (error is BleError) { - processLifecycle.coroutineScope.launch(dispatchers.default) { _connectionError.emit(error) } - onDisconnect(!error.shouldReconnect) - } else { - onDisconnect(isPermanent = true) - } - } - /** Start our configured interface (if it isn't already running) */ private fun startInterface() { if (radioIf !is NopInterface) { diff --git a/app/src/main/java/com/geeksville/mesh/repository/radio/MeshtasticRadioProfile.kt b/app/src/main/java/com/geeksville/mesh/repository/radio/MeshtasticRadioProfile.kt new file mode 100644 index 000000000..512b04fdd --- /dev/null +++ b/app/src/main/java/com/geeksville/mesh/repository/radio/MeshtasticRadioProfile.kt @@ -0,0 +1,33 @@ +/* + * Copyright (c) 2025-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 com.geeksville.mesh.repository.radio + +import kotlinx.coroutines.flow.Flow + +/** A definition of the Meshtastic BLE Service profile. */ +interface MeshtasticRadioProfile { + interface State { + /** The flow of incoming packets from the radio. */ + val fromRadio: Flow + + /** The flow of incoming log packets from the radio. */ + val logRadio: Flow + + /** Sends a packet to the radio. */ + suspend fun sendToRadio(packet: ByteArray) + } +} diff --git a/app/src/main/java/com/geeksville/mesh/repository/radio/MeshtasticRadioServiceImpl.kt b/app/src/main/java/com/geeksville/mesh/repository/radio/MeshtasticRadioServiceImpl.kt new file mode 100644 index 000000000..266df6651 --- /dev/null +++ b/app/src/main/java/com/geeksville/mesh/repository/radio/MeshtasticRadioServiceImpl.kt @@ -0,0 +1,94 @@ +/* + * Copyright (c) 2025-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 com.geeksville.mesh.repository.radio + +import kotlinx.coroutines.flow.Flow +import kotlinx.coroutines.flow.MutableSharedFlow +import kotlinx.coroutines.flow.channelFlow +import kotlinx.coroutines.launch +import no.nordicsemi.kotlin.ble.client.RemoteCharacteristic +import no.nordicsemi.kotlin.ble.client.RemoteService +import no.nordicsemi.kotlin.ble.core.WriteType +import org.meshtastic.core.ble.MeshtasticBleConstants.FROMNUM_CHARACTERISTIC +import org.meshtastic.core.ble.MeshtasticBleConstants.FROMRADIOSYNC_CHARACTERISTIC +import org.meshtastic.core.ble.MeshtasticBleConstants.FROMRADIO_CHARACTERISTIC +import org.meshtastic.core.ble.MeshtasticBleConstants.LOGRADIO_CHARACTERISTIC +import org.meshtastic.core.ble.MeshtasticBleConstants.TORADIO_CHARACTERISTIC + +class MeshtasticRadioServiceImpl(private val remoteService: RemoteService) : MeshtasticRadioProfile.State { + + private val toRadioCharacteristic: RemoteCharacteristic = + remoteService.characteristics.first { it.uuid == TORADIO_CHARACTERISTIC } + private val fromRadioCharacteristic: RemoteCharacteristic = + remoteService.characteristics.first { it.uuid == FROMRADIO_CHARACTERISTIC } + private val fromRadioSyncCharacteristic: RemoteCharacteristic? = + remoteService.characteristics.firstOrNull { it.uuid == FROMRADIOSYNC_CHARACTERISTIC } + private val fromNumCharacteristic: RemoteCharacteristic? = + if (fromRadioSyncCharacteristic == null) { + remoteService.characteristics.first { it.uuid == FROMNUM_CHARACTERISTIC } + } else { + null + } + private val logRadioCharacteristic: RemoteCharacteristic = + remoteService.characteristics.first { it.uuid == LOGRADIO_CHARACTERISTIC } + + private val triggerDrain = MutableSharedFlow(extraBufferCapacity = 64) + + init { + require(toRadioCharacteristic.isWritable()) { "TORADIO must be writable" } + require(fromRadioCharacteristic.isReadable()) { "FROMRADIO must be readable" } + fromRadioSyncCharacteristic?.let { require(it.isSubscribable()) { "FROMRADIOSYNC must be subscribable" } } + fromNumCharacteristic?.let { require(it.isSubscribable()) { "FROMNUM must be subscribable" } } + require(logRadioCharacteristic.isSubscribable()) { "LOGRADIO must be subscribable" } + } + + override val fromRadio: Flow = + if (fromRadioSyncCharacteristic != null) { + fromRadioSyncCharacteristic.subscribe() + } else { + // Legacy path: drain fromRadio characteristic when notified or after write + channelFlow { + launch { fromNumCharacteristic!!.subscribe().collect { triggerDrain.tryEmit(Unit) } } + + triggerDrain.collect { + var keepReading = true + while (keepReading) { + try { + val packet = fromRadioCharacteristic.read() + if (packet.isEmpty()) { + keepReading = false + } else { + send(packet) + } + } catch (@Suppress("TooGenericExceptionCaught") e: Exception) { + co.touchlab.kermit.Logger.e(e) { "BLE: Failed to read from FROMRADIO" } + keepReading = false + } + } + } + } + } + + override val logRadio: Flow = logRadioCharacteristic.subscribe() + + override suspend fun sendToRadio(packet: ByteArray) { + toRadioCharacteristic.write(packet, WriteType.WITHOUT_RESPONSE) + if (fromRadioSyncCharacteristic == null) { + triggerDrain.tryEmit(Unit) + } + } +} diff --git a/app/src/main/java/com/geeksville/mesh/repository/radio/NordicBleInterface.kt b/app/src/main/java/com/geeksville/mesh/repository/radio/NordicBleInterface.kt index aa72dfdd4..7e06206ba 100644 --- a/app/src/main/java/com/geeksville/mesh/repository/radio/NordicBleInterface.kt +++ b/app/src/main/java/com/geeksville/mesh/repository/radio/NordicBleInterface.kt @@ -20,41 +20,26 @@ import android.annotation.SuppressLint import co.touchlab.kermit.Logger import dagger.assisted.Assisted import dagger.assisted.AssistedInject -import kotlinx.coroutines.CompletableDeferred import kotlinx.coroutines.CoroutineExceptionHandler import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.SupervisorJob import kotlinx.coroutines.cancel import kotlinx.coroutines.delay -import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.catch -import kotlinx.coroutines.flow.channelFlow -import kotlinx.coroutines.flow.collect +import kotlinx.coroutines.flow.first import kotlinx.coroutines.flow.firstOrNull import kotlinx.coroutines.flow.launchIn import kotlinx.coroutines.flow.onEach -import kotlinx.coroutines.isActive import kotlinx.coroutines.launch -import kotlinx.coroutines.runBlocking import kotlinx.coroutines.sync.Mutex import kotlinx.coroutines.sync.withLock -import kotlinx.coroutines.withTimeout -import no.nordicsemi.kotlin.ble.client.RemoteCharacteristic import no.nordicsemi.kotlin.ble.client.android.CentralManager import no.nordicsemi.kotlin.ble.client.android.Peripheral -import no.nordicsemi.kotlin.ble.client.exception.InvalidAttributeException -import no.nordicsemi.kotlin.ble.core.CharacteristicProperty import no.nordicsemi.kotlin.ble.core.ConnectionState import no.nordicsemi.kotlin.ble.core.WriteType import org.meshtastic.core.ble.BleConnection -import org.meshtastic.core.ble.BleError import org.meshtastic.core.ble.BleScanner -import org.meshtastic.core.ble.MeshtasticBleConstants.FROMNUM_CHARACTERISTIC -import org.meshtastic.core.ble.MeshtasticBleConstants.FROMRADIOSYNC_CHARACTERISTIC -import org.meshtastic.core.ble.MeshtasticBleConstants.FROMRADIO_CHARACTERISTIC -import org.meshtastic.core.ble.MeshtasticBleConstants.LOGRADIO_CHARACTERISTIC import org.meshtastic.core.ble.MeshtasticBleConstants.SERVICE_UUID -import org.meshtastic.core.ble.MeshtasticBleConstants.TORADIO_CHARACTERISTIC import org.meshtastic.core.ble.retryBleOperation import org.meshtastic.core.common.util.nowMillis import org.meshtastic.core.model.RadioNotConnectedException @@ -70,7 +55,11 @@ private val SCAN_TIMEOUT = 5.seconds * A [IRadioInterface] implementation for BLE devices using Nordic Kotlin BLE Library. * https://github.com/NordicSemiconductor/Kotlin-BLE-Library. * - * This class is responsible for connecting to and communicating with a Meshtastic device over BLE. + * This class handles the high-level connection lifecycle for Meshtastic radios over BLE, including: + * - Bonding and discovery. + * - Automatic reconnection logic. + * - MTU and connection parameter monitoring. + * - Routing raw byte packets between the radio and [RadioInterfaceService]. * * @param serviceScope The coroutine scope to use for launching coroutines. * @param centralManager The central manager provided by Nordic BLE Library. @@ -96,13 +85,13 @@ constructor( Logger.w(e) { "[$address] Failed to disconnect in exception handler" } } } - service.onDisconnect(error = BleError.from(throwable)) + val (isPermanent, msg) = throwable.toDisconnectReason() + service.onDisconnect(isPermanent, errorMessage = msg) } private val connectionScope: CoroutineScope = CoroutineScope(serviceScope.coroutineContext + SupervisorJob() + exceptionHandler) private val bleConnection: BleConnection = BleConnection(centralManager, connectionScope, address) - private val drainMutex: Mutex = Mutex() private val writeMutex: Mutex = Mutex() private var connectionStartTime: Long = 0 @@ -111,66 +100,10 @@ constructor( private var bytesReceived: Long = 0 private var bytesSent: Long = 0 - private var toRadioCharacteristic: RemoteCharacteristic? = null - private var fromNumCharacteristic: RemoteCharacteristic? = null - private var fromRadioCharacteristic: RemoteCharacteristic? = null - private var logRadioCharacteristic: RemoteCharacteristic? = null - private var fromRadioSyncCharacteristic: RemoteCharacteristic? = null - init { connect() } - // --- Packet Flow Management --- - - private fun fromRadioPacketFlow(): Flow = channelFlow { - while (isActive) { - val packet = - try { - fromRadioCharacteristic?.read()?.takeIf { it.isNotEmpty() } - } catch (e: InvalidAttributeException) { - Logger.w(e) { "[$address] Attribute invalidated during read, clearing characteristics" } - handleInvalidAttribute(e) - null - } catch (e: Exception) { - Logger.w(e) { "[$address] Error reading fromRadioCharacteristic (likely disconnected)" } - null - } - - if (packet == null) { - Logger.d { "[$address] fromRadio queue drain complete or error reading characteristic" } - break - } - send(packet) - } - } - - private fun dispatchPacket(packet: ByteArray) { - packetsReceived++ - bytesReceived += packet.size - Logger.d { - "[$address] Dispatching packet to service.handleFromRadio() - " + - "Packet #$packetsReceived, ${packet.size} bytes (Total: $bytesReceived bytes)" - } - try { - service.handleFromRadio(packet) - } catch (t: Throwable) { - Logger.e(t) { "[$address] Failed to execute service.handleFromRadio()" } - } - } - - private suspend fun drainPacketQueueAndDispatch() { - drainMutex.withLock { - fromRadioPacketFlow() - .onEach { packet -> - Logger.d { "[$address] Read packet from queue (${packet.size} bytes)" } - dispatchPacket(packet) - } - .catch { ex -> Logger.w(ex) { "[$address] Exception while draining packet queue" } } - .collect() - } - } - // --- Connection & Discovery Logic --- /** Robustly finds the peripheral. First checks bonded devices, then performs a short scan if not found. */ @@ -211,11 +144,11 @@ constructor( } .catch { e -> Logger.w(e) { "[$address] bleConnection.connectionState flow crashed!" } - service.onDisconnect(BleError.from(e)) + handleFailure(e) } .launchIn(connectionScope) - val p = retryBleOperation(tag = address) { findPeripheral() } + val p = findPeripheral() val state = bleConnection.connectAndAwait(p, CONNECTION_TIMEOUT_MS) if (state !is ConnectionState.Connected) { throw RadioNotConnectedException("Failed to connect to device at address $address") @@ -226,14 +159,14 @@ constructor( } catch (e: Exception) { val failureTime = nowMillis - connectionStartTime Logger.w(e) { "[$address] Failed to connect to peripheral after ${failureTime}ms" } - service.onDisconnect(BleError.from(e)) + handleFailure(e) } } } private suspend fun onConnected() { try { - bleConnection.peripheral?.let { p -> + bleConnection.peripheralFlow.first()?.let { p -> val rssi = retryBleOperation(tag = address) { p.readRssi() } Logger.d { "[$address] Connection confirmed. Initial RSSI: $rssi dBm" } } @@ -243,7 +176,7 @@ constructor( } private fun onDisconnected(state: ConnectionState.Disconnected) { - clearCharacteristics() + radioService = null val uptime = if (connectionStartTime > 0) { @@ -257,117 +190,64 @@ constructor( "Packets RX: $packetsReceived ($bytesReceived bytes), " + "Packets TX: $packetsSent ($bytesSent bytes)" } - service.onDisconnect(error = BleError.Disconnected(reason = state.reason)) + val (isPermanent, msg) = + when (val reason = state.reason) { + is ConnectionState.Disconnected.Reason.InsufficientAuthentication -> + Pair(true, "Insufficient authentication: please unpair and repair the device") + is ConnectionState.Disconnected.Reason.RequiredServiceNotFound -> + Pair(false, "Required characteristic missing") + else -> Pair(false, reason.toString()) + } + service.onDisconnect(isPermanent, errorMessage = msg) } private suspend fun discoverServicesAndSetupCharacteristics() { try { - val chars = - bleConnection.discoverCharacteristics( - serviceUuid = SERVICE_UUID, - requiredUuids = - listOf( - TORADIO_CHARACTERISTIC, - FROMNUM_CHARACTERISTIC, - FROMRADIO_CHARACTERISTIC, - LOGRADIO_CHARACTERISTIC, - ), - optionalUuids = listOf(FROMRADIOSYNC_CHARACTERISTIC), - ) + bleConnection.profile(serviceUuid = SERVICE_UUID) { service -> + val radioService = MeshtasticRadioServiceImpl(service) - if (chars != null) { - toRadioCharacteristic = chars[TORADIO_CHARACTERISTIC] - fromNumCharacteristic = chars[FROMNUM_CHARACTERISTIC] - fromRadioCharacteristic = chars[FROMRADIO_CHARACTERISTIC] - logRadioCharacteristic = chars[LOGRADIO_CHARACTERISTIC] - fromRadioSyncCharacteristic = chars[FROMRADIOSYNC_CHARACTERISTIC] + // Wire up notifications + radioService.fromRadio + .onEach { packet -> + Logger.d { "[$address] Received packet fromRadio (${packet.size} bytes)" } + dispatchPacket(packet) + } + .catch { e -> + Logger.w(e) { "[$address] Error in fromRadio flow" } + handleFailure(e) + } + .launchIn(this) - Logger.d { "[$address] Characteristics discovered successfully" } - setupNotifications() - service.onConnect() - } else { - Logger.w { "[$address] Discovery failed: missing required characteristics" } - service.onDisconnect(error = BleError.DiscoveryFailed("One or more characteristics not found")) + radioService.logRadio + .onEach { packet -> + Logger.d { "[$address] Received packet logRadio (${packet.size} bytes)" } + dispatchPacket(packet) + } + .catch { e -> + Logger.w(e) { "[$address] Error in logRadio flow" } + handleFailure(e) + } + .launchIn(this) + + // Store reference for handleSendToRadio + this@NordicBleInterface.radioService = radioService + + Logger.i { "[$address] Profile service active and characteristics subscribed" } + + // Log negotiated MTU for diagnostics + val maxLen = bleConnection.maximumWriteValueLength(WriteType.WITHOUT_RESPONSE) + Logger.i { "[$address] BLE Radio Session Ready. Max write length (WITHOUT_RESPONSE): $maxLen bytes" } + + this@NordicBleInterface.service.onConnect() } } catch (e: Exception) { - Logger.w(e) { "[$address] Service discovery failed" } + Logger.w(e) { "[$address] Profile service discovery or operation failed" } bleConnection.disconnect() - service.onDisconnect(error = BleError.from(e)) + handleFailure(e) } } - // --- Notification Setup --- - - @Suppress("LongMethod") - private suspend fun setupNotifications() { - val fromRadioReady = CompletableDeferred() - val logRadioReady = CompletableDeferred() - - // 1. Prefer FromRadioSync (Indicate) if available - if (fromRadioSyncCharacteristic != null) { - Logger.i { "[$address] Using FromRadioSync for packet reception" } - fromRadioSyncCharacteristic - ?.subscribe { - Logger.d { "[$address] FromRadioSync subscription active" } - fromRadioReady.complete(Unit) - } - ?.onEach { payload -> - Logger.d { "[$address] FromRadioSync Indication (${payload.size} bytes)" } - dispatchPacket(payload) - } - ?.catch { e -> - if (!fromRadioReady.isCompleted) fromRadioReady.completeExceptionally(e) - Logger.w(e) { "[$address] Error in fromRadioSyncCharacteristic subscription" } - service.onDisconnect(BleError.from(e)) - } - ?.launchIn(connectionScope) ?: fromRadioReady.complete(Unit) - } else { - // 2. Fallback to legacy FromNum (Notify) + FromRadio (Read) - Logger.i { "[$address] Using legacy FromNum/FromRadio for packet reception" } - fromNumCharacteristic - ?.subscribe { - Logger.d { "[$address] FromNum subscription active" } - fromRadioReady.complete(Unit) - } - ?.onEach { notifyBytes -> - Logger.d { "[$address] FromNum Notification (${notifyBytes.size} bytes), draining queue" } - connectionScope.launch { drainPacketQueueAndDispatch() } - } - ?.catch { e -> - if (!fromRadioReady.isCompleted) fromRadioReady.completeExceptionally(e) - Logger.w(e) { "[$address] Error in fromNumCharacteristic subscription" } - service.onDisconnect(BleError.from(e)) - } - ?.launchIn(connectionScope) ?: fromRadioReady.complete(Unit) - } - - logRadioCharacteristic - ?.subscribe { - Logger.d { "[$address] LogRadio subscription active" } - logRadioReady.complete(Unit) - } - ?.onEach { notifyBytes -> - Logger.d { "[$address] LogRadio Notification (${notifyBytes.size} bytes), dispatching packet" } - dispatchPacket(notifyBytes) - } - ?.catch { e -> - if (!logRadioReady.isCompleted) logRadioReady.completeExceptionally(e) - Logger.w(e) { "[$address] Error in logRadioCharacteristic subscription" } - service.onDisconnect(BleError.from(e)) - } - ?.launchIn(connectionScope) ?: logRadioReady.complete(Unit) - - try { - withTimeout(CONNECTION_TIMEOUT_MS) { - fromRadioReady.await() - logRadioReady.await() - } - Logger.d { "[$address] All notifications successfully subscribed" } - } catch (e: Exception) { - Logger.e(e) { "[$address] Timeout or error waiting for characteristic subscriptions" } - throw e - } - } + private var radioService: MeshtasticRadioProfile.State? = null // --- IRadioInterface Implementation --- @@ -377,44 +257,31 @@ constructor( * @param p The packet to send. */ override fun handleSendToRadio(p: ByteArray) { - toRadioCharacteristic?.let { characteristic -> + val currentService = radioService + if (currentService != null) { connectionScope.launch { writeMutex.withLock { try { - val writeType = - if (characteristic.properties.contains(CharacteristicProperty.WRITE_WITHOUT_RESPONSE)) { - WriteType.WITHOUT_RESPONSE - } else { - WriteType.WITH_RESPONSE - } - - retryBleOperation(tag = address) { characteristic.write(p, writeType = writeType) } - + retryBleOperation(tag = address) { currentService.sendToRadio(p) } packetsSent++ bytesSent += p.size Logger.d { "[$address] Successfully wrote packet #$packetsSent " + - "to toRadioCharacteristic with $writeType - " + + "to toRadioCharacteristic - " + "${p.size} bytes (Total TX: $bytesSent bytes)" } - - // Only manually drain if we are using the legacy FromNum/FromRadio flow - if (fromRadioSyncCharacteristic == null) { - drainPacketQueueAndDispatch() - } - } catch (e: InvalidAttributeException) { - Logger.w(e) { "[$address] Attribute invalidated during write, clearing characteristics" } - handleInvalidAttribute(e) } catch (e: Exception) { Logger.w(e) { "[$address] Failed to write packet to toRadioCharacteristic after " + "$packetsSent successful writes" } - service.onDisconnect(BleError.from(e)) + handleFailure(e) } } } - } ?: Logger.w { "[$address] toRadio characteristic unavailable, can't send data" } + } else { + Logger.w { "[$address] toRadio characteristic unavailable, can't send data" } + } } override fun keepAlive() { @@ -423,35 +290,53 @@ constructor( /** Closes the connection to the device. */ override fun close() { - runBlocking { - val uptime = - if (connectionStartTime > 0) { - nowMillis - connectionStartTime - } else { - 0 - } - Logger.i { - "[$address] BLE close() called - " + - "Uptime: ${uptime}ms, " + - "Packets RX: $packetsReceived ($bytesReceived bytes), " + - "Packets TX: $packetsSent ($bytesSent bytes)" + val uptime = + if (connectionStartTime > 0) { + nowMillis - connectionStartTime + } else { + 0 } + Logger.i { + "[$address] BLE close() called - " + + "Uptime: ${uptime}ms, " + + "Packets RX: $packetsReceived ($bytesReceived bytes), " + + "Packets TX: $packetsSent ($bytesSent bytes)" + } + serviceScope.launch { connectionScope.cancel() bleConnection.disconnect() service.onDisconnect(true) } } - private fun handleInvalidAttribute(e: InvalidAttributeException) { - clearCharacteristics() - service.onDisconnect(BleError.from(e)) + private fun dispatchPacket(packet: ByteArray) { + packetsReceived++ + bytesReceived += packet.size + Logger.d { + "[$address] Dispatching packet to service.handleFromRadio() - " + + "Packet #$packetsReceived, ${packet.size} bytes (Total: $bytesReceived bytes)" + } + service.handleFromRadio(packet) } - private fun clearCharacteristics() { - toRadioCharacteristic = null - fromNumCharacteristic = null - fromRadioCharacteristic = null - logRadioCharacteristic = null - fromRadioSyncCharacteristic = null + private fun handleFailure(throwable: Throwable) { + val (isPermanent, msg) = throwable.toDisconnectReason() + service.onDisconnect(isPermanent, errorMessage = msg) + } + + private fun Throwable.toDisconnectReason(): Pair { + val isPermanent = + this is no.nordicsemi.kotlin.ble.core.exception.BluetoothUnavailableException || + this is no.nordicsemi.kotlin.ble.core.exception.ManagerClosedException + val msg = + when (this) { + is RadioNotConnectedException -> this.message ?: "Device not found" + is NoSuchElementException, + is IllegalArgumentException, + -> "Required characteristic missing" + is no.nordicsemi.kotlin.ble.core.exception.GattException -> "GATT Error: ${this.message}" + else -> this.message ?: this.javaClass.simpleName + } + return Pair(isPermanent, msg) } } diff --git a/app/src/test/java/com/geeksville/mesh/repository/radio/NordicBleInterfaceRetryTest.kt b/app/src/test/java/com/geeksville/mesh/repository/radio/NordicBleInterfaceRetryTest.kt index 41cceafe2..244167e5c 100644 --- a/app/src/test/java/com/geeksville/mesh/repository/radio/NordicBleInterfaceRetryTest.kt +++ b/app/src/test/java/com/geeksville/mesh/repository/radio/NordicBleInterfaceRetryTest.kt @@ -38,7 +38,6 @@ import no.nordicsemi.kotlin.ble.core.LegacyAdvertisingSetParameters import no.nordicsemi.kotlin.ble.core.Permission import org.junit.Before import org.junit.Test -import org.meshtastic.core.ble.BleError import org.meshtastic.core.ble.MeshtasticBleConstants.FROMNUM_CHARACTERISTIC import org.meshtastic.core.ble.MeshtasticBleConstants.FROMRADIO_CHARACTERISTIC import org.meshtastic.core.ble.MeshtasticBleConstants.LOGRADIO_CHARACTERISTIC @@ -169,7 +168,7 @@ class NordicBleInterfaceRetryTest { assert(writtenValue!!.contentEquals(dataToSend)) // Verify we didn't disconnect due to the retryable error - verify(exactly = 0) { service.onDisconnect(any()) } + verify(exactly = 0) { service.onDisconnect(any(), any()) } nordicInterface.close() } @@ -274,7 +273,7 @@ class NordicBleInterfaceRetryTest { // Verify onDisconnect was called after retries exhausted // Nordic BLE wraps RuntimeException in BluetoothException - verify { service.onDisconnect(any()) } + verify { service.onDisconnect(any(), any()) } nordicInterface.close() } diff --git a/app/src/test/java/com/geeksville/mesh/repository/radio/NordicBleInterfaceTest.kt b/app/src/test/java/com/geeksville/mesh/repository/radio/NordicBleInterfaceTest.kt index 1ee5ff9ee..1bf2f5a29 100644 --- a/app/src/test/java/com/geeksville/mesh/repository/radio/NordicBleInterfaceTest.kt +++ b/app/src/test/java/com/geeksville/mesh/repository/radio/NordicBleInterfaceTest.kt @@ -40,7 +40,6 @@ import no.nordicsemi.kotlin.ble.core.and import no.nordicsemi.kotlin.ble.environment.android.mock.MockAndroidEnvironment import org.junit.Before import org.junit.Test -import org.meshtastic.core.ble.BleError import org.meshtastic.core.ble.MeshtasticBleConstants.FROMNUM_CHARACTERISTIC import org.meshtastic.core.ble.MeshtasticBleConstants.FROMRADIOSYNC_CHARACTERISTIC import org.meshtastic.core.ble.MeshtasticBleConstants.FROMRADIO_CHARACTERISTIC @@ -400,8 +399,7 @@ class NordicBleInterfaceTest { advanceUntilIdle() // Verify onDisconnect was called on the service - // NordicBleInterface calls onDisconnect(BleError.Disconnected) - verify { service.onDisconnect(any()) } + verify { service.onDisconnect(any(), any()) } nordicInterface.close() } @@ -481,7 +479,7 @@ class NordicBleInterfaceTest { advanceUntilIdle() // Verify that discovery failed - verify { service.onDisconnect(any()) } + verify { service.onDisconnect(false, "Required characteristic missing") } nordicInterface.close() } @@ -575,7 +573,7 @@ class NordicBleInterfaceTest { advanceUntilIdle() // Verify onDisconnect was called with error - verify { service.onDisconnect(any()) } + verify { service.onDisconnect(any(), any()) } nordicInterface.close() } diff --git a/core/ble/README.md b/core/ble/README.md index 9989025e3..8b6f34062 100644 --- a/core/ble/README.md +++ b/core/ble/README.md @@ -31,33 +31,32 @@ This modernization replaces legacy callback-based implementations with robust, C ## Key Components -### 1. `NordicBleInterface` -The primary implementation of `IRadioInterface` for BLE devices. It acts as the bridge between the app's `RadioInterfaceService` and the physical Bluetooth device. +### 1. `BleConnection` +A robust wrapper around Nordic's `Peripheral` and `CentralManager` that simplifies the connection lifecycle and service discovery using modern Coroutine APIs. -- **Responsibility:** - - Managing the connection lifecycle. - - Discovering GATT services and characteristics. - - Handling data transmission (ToRadio) and reception (FromRadio). - - Managing MTU negotiation and connection priority. +- **Features:** + - **Connection & Await:** Provides suspend functions to connect and wait for a terminal state (Connected or Disconnected). + - **Unified Profile Helper:** A `profile` function that manages service discovery, characteristic setup, and lifecycle in a single block, with automatic timeout and error handling. + - **Observability:** Exposes `peripheralFlow` and `connectionState` as Flows for reactive UI and service updates. + - **Connection Management:** Handles PHY updates, MTU logging, and connection priority requests automatically. ### 2. `BluetoothRepository` A Singleton repository responsible for the global state of Bluetooth on the Android device. - **Features:** - **State Management:** Exposes a `StateFlow` reflecting whether Bluetooth is enabled, permissions are granted, and which devices are bonded. - - **Scanning:** Uses Nordic's `Scanner` to find devices. - - **Bonding:** Handles the creation of bonds with peripherals. + - **Permission Handling:** Centralizes logic for checking Bluetooth and Location permissions across different Android versions. + - **Bonding:** Simplifies the process of creating bonds with peripherals. -### 3. `BleConnection` -A wrapper around Nordic's `ClientBleGatt` that simplifies the connection process. - -- **Features:** - - **Connection & Await:** Provides suspend functions to connect and wait for a specific connection state. - - **Service Discovery:** Helper functions to discover specific services and characteristics with timeouts and retries. - - **Observability:** Logs connection parameters, PHY updates, and state changes. +### 3. `BleScanner` +A wrapper around Nordic's `CentralManager` scanning capabilities to provide a consistent and easy-to-use API for BLE scanning with built-in peripheral deduplication. ### 4. `BleRetry` -A utility for executing BLE operations with exponential backoff and retry logic. This is crucial for handling the inherent unreliability of wireless communication. +A utility for executing BLE operations with retry logic, essential for handling the inherent unreliability of wireless communication. + +## Integration in `app` + +The `:core:ble` module is used by `NordicBleInterface` in the main application module to implement the `IRadioInterface` for Bluetooth devices. ## Usage diff --git a/core/ble/src/main/kotlin/org/meshtastic/core/ble/BleConnection.kt b/core/ble/src/main/kotlin/org/meshtastic/core/ble/BleConnection.kt index 1ec635cc6..e31ef96ef 100644 --- a/core/ble/src/main/kotlin/org/meshtastic/core/ble/BleConnection.kt +++ b/core/ble/src/main/kotlin/org/meshtastic/core/ble/BleConnection.kt @@ -17,33 +17,29 @@ package org.meshtastic.core.ble import co.touchlab.kermit.Logger +import kotlinx.coroutines.CompletableDeferred import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Job import kotlinx.coroutines.NonCancellable +import kotlinx.coroutines.awaitCancellation +import kotlinx.coroutines.flow.MutableSharedFlow import kotlinx.coroutines.flow.SharedFlow -import kotlinx.coroutines.flow.SharingStarted import kotlinx.coroutines.flow.asSharedFlow -import kotlinx.coroutines.flow.filter -import kotlinx.coroutines.flow.filterNotNull import kotlinx.coroutines.flow.first -import kotlinx.coroutines.flow.flatMapLatest -import kotlinx.coroutines.flow.flowOf import kotlinx.coroutines.flow.launchIn import kotlinx.coroutines.flow.onEach -import kotlinx.coroutines.flow.shareIn +import kotlinx.coroutines.launch import kotlinx.coroutines.withContext import kotlinx.coroutines.withTimeout import no.nordicsemi.android.common.core.simpleSharedFlow -import no.nordicsemi.kotlin.ble.client.RemoteCharacteristic -import no.nordicsemi.kotlin.ble.client.RemoteService import no.nordicsemi.kotlin.ble.client.android.CentralManager import no.nordicsemi.kotlin.ble.client.android.ConnectionPriority import no.nordicsemi.kotlin.ble.client.android.Peripheral import no.nordicsemi.kotlin.ble.core.ConnectionState +import no.nordicsemi.kotlin.ble.core.WriteType +import kotlin.time.Duration.Companion.seconds import kotlin.uuid.Uuid -private const val SERVICE_DISCOVERY_TIMEOUT_MS = 10_000L - /** * Encapsulates a BLE connection to a [Peripheral]. Handles connection lifecycle, state monitoring, and service * discovery. @@ -61,12 +57,18 @@ class BleConnection( var peripheral: Peripheral? = null private set + private val _peripheral = MutableSharedFlow(replay = 1) + + /** A flow of the current peripheral. */ + val peripheralFlow = _peripheral.asSharedFlow() + private val _connectionState = simpleSharedFlow() /** A flow of [ConnectionState] changes for the current [peripheral]. */ val connectionState: SharedFlow = _connectionState.asSharedFlow() private var stateJob: Job? = null + private var profileJob: Job? = null /** * Connects to the given [Peripheral]. Note that this method returns as soon as the connection attempt is initiated. @@ -77,6 +79,7 @@ class BleConnection( suspend fun connect(p: Peripheral) = withContext(NonCancellable) { stateJob?.cancel() peripheral = p + _peripheral.emit(p) centralManager.connect( peripheral = p, @@ -103,57 +106,32 @@ class BleConnection( * * @param p The peripheral to connect to. * @param timeoutMs The maximum time to wait for a connection in milliseconds. + * @param onRegister Optional block to run before connecting, allowing for profile registration. * @return The final [ConnectionState]. * @throws kotlinx.coroutines.TimeoutCancellationException if the timeout is reached. */ - suspend fun connectAndAwait(p: Peripheral, timeoutMs: Long): ConnectionState { + suspend fun connectAndAwait(p: Peripheral, timeoutMs: Long, onRegister: suspend () -> Unit = {}): ConnectionState { + onRegister() connect(p) return withTimeout(timeoutMs) { connectionState.first { it is ConnectionState.Connected || it is ConnectionState.Disconnected } } } - /** A flow of discovered services. Useful for reacting to "Service Changed" indications. */ - val services: SharedFlow> = - _connectionState - .asSharedFlow() - .filter { it is ConnectionState.Connected } - .flatMapLatest { peripheral?.services() ?: flowOf(emptyList()) } - .filterNotNull() - .shareIn(scope, SharingStarted.WhileSubscribed(), replay = 1) - - /** Discovers characteristics for a specific service. */ - suspend fun discoverCharacteristics( - serviceUuid: Uuid, - requiredUuids: List, - optionalUuids: List = emptyList(), - ): Map? { - val p = peripheral ?: return null - - return retryBleOperation(tag = tag) { - val allRequested = requiredUuids + optionalUuids - val serviceList = - withTimeout(SERVICE_DISCOVERY_TIMEOUT_MS) { p.services(listOf(serviceUuid)).filterNotNull().first() } - val service = serviceList.find { it.uuid == serviceUuid } ?: return@retryBleOperation null - - val result = mutableMapOf() - for (uuid in allRequested) { - val char = service.characteristics.find { it.uuid == uuid } - if (char != null) { - result[uuid] = char - } - } - - val hasAllRequired = requiredUuids.all { result.containsKey(it) } - if (hasAllRequired) result else null - } - } - + @Suppress("TooGenericExceptionCaught") private fun observePeripheralDetails(p: Peripheral) { p.phy.onEach { phy -> Logger.i { "[$tag] BLE PHY changed to $phy" } }.launchIn(scope) p.connectionParameters - .onEach { params -> Logger.i { "[$tag] BLE connection parameters changed to $params" } } + .onEach { params -> + Logger.i { "[$tag] BLE connection parameters changed to $params" } + try { + val maxWriteLen = p.maximumWriteValueLength(WriteType.WITHOUT_RESPONSE) + Logger.i { "[$tag] Negotiated MTU (Write): $maxWriteLen bytes" } + } catch (e: Exception) { + Logger.d { "[$tag] Could not read MTU: ${e.message}" } + } + } .launchIn(scope) } @@ -161,7 +139,65 @@ class BleConnection( suspend fun disconnect() = withContext(NonCancellable) { stateJob?.cancel() stateJob = null + profileJob?.cancel() + profileJob = null peripheral?.disconnect() peripheral = null + _peripheral.emit(null) + } + + /** + * Executes a block within a discovered profile. Handles peripheral readiness, discovery with a timeout, and cleans + * up the profile job if discovery fails. + * + * @param serviceUuid The UUID of the service to discover. + * @param timeout The duration to wait for discovery. + * @param block The block to execute with the discovered service. + */ + @Suppress("TooGenericExceptionCaught") + suspend fun profile( + serviceUuid: Uuid, + timeout: kotlin.time.Duration = 10.seconds, + setup: suspend CoroutineScope.(no.nordicsemi.kotlin.ble.client.RemoteService) -> T, + ): T { + val p = peripheralFlow.first { it != null }!! + val serviceReady = CompletableDeferred() + + profileJob?.cancel() + val job = + scope.launch { + try { + val profileScope = this + p.profile(serviceUuid = serviceUuid, required = true, scope = profileScope) { service -> + try { + val result = setup(service) + serviceReady.complete(result) + // Keep the profile active until this launch scope (profileJob) is cancelled + awaitCancellation() + } catch (e: Throwable) { + if (!serviceReady.isCompleted) serviceReady.completeExceptionally(e) + throw e + } + } + } catch (e: Throwable) { + if (!serviceReady.isCompleted) serviceReady.completeExceptionally(e) + } + } + profileJob = job + + return try { + withTimeout(timeout) { serviceReady.await() } + } catch (e: Throwable) { + profileJob?.cancel() + throw e + } + } + + /** Returns the maximum write value length for the given write type. */ + fun maximumWriteValueLength(writeType: WriteType): Int? = peripheral?.maximumWriteValueLength(writeType) + + /** Requests a new connection priority for the current peripheral. */ + suspend fun requestConnectionPriority(priority: ConnectionPriority) { + peripheral?.requestConnectionPriority(priority) } } diff --git a/core/ble/src/main/kotlin/org/meshtastic/core/ble/BleError.kt b/core/ble/src/main/kotlin/org/meshtastic/core/ble/BleError.kt deleted file mode 100644 index 4bbf155c8..000000000 --- a/core/ble/src/main/kotlin/org/meshtastic/core/ble/BleError.kt +++ /dev/null @@ -1,135 +0,0 @@ -/* - * Copyright (c) 2025-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.ble - -import no.nordicsemi.kotlin.ble.client.exception.ConnectionFailedException -import no.nordicsemi.kotlin.ble.client.exception.InvalidAttributeException -import no.nordicsemi.kotlin.ble.client.exception.OperationFailedException -import no.nordicsemi.kotlin.ble.client.exception.PeripheralNotConnectedException -import no.nordicsemi.kotlin.ble.client.exception.ScanningException -import no.nordicsemi.kotlin.ble.client.exception.ValueDoesNotMatchException -import no.nordicsemi.kotlin.ble.core.ConnectionState -import no.nordicsemi.kotlin.ble.core.exception.BluetoothException -import no.nordicsemi.kotlin.ble.core.exception.BluetoothUnavailableException -import no.nordicsemi.kotlin.ble.core.exception.GattException -import no.nordicsemi.kotlin.ble.core.exception.ManagerClosedException - -/** - * Represents specific BLE failures, modeled after the iOS implementation's AccessoryError. This allows for more - * granular error handling and intelligent reconnection strategies. - */ -sealed class BleError(val message: String, val shouldReconnect: Boolean) { - - /** - * An error indicating that the peripheral was not found. This is a non-recoverable error and should not trigger a - * reconnect. - */ - data object PeripheralNotFound : BleError("Peripheral not found", shouldReconnect = false) - - /** - * An error indicating a failure during the connection attempt. This may be recoverable, so a reconnect attempt is - * warranted. - */ - class ConnectionFailed(exception: Throwable) : - BleError("Connection failed: ${exception.message}", shouldReconnect = true) - - /** - * An error indicating a failure during the service discovery process. This may be recoverable, so a reconnect - * attempt is warranted. - */ - class DiscoveryFailed(message: String) : BleError("Discovery failed: $message", shouldReconnect = true) - - /** - * An error indicating a disconnection initiated by the peripheral. This may be recoverable, so a reconnect attempt - * is warranted. - */ - class Disconnected(reason: ConnectionState.Disconnected.Reason?) : - BleError("Disconnected: ${reason ?: "Unknown reason"}", shouldReconnect = true) - - /** - * Wraps a generic GattException. The reconnection strategy depends on the nature of the Gatt error. - * - * @param exception The underlying GattException. - */ - class GattError(exception: GattException) : BleError("Gatt exception: ${exception.message}", shouldReconnect = true) - - /** - * Wraps a generic BluetoothException. The reconnection strategy depends on the nature of the Bluetooth error. - * - * @param exception The underlying BluetoothException. - */ - class BluetoothError(exception: BluetoothException) : - BleError("Bluetooth exception: ${exception.message}", shouldReconnect = true) - - /** The BLE manager was closed. This is a non-recoverable error. */ - class ManagerClosed(exception: ManagerClosedException) : - BleError("Manager closed: ${exception.message}", shouldReconnect = false) - - /** A BLE operation failed. This may be recoverable. */ - class OperationFailed(exception: OperationFailedException) : - BleError("Operation failed: ${exception.message}", shouldReconnect = true) - - /** - * An invalid attribute was used. This usually happens when the GATT handles become stale (e.g. after a service - * change or an unexpected disconnect). This is recoverable via a fresh connection and discovery. - */ - class InvalidAttribute(exception: InvalidAttributeException) : - BleError("Invalid attribute: ${exception.message}", shouldReconnect = true) - - /** An error occurred while scanning for devices. This may be recoverable. */ - class Scanning(exception: ScanningException) : - BleError("Scanning error: ${exception.message}", shouldReconnect = true) - - /** Bluetooth is unavailable on the device. This is a non-recoverable error. */ - class BluetoothUnavailable(exception: BluetoothUnavailableException) : - BleError("Bluetooth unavailable: ${exception.message}", shouldReconnect = false) - - /** The peripheral is not connected. This may be recoverable. */ - class PeripheralNotConnected(exception: PeripheralNotConnectedException) : - BleError("Peripheral not connected: ${exception.message}", shouldReconnect = true) - - /** A value did not match what was expected. This may be recoverable. */ - class ValueDoesNotMatch(exception: ValueDoesNotMatchException) : - BleError("Value does not match: ${exception.message}", shouldReconnect = true) - - /** A generic error for other exceptions that may occur. */ - class GenericError(exception: Throwable) : - BleError("An unexpected error occurred: ${exception.message}", shouldReconnect = true) - - companion object { - fun from(exception: Throwable): BleError = when (exception) { - is GattException -> { - when (exception) { - is ConnectionFailedException -> ConnectionFailed(exception) - is PeripheralNotConnectedException -> PeripheralNotConnected(exception) - is OperationFailedException -> OperationFailed(exception) - is ValueDoesNotMatchException -> ValueDoesNotMatch(exception) - else -> GattError(exception) - } - } - is BluetoothException -> { - when (exception) { - is BluetoothUnavailableException -> BluetoothUnavailable(exception) - is InvalidAttributeException -> InvalidAttribute(exception) - is ScanningException -> Scanning(exception) - else -> BluetoothError(exception) - } - } - else -> GenericError(exception) - } - } -} diff --git a/core/ble/src/main/kotlin/org/meshtastic/core/ble/BleModule.kt b/core/ble/src/main/kotlin/org/meshtastic/core/ble/BleModule.kt index 0086932f9..4970cfa89 100644 --- a/core/ble/src/main/kotlin/org/meshtastic/core/ble/BleModule.kt +++ b/core/ble/src/main/kotlin/org/meshtastic/core/ble/BleModule.kt @@ -23,12 +23,12 @@ import dagger.hilt.InstallIn import dagger.hilt.android.qualifiers.ApplicationContext import dagger.hilt.components.SingletonComponent import kotlinx.coroutines.CoroutineScope -import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.SupervisorJob import no.nordicsemi.kotlin.ble.client.android.CentralManager import no.nordicsemi.kotlin.ble.client.android.native import no.nordicsemi.kotlin.ble.core.android.AndroidEnvironment import no.nordicsemi.kotlin.ble.environment.android.NativeAndroidEnvironment +import org.meshtastic.core.di.CoroutineDispatchers import javax.inject.Singleton @Module @@ -47,5 +47,6 @@ object BleModule { @Provides @Singleton - fun provideBleSingletonCoroutineScope(): CoroutineScope = CoroutineScope(SupervisorJob() + Dispatchers.Default) + fun provideBleSingletonCoroutineScope(dispatchers: CoroutineDispatchers): CoroutineScope = + CoroutineScope(SupervisorJob() + dispatchers.default) } diff --git a/core/ble/src/main/kotlin/org/meshtastic/core/ble/BleRetry.kt b/core/ble/src/main/kotlin/org/meshtastic/core/ble/BleRetry.kt index 5cde0ca9f..c636d4718 100644 --- a/core/ble/src/main/kotlin/org/meshtastic/core/ble/BleRetry.kt +++ b/core/ble/src/main/kotlin/org/meshtastic/core/ble/BleRetry.kt @@ -30,7 +30,6 @@ import kotlinx.coroutines.delay * @return The result of the operation. * @throws Exception if the operation fails after all attempts. */ -@Suppress("TooGenericExceptionCaught") suspend fun retryBleOperation( count: Int = 3, delayMs: Long = 500L, @@ -43,7 +42,7 @@ suspend fun retryBleOperation( return block() } catch (e: CancellationException) { throw e - } catch (e: Exception) { + } catch (@Suppress("TooGenericExceptionCaught") e: Exception) { currentAttempt++ if (currentAttempt >= count) { Logger.w(e) { "[$tag] BLE operation failed after $count attempts, giving up" } diff --git a/core/ble/src/main/kotlin/org/meshtastic/core/ble/BluetoothRepository.kt b/core/ble/src/main/kotlin/org/meshtastic/core/ble/BluetoothRepository.kt index 8861b8a11..dbf68f811 100644 --- a/core/ble/src/main/kotlin/org/meshtastic/core/ble/BluetoothRepository.kt +++ b/core/ble/src/main/kotlin/org/meshtastic/core/ble/BluetoothRepository.kt @@ -25,6 +25,7 @@ import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.StateFlow import kotlinx.coroutines.flow.asStateFlow import kotlinx.coroutines.launch +import no.nordicsemi.kotlin.ble.client.RemoteServices import no.nordicsemi.kotlin.ble.client.android.CentralManager import no.nordicsemi.kotlin.ble.client.android.Peripheral import no.nordicsemi.kotlin.ble.core.android.AndroidEnvironment @@ -85,13 +86,7 @@ constructor( } internal suspend fun updateBluetoothState() { - val hasPerms = - if (androidEnvironment.requiresBluetoothRuntimePermissions) { - androidEnvironment.isBluetoothScanPermissionGranted && - androidEnvironment.isBluetoothConnectPermissionGranted - } else { - androidEnvironment.isLocationPermissionGranted - } + val hasPerms = hasRequiredPermissions() val enabled = androidEnvironment.isBluetoothEnabled val newState = BluetoothState( @@ -116,13 +111,7 @@ constructor( @SuppressLint("MissingPermission") fun isBonded(address: String): Boolean { val enabled = androidEnvironment.isBluetoothEnabled - val hasPerms = - if (androidEnvironment.requiresBluetoothRuntimePermissions) { - androidEnvironment.isBluetoothScanPermissionGranted && - androidEnvironment.isBluetoothConnectPermissionGranted - } else { - androidEnvironment.isLocationPermissionGranted - } + val hasPerms = hasRequiredPermissions() return if (enabled && hasPerms) { centralManager.getBondedPeripherals().any { it.address == address } } else { @@ -130,10 +119,19 @@ constructor( } } + private fun hasRequiredPermissions(): Boolean = if (androidEnvironment.requiresBluetoothRuntimePermissions) { + androidEnvironment.isBluetoothScanPermissionGranted && + androidEnvironment.isBluetoothConnectPermissionGranted + } else { + androidEnvironment.isLocationPermissionGranted + } + /** Checks if a peripheral is one of ours, either by its advertised name or by the services it provides. */ private fun isMatchingPeripheral(peripheral: Peripheral): Boolean { val nameMatches = peripheral.name?.matches(Regex(BLE_NAME_PATTERN)) ?: false - val hasRequiredService = peripheral.services(listOf(SERVICE_UUID)).value?.isNotEmpty() ?: false + val hasRequiredService = + (peripheral.services(listOf(SERVICE_UUID)).value as? RemoteServices.Discovered)?.services?.isNotEmpty() + ?: false return nameMatches || hasRequiredService } diff --git a/core/ble/src/main/kotlin/org/meshtastic/core/ble/MeshtasticBleConstants.kt b/core/ble/src/main/kotlin/org/meshtastic/core/ble/MeshtasticBleConstants.kt index 789110ac6..389516521 100644 --- a/core/ble/src/main/kotlin/org/meshtastic/core/ble/MeshtasticBleConstants.kt +++ b/core/ble/src/main/kotlin/org/meshtastic/core/ble/MeshtasticBleConstants.kt @@ -39,4 +39,15 @@ object MeshtasticBleConstants { val LOGRADIO_CHARACTERISTIC: Uuid = Uuid.parse("5a3d6e49-06e6-4423-9944-e9de8cdf9547") val FROMRADIOSYNC_CHARACTERISTIC: Uuid = Uuid.parse("888a50c3-982d-45db-9963-c7923769165d") + + // --- OTA Characteristics --- + + /** The Meshtastic OTA service UUID (ESP32 Unified OTA). */ + val OTA_SERVICE_UUID: Uuid = Uuid.parse("4FAFC201-1FB5-459E-8FCC-C5C9C331914B") + + /** Characteristic for writing OTA commands and firmware data. */ + val OTA_WRITE_CHARACTERISTIC: Uuid = Uuid.parse("62ec0272-3ec5-11eb-b378-0242ac130005") + + /** Characteristic for receiving OTA status notifications/ACKs. */ + val OTA_NOTIFY_CHARACTERISTIC: Uuid = Uuid.parse("62ec0272-3ec5-11eb-b378-0242ac130003") } diff --git a/core/ble/src/test/kotlin/org/meshtastic/core/ble/BluetoothRepositoryTest.kt b/core/ble/src/test/kotlin/org/meshtastic/core/ble/BluetoothRepositoryTest.kt index a4477c5e7..84b2d697b 100644 --- a/core/ble/src/test/kotlin/org/meshtastic/core/ble/BluetoothRepositoryTest.kt +++ b/core/ble/src/test/kotlin/org/meshtastic/core/ble/BluetoothRepositoryTest.kt @@ -124,4 +124,37 @@ class BluetoothRepositoryTest { assertEquals("Should find 1 bonded device", 1, state.bondedDevices.size) assertEquals(address, state.bondedDevices.first().address) } + + @Test + fun `isBonded returns false when permissions are not granted`() = runTest(testDispatcher) { + val noPermsEnv = + MockAndroidEnvironment.Api31( + isBluetoothEnabled = true, + isBluetoothScanPermissionGranted = false, + isBluetoothConnectPermissionGranted = false, + ) + val centralManager = CentralManager.mock(noPermsEnv, backgroundScope) + + val repository = BluetoothRepository(dispatchers, lifecycleOwner.lifecycle, centralManager, noPermsEnv) + runCurrent() + + assertFalse(repository.isBonded("C0:00:00:00:00:03")) + } + + @Test + fun `state has no permissions when bluetooth permissions denied`() = runTest(testDispatcher) { + val noPermsEnv = + MockAndroidEnvironment.Api31( + isBluetoothEnabled = true, + isBluetoothScanPermissionGranted = true, + isBluetoothConnectPermissionGranted = false, + ) + val centralManager = CentralManager.mock(noPermsEnv, backgroundScope) + + val repository = BluetoothRepository(dispatchers, lifecycleOwner.lifecycle, centralManager, noPermsEnv) + runCurrent() + + val state = repository.state.value + assertFalse("hasPermissions should be false when connect permission is denied", state.hasPermissions) + } } 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 787863341..863761bef 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 @@ -59,10 +59,7 @@ interface RadioInterfaceService { fun onConnect() /** Called by an interface when it has disconnected. */ - fun onDisconnect(isPermanent: Boolean) - - /** Called by an interface when it has disconnected with an error. */ - fun onDisconnect(error: Any) + fun onDisconnect(isPermanent: Boolean, errorMessage: String? = null) /** Called by an interface when it has received raw data from the radio. */ fun handleFromRadio(bytes: ByteArray) diff --git a/feature/firmware/README.md b/feature/firmware/README.md index 1c811faf2..99479ba2d 100644 --- a/feature/firmware/README.md +++ b/feature/firmware/README.md @@ -42,11 +42,12 @@ The `:feature:firmware` module provides a unified interface for updating Meshtas Meshtastic-Android supports three primary firmware update flows: #### 1. ESP32 Unified OTA (WiFi & BLE) -Used for modern ESP32 devices (e.g., Heltec V3, T-Beam S3). This method utilizes the **Unified OTA Protocol**, which enables high-speed transfers over TCP (port 3232) or BLE. The BLE transport uses the **Nordic Semiconductor Kotlin-BLE-Library** for architectural consistency with the rest of the application. +Used for modern ESP32 devices (e.g., Heltec V3, T-Beam S3). This method utilizes the **Unified OTA Protocol**, which enables high-speed transfers over TCP (port 3232) or BLE. The BLE transport uses the **Nordic Semiconductor Kotlin-BLE-Library** for architectural consistency and modern coroutine support. **Key Features:** - **Pre-shared Hash Verification**: The app sends the firmware SHA256 hash in an initial `AdminMessage` trigger. The device stores this in NVS and verifies the incoming stream against it. - **Connection Retry**: Robust logic to wait for the device to reboot and start the OTA listener. +- **Automatic MTU Handling & Fragmentation**: The BLE transport automatically detects the negotiated MTU and fragments data chunks into packets that fit. It carefully manages acknowledgments for each fragmented packet to ensure reliability even on congested connections. ```mermaid sequenceDiagram diff --git a/feature/firmware/src/main/kotlin/org/meshtastic/feature/firmware/ota/BleOtaTransport.kt b/feature/firmware/src/main/kotlin/org/meshtastic/feature/firmware/ota/BleOtaTransport.kt index 0b07b4146..af6df6cba 100644 --- a/feature/firmware/src/main/kotlin/org/meshtastic/feature/firmware/ota/BleOtaTransport.kt +++ b/feature/firmware/src/main/kotlin/org/meshtastic/feature/firmware/ota/BleOtaTransport.kt @@ -32,13 +32,16 @@ import kotlinx.coroutines.flow.onEach import kotlinx.coroutines.withTimeout import no.nordicsemi.kotlin.ble.client.RemoteCharacteristic import no.nordicsemi.kotlin.ble.client.android.CentralManager +import no.nordicsemi.kotlin.ble.client.android.ConnectionPriority import no.nordicsemi.kotlin.ble.client.android.Peripheral import no.nordicsemi.kotlin.ble.core.ConnectionState import no.nordicsemi.kotlin.ble.core.WriteType import org.meshtastic.core.ble.BleConnection import org.meshtastic.core.ble.BleScanner +import org.meshtastic.core.ble.MeshtasticBleConstants.OTA_NOTIFY_CHARACTERISTIC +import org.meshtastic.core.ble.MeshtasticBleConstants.OTA_SERVICE_UUID +import org.meshtastic.core.ble.MeshtasticBleConstants.OTA_WRITE_CHARACTERISTIC import kotlin.time.Duration.Companion.seconds -import kotlin.uuid.Uuid /** * BLE transport implementation for ESP32 Unified OTA protocol. Uses Nordic Kotlin-BLE-Library for modern coroutine @@ -161,57 +164,81 @@ class BleOtaTransport( Logger.i { "BLE OTA: Connected to ${p.address}, discovering services..." } - // Discover services - val chars = - bleConnection.discoverCharacteristics(SERVICE_UUID, listOf(OTA_CHARACTERISTIC_UUID, TX_CHARACTERISTIC_UUID)) - ?: throw OtaProtocolException.ConnectionFailed("Required OTA service or characteristics not found") + // Increase connection priority for OTA + bleConnection.requestConnectionPriority(ConnectionPriority.HIGH) - otaCharacteristic = chars[OTA_CHARACTERISTIC_UUID] - val txChar = chars[TX_CHARACTERISTIC_UUID] - - if (otaCharacteristic == null || txChar == null) { - throw OtaProtocolException.ConnectionFailed("Required characteristics not found") - } - - // Enable notifications and collect responses - val subscribed = CompletableDeferred() - txChar - .subscribe { - Logger.d { "BLE OTA: TX characteristic subscribed" } - subscribed.complete(Unit) - } - .onEach { notifyBytes -> - try { - val response = notifyBytes.decodeToString() - Logger.d { "BLE OTA: Received response: $response" } - responseChannel.trySend(response) - } catch (@Suppress("TooGenericExceptionCaught") e: Exception) { - Logger.e(e) { "BLE OTA: Failed to decode response bytes" } + // Discover services using our unified profile helper + bleConnection.profile(OTA_SERVICE_UUID) { service -> + val ota = + requireNotNull(service.characteristics.firstOrNull { it.uuid == OTA_WRITE_CHARACTERISTIC }) { + "OTA characteristic not found" + } + val txChar = + requireNotNull(service.characteristics.firstOrNull { it.uuid == OTA_NOTIFY_CHARACTERISTIC }) { + "TX characteristic not found" } - } - .catch { e -> - if (!subscribed.isCompleted) subscribed.completeExceptionally(e) - Logger.e(e) { "BLE OTA: Error in TX characteristic subscription" } - } - .launchIn(transportScope) - subscribed.await() - Logger.i { "BLE OTA: Service discovered and ready" } + otaCharacteristic = ota + + // Log negotiated MTU for diagnostics + val maxLen = bleConnection.maximumWriteValueLength(WriteType.WITHOUT_RESPONSE) + Logger.i { "BLE OTA: Service ready. Max write value length: $maxLen bytes" } + + // Enable notifications and collect responses + val subscribed = CompletableDeferred() + txChar + .subscribe { + Logger.d { "BLE OTA: TX characteristic subscribed" } + subscribed.complete(Unit) + } + .onEach { notifyBytes -> + try { + val response = notifyBytes.decodeToString() + Logger.d { "BLE OTA: Received response: $response" } + responseChannel.trySend(response) + } catch (@Suppress("TooGenericExceptionCaught") e: Exception) { + Logger.e(e) { "BLE OTA: Failed to decode response bytes" } + } + } + .catch { e -> + if (!subscribed.isCompleted) subscribed.completeExceptionally(e) + Logger.e(e) { "BLE OTA: Error in TX characteristic subscription" } + } + .launchIn(this) + + subscribed.await() + Logger.i { "BLE OTA: Service discovered and ready" } + } } + /** + * Initiates the OTA update by sending the size and hash. + * + * Note: If the start command is fragmented into multiple BLE packets, the protocol may send multiple responses + * (usually one ACK per packet followed by a final OK/ERASING). + */ + @Suppress("CyclomaticComplexMethod") override suspend fun startOta( sizeBytes: Long, sha256Hash: String, onHandshakeStatus: suspend (OtaHandshakeStatus) -> Unit, ): Result = runCatching { val command = OtaCommand.StartOta(sizeBytes, sha256Hash) - sendCommand(command) + val packetsSent = sendCommand(command) var handshakeComplete = false + var responsesReceived = 0 while (!handshakeComplete) { val response = waitForResponse(ERASING_TIMEOUT_MS) + responsesReceived++ when (val parsed = OtaResponse.parse(response)) { - is OtaResponse.Ok -> handshakeComplete = true + is OtaResponse.Ok -> { + // Only consider handshake complete after consuming all potential fragmented responses + if (responsesReceived >= packetsSent) { + handshakeComplete = true + } + } + is OtaResponse.Erasing -> { Logger.i { "BLE OTA: Device erasing flash..." } onHandshakeStatus(OtaHandshakeStatus.Erasing) @@ -231,6 +258,14 @@ class BleOtaTransport( } } + /** + * Streams the firmware data in chunks. + * + * Each chunk is potentially fragmented into multiple BLE packets based on the negotiated MTU. The transport ensures + * that every fragmented packet is acknowledged by the device before proceeding, preventing buffer overflows on the + * radio. + */ + @Suppress("CyclomaticComplexMethod") override suspend fun streamFirmware( data: ByteArray, chunkSize: Int, @@ -248,43 +283,49 @@ class BleOtaTransport( val currentChunkSize = minOf(chunkSize, remainingBytes) val chunk = data.copyOfRange(sentBytes, sentBytes + currentChunkSize) - // Write chunk - writeData(chunk, WriteType.WITHOUT_RESPONSE) + // Write chunk (potentially fragmented into multiple BLE packets) + val packetsSentForChunk = writeData(chunk, WriteType.WITHOUT_RESPONSE) - // Wait for response (ACK or OK for last chunk) - val response = waitForResponse(ACK_TIMEOUT_MS) + // Wait for responses (The protocol expects one response per GATT write) val nextSentBytes = sentBytes + currentChunkSize - when (val parsed = OtaResponse.parse(response)) { - is OtaResponse.Ack -> { - // Normal chunk success - } + repeat(packetsSentForChunk) { i -> + val response = waitForResponse(ACK_TIMEOUT_MS) + val isLastPacketOfChunk = i == packetsSentForChunk - 1 - is OtaResponse.Ok -> { - // OK indicates completion (usually on last chunk) - if (nextSentBytes >= totalBytes) { - sentBytes = nextSentBytes - onProgress(1.0f) - return@runCatching Unit - } else { - throw OtaProtocolException.TransferFailed("Premature OK received at offset $nextSentBytes") + when (val parsed = OtaResponse.parse(response)) { + is OtaResponse.Ack -> { + // Normal packet success } - } - is OtaResponse.Error -> { - if (parsed.message.contains("Hash Mismatch", ignoreCase = true)) { - throw OtaProtocolException.VerificationFailed("Firmware hash mismatch after transfer") + is OtaResponse.Ok -> { + // OK indicates completion (usually on last packet of last chunk) + if (nextSentBytes >= totalBytes && isLastPacketOfChunk) { + sentBytes = nextSentBytes + onProgress(1.0f) + return@runCatching Unit + } else if (!isLastPacketOfChunk) { + // Intermediate OK might happen if the device treats packets as chunks + } else { + throw OtaProtocolException.TransferFailed("Premature OK received at offset $nextSentBytes") + } } - throw OtaProtocolException.TransferFailed("Transfer failed: ${parsed.message}") - } - else -> throw OtaProtocolException.TransferFailed("Unexpected response: $response") + is OtaResponse.Error -> { + if (parsed.message.contains("Hash Mismatch", ignoreCase = true)) { + throw OtaProtocolException.VerificationFailed("Firmware hash mismatch after transfer") + } + throw OtaProtocolException.TransferFailed("Transfer failed: ${parsed.message}") + } + + else -> throw OtaProtocolException.TransferFailed("Unexpected response: $response") + } } sentBytes = nextSentBytes onProgress(sentBytes.toFloat() / totalBytes) } - // If we finished the loop without receiving OK, wait for it now + // If we finished the loop without receiving OK, wait for it now (verification stage) val finalResponse = waitForResponse(VERIFICATION_TIMEOUT_MS) when (val parsed = OtaResponse.parse(finalResponse)) { is OtaResponse.Ok -> Unit @@ -305,20 +346,37 @@ class BleOtaTransport( transportScope.cancel() } - private suspend fun sendCommand(command: OtaCommand) { + private suspend fun sendCommand(command: OtaCommand): Int { val data = command.toString().toByteArray() - writeData(data, WriteType.WITH_RESPONSE) + return writeData(data, WriteType.WITH_RESPONSE) } - private suspend fun writeData(data: ByteArray, writeType: WriteType) { + /** + * Writes data to the OTA characteristic, fragmenting the data into multiple BLE packets if it exceeds the + * negotiated MTU (maximum write length). + * + * @return The number of packets sent. + */ + private suspend fun writeData(data: ByteArray, writeType: WriteType): Int { val characteristic = otaCharacteristic ?: throw OtaProtocolException.ConnectionFailed("OTA characteristic not available") + val maxLen = bleConnection.maximumWriteValueLength(writeType) ?: data.size + var offset = 0 + var packetsSent = 0 + try { - characteristic.write(data, writeType = writeType) + while (offset < data.size) { + val chunkSize = minOf(data.size - offset, maxLen) + val packet = data.copyOfRange(offset, offset + chunkSize) + characteristic.write(packet, writeType = writeType) + offset += chunkSize + packetsSent++ + } } catch (@Suppress("TooGenericExceptionCaught") e: Exception) { - throw OtaProtocolException.TransferFailed("Failed to write data", e) + throw OtaProtocolException.TransferFailed("Failed to write data at offset $offset", e) } + return packetsSent } private suspend fun waitForResponse(timeoutMs: Long): String = try { @@ -328,11 +386,6 @@ class BleOtaTransport( } companion object { - // Service and Characteristic UUIDs from ESP32 Unified OTA spec - private val SERVICE_UUID = Uuid.parse("4FAFC201-1FB5-459E-8FCC-C5C9C331914B") - private val OTA_CHARACTERISTIC_UUID = Uuid.parse("62ec0272-3ec5-11eb-b378-0242ac130005") - private val TX_CHARACTERISTIC_UUID = Uuid.parse("62ec0272-3ec5-11eb-b378-0242ac130003") - // Timeouts and retries private val SCAN_TIMEOUT = 10.seconds private const val CONNECTION_TIMEOUT_MS = 15_000L diff --git a/feature/firmware/src/test/kotlin/org/meshtastic/feature/firmware/ota/BleOtaTransportServiceDiscoveryTest.kt b/feature/firmware/src/test/kotlin/org/meshtastic/feature/firmware/ota/BleOtaTransportServiceDiscoveryTest.kt new file mode 100644 index 000000000..3b33ed5b6 --- /dev/null +++ b/feature/firmware/src/test/kotlin/org/meshtastic/feature/firmware/ota/BleOtaTransportServiceDiscoveryTest.kt @@ -0,0 +1,209 @@ +/* + * 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.feature.firmware.ota + +import co.touchlab.kermit.Logger +import co.touchlab.kermit.Severity +import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.test.StandardTestDispatcher +import kotlinx.coroutines.test.runTest +import no.nordicsemi.kotlin.ble.client.android.CentralManager +import no.nordicsemi.kotlin.ble.client.android.mock.mock +import no.nordicsemi.kotlin.ble.client.mock.ConnectionResult +import no.nordicsemi.kotlin.ble.client.mock.PeripheralSpec +import no.nordicsemi.kotlin.ble.client.mock.PeripheralSpecEventHandler +import no.nordicsemi.kotlin.ble.client.mock.Proximity +import no.nordicsemi.kotlin.ble.core.CharacteristicProperty +import no.nordicsemi.kotlin.ble.core.LegacyAdvertisingSetParameters +import no.nordicsemi.kotlin.ble.core.Permission +import no.nordicsemi.kotlin.ble.core.and +import no.nordicsemi.kotlin.ble.environment.android.mock.MockAndroidEnvironment +import org.junit.Assert.assertTrue +import org.junit.Before +import org.junit.Test +import kotlin.time.Duration.Companion.milliseconds +import kotlin.uuid.Uuid + +private val SERVICE_UUID = Uuid.parse("4FAFC201-1FB5-459E-8FCC-C5C9C331914B") +private val OTA_CHARACTERISTIC_UUID = Uuid.parse("62ec0272-3ec5-11eb-b378-0242ac130005") +private val TX_CHARACTERISTIC_UUID = Uuid.parse("62ec0272-3ec5-11eb-b378-0242ac130003") + +/** + * Tests for BleOtaTransport service discovery via Nordic's Peripheral.profile() API. These validate the refactored + * connect() path that replaced discoverCharacteristics(). + */ +@OptIn(ExperimentalCoroutinesApi::class) +class BleOtaTransportServiceDiscoveryTest { + + private val testDispatcher = StandardTestDispatcher() + private val address = "00:11:22:33:44:55" + + @Before + fun setup() { + Logger.setLogWriters( + object : co.touchlab.kermit.LogWriter() { + override fun log(severity: Severity, message: String, tag: String, throwable: Throwable?) { + println("[$severity] $tag: $message") + throwable?.printStackTrace() + } + }, + ) + } + + @Test + fun `connect fails when OTA service not found on device`() = runTest(testDispatcher) { + val mockEnvironment = MockAndroidEnvironment.Api31(isBluetoothEnabled = true) + val centralManager = CentralManager.mock(mockEnvironment, scope = backgroundScope) + + // Create a peripheral with a DIFFERENT service UUID (not the OTA service) + val wrongServiceUuid = Uuid.parse("0000180A-0000-1000-8000-00805F9B34FB") // Device Info + val otaPeripheral = + PeripheralSpec.simulatePeripheral(identifier = address, proximity = Proximity.IMMEDIATE) { + advertising( + parameters = LegacyAdvertisingSetParameters(connectable = true, interval = 100.milliseconds), + ) { + CompleteLocalName("ESP32-OTA") + } + connectable( + name = "ESP32-OTA", + eventHandler = object : PeripheralSpecEventHandler {}, + isBonded = true, + ) { + Service(uuid = wrongServiceUuid) { + Characteristic( + uuid = OTA_CHARACTERISTIC_UUID, + properties = + CharacteristicProperty.WRITE and CharacteristicProperty.WRITE_WITHOUT_RESPONSE, + permission = Permission.WRITE, + ) + } + } + } + + centralManager.simulatePeripherals(listOf(otaPeripheral)) + + val transport = BleOtaTransport(centralManager, address, testDispatcher) + val result = transport.connect() + + assertTrue("Connect should fail when OTA service is missing", result.isFailure) + transport.close() + } + + @Test + fun `connect fails when TX characteristic is missing`() = runTest(testDispatcher) { + val mockEnvironment = MockAndroidEnvironment.Api31(isBluetoothEnabled = true) + val centralManager = CentralManager.mock(mockEnvironment, scope = backgroundScope) + + // Create a peripheral with the OTA service but only the OTA characteristic (no TX) + val otaPeripheral = + PeripheralSpec.simulatePeripheral(identifier = address, proximity = Proximity.IMMEDIATE) { + advertising( + parameters = LegacyAdvertisingSetParameters(connectable = true, interval = 100.milliseconds), + ) { + CompleteLocalName("ESP32-OTA") + } + connectable( + name = "ESP32-OTA", + eventHandler = object : PeripheralSpecEventHandler {}, + isBonded = true, + ) { + Service(uuid = SERVICE_UUID) { + Characteristic( + uuid = OTA_CHARACTERISTIC_UUID, + properties = + CharacteristicProperty.WRITE and CharacteristicProperty.WRITE_WITHOUT_RESPONSE, + permission = Permission.WRITE, + ) + // TX_CHARACTERISTIC intentionally omitted + } + } + } + + centralManager.simulatePeripherals(listOf(otaPeripheral)) + + val transport = BleOtaTransport(centralManager, address, testDispatcher) + val result = transport.connect() + + assertTrue("Connect should fail when TX characteristic is missing", result.isFailure) + transport.close() + } + + @Test + fun `connect fails when device is not found during scan`() = runTest(testDispatcher) { + val mockEnvironment = MockAndroidEnvironment.Api31(isBluetoothEnabled = true) + val centralManager = CentralManager.mock(mockEnvironment, scope = backgroundScope) + + // Don't simulate any peripherals — scan will find nothing + val transport = BleOtaTransport(centralManager, address, testDispatcher) + val result = transport.connect() + + assertTrue("Connect should fail when device is not found", result.isFailure) + val exception = result.exceptionOrNull() + assertTrue( + "Should be ConnectionFailed, got: $exception", + exception is OtaProtocolException.ConnectionFailed, + ) + transport.close() + } + + @Test + fun `connect succeeds with valid OTA service and characteristics`() = runTest(testDispatcher) { + val mockEnvironment = MockAndroidEnvironment.Api31(isBluetoothEnabled = true) + val centralManager = CentralManager.mock(mockEnvironment, scope = backgroundScope) + + val otaPeripheral = + PeripheralSpec.simulatePeripheral(identifier = address, proximity = Proximity.IMMEDIATE) { + advertising( + parameters = LegacyAdvertisingSetParameters(connectable = true, interval = 100.milliseconds), + ) { + CompleteLocalName("ESP32-OTA") + } + connectable( + name = "ESP32-OTA", + eventHandler = + object : PeripheralSpecEventHandler { + override fun onConnectionRequest( + preferredPhy: List, + ): ConnectionResult = ConnectionResult.Accept + }, + isBonded = true, + ) { + Service(uuid = SERVICE_UUID) { + Characteristic( + uuid = OTA_CHARACTERISTIC_UUID, + properties = + CharacteristicProperty.WRITE and CharacteristicProperty.WRITE_WITHOUT_RESPONSE, + permission = Permission.WRITE, + ) + Characteristic( + uuid = TX_CHARACTERISTIC_UUID, + property = CharacteristicProperty.NOTIFY, + permission = Permission.READ, + ) + } + } + } + + centralManager.simulatePeripherals(listOf(otaPeripheral)) + + val transport = BleOtaTransport(centralManager, address, testDispatcher) + val result = transport.connect() + + assertTrue("Connect should succeed: ${result.exceptionOrNull()}", result.isSuccess) + transport.close() + } +} diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml index 710c9fda8..daa0a459c 100644 --- a/gradle/libs.versions.toml +++ b/gradle/libs.versions.toml @@ -58,7 +58,7 @@ spotless = "8.3.0" wire = "6.0.0-alpha03" vico = "3.0.2" dependency-guard = "0.5.0" -nordic-ble = "2.0.0-alpha15" +nordic-ble = "2.0.0-alpha16" nordic-common = "2.9.2"