From b6926700ca0f7c48be8ace6562cd160144c98ffa Mon Sep 17 00:00:00 2001 From: James Rich <2199651+jamesarich@users.noreply.github.com> Date: Tue, 23 Jun 2026 12:45:04 -0500 Subject: [PATCH] fix(firmware): harden ESP32 OTA + nRF DFU update paths (hardware-validated) (#5915) Co-authored-by: Claude Opus 4.8 --- .gitignore | 4 + .skills/compose-ui/strings-index.txt | 1 + .../composeResources/values/strings.xml | 1 + .../feature/firmware/FirmwareManifest.kt | 16 +-- .../feature/firmware/FirmwareRetriever.kt | 42 +++--- .../feature/firmware/FirmwareUpdateScreen.kt | 11 ++ .../feature/firmware/FirmwareUpdateState.kt | 2 + .../feature/firmware/ota/BleOtaTransport.kt | 118 +++++++++------- .../firmware/ota/dfu/DfuUploadTransport.kt | 7 + .../firmware/ota/dfu/LegacyDfuTransport.kt | 62 ++++---- .../firmware/ota/dfu/SecureDfuHandler.kt | 17 ++- .../firmware/ota/dfu/SecureDfuProtocol.kt | 3 - .../firmware/ota/dfu/SecureDfuTransport.kt | 12 +- .../firmware/CommonFirmwareRetrieverTest.kt | 24 +++- .../feature/firmware/FirmwareManifestTest.kt | 57 ++++---- .../firmware/ota/BleOtaTransportTest.kt | 124 +++++++++++++++- .../ota/dfu/LegacyDfuTransportTest.kt | 26 ++++ workpad.md | 132 ------------------ 18 files changed, 357 insertions(+), 302 deletions(-) delete mode 100644 workpad.md diff --git a/.gitignore b/.gitignore index 1e7e47d46..654deb00b 100644 --- a/.gitignore +++ b/.gitignore @@ -95,3 +95,7 @@ offline-repository/ build-scan-*.scan # burningmesh capture replay assets (private data — generated via replay_server.py --export) *.fromradio + +# craft pipeline scratch workpad (per-task working notes, not source) +/workpad.md +/workpad.*.md diff --git a/.skills/compose-ui/strings-index.txt b/.skills/compose-ui/strings-index.txt index 2af559b4c..3da5cd2cf 100644 --- a/.skills/compose-ui/strings-index.txt +++ b/.skills/compose-ui/strings-index.txt @@ -607,6 +607,7 @@ firmware_update_retrieval_failed firmware_update_retry firmware_update_save_dfu_file firmware_update_select_file +firmware_update_slow_bootloader_hint firmware_update_source_local firmware_update_stable firmware_update_starting_dfu diff --git a/core/resources/src/commonMain/composeResources/values/strings.xml b/core/resources/src/commonMain/composeResources/values/strings.xml index d8ef3d42e..b5b52f9c9 100644 --- a/core/resources/src/commonMain/composeResources/values/strings.xml +++ b/core/resources/src/commonMain/composeResources/values/strings.xml @@ -631,6 +631,7 @@ Retry Please save the .uf2 file to your device's DFU drive. Select Local File + Updates are slow on this device's bootloader. Flashing the OTAFIX bootloader enables much faster BLE updates. Source: Local File Stable Starting DFU... diff --git a/feature/firmware/src/commonMain/kotlin/org/meshtastic/feature/firmware/FirmwareManifest.kt b/feature/firmware/src/commonMain/kotlin/org/meshtastic/feature/firmware/FirmwareManifest.kt index 110d5cf9e..ab60e9bf3 100644 --- a/feature/firmware/src/commonMain/kotlin/org/meshtastic/feature/firmware/FirmwareManifest.kt +++ b/feature/firmware/src/commonMain/kotlin/org/meshtastic/feature/firmware/FirmwareManifest.kt @@ -26,20 +26,20 @@ import kotlinx.serialization.Serializable * we fetch the manifest on-demand, locate the `app0` partition entry, and use its [FirmwareManifestFile.name] as the * exact filename to download. * + * Only [files] is modelled. The manifest also carries decorative metadata (`hwModel`, `mcu`, `build_epoch`, the + * partition table, …) whose scalar types vary across firmware versions — notably `hwModel` is an **integer** in current + * releases (the `HardwareModel` enum value) though it was historically a string. Modelling those would couple parsing + * to a schema we don't control; instead [Json.ignoreUnknownKeys] drops them so an upstream type change can never break + * OTA resolution. (A `hwModel: String` field previously made every manifest fail to parse, silently falling back to + * filename heuristics — see `FirmwareRetriever`.) + * * Example URL: * ``` * https://raw.githubusercontent.com/meshtastic/meshtastic.github.io/master/ * firmware-2.7.17/firmware-t-deck-2.7.17.mt.json * ``` */ -@Serializable -internal data class FirmwareManifest( - @SerialName("hwModel") val hwModel: String = "", - val architecture: String = "", - @SerialName("platformioTarget") val platformioTarget: String = "", - val mcu: String = "", - val files: List = emptyList(), -) +@Serializable internal data class FirmwareManifest(val files: List = emptyList()) /** * A single partition file entry inside a [FirmwareManifest]. diff --git a/feature/firmware/src/commonMain/kotlin/org/meshtastic/feature/firmware/FirmwareRetriever.kt b/feature/firmware/src/commonMain/kotlin/org/meshtastic/feature/firmware/FirmwareRetriever.kt index 66c68cc05..a206cb636 100644 --- a/feature/firmware/src/commonMain/kotlin/org/meshtastic/feature/firmware/FirmwareRetriever.kt +++ b/feature/firmware/src/commonMain/kotlin/org/meshtastic/feature/firmware/FirmwareRetriever.kt @@ -76,10 +76,13 @@ class FirmwareRetriever(private val fileHandler: FirmwareFileHandler) { /** * Download the ESP32 OTA firmware binary. Tries in order: - * 1. `.mt.json` manifest resolution (2.7.17+) - * 2. Current naming convention (`firmware--.bin`) - * 3. Legacy naming (`firmware---update.bin`) - * 4. Any matching `.bin` from the release zip + * 1. `.mt.json` manifest resolution (2.7.17+) — the authoritative `app0` partition image. + * 2. `firmware---update.bin` — the bare app image. Preferred over the plain `.bin` because on + * pre-2.7.17 releases (which ship no manifest) `firmware--.bin` is a *merged* image + * (bootloader + partition table + app); flashing that to the `app0` partition leaves it misaligned and the + * device's `esp_ota_end` rejects it. `-update.bin` is the OTA-able app image whenever it is published. + * 3. `firmware--.bin` — the bare app image on 2.7.17+ (which publish no `-update.bin`). + * 4. Any matching `.bin` from the release zip. * * @return The downloaded `.bin` [FirmwareArtifact], or `null` if the file could not be resolved. */ @@ -97,7 +100,22 @@ class FirmwareRetriever(private val fileHandler: FirmwareFileHandler) { return it } - // ── Fallback 1: current naming (2.7.17+) ──────────────────────────── + // ── Fallback 1: bare app image `-update.bin` (unambiguous OTA image when published; required for pre-2.7.17, + // whose plain `.bin` is a merged bootloader+app image that esp_ota_end rejects) ── + val updateFilename = "firmware-$target-$version-update.bin" + retrieveArtifact( + release = release, + hardware = hardware, + onProgress = onProgress, + fileSuffix = "-update.bin", + internalFileExtension = "-update.bin", + preferredFilename = updateFilename, + ) + ?.let { + return it + } + + // ── Fallback 2: plain `firmware--.bin` (the app image on 2.7.17+) ── val currentFilename = "firmware-$target-$version.bin" retrieveArtifact( release = release, @@ -111,20 +129,6 @@ class FirmwareRetriever(private val fileHandler: FirmwareFileHandler) { return it } - // ── Fallback 2: legacy naming (pre-2.7.17) ────────────────────────── - val legacyFilename = "firmware-$target-$version-update.bin" - retrieveArtifact( - release = release, - hardware = hardware, - onProgress = onProgress, - fileSuffix = "-update.bin", - internalFileExtension = "-update.bin", - preferredFilename = legacyFilename, - ) - ?.let { - return it - } - // ── Fallback 3: any matching .bin from the release zip ─────────────── return retrieveArtifact( release = release, diff --git a/feature/firmware/src/commonMain/kotlin/org/meshtastic/feature/firmware/FirmwareUpdateScreen.kt b/feature/firmware/src/commonMain/kotlin/org/meshtastic/feature/firmware/FirmwareUpdateScreen.kt index 45aa27cd2..76e74ebff 100644 --- a/feature/firmware/src/commonMain/kotlin/org/meshtastic/feature/firmware/FirmwareUpdateScreen.kt +++ b/feature/firmware/src/commonMain/kotlin/org/meshtastic/feature/firmware/FirmwareUpdateScreen.kt @@ -728,6 +728,17 @@ private fun ProgressContent( ) } + val hint = progressState.hint + if (hint != null) { + Spacer(Modifier.height(8.dp)) + Text( + text = hint.asString(), + style = MaterialTheme.typography.bodySmall, + color = MaterialTheme.colorScheme.primary, + textAlign = TextAlign.Center, + ) + } + Spacer(Modifier.height(12.dp)) if (isDownloading || isUpdating) { diff --git a/feature/firmware/src/commonMain/kotlin/org/meshtastic/feature/firmware/FirmwareUpdateState.kt b/feature/firmware/src/commonMain/kotlin/org/meshtastic/feature/firmware/FirmwareUpdateState.kt index adb741cc0..1cb212b08 100644 --- a/feature/firmware/src/commonMain/kotlin/org/meshtastic/feature/firmware/FirmwareUpdateState.kt +++ b/feature/firmware/src/commonMain/kotlin/org/meshtastic/feature/firmware/FirmwareUpdateState.kt @@ -26,11 +26,13 @@ import org.meshtastic.core.resources.UiText * @property message A high-level status message (e.g., "Downloading..."). * @property progress A value between 0.0 and 1.0 representing completion percentage. * @property details Optional high-frequency detail text (e.g., "1.2 MiB/s, 45%"). + * @property hint Optional persistent advisory shown alongside the progress (e.g. a slow-bootloader tip). */ data class ProgressState( val message: UiText = UiText.DynamicString(""), val progress: Float = 0f, val details: String? = null, + val hint: UiText? = null, ) /** State machine for the firmware update flow, observed by [FirmwareUpdateScreen]. */ diff --git a/feature/firmware/src/commonMain/kotlin/org/meshtastic/feature/firmware/ota/BleOtaTransport.kt b/feature/firmware/src/commonMain/kotlin/org/meshtastic/feature/firmware/ota/BleOtaTransport.kt index d78154b85..39d6dc6c8 100644 --- a/feature/firmware/src/commonMain/kotlin/org/meshtastic/feature/firmware/ota/BleOtaTransport.kt +++ b/feature/firmware/src/commonMain/kotlin/org/meshtastic/feature/firmware/ota/BleOtaTransport.kt @@ -38,6 +38,7 @@ 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 org.meshtastic.core.common.util.safeCatching +import kotlin.concurrent.Volatile import kotlin.time.Duration import kotlin.time.Duration.Companion.milliseconds import kotlin.time.Duration.Companion.seconds @@ -58,7 +59,9 @@ class BleOtaTransport( private val responseChannel = Channel(Channel.UNLIMITED) - private var isConnected = false + // Written from the connectionState collector (Dispatchers.Default) and read by the streaming loop's + // connection-loss guard (Dispatchers.IO); @Volatile ensures the guard sees a mid-transfer disconnect. + @Volatile private var isConnected = false /** Scan for the device by MAC address (or MAC+1 for OTA mode) with retries. */ private suspend fun scanForOtaDevice(): BleDevice? { @@ -66,13 +69,7 @@ class BleOtaTransport( val targetAddresses = setOf(address, otaAddress) Logger.i { "BLE OTA: Will match addresses: $targetAddresses" } - return scanForBleDevice( - scanner = scanner, - tag = "BLE OTA", - serviceUuid = OTA_SERVICE_UUID, - retryCount = SCAN_RETRY_COUNT, - retryDelay = SCAN_RETRY_DELAY, - ) { + return scanForBleDevice(scanner = scanner, tag = "BLE OTA", serviceUuid = OTA_SERVICE_UUID) { it.address in targetAddresses } } @@ -187,6 +184,20 @@ class BleOtaTransport( } } + /** + * Streams firmware to the device. The ESP32 OTA loader is an ACK-paced byte stream: it drains its receive buffer + * and emits exactly ONE response per drain — an `ACK` while more data is expected, or the terminal `OK` once the + * final byte is received and the image verified. To keep a deterministic 1-write → 1-response cadence at any MTU, + * each chunk is sent as a single GATT write no larger than the negotiated write payload. A chunk that exceeded the + * payload would fragment into multiple writes that the device coalesces into one response, desyncing our response + * accounting (the cause of the prior ACK-timeout hang at MTU 512, where a 512-byte chunk split into [509, 3]). + * + * Because we write one chunk then await its single response before writing the next, only one chunk is ever + * outstanding, so the device's byte-stream receive buffer holds exactly one chunk per drain — keeping the cadence a + * deterministic 1 write → 1 response. Branch on response *type* (never on a fragment count): success requires an + * explicit terminal `OK`, and any `ERR` fails the transfer, so a late device error can never be reported as + * success. + */ @Suppress("MagicNumber") override suspend fun streamFirmware( data: ByteArray, @@ -196,63 +207,65 @@ class BleOtaTransport( val totalBytes = data.size var sentBytes = 0 + val writePayload = bleConnection.maximumWriteValueLength(BleWriteType.WITHOUT_RESPONSE) ?: SAFE_WRITE_PAYLOAD + val effectiveChunkSize = minOf(chunkSize, writePayload).coerceAtLeast(1) + while (sentBytes < totalBytes) { if (!isConnected) { throw OtaProtocolException.TransferFailed("Connection lost during transfer") } - val remainingBytes = totalBytes - sentBytes - val currentChunkSize = minOf(chunkSize, remainingBytes) + val currentChunkSize = minOf(effectiveChunkSize, totalBytes - sentBytes) val chunk = data.copyOfRange(sentBytes, sentBytes + currentChunkSize) + val isLastChunk = sentBytes + currentChunkSize >= totalBytes - val packetsSentForChunk = writeData(chunk, BleWriteType.WITHOUT_RESPONSE) - - val nextSentBytes = sentBytes + currentChunkSize - repeat(packetsSentForChunk) { i -> - val response = waitForResponse(ACK_TIMEOUT) - val isLastPacketOfChunk = i == packetsSentForChunk - 1 - - when (val parsed = OtaResponse.parse(response)) { - is OtaResponse.Ack -> {} - - is OtaResponse.Ok -> { - if (nextSentBytes >= totalBytes && isLastPacketOfChunk) { - sentBytes = nextSentBytes - onProgress(1.0f) - return@safeCatching Unit - } - } - - 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") - } + val packetsSent = writeData(chunk, BleWriteType.WITHOUT_RESPONSE) + if (packetsSent != 1) { + throw OtaProtocolException.TransferFailed("Chunk produced $packetsSent writes, expected exactly one") } - sentBytes = nextSentBytes + when (val parsed = OtaResponse.parse(waitForResponse(ACK_TIMEOUT))) { + is OtaResponse.Ack -> Unit + + is OtaResponse.Ok -> + if (isLastChunk) { + sentBytes += currentChunkSize + onProgress(1.0f) + return@safeCatching Unit + } else { + // The device sends OK only once the full image is received; an OK before the final chunk + // means its size accounting disagrees with ours — fail closed rather than treat it as an ACK. + throw OtaProtocolException.TransferFailed("Received OK before the final chunk") + } + + is OtaResponse.Error -> throw transferException(parsed) + + else -> throw OtaProtocolException.TransferFailed("Unexpected response during transfer: $parsed") + } + + sentBytes += currentChunkSize onProgress(sentBytes.toFloat() / totalBytes) } - val finalResponse = waitForResponse(VERIFICATION_TIMEOUT) - when (val parsed = OtaResponse.parse(finalResponse)) { + // Every chunk was acknowledged with ACK but the terminal OK has not arrived (e.g. the device expected more + // bytes than we sent) — wait for the device's final verification result rather than reporting success. + when (val parsed = OtaResponse.parse(waitForResponse(VERIFICATION_TIMEOUT))) { is OtaResponse.Ok -> Unit - - is OtaResponse.Error -> { - if (parsed.message.contains("Hash Mismatch", ignoreCase = true)) { - throw OtaProtocolException.VerificationFailed("Firmware hash mismatch after transfer") - } - throw OtaProtocolException.TransferFailed("Verification failed: ${parsed.message}") - } - + is OtaResponse.Error -> throw transferException(parsed) else -> throw OtaProtocolException.TransferFailed("Expected OK after transfer, got: $parsed") } } + /** + * Maps a device `ERR` response to the appropriate transfer exception (hash mismatch is verification, else generic). + */ + private fun transferException(error: OtaResponse.Error): OtaProtocolException = + if (error.message.contains("Hash Mismatch", ignoreCase = true)) { + OtaProtocolException.VerificationFailed("Firmware hash mismatch after transfer") + } else { + OtaProtocolException.TransferFailed("Transfer failed: ${error.message}") + } + override suspend fun close() { bleConnection.disconnect() isConnected = false @@ -265,7 +278,9 @@ class BleOtaTransport( } private suspend fun writeData(data: ByteArray, writeType: BleWriteType): Int { - val maxLen = bleConnection.maximumWriteValueLength(writeType) ?: data.size + // takeIf { it > 0 }: a non-positive negotiated length would stall the loop (offset never advances); fall + // back to a single whole-buffer write instead of looping forever. + val maxLen = bleConnection.maximumWriteValueLength(writeType)?.takeIf { it > 0 } ?: data.size var offset = 0 var packetsSent = 0 @@ -298,8 +313,9 @@ class BleOtaTransport( 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 val SCAN_RETRY_DELAY = 2.seconds const val RECOMMENDED_CHUNK_SIZE = 512 + + /** Fallback write payload when the MTU has not been negotiated (23-byte ATT MTU minus the 3-byte header). */ + private const val SAFE_WRITE_PAYLOAD = 20 } } diff --git a/feature/firmware/src/commonMain/kotlin/org/meshtastic/feature/firmware/ota/dfu/DfuUploadTransport.kt b/feature/firmware/src/commonMain/kotlin/org/meshtastic/feature/firmware/ota/dfu/DfuUploadTransport.kt index aea70244e..a772165a2 100644 --- a/feature/firmware/src/commonMain/kotlin/org/meshtastic/feature/firmware/ota/dfu/DfuUploadTransport.kt +++ b/feature/firmware/src/commonMain/kotlin/org/meshtastic/feature/firmware/ota/dfu/DfuUploadTransport.kt @@ -24,6 +24,13 @@ package org.meshtastic.feature.firmware.ota.dfu * on which DFU service is exposed. */ interface DfuUploadTransport { + /** + * True once connected if the bootloader can only transfer in small (≤20-byte) blocks — i.e. it did not negotiate a + * larger ATT MTU. Used to surface a "slow bootloader" advisory to the user. Valid after [connectToDfuMode]. + */ + val isLowSpeedTransfer: Boolean + get() = false + /** Establish the GATT session with the device in DFU mode. */ suspend fun connectToDfuMode(): Result diff --git a/feature/firmware/src/commonMain/kotlin/org/meshtastic/feature/firmware/ota/dfu/LegacyDfuTransport.kt b/feature/firmware/src/commonMain/kotlin/org/meshtastic/feature/firmware/ota/dfu/LegacyDfuTransport.kt index e54fb157c..0444a57d7 100644 --- a/feature/firmware/src/commonMain/kotlin/org/meshtastic/feature/firmware/ota/dfu/LegacyDfuTransport.kt +++ b/feature/firmware/src/commonMain/kotlin/org/meshtastic/feature/firmware/ota/dfu/LegacyDfuTransport.kt @@ -269,33 +269,36 @@ class LegacyDfuTransport( * that will never complete. */ + /** + * Low-speed when the bootloader did not negotiate a larger MTU, leaving us on the 20-byte packet floor. Valid once + * [connectToDfuMode] has established the connection (the MTU is known by then). + */ + override val isLowSpeedTransfer: Boolean + get() = computeStreamPacketSize() <= LEGACY_DFU_PACKET_SIZE + /** * Determine the per-packet size for firmware streaming. * - * Returns [LEGACY_DFU_PACKET_SIZE] (20) by default for safety against stock Nordic / pre-2.1 OTAFIX bootloaders - * that overrun their flash buffer on larger writes. When the device advertises an OTAFIX-2.1+ name (`_DFU` - * suffix per https://github.com/oltaco/Adafruit_nRF52_Bootloader_OTAFIX#changes-in-otafix-21) and the connection - * has negotiated a larger ATT MTU, returns that MTU − 3 (ATT header overhead) capped at 244 bytes. + * Uses the negotiated ATT MTU − 3 (the largest `WITHOUT_RESPONSE` write the link allows), capped at + * [MAX_HIGH_MTU_PACKET_SIZE] and floored to a 4-byte boundary. This is **self-gating**: a bootloader that cannot + * accept large DFU writes never negotiates a large MTU, so we fall back to the 20-byte [LEGACY_DFU_PACKET_SIZE] + * floor. The advertised name (`AdaDFU` vs OTAFIX `_DFU`) is NOT used — the negotiated MTU is the direct capability + * signal. The Adafruit nRF52 bootloader's DFU data characteristic accepts up to MTU−3 = 244 bytes (it copies the + * actual write length into a 600-byte pool buffer), but **rejects any write whose length is not a multiple of 4** + * (`BLE_DFU_RESP_VAL_NOT_SUPPORTED`) — hence the word-alignment floor. */ private fun computeStreamPacketSize(): Int { - val name = dfuAdvertisedName.orEmpty() - val isOtafix21 = name.endsWith(OTAFIX_NAME_SUFFIX, ignoreCase = true) - if (!isOtafix21) return LEGACY_DFU_PACKET_SIZE val negotiated = bleConnection.maximumWriteValueLength(BleWriteType.WITHOUT_RESPONSE) ?: return LEGACY_DFU_PACKET_SIZE - return negotiated.coerceIn(LEGACY_DFU_PACKET_SIZE, MAX_HIGH_MTU_PACKET_SIZE) + val sized = negotiated.coerceIn(LEGACY_DFU_PACKET_SIZE, MAX_HIGH_MTU_PACKET_SIZE) + return sized - (sized % DFU_PACKET_WORD_ALIGNMENT) } @Suppress("CyclomaticComplexMethod", "NestedBlockDepth", "LongMethod") private suspend fun streamFirmware(firmware: ByteArray, onProgress: suspend (Float) -> Unit) { - // Default: 20-byte ATT packets (the only size accepted by stock Nordic / Adafruit pre-OTAFIX-2.1 - // bootloaders). Sending larger packets to those bootloaders overruns the flash-write buffer and - // silently bricks the device. See .agent_refs/Android-DFU-Library/.../BaseCustomDfuImpl.java:417. - // - // OTAFIX 2.1+ explicitly supports high-MTU packets ("Enables larger DFU packets for improved - // throughput when supported by the client" per the OTAFIX README). It re-uses the standard - // `_DFU` advertising-name suffix as a marker for the 2.1 feature set, so we opportunistically - // bump the packet size to the negotiated ATT MTU (minus 3 for the ATT header) when we see that name. + // Packet size = negotiated ATT MTU − 3, word-aligned and capped at 244 (see computeStreamPacketSize). Falls + // back to 20 bytes when the bootloader did not negotiate a larger MTU, which is the self-gating safety against + // bootloaders that can't accept large DFU writes — the 20-byte path is the slow but universally-safe default. val mtu = computeStreamPacketSize() var offset = 0 Logger.i { @@ -472,8 +475,6 @@ class LegacyDfuTransport( scanner = scanner, tag = "Legacy DFU", serviceUuid = LegacyDfuUuids.SERVICE, - retryCount = SCAN_RETRY_COUNT, - retryDelay = SCAN_RETRY_DELAY, predicate = predicate, ) @@ -487,8 +488,6 @@ class LegacyDfuTransport( private val START_RESPONSE_TIMEOUT = 30.seconds private val VALIDATE_TIMEOUT = 60.seconds private val SUBSCRIPTION_SETTLE = 500.milliseconds - private const val SCAN_RETRY_COUNT = 3 - private val SCAN_RETRY_DELAY = 2.seconds /** * Wall-clock budget for a full firmware streaming session. Must comfortably exceed the upload duration for the @@ -512,28 +511,21 @@ class LegacyDfuTransport( internal const val PRN_INTERVAL_PACKETS = 30 /** - * Default Legacy DFU packet size (20 bytes — the original ATT_MTU minus the 3-byte ATT header). - * - * Stock Nordic / pre-OTAFIX-2.1 bootloaders only support this size; sending larger packets to those bootloaders - * overruns the flash-write buffer and silently bricks the device. For OTAFIX 2.1+ bootloaders (detected via - * `OTAFIX_NAME_SUFFIX`), [computeStreamPacketSize] bumps the per-packet size up to the negotiated MTU (capped - * by [MAX_HIGH_MTU_PACKET_SIZE]) for a ~12× throughput win. + * Universally-safe Legacy DFU packet size (20 bytes — the original ATT_MTU minus the 3-byte ATT header). Used + * as the floor when the link did not negotiate a larger MTU; [computeStreamPacketSize] raises the per-packet + * size to the negotiated MTU (capped by [MAX_HIGH_MTU_PACKET_SIZE]) for a ~12× throughput win when available. */ internal const val LEGACY_DFU_PACKET_SIZE = 20 /** - * Cap on the high-MTU packet size used when an OTAFIX-2.1+ bootloader is detected. The largest ATT MTU the BLE - * 5.0 LE Data Length extension can give us is 247 bytes (244 of payload), and Adafruit's own flash accumulation - * buffer is 240 bytes, so capping at 244 keeps each write to one ATT PDU and one flash-write boundary. + * Cap on the high-MTU DFU packet size. The largest ATT MTU the BLE 5.0 LE Data Length extension gives us is 247 + * bytes (244 of payload); the Adafruit bootloader's DFU pool buffer (600 B) comfortably holds it, so 244 keeps + * each write to one ATT PDU. Stays a multiple of 4 — the bootloader rejects non-word-aligned writes. */ internal const val MAX_HIGH_MTU_PACKET_SIZE = 244 - /** - * Suffix used by the OTAFIX 2.1+ bootloader on every supported board's DFU-mode advertising name (e.g. - * `4631_DFU`, `T114_DFU`, `XIAO_DFU`). The 2.0 release advertised generic `AdaDFU`/board names without this - * suffix, so its presence is a reliable in-band signal that high-MTU is supported. - */ - internal const val OTAFIX_NAME_SUFFIX = "_DFU" + /** DFU data writes must be a whole number of 32-bit words; [computeStreamPacketSize] floors to this. */ + private const val DFU_PACKET_WORD_ALIGNMENT = 4 /** Minimum DFU Version we support; older bootloaders use the SDK ≤ 6 single-shot init flow. */ private const val MIN_SUPPORTED_DFU_VERSION = 5 diff --git a/feature/firmware/src/commonMain/kotlin/org/meshtastic/feature/firmware/ota/dfu/SecureDfuHandler.kt b/feature/firmware/src/commonMain/kotlin/org/meshtastic/feature/firmware/ota/dfu/SecureDfuHandler.kt index 47b544649..5a9bd8e29 100644 --- a/feature/firmware/src/commonMain/kotlin/org/meshtastic/feature/firmware/ota/dfu/SecureDfuHandler.kt +++ b/feature/firmware/src/commonMain/kotlin/org/meshtastic/feature/firmware/ota/dfu/SecureDfuHandler.kt @@ -38,6 +38,7 @@ import org.meshtastic.core.resources.firmware_update_downloading_percent import org.meshtastic.core.resources.firmware_update_enabling_dfu import org.meshtastic.core.resources.firmware_update_not_found_in_release import org.meshtastic.core.resources.firmware_update_ota_failed +import org.meshtastic.core.resources.firmware_update_slow_bootloader_hint import org.meshtastic.core.resources.firmware_update_starting_dfu import org.meshtastic.core.resources.firmware_update_uploading import org.meshtastic.core.resources.firmware_update_validating @@ -157,7 +158,15 @@ class SecureDfuHandler( // ── 7. Firmware ─────────────────────────────────────────────── val uploadMsg = UiText.Resource(Res.string.firmware_update_uploading) - updateState(FirmwareUpdateState.Updating(ProgressState(uploadMsg, 0f))) + // Surface a "slow bootloader" tip during the upload when the link couldn't negotiate a larger MTU + // (e.g. a stock Adafruit bootloader capped at MTU 23 → 20-byte packets, ~3.7 KB/s). + val slowHint = + if (transport.isLowSpeedTransfer) { + UiText.Resource(Res.string.firmware_update_slow_bootloader_hint) + } else { + null + } + updateState(FirmwareUpdateState.Updating(ProgressState(uploadMsg, 0f, hint = slowHint))) val firmwareSize = pkg.firmware.size val throughputTracker = ThroughputTracker() @@ -183,7 +192,11 @@ class SecureDfuHandler( } } - updateState(FirmwareUpdateState.Updating(ProgressState(uploadMsg, progress, details))) + updateState( + FirmwareUpdateState.Updating( + ProgressState(uploadMsg, progress, details, hint = slowHint), + ), + ) } .getOrThrow() diff --git a/feature/firmware/src/commonMain/kotlin/org/meshtastic/feature/firmware/ota/dfu/SecureDfuProtocol.kt b/feature/firmware/src/commonMain/kotlin/org/meshtastic/feature/firmware/ota/dfu/SecureDfuProtocol.kt index 9477acfdd..1f3f8e261 100644 --- a/feature/firmware/src/commonMain/kotlin/org/meshtastic/feature/firmware/ota/dfu/SecureDfuProtocol.kt +++ b/feature/firmware/src/commonMain/kotlin/org/meshtastic/feature/firmware/ota/dfu/SecureDfuProtocol.kt @@ -38,9 +38,6 @@ internal object SecureDfuUuids { /** Buttonless DFU – no bond required. Write 0x01 to reboot into DFU mode. */ val BUTTONLESS_NO_BONDS: Uuid = Uuid.parse("8EC90003-F315-4F60-9FB8-838830DAEA50") - - /** Buttonless DFU – bond required variant. */ - val BUTTONLESS_WITH_BONDS: Uuid = Uuid.parse("8EC90004-F315-4F60-9FB8-838830DAEA50") } /** diff --git a/feature/firmware/src/commonMain/kotlin/org/meshtastic/feature/firmware/ota/dfu/SecureDfuTransport.kt b/feature/firmware/src/commonMain/kotlin/org/meshtastic/feature/firmware/ota/dfu/SecureDfuTransport.kt index 3be3051dd..277e0c200 100644 --- a/feature/firmware/src/commonMain/kotlin/org/meshtastic/feature/firmware/ota/dfu/SecureDfuTransport.kt +++ b/feature/firmware/src/commonMain/kotlin/org/meshtastic/feature/firmware/ota/dfu/SecureDfuTransport.kt @@ -634,14 +634,8 @@ class SecureDfuTransport( // Scanning helpers // --------------------------------------------------------------------------- - private suspend fun scanForDevice(predicate: (BleDevice) -> Boolean): BleDevice? = scanForBleDevice( - scanner = scanner, - tag = "DFU", - serviceUuid = SecureDfuUuids.SERVICE, - retryCount = SCAN_RETRY_COUNT, - retryDelay = SCAN_RETRY_DELAY, - predicate = predicate, - ) + private suspend fun scanForDevice(predicate: (BleDevice) -> Boolean): BleDevice? = + scanForBleDevice(scanner = scanner, tag = "DFU", serviceUuid = SecureDfuUuids.SERVICE, predicate = predicate) // --------------------------------------------------------------------------- // Constants @@ -662,8 +656,6 @@ class SecureDfuTransport( * ([BUTTONLESS_RESPONSE_TIMEOUT]) plus settle/write overhead. */ private val TRIGGER_TIMEOUT = 5.seconds - private const val SCAN_RETRY_COUNT = 3 - private val SCAN_RETRY_DELAY = 2.seconds private val RETRY_DELAY = 2.seconds private val FIRST_CHUNK_DELAY = 400.milliseconds diff --git a/feature/firmware/src/commonTest/kotlin/org/meshtastic/feature/firmware/CommonFirmwareRetrieverTest.kt b/feature/firmware/src/commonTest/kotlin/org/meshtastic/feature/firmware/CommonFirmwareRetrieverTest.kt index e2705f553..9410dcaaa 100644 --- a/feature/firmware/src/commonTest/kotlin/org/meshtastic/feature/firmware/CommonFirmwareRetrieverTest.kt +++ b/feature/firmware/src/commonTest/kotlin/org/meshtastic/feature/firmware/CommonFirmwareRetrieverTest.kt @@ -106,17 +106,33 @@ abstract class CommonFirmwareRetrieverTest { } @Test - fun `retrieveEsp32Firmware falls back to legacy naming when current naming fails`() = runTest { + fun `retrieveEsp32Firmware resolves update bin when no manifest and no plain bin`() = runTest { val handler = FakeFirmwareFileHandler() val retriever = FirmwareRetriever(handler) - // No manifest, no current naming - // Legacy naming succeeds + // No manifest, no plain .bin; only the bare app `-update.bin` is published. handler.existingUrls.add("$BASE_URL/firmware-2.7.17/firmware-heltec-v3-2.7.17-update.bin") val result = retriever.retrieveEsp32Firmware(TEST_RELEASE, TEST_HARDWARE) {} - assertNotNull(result, "Should resolve firmware via legacy naming fallback") + assertNotNull(result, "Should resolve firmware via -update.bin") + assertEquals("firmware-heltec-v3-2.7.17-update.bin", result.fileName) + } + + @Test + fun `retrieveEsp32Firmware prefers update bin over plain bin when both exist`() = runTest { + val handler = FakeFirmwareFileHandler() + val retriever = FirmwareRetriever(handler) + + // No manifest. Both files exist: on pre-2.7.17 releases the plain `.bin` is a *merged* bootloader+app image + // (which esp_ota_end rejects when flashed to app0), while `-update.bin` is the bare app image. Confirmed on + // hardware with 2.7.15: the merged image failed `OTA End`, the -update.bin app image is the OTA-able one. + handler.existingUrls.add("$BASE_URL/firmware-2.7.17/firmware-heltec-v3-2.7.17.bin") + handler.existingUrls.add("$BASE_URL/firmware-2.7.17/firmware-heltec-v3-2.7.17-update.bin") + + val result = retriever.retrieveEsp32Firmware(TEST_RELEASE, TEST_HARDWARE) {} + + assertNotNull(result) assertEquals("firmware-heltec-v3-2.7.17-update.bin", result.fileName) } diff --git a/feature/firmware/src/commonTest/kotlin/org/meshtastic/feature/firmware/FirmwareManifestTest.kt b/feature/firmware/src/commonTest/kotlin/org/meshtastic/feature/firmware/FirmwareManifestTest.kt index dd75b4ef0..358f33e69 100644 --- a/feature/firmware/src/commonTest/kotlin/org/meshtastic/feature/firmware/FirmwareManifestTest.kt +++ b/feature/firmware/src/commonTest/kotlin/org/meshtastic/feature/firmware/FirmwareManifestTest.kt @@ -26,46 +26,42 @@ private val json = Json { ignoreUnknownKeys = true } class FirmwareManifestTest { + /** + * Regression for the manifest-never-parses bug: the real published `.mt.json` has `hwModel` as an **integer** (the + * HardwareModel enum value) plus assorted scalar/array metadata. A previous `hwModel: String` field threw + * `JsonDecodingException` on every real manifest, silently falling back to filename heuristics. The model now only + * consumes `files`, so the real shape must parse and resolve `app0`. + */ @Test - fun `deserialize full manifest with all fields`() { + fun `real published manifest shape parses and resolves app0`() { val raw = """ { - "hwModel": "HELTEC_V3", - "architecture": "esp32-s3", + "version": "2.7.25.104df5f", + "build_epoch": 1780963200, + "hwModel": 43, "platformioTarget": "heltec-v3", "mcu": "esp32s3", + "repo": "meshtastic/firmware", + "has_mui": false, "files": [ - { - "name": "firmware-heltec-v3-2.7.17.bin", - "part_name": "app0", - "md5": "abc123def456", - "bytes": 2097152 - }, - { - "name": "mt-esp32s3-ota.bin", - "part_name": "app1", - "md5": "789xyz", - "bytes": 636928 - }, - { - "name": "littlefs-heltec-v3-2.7.17.bin", - "part_name": "spiffs", - "md5": "000111", - "bytes": 1048576 - } + { "name": "firmware-heltec-v3-2.7.25.104df5f.elf", "md5": "a22e", "bytes": 22268744 }, + { "name": "firmware-heltec-v3-2.7.25.104df5f.bin", "md5": "6a45", "bytes": 2108864, "part_name": "app0" }, + { "name": "mt-esp32s3-ota.bin", "md5": "497d", "bytes": 636544, "part_name": "app1" } + ], + "part": [ + { "name": "app0", "type": "app", "subtype": "ota_0", "offset": "0x10000", "size": "0x330000" } ] } """ .trimIndent() val manifest = json.decodeFromString(raw) + val otaEntry = manifest.files.firstOrNull { it.partName == "app0" } - assertEquals("HELTEC_V3", manifest.hwModel) - assertEquals("esp32-s3", manifest.architecture) - assertEquals("heltec-v3", manifest.platformioTarget) - assertEquals("esp32s3", manifest.mcu) assertEquals(3, manifest.files.size) + assertEquals("firmware-heltec-v3-2.7.25.104df5f.bin", otaEntry?.name) + assertEquals(2108864L, otaEntry?.bytes) } @Test @@ -116,22 +112,18 @@ class FirmwareManifestTest { } @Test - fun `missing optional fields use defaults`() { + fun `missing files field uses default`() { val raw = """{}""" val manifest = json.decodeFromString(raw) - assertEquals("", manifest.hwModel) - assertEquals("", manifest.architecture) - assertEquals("", manifest.platformioTarget) - assertEquals("", manifest.mcu) assertTrue(manifest.files.isEmpty()) } @Test - fun `unknown keys are ignored`() { + fun `unknown keys including scalar metadata are ignored`() { val raw = """ { - "hwModel": "RAK4631", + "hwModel": 9, "unknown_field": "whatever", "files": [ { "name": "firmware.bin", "part_name": "app0", "extra": true } @@ -141,7 +133,6 @@ class FirmwareManifestTest { .trimIndent() val manifest = json.decodeFromString(raw) - assertEquals("RAK4631", manifest.hwModel) assertEquals(1, manifest.files.size) assertEquals("firmware.bin", manifest.files[0].name) } diff --git a/feature/firmware/src/commonTest/kotlin/org/meshtastic/feature/firmware/ota/BleOtaTransportTest.kt b/feature/firmware/src/commonTest/kotlin/org/meshtastic/feature/firmware/ota/BleOtaTransportTest.kt index ce2f69d91..d52d35e95 100644 --- a/feature/firmware/src/commonTest/kotlin/org/meshtastic/feature/firmware/ota/BleOtaTransportTest.kt +++ b/feature/firmware/src/commonTest/kotlin/org/meshtastic/feature/firmware/ota/BleOtaTransportTest.kt @@ -212,11 +212,8 @@ class BleOtaTransportTest { val progressValues = mutableListOf() val firmwareData = byteArrayOf(0x01, 0x02, 0x03, 0x04) - // For a 4-byte firmware with chunkSize=4 and maxWriteValueLength=512: - // 1 chunk → 1 packet → 1 ACK expected. - // Then the code checks if it's the last packet of the last chunk — - // if OK is received with isLastPacketOfChunk=true and nextSentBytes>=totalBytes, - // it returns early. + // 4-byte firmware, chunkSize 4, payload 512 → one chunk = one write expecting one response. + // The terminal OK on the last (only) chunk completes the transfer. emitResponse(connection, "OK") val result = transport.streamFirmware(firmwareData, 4) { progress -> progressValues.add(progress) } @@ -308,6 +305,123 @@ class BleOtaTransportTest { assertIs(result.exceptionOrNull()) } + @Test + fun `streamFirmware clamps chunk to negotiated write payload and acks once per chunk`() = runTest { + val scanner = FakeBleScanner() + val connection = FakeBleConnection() + val (transport) = createTransport(scanner, connection) + // Negotiated payload (200) smaller than the requested chunk (512) — the real MTU-512 case is payload 509. + connection.maxWriteValueLength = 200 + scanner.emitDevice(FakeBleDevice(address)) + transport.connect().getOrThrow() + + emitResponse(connection, "OK") + transport.startOta(600L, "hash") + + // The transport must clamp 512 → 200, yielding three 200-byte chunks, each ONE write expecting ONE + // response. Before the fix the 512-byte chunk fragmented into multiple writes and the loop waited for a + // response per fragment, desyncing against the device's one-response-per-chunk cadence (10s ACK-timeout hang). + emitResponse(connection, "ACK") + emitResponse(connection, "ACK") + emitResponse(connection, "OK") + + val progress = mutableListOf() + val result = transport.streamFirmware(ByteArray(600) { it.toByte() }, 512) { progress.add(it) } + + assertTrue(result.isSuccess, "streamFirmware failed: ${result.exceptionOrNull()}") + assertEquals(1.0f, progress.last()) + + val dataWrites = connection.service.writes.filter { it.writeType == BleWriteType.WITHOUT_RESPONSE } + assertEquals(3, dataWrites.size, "expected exactly one write per 200-byte chunk") + assertTrue(dataWrites.all { it.data.size <= 200 }, "no write may exceed the negotiated payload") + } + + @Test + fun `streamFirmware surfaces a final-chunk error instead of reporting success`() = runTest { + val scanner = FakeBleScanner() + val connection = FakeBleConnection() + val (transport) = createTransport(scanner, connection) + connection.maxWriteValueLength = 200 + scanner.emitDevice(FakeBleDevice(address)) + transport.connect().getOrThrow() + + emitResponse(connection, "OK") + transport.startOta(600L, "hash") + + // First two chunks ack; the device then rejects the final image hash. Must fail, never report success. + emitResponse(connection, "ACK") + emitResponse(connection, "ACK") + emitResponse(connection, "ERR Hash Mismatch") + + val result = transport.streamFirmware(ByteArray(600) { it.toByte() }, 512) {} + + assertTrue(result.isFailure) + assertIs(result.exceptionOrNull()) + } + + @Test + fun `streamFirmware succeeds when the last chunk is ACKed then a separate terminal OK arrives`() = runTest { + val scanner = FakeBleScanner() + val connection = FakeBleConnection() + val (transport) = createTransport(scanner, connection) + connectTransport(transport, scanner, connection) + + emitResponse(connection, "OK") + transport.startOta(8L, "hash") + + // Last chunk is ACKed (not OK); the device then sends a separate terminal OK. Exercises the post-loop + // verification wait, which must complete successfully. + emitResponse(connection, "ACK") + emitResponse(connection, "ACK") + emitResponse(connection, "OK") + + val result = transport.streamFirmware(ByteArray(8) { it.toByte() }, 4) {} + + assertTrue(result.isSuccess, "streamFirmware failed: ${result.exceptionOrNull()}") + } + + @Test + fun `streamFirmware fails when a terminal error arrives after the last chunk is ACKed`() = runTest { + val scanner = FakeBleScanner() + val connection = FakeBleConnection() + val (transport) = createTransport(scanner, connection) + connectTransport(transport, scanner, connection) + + emitResponse(connection, "OK") + transport.startOta(8L, "hash") + + // All chunks ACKed, then the device rejects the image post-transfer. Exercises the post-loop error branch — + // a late error must surface as failure, never success. + emitResponse(connection, "ACK") + emitResponse(connection, "ACK") + emitResponse(connection, "ERR Hash Mismatch") + + val result = transport.streamFirmware(ByteArray(8) { it.toByte() }, 4) {} + + assertTrue(result.isFailure) + assertIs(result.exceptionOrNull()) + } + + @Test + fun `streamFirmware fails when a non-final chunk receives OK`() = runTest { + val scanner = FakeBleScanner() + val connection = FakeBleConnection() + val (transport) = createTransport(scanner, connection) + connectTransport(transport, scanner, connection) + + emitResponse(connection, "OK") + transport.startOta(8L, "hash") + + // A premature OK on the first of two chunks: the device sends OK only at completion, so this signals a + // size disagreement and must fail rather than be treated as an ACK. + emitResponse(connection, "OK") + + val result = transport.streamFirmware(ByteArray(8) { it.toByte() }, 4) {} + + assertTrue(result.isFailure) + assertIs(result.exceptionOrNull()) + } + // ----------------------------------------------------------------------- // close() // ----------------------------------------------------------------------- diff --git a/feature/firmware/src/commonTest/kotlin/org/meshtastic/feature/firmware/ota/dfu/LegacyDfuTransportTest.kt b/feature/firmware/src/commonTest/kotlin/org/meshtastic/feature/firmware/ota/dfu/LegacyDfuTransportTest.kt index f56922fbf..cbb89aefe 100644 --- a/feature/firmware/src/commonTest/kotlin/org/meshtastic/feature/firmware/ota/dfu/LegacyDfuTransportTest.kt +++ b/feature/firmware/src/commonTest/kotlin/org/meshtastic/feature/firmware/ota/dfu/LegacyDfuTransportTest.kt @@ -216,6 +216,32 @@ class LegacyDfuTransportTest { env.packetWrites().forEach { assertEquals(BleWriteType.WITHOUT_RESPONSE, it.writeType) } } + @Test + fun `transferFirmware uses high-MTU packets when the link negotiates a larger MTU`() = runTest { + val env = createConnectedTransport(mtu = 244) + env.responder.scheme = LegacyResponderScheme.HappyPath + + env.transport.transferInitPacket(ByteArray(14) { it.toByte() }).getOrThrow() + val result = env.transport.transferFirmware(ByteArray(600) { it.toByte() }) {} + + assertTrue(result.isSuccess, "transferFirmware failed: ${result.exceptionOrNull()}") + // Firmware streams in 244-byte packets (negotiated MTU − 3) instead of the 20-byte default — the ~12× win. + assertEquals(244, env.packetWrites().maxOf { it.data.size }) + } + + @Test + fun `transferFirmware floors the packet size to a 4-byte boundary`() = runTest { + val env = createConnectedTransport(mtu = 242) // not a multiple of 4 + env.responder.scheme = LegacyResponderScheme.HappyPath + + env.transport.transferInitPacket(ByteArray(14) { it.toByte() }).getOrThrow() + val result = env.transport.transferFirmware(ByteArray(600) { it.toByte() }) {} + + assertTrue(result.isSuccess, "transferFirmware failed: ${result.exceptionOrNull()}") + // 242 → 240: the Adafruit DFU data path rejects writes whose length isn't a multiple of 4. + assertEquals(240, env.packetWrites().maxOf { it.data.size }) + } + // ----------------------------------------------------------------------- // Phase 4: error paths // ----------------------------------------------------------------------- diff --git a/workpad.md b/workpad.md deleted file mode 100644 index fee7c2969..000000000 --- a/workpad.md +++ /dev/null @@ -1,132 +0,0 @@ -# Craft Workpad: Live BT/Wi-Fi adapter-state detection - -> Generated by /craft · Started: 2026-06-18 · Branch: claude/gallant-thompson-d1b2ea (PR #5851) - ---- - -## Task - -Make the Bluetooth/Wi-Fi adapter-disabled detection **live** instead of `ON_RESUME`-polled — add a `BroadcastReceiver` for Bluetooth adapter state and a `ConnectivityManager.NetworkCallback` for network availability, so the Connections recovery banners update in real time (e.g. toggling BT from the quick-settings shade) rather than only when the activity resumes. - -Scope: `isBluetoothDisabled()` and `isWifiUnavailable()` in `core/ui/src/androidMain/.../util/PlatformUtils.kt`. Follow-up to the adapter-state feature added in commit 2c06a8019 on PR #5851. - ---- - -## Exploration Report - -### Key Facts - -- Both target functions live in `core/ui/src/androidMain/.../util/PlatformUtils.kt`: `isBluetoothDisabled()` (reads `BluetoothManager.adapter.isEnabled`) and `isWifiUnavailable()` (reads `ConnectivityManager.activeNetwork` transports). Both currently wrap their read in the private `rememberOnResumeState { ... }` helper — recomputed only on `Lifecycle.Event.ON_RESUME`. -- `rememberOnResumeState(check)` is the only consumer-shared refresh primitive; also used by `isGpsDisabled()`. Changing the two BT/Wi-Fi functions must NOT change `isGpsDisabled()` behavior (out of scope). -- **NetworkCallback pattern already exists** in `core/network/.../ConnectivityManager.kt`: `observeNetworks()` uses `callbackFlow { … registerNetworkCallback(req, cb); awaitClose { unregisterNetworkCallback(cb) } }` with `onAvailable`/`onLost`/`onCapabilitiesChanged`. Mirror it with a `DisposableEffect` in the composable (or `registerDefaultNetworkCallback` for the single active network). -- **BroadcastReceiver pattern already exists** in `core/ble/.../AndroidBluetoothRepository.kt`: registers via `ContextCompat.registerReceiver(context, receiver, filter, ContextCompat.RECEIVER_NOT_EXPORTED)` and `context.unregisterReceiver(...)`. For adapter state, the action is `BluetoothAdapter.ACTION_STATE_CHANGED`. -- The two functions are consumed only by `ConnectionsScreen.kt` (hoisted as `bluetoothDisabled`/`wifiUnavailable` booleans driving inline `RecoveryCard` banners + the BLE toggle routing). No other callers — the public contract (`@Composable expect fun … : Boolean`) is unchanged; only the androidMain implementation changes. -- `androidApp/src/main/AndroidManifest.xml` already declares Bluetooth + network permissions; `ACTION_STATE_CHANGED` and a `NetworkCallback` need no extra permission beyond what's declared (`ACCESS_NETWORK_STATE` is present for connectivity callbacks — verify). - -### Key Files - -- `core/ui/src/androidMain/kotlin/org/meshtastic/core/ui/util/PlatformUtils.kt` — the two functions + `rememberOnResumeState`; the only file that changes. -- `core/network/.../repository/ConnectivityManager.kt` — reference NetworkCallback/callbackFlow pattern. -- `core/ble/.../AndroidBluetoothRepository.kt` — reference registerReceiver/RECEIVER_NOT_EXPORTED pattern. -- `feature/connections/.../ui/ConnectionsScreen.kt` — sole consumer (no change needed; reads the same booleans, now updated live). -- jvm/ios actuals already return constant `false` — unaffected. - ---- - -## Requirements - -R1: `isBluetoothDisabled()` updates reactively — when the Bluetooth adapter is turned on/off (incl. from the quick-settings shade while the app is foregrounded), the returned value changes without waiting for `ON_RESUME`. - Source: human confirmed - Verification: review confirms a `BroadcastReceiver` on `BluetoothAdapter.ACTION_STATE_CHANGED` drives the state; manual toggle from shade flips the banner. - -R2: `isWifiUnavailable()` updates reactively — connecting/disconnecting Wi-Fi (or losing the active local network) changes the returned value live. - Source: human confirmed - Verification: review confirms a `ConnectivityManager` `NetworkCallback` (default-network) drives the state. - -R3: `ON_RESUME` polling is removed for these two functions; the receiver/callback is registered and unregistered with the composable's lifetime via `DisposableEffect` (no leaks). `isGpsDisabled()` continues to use `rememberOnResumeState` unchanged. - Source: human confirmed - Verification: `grep` shows no `rememberOnResumeState` in the two functions; `awaitClose`/`onDispose` unregisters; `isGpsDisabled` untouched. - -R4: Registration follows existing repo conventions — `ContextCompat.registerReceiver(..., RECEIVER_NOT_EXPORTED)` for the BT receiver; `registerDefaultNetworkCallback`/`unregisterNetworkCallback` for the network callback. No new permissions (ACCESS_NETWORK_STATE already declared). - Source: craft-clarify recommendation - Verification: review confirms the registration calls + flag matches `RECEIVER_NOT_EXPORTED`. - -R5: The public `expect` contract and all consumers are unchanged — `ConnectionsScreen` reads the same `Boolean`s, now live. jvm/ios actuals stay constant `false`. - Source: craft-clarify recommendation - Verification: no change to commonMain expect or jvm/iosMain; ConnectionsScreen diff empty; both flavors assemble. - ---- - -## Architectural Decision - -- **Chosen approach:** Extract a private `rememberObservedFlag(read, subscribe)` primitive (DisposableEffect + mutableStateOf, re-seed on registration); express `isBluetoothDisabled()` via a `BroadcastReceiver` on `ACTION_STATE_CHANGED` (RECEIVER_NOT_EXPORTED, main-thread delivery) and `isWifiUnavailable()` via `registerDefaultNetworkCallback(callback, mainHandler)`. -- **Approved:** 2026-06-18 -- **Adversarial:** P1-ish threading risk (NetworkCallback on background thread) mitigated by main-thread Handler; leaks prevented by onDispose unregister; cold-start staleness prevented by read() re-seed. No blocker. -- **Files:** `core/ui/src/androidMain/kotlin/org/meshtastic/core/ui/util/PlatformUtils.kt` only. -- **Test:** build both flavors + detekt/spotless; manual shade-toggle verification (no headless test feasible — `core/ui` has no instrumentation). - ---- - -## Acceptance Criteria - -AC1: [R1] `isBluetoothDisabled()` is driven by a `BroadcastReceiver` on `BluetoothAdapter.ACTION_STATE_CHANGED`. Auto-verify: grep ACTION_STATE_CHANGED + BroadcastReceiver in the function. -AC2: [R2] `isWifiUnavailable()` is driven by `registerDefaultNetworkCallback`. Auto-verify: grep registerDefaultNetworkCallback. -AC3: [R3] neither function uses `rememberOnResumeState`; observer torn down in `onDispose`; `isGpsDisabled()` still uses `rememberOnResumeState`. Auto-verify: grep. -AC4: [R4] BT receiver uses `RECEIVER_NOT_EXPORTED`; network callback uses a main-thread `Handler`. Auto-verify: grep. -AC5: [R5] commonMain expect + jvm/ios actuals + ConnectionsScreen unchanged; both flavors assemble; detekt/spotless clean. Auto-verify: git diff scope + build. - ---- - -## Implementation Plan - -**Branch:** claude/gallant-thompson-d1b2ea **Base commit:** 2c06a8019 - -### Task list - -### Pre-existing failures (do not fix — out of scope) - ---- - -## Completion Bar - -1. [x] All planned files created/modified -2. [x] Linter clean -3. [x] Tests pass (no new tests feasible; regressions green) -4. [x] Every AC has a completion note -5. [x] No open markers remain -6. [x] Scope discipline honored - ---- - -## Review - -Reviewer: concurrency (right-sized — single-file ~40-line change on established repo patterns; build+detekt covered the rest). - -Verdict: **SOUND.** Threading main-thread-confined (BT receiver no-Handler → main; NetworkCallback with main-Looper Handler); register/unregister balanced 1:1 via `DisposableEffect(Unit)`/`onDispose`; no leak or double-unregister; `read()` re-seed correct. - -Findings (both P3, no action): -- F-1 [P3] `rememberUpdatedState(subscribe)` freshness is never exercised (subscribe runs once). NOT removed: accessing `subscribe` directly inside `DisposableEffect` re-triggers the `LambdaParameterInRestartableEffect` detekt rule, so the wrapper is required for lint. `LocalContext` is stable, so no stale-context defect. Kept as-is. -- F-2 [P3] `registerDefaultNetworkCallback` `TooManyRequests` — structurally bounded (1:1 with a single live banner); no retry loop. No guard needed. - -### Acceptance criteria status -- AC1 ✓ BroadcastReceiver on ACTION_STATE_CHANGED drives isBluetoothDisabled. -- AC2 ✓ registerDefaultNetworkCallback drives isWifiUnavailable. -- AC3 ✓ neither uses rememberOnResumeState; onDispose unregisters; isGpsDisabled untouched. -- AC4 ✓ RECEIVER_NOT_EXPORTED + main-thread Handler. -- AC5 ✓ expect/jvm/ios/ConnectionsScreen unchanged; both flavors assemble; detekt/spotless clean. - ---- - -## Deferred Items - ---- - -## Phase Log - -- explore: done — 2026-06-18 — lean direct explore (deep prior context). Both reactive patterns already in repo (NetworkCallback callbackFlow in core/network; registerReceiver RECEIVER_NOT_EXPORTED in core/ble). Only androidMain PlatformUtils changes. -- clarify: done — 2026-06-18 — Q1 confirmed (replace ON_RESUME entirely). R1–R5 recorded. -- architect: done — 2026-06-18 — single approach (rememberObservedFlag primitive) + self-adversarial pass (threading via main Handler). Approved. -- implement: done — 2026-06-18 — one file (core/ui androidMain PlatformUtils). All 5 ACs met; build+detekt+spotless+both flavors green. -- review: done — 2026-06-18 — concurrency reviewer: SOUND. 2×P3 non-actionable (lint-required wrapper; bounded TooManyRequests). -- refine: done — 2026-06-18 — fast path, no actionable findings. P3s recorded as considered/declined. -- pr: done — 2026-06-18 — pushed to existing PR #5851 (174db32ae); no new PR (same branch/feature).