fix(transport): Kable BLE audit + thread-safety, MQTT, and logging fixes across transport layers (#5071)

This commit is contained in:
James Rich 2026-04-11 17:56:29 -05:00 committed by GitHub
parent 5f0e60eb21
commit a3c0a4832d
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
44 changed files with 1123 additions and 513 deletions

View file

@ -38,6 +38,9 @@ import org.meshtastic.core.ble.BleWriteType
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
import kotlin.time.Duration.Companion.milliseconds
import kotlin.time.Duration.Companion.seconds
/** BLE transport implementation for ESP32 Unified OTA protocol using Kable. */
class BleOtaTransport(
@ -68,7 +71,7 @@ class BleOtaTransport(
tag = "BLE OTA",
serviceUuid = OTA_SERVICE_UUID,
retryCount = SCAN_RETRY_COUNT,
retryDelayMs = SCAN_RETRY_DELAY_MS,
retryDelay = SCAN_RETRY_DELAY,
) {
it.address in targetAddresses
}
@ -76,8 +79,8 @@ class BleOtaTransport(
@Suppress("MagicNumber")
override suspend fun connect(): Result<Unit> = runCatching {
Logger.i { "BLE OTA: Waiting ${REBOOT_DELAY_MS}ms for device to reboot into OTA mode..." }
delay(REBOOT_DELAY_MS)
Logger.i { "BLE OTA: Waiting $REBOOT_DELAY for device to reboot into OTA mode..." }
delay(REBOOT_DELAY)
Logger.i { "BLE OTA: Connecting to $address using Kable..." }
@ -96,7 +99,7 @@ class BleOtaTransport(
.launchIn(transportScope)
try {
val finalState = bleConnection.connectAndAwait(device, CONNECTION_TIMEOUT_MS)
val finalState = bleConnection.connectAndAwait(device, CONNECTION_TIMEOUT)
if (finalState is BleConnectionState.Disconnected) {
Logger.w { "BLE OTA: Failed to connect to ${device.address} (state=$finalState)" }
throw OtaProtocolException.ConnectionFailed("Failed to connect to device at address ${device.address}")
@ -137,7 +140,7 @@ class BleOtaTransport(
.launchIn(this)
// Allow time for the BLE subscription to be established before proceeding.
delay(SUBSCRIPTION_SETTLE_MS)
delay(SUBSCRIPTION_SETTLE)
if (!subscribed.isCompleted) subscribed.complete(Unit)
subscribed.await()
@ -156,7 +159,7 @@ class BleOtaTransport(
var handshakeComplete = false
var responsesReceived = 0
while (!handshakeComplete) {
val response = waitForResponse(ERASING_TIMEOUT_MS)
val response = waitForResponse(ERASING_TIMEOUT)
responsesReceived++
when (val parsed = OtaResponse.parse(response)) {
is OtaResponse.Ok -> {
@ -203,7 +206,7 @@ class BleOtaTransport(
val nextSentBytes = sentBytes + currentChunkSize
repeat(packetsSentForChunk) { i ->
val response = waitForResponse(ACK_TIMEOUT_MS)
val response = waitForResponse(ACK_TIMEOUT)
val isLastPacketOfChunk = i == packetsSentForChunk - 1
when (val parsed = OtaResponse.parse(response)) {
@ -229,7 +232,7 @@ class BleOtaTransport(
onProgress(sentBytes.toFloat() / totalBytes)
}
val finalResponse = waitForResponse(VERIFICATION_TIMEOUT_MS)
val finalResponse = waitForResponse(VERIFICATION_TIMEOUT)
when (val parsed = OtaResponse.parse(finalResponse)) {
is OtaResponse.Ok -> Unit
is OtaResponse.Error -> {
@ -274,21 +277,21 @@ class BleOtaTransport(
return packetsSent
}
private suspend fun waitForResponse(timeoutMs: Long): String = try {
withTimeout(timeoutMs) { responseChannel.receive() }
private suspend fun waitForResponse(timeout: Duration): String = try {
withTimeout(timeout) { responseChannel.receive() }
} catch (@Suppress("SwallowedException") e: kotlinx.coroutines.TimeoutCancellationException) {
throw OtaProtocolException.Timeout("Timeout waiting for response after ${timeoutMs}ms")
throw OtaProtocolException.Timeout("Timeout waiting for response after $timeout")
}
companion object {
private const val CONNECTION_TIMEOUT_MS = 15_000L
private const val SUBSCRIPTION_SETTLE_MS = 500L
private const val ERASING_TIMEOUT_MS = 60_000L
private const val ACK_TIMEOUT_MS = 10_000L
private const val VERIFICATION_TIMEOUT_MS = 10_000L
private const val REBOOT_DELAY_MS = 5_000L
private val CONNECTION_TIMEOUT = 15.seconds
private val SUBSCRIPTION_SETTLE = 500.milliseconds
private val ERASING_TIMEOUT = 60.seconds
private val ACK_TIMEOUT = 10.seconds
private val VERIFICATION_TIMEOUT = 10.seconds
private val REBOOT_DELAY = 5.seconds
private const val SCAN_RETRY_COUNT = 3
private const val SCAN_RETRY_DELAY_MS = 2_000L
private val SCAN_RETRY_DELAY = 2.seconds
const val RECOMMENDED_CHUNK_SIZE = 512
}
}

View file

@ -26,7 +26,7 @@ import kotlin.time.Duration
import kotlin.time.Duration.Companion.seconds
internal const val DEFAULT_SCAN_RETRY_COUNT = 3
internal const val DEFAULT_SCAN_RETRY_DELAY_MS = 2_000L
internal val DEFAULT_SCAN_RETRY_DELAY: Duration = 2.seconds
internal val DEFAULT_SCAN_TIMEOUT: Duration = 10.seconds
private const val MAC_PARTS_COUNT = 6
@ -59,7 +59,7 @@ internal suspend fun scanForBleDevice(
tag: String,
serviceUuid: kotlin.uuid.Uuid,
retryCount: Int = DEFAULT_SCAN_RETRY_COUNT,
retryDelayMs: Long = DEFAULT_SCAN_RETRY_DELAY_MS,
retryDelay: Duration = DEFAULT_SCAN_RETRY_DELAY,
scanTimeout: Duration = DEFAULT_SCAN_TIMEOUT,
predicate: (BleDevice) -> Boolean,
): BleDevice? {
@ -80,7 +80,7 @@ internal suspend fun scanForBleDevice(
return device
}
Logger.w { "$tag: Target not in ${foundDevices.size} devices found" }
if (attempt < retryCount - 1) delay(retryDelayMs)
if (attempt < retryCount - 1) delay(retryDelay)
}
return null
}

View file

@ -48,6 +48,9 @@ import org.meshtastic.core.ble.BleWriteType
import org.meshtastic.core.ble.DEFAULT_BLE_WRITE_VALUE_LENGTH
import org.meshtastic.feature.firmware.ota.calculateMacPlusOne
import org.meshtastic.feature.firmware.ota.scanForBleDevice
import kotlin.time.Duration
import kotlin.time.Duration.Companion.milliseconds
import kotlin.time.Duration.Companion.seconds
/**
* Kable-based transport for the Nordic Secure DFU (Secure DFU over BLE) protocol.
@ -96,7 +99,7 @@ class SecureDfuTransport(
?: throw DfuException.ConnectionFailed("Device $address not found for buttonless DFU trigger")
Logger.i { "DFU: Connecting to $address to trigger buttonless DFU..." }
bleConnection.connectAndAwait(device, CONNECT_TIMEOUT_MS)
bleConnection.connectAndAwait(device, CONNECT_TIMEOUT)
bleConnection.profile(SecureDfuUuids.SERVICE) { service ->
val buttonlessChar = service.characteristic(SecureDfuUuids.BUTTONLESS_NO_BONDS)
@ -111,7 +114,7 @@ class SecureDfuTransport(
.catch { e -> Logger.d(e) { "DFU: Buttonless indication stream ended (expected on disconnect)" } }
.launchIn(this)
delay(SUBSCRIPTION_SETTLE_MS)
delay(SUBSCRIPTION_SETTLE)
Logger.i { "DFU: Writing buttonless DFU trigger..." }
service.write(buttonlessChar, byteArrayOf(0x01), BleWriteType.WITH_RESPONSE)
@ -119,7 +122,7 @@ class SecureDfuTransport(
// Wait for the indication response (0x20-01-STATUS). The device may disconnect before we receive it —
// that's expected and treated as success, matching the Nordic DFU library's behavior.
try {
withTimeout(BUTTONLESS_RESPONSE_TIMEOUT_MS) {
withTimeout(BUTTONLESS_RESPONSE_TIMEOUT) {
val response = indicationChannel.receive()
if (response.size >= 3 && response[0] == BUTTONLESS_RESPONSE_CODE && response[2] != 0x01.toByte()) {
Logger.w { "DFU: Buttonless DFU response indicates error: ${response.toHexString()}" }
@ -162,7 +165,7 @@ class SecureDfuTransport(
bleConnection.connectionState.onEach { Logger.d { "DFU: Connection state → $it" } }.launchIn(transportScope)
val connected = bleConnection.connectAndAwait(device, CONNECT_TIMEOUT_MS)
val connected = bleConnection.connectAndAwait(device, CONNECT_TIMEOUT)
if (connected is BleConnectionState.Disconnected) {
throw DfuException.ConnectionFailed("Failed to connect to DFU device ${device.address}")
}
@ -188,7 +191,7 @@ class SecureDfuTransport(
}
.launchIn(this)
delay(SUBSCRIPTION_SETTLE_MS)
delay(SUBSCRIPTION_SETTLE)
if (!subscribed.isCompleted) subscribed.complete(Unit)
subscribed.await()
@ -286,7 +289,7 @@ class SecureDfuTransport(
} catch (e: Throwable) {
lastError = e
Logger.w(e) { "DFU: Object transfer failed (attempt ${attempt + 1}/$OBJECT_RETRY_COUNT): ${e.message}" }
if (attempt < OBJECT_RETRY_COUNT - 1) delay(RETRY_DELAY_MS)
if (attempt < OBJECT_RETRY_COUNT - 1) delay(RETRY_DELAY)
}
}
throw lastError ?: DfuException.TransferFailed("Object transfer failed after $OBJECT_RETRY_COUNT attempts")
@ -347,7 +350,7 @@ class SecureDfuTransport(
// First-chunk delay: some older bootloaders need time to prepare flash after Create.
// The Nordic DFU library uses 400ms for the first chunk.
if (isFirstChunk) {
delay(FIRST_CHUNK_DELAY_MS)
delay(FIRST_CHUNK_DELAY)
isFirstChunk = false
}
@ -399,7 +402,7 @@ class SecureDfuTransport(
} catch (e: DfuException.ProtocolError) {
if (e.resultCode == DfuResultCode.INVALID_OBJECT && offset + objectSize >= totalBytes) {
Logger.w { "DFU: Execute returned INVALID_OBJECT on final object, retrying once..." }
delay(RETRY_DELAY_MS)
delay(RETRY_DELAY)
sendExecute()
} else {
throw e
@ -440,7 +443,7 @@ class SecureDfuTransport(
// Wait for the device's PRN receipt notification, then validate CRC.
// Skip the wait on the last packet — the final CALCULATE_CHECKSUM covers it.
if (prnInterval > 0 && packetsSincePrn >= prnInterval && pos < until) {
val response = awaitNotification(COMMAND_TIMEOUT_MS)
val response = awaitNotification(COMMAND_TIMEOUT)
if (response is DfuResponse.ChecksumResult) {
val expectedCrc = DfuCrc32.calculate(data, length = pos)
if (response.offset != pos || response.crc32 != expectedCrc) {
@ -459,7 +462,7 @@ class SecureDfuTransport(
val controlChar = service.characteristic(SecureDfuUuids.CONTROL_POINT)
service.write(controlChar, payload, BleWriteType.WITH_RESPONSE)
}
return awaitNotification(COMMAND_TIMEOUT_MS)
return awaitNotification(COMMAND_TIMEOUT)
}
private suspend fun setPrn(value: Int) {
@ -506,13 +509,13 @@ class SecureDfuTransport(
Logger.d { "DFU: Object executed." }
}
private suspend fun awaitNotification(timeoutMs: Long): DfuResponse = try {
withTimeout(timeoutMs) {
private suspend fun awaitNotification(timeout: Duration): DfuResponse = try {
withTimeout(timeout) {
val bytes = notificationChannel.receive()
DfuResponse.parse(bytes).also { Logger.d { "DFU: Notification → $it" } }
}
} catch (_: TimeoutCancellationException) {
throw DfuException.Timeout("No response from Control Point after ${timeoutMs}ms")
throw DfuException.Timeout("No response from Control Point after $timeout")
}
private fun DfuResponse.requireSuccess(expectedOpcode: Byte) {
@ -541,7 +544,7 @@ class SecureDfuTransport(
tag = "DFU",
serviceUuid = SecureDfuUuids.SERVICE,
retryCount = SCAN_RETRY_COUNT,
retryDelayMs = SCAN_RETRY_DELAY_MS,
retryDelay = SCAN_RETRY_DELAY,
predicate = predicate,
)
@ -550,14 +553,14 @@ class SecureDfuTransport(
// ---------------------------------------------------------------------------
companion object {
private const val CONNECT_TIMEOUT_MS = 15_000L
private const val COMMAND_TIMEOUT_MS = 30_000L
private const val SUBSCRIPTION_SETTLE_MS = 500L
private const val BUTTONLESS_RESPONSE_TIMEOUT_MS = 3_000L
private val CONNECT_TIMEOUT = 15.seconds
private val COMMAND_TIMEOUT = 30.seconds
private val SUBSCRIPTION_SETTLE = 500.milliseconds
private val BUTTONLESS_RESPONSE_TIMEOUT = 3.seconds
private const val SCAN_RETRY_COUNT = 3
private const val SCAN_RETRY_DELAY_MS = 2_000L
private const val RETRY_DELAY_MS = 2_000L
private const val FIRST_CHUNK_DELAY_MS = 400L
private val SCAN_RETRY_DELAY = 2.seconds
private val RETRY_DELAY = 2.seconds
private val FIRST_CHUNK_DELAY = 400.milliseconds
/** Response code prefix for Buttonless DFU indications (0x20 = response). */
private const val BUTTONLESS_RESPONSE_CODE: Byte = 0x20

View file

@ -614,8 +614,8 @@ class SecureDfuTransportTest {
override suspend fun connect(device: BleDevice) = delegate.connect(device)
override suspend fun connectAndAwait(device: BleDevice, timeoutMs: Long) =
delegate.connectAndAwait(device, timeoutMs)
override suspend fun connectAndAwait(device: BleDevice, timeout: Duration) =
delegate.connectAndAwait(device, timeout)
override suspend fun disconnect() = delegate.disconnect()

View file

@ -16,6 +16,8 @@
*/
package org.meshtastic.feature.wifiprovision
import kotlin.time.Duration.Companion.milliseconds
import kotlin.time.Duration.Companion.seconds
import kotlin.uuid.Uuid
/**
@ -62,14 +64,14 @@ internal object NymeaBleConstants {
/** JSON stream terminator — marks the end of a reassembled message. */
const val STREAM_TERMINATOR = '\n'
/** Scan + connect timeout in milliseconds. */
const val SCAN_TIMEOUT_MS = 10_000L
/** Scan + connect timeout. */
val SCAN_TIMEOUT = 10.seconds
/** Maximum time to wait for a command response. */
const val RESPONSE_TIMEOUT_MS = 15_000L
val RESPONSE_TIMEOUT = 15.seconds
/** Settle time after subscribing to notifications before sending commands. */
const val SUBSCRIPTION_SETTLE_MS = 300L
val SUBSCRIPTION_SETTLE = 300.milliseconds
// endregion
// region Wireless Commander command codes

View file

@ -43,14 +43,13 @@ import org.meshtastic.feature.wifiprovision.NymeaBleConstants.CMD_GET_NETWORKS
import org.meshtastic.feature.wifiprovision.NymeaBleConstants.CMD_SCAN
import org.meshtastic.feature.wifiprovision.NymeaBleConstants.COMMANDER_RESPONSE_UUID
import org.meshtastic.feature.wifiprovision.NymeaBleConstants.RESPONSE_SUCCESS
import org.meshtastic.feature.wifiprovision.NymeaBleConstants.RESPONSE_TIMEOUT_MS
import org.meshtastic.feature.wifiprovision.NymeaBleConstants.SCAN_TIMEOUT_MS
import org.meshtastic.feature.wifiprovision.NymeaBleConstants.SUBSCRIPTION_SETTLE_MS
import org.meshtastic.feature.wifiprovision.NymeaBleConstants.RESPONSE_TIMEOUT
import org.meshtastic.feature.wifiprovision.NymeaBleConstants.SCAN_TIMEOUT
import org.meshtastic.feature.wifiprovision.NymeaBleConstants.SUBSCRIPTION_SETTLE
import org.meshtastic.feature.wifiprovision.NymeaBleConstants.WIRELESS_COMMANDER_UUID
import org.meshtastic.feature.wifiprovision.NymeaBleConstants.WIRELESS_SERVICE_UUID
import org.meshtastic.feature.wifiprovision.model.ProvisionResult
import org.meshtastic.feature.wifiprovision.model.WifiNetwork
import kotlin.time.Duration.Companion.milliseconds
/**
* GATT client for the nymea-networkmanager WiFi provisioning profile.
@ -87,26 +86,20 @@ class NymeaWifiService(
*
* @param address Optional MAC address filter. If null, the first advertising device is used.
* @return The discovered device's advertised name on success.
* @throws IllegalStateException if no device is found within [SCAN_TIMEOUT_MS].
* @throws IllegalStateException if no device is found within [SCAN_TIMEOUT].
*/
suspend fun connect(address: String? = null): Result<String> = runCatching {
Logger.i { "$TAG: Scanning for nymea-networkmanager device (address=$address)…" }
val device =
withTimeout(SCAN_TIMEOUT_MS) {
scanner
.scan(
timeout = SCAN_TIMEOUT_MS.milliseconds,
serviceUuid = WIRELESS_SERVICE_UUID,
address = address,
)
.first()
withTimeout(SCAN_TIMEOUT) {
scanner.scan(timeout = SCAN_TIMEOUT, serviceUuid = WIRELESS_SERVICE_UUID, address = address).first()
}
val deviceName = device.name ?: device.address
Logger.i { "$TAG: Found device: ${device.name} @ ${device.address}" }
val state = bleConnection.connectAndAwait(device, SCAN_TIMEOUT_MS)
val state = bleConnection.connectAndAwait(device, SCAN_TIMEOUT)
check(state is BleConnectionState.Connected) { "Failed to connect to ${device.address} — final state: $state" }
Logger.i { "$TAG: Connected. Discovering wireless service…" }
@ -130,7 +123,7 @@ class NymeaWifiService(
}
.launchIn(this)
delay(SUBSCRIPTION_SETTLE_MS)
delay(SUBSCRIPTION_SETTLE)
if (!subscribed.isCompleted) subscribed.complete(Unit)
subscribed.await()
@ -235,8 +228,8 @@ class NymeaWifiService(
}
}
/** Wait up to [RESPONSE_TIMEOUT_MS] for a complete JSON response from the notification channel. */
private suspend fun waitForResponse(): String = withTimeout(RESPONSE_TIMEOUT_MS) { responseChannel.receive() }
/** Wait up to [RESPONSE_TIMEOUT] for a complete JSON response from the notification channel. */
private suspend fun waitForResponse(): String = withTimeout(RESPONSE_TIMEOUT) { responseChannel.receive() }
private fun nymeaErrorMessage(code: Int): String = when (code) {
NymeaBleConstants.RESPONSE_INVALID_COMMAND -> "Invalid command"