mirror of
https://github.com/meshtastic/Meshtastic-Android.git
synced 2026-06-25 22:15:33 -04:00
fix(firmware): harden ESP32 OTA + nRF DFU update paths (hardware-validated) (#5915)
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
James Rich
4
.gitignore
vendored
4
.gitignore
vendored
@@ -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
|
||||
|
||||
1
.skills/compose-ui/strings-index.txt
generated
1
.skills/compose-ui/strings-index.txt
generated
@@ -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
|
||||
|
||||
@@ -631,6 +631,7 @@
|
||||
<string name="firmware_update_retry">Retry</string>
|
||||
<string name="firmware_update_save_dfu_file">Please save the .uf2 file to your device's DFU drive.</string>
|
||||
<string name="firmware_update_select_file">Select Local File</string>
|
||||
<string name="firmware_update_slow_bootloader_hint">Updates are slow on this device's bootloader. Flashing the OTAFIX bootloader enables much faster BLE updates.</string>
|
||||
<string name="firmware_update_source_local">Source: Local File</string>
|
||||
<string name="firmware_update_stable">Stable</string>
|
||||
<string name="firmware_update_starting_dfu">Starting DFU...</string>
|
||||
|
||||
@@ -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<FirmwareManifestFile> = emptyList(),
|
||||
)
|
||||
@Serializable internal data class FirmwareManifest(val files: List<FirmwareManifestFile> = emptyList())
|
||||
|
||||
/**
|
||||
* A single partition file entry inside a [FirmwareManifest].
|
||||
|
||||
@@ -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-<target>-<version>.bin`)
|
||||
* 3. Legacy naming (`firmware-<target>-<version>-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-<target>-<version>-update.bin` — the bare app image. Preferred over the plain `.bin` because on
|
||||
* pre-2.7.17 releases (which ship no manifest) `firmware-<target>-<version>.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-<target>-<version>.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-<target>-<version>.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,
|
||||
|
||||
@@ -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) {
|
||||
|
||||
@@ -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]. */
|
||||
|
||||
@@ -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<String>(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
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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<Unit>
|
||||
|
||||
|
||||
@@ -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 (`<board>_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
|
||||
// `<board>_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
|
||||
|
||||
@@ -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()
|
||||
|
||||
|
||||
@@ -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")
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
@@ -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
|
||||
|
||||
|
||||
@@ -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)
|
||||
}
|
||||
|
||||
|
||||
@@ -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<FirmwareManifest>(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<FirmwareManifest>(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<FirmwareManifest>(raw)
|
||||
assertEquals("RAK4631", manifest.hwModel)
|
||||
assertEquals(1, manifest.files.size)
|
||||
assertEquals("firmware.bin", manifest.files[0].name)
|
||||
}
|
||||
|
||||
@@ -212,11 +212,8 @@ class BleOtaTransportTest {
|
||||
val progressValues = mutableListOf<Float>()
|
||||
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<OtaProtocolException.TransferFailed>(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<Float>()
|
||||
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<OtaProtocolException.VerificationFailed>(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<OtaProtocolException.VerificationFailed>(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<OtaProtocolException.TransferFailed>(result.exceptionOrNull())
|
||||
}
|
||||
|
||||
// -----------------------------------------------------------------------
|
||||
// close()
|
||||
// -----------------------------------------------------------------------
|
||||
|
||||
@@ -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
|
||||
// -----------------------------------------------------------------------
|
||||
|
||||
132
workpad.md
132
workpad.md
@@ -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).
|
||||
Reference in New Issue
Block a user