From a276cda62c5e80e7eb044503b71096b8997fa005 Mon Sep 17 00:00:00 2001
From: James Rich <2199651+jamesarich@users.noreply.github.com>
Date: Fri, 21 Aug 2026 15:15:57 +0000
Subject: [PATCH] fix(firmware): show the erase wait and upload retries during
Legacy DFU (#6812)
---
.skills/compose-ui/strings-index.txt | 2 +
.../composeResources/values/strings.xml | 2 +
.../firmware/ota/dfu/DfuUploadTransport.kt | 20 ++++
.../firmware/ota/dfu/LegacyDfuTransport.kt | 92 ++++++++++---------
.../firmware/ota/dfu/SecureDfuHandler.kt | 33 ++++++-
.../ota/dfu/DfuUploadPhaseStateTest.kt | 43 +++++++++
.../ota/dfu/LegacyDfuRetryPolicyTest.kt | 59 ++++++++++++
7 files changed, 208 insertions(+), 43 deletions(-)
create mode 100644 feature/firmware/src/commonTest/kotlin/org/meshtastic/feature/firmware/ota/dfu/DfuUploadPhaseStateTest.kt
diff --git a/.skills/compose-ui/strings-index.txt b/.skills/compose-ui/strings-index.txt
index 6939f63f35..d6710ec175 100644
--- a/.skills/compose-ui/strings-index.txt
+++ b/.skills/compose-ui/strings-index.txt
@@ -713,6 +713,7 @@ firmware_update_open
firmware_update_open_flasher
firmware_update_ota_failed
firmware_update_ota_unsupported_reason
+firmware_update_preparing_flash
firmware_update_rak4631_bootloader_hint
firmware_update_rebooting
firmware_update_release_notes
@@ -721,6 +722,7 @@ firmware_update_requires_ota_zip
firmware_update_requires_uf2
firmware_update_retrieval_failed
firmware_update_retry
+firmware_update_retrying_upload
firmware_update_save_dfu_file
firmware_update_searching_device
firmware_update_select_file
diff --git a/core/resources/src/commonMain/composeResources/values/strings.xml b/core/resources/src/commonMain/composeResources/values/strings.xml
index b2db0c0c3b..376170b353 100644
--- a/core/resources/src/commonMain/composeResources/values/strings.xml
+++ b/core/resources/src/commonMain/composeResources/values/strings.xml
@@ -743,6 +743,7 @@
Open Meshtastic Flasher
OTA update failed: %1$s
This device's OTA loader rejected the requested update method: %1$s
+ Preparing device: erasing flash. This can take up to a minute...
For RAK WisBlock RAK4631, the vendor's bootloader .zip has to be flashed with a serial DFU tool such as adafruit-nrfutil — copying that .zip to the device's drive won't work. Alternatively, connect this device over USB and use the bootloader upgrade in this app.
Rebooting to DFU...
Release Notes
@@ -751,6 +752,7 @@
%1$s requires a target-matching .uf2 file.
Could not retrieve firmware file.
Retry
+ Device did not respond. Retrying upload (attempt %1$d/%2$d)...
Please save the .uf2 file to your device's DFU drive.
Searching for OTA device on the network...
Select Local File
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 9b8d381727..35cd67f4f2 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
@@ -43,6 +43,17 @@ interface DfuUploadTransport {
*/
suspend fun transferFirmware(firmware: ByteArray, onProgress: suspend (Float) -> Unit): Result
+ /**
+ * As [transferFirmware], additionally reporting coarse [DfuUploadPhase] transitions so the UI can explain a wait
+ * that produces no progress (Legacy DFU erases the whole application region before it accepts a single byte).
+ * Transports without a meaningful prepare step keep the default, which never reports a phase.
+ */
+ suspend fun transferFirmware(
+ firmware: ByteArray,
+ onPhase: suspend (DfuUploadPhase) -> Unit,
+ onProgress: suspend (Float) -> Unit,
+ ): Result = transferFirmware(firmware, onProgress)
+
/**
* Best-effort abort. Operational transport exceptions are swallowed; structured-concurrency cancellation and Error
* subtypes propagate.
@@ -52,3 +63,12 @@ interface DfuUploadTransport {
/** Disconnect and release resources. */
suspend fun close()
}
+
+/** Coarse phases inside [DfuUploadTransport.transferFirmware], for UI that must explain a silent wait. */
+enum class DfuUploadPhase {
+ /** Start request sent; the bootloader is erasing flash and will not accept data until it has finished. */
+ PREPARING,
+
+ /** The bootloader accepted the start; firmware bytes are streaming and progress callbacks follow. */
+ STREAMING,
+}
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 765de1b025..3bca01469c 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
@@ -237,58 +237,68 @@ internal constructor(
* acknowledgement; treat that operational Exception as expected success (structured cancellation and Error
* subtypes still propagate).
*/
- @Suppress("LongMethod")
override suspend fun transferFirmware(firmware: ByteArray, onProgress: suspend (Float) -> Unit): Result =
- safeCatching {
- val initPacket =
- pendingInitPacket
- ?: throw DfuException.TransferFailed("transferInitPacket must be called before transferFirmware")
- Logger.i { "Legacy DFU: Starting upload (init=${initPacket.size}B, firmware=${firmware.size}B)..." }
+ transferFirmware(firmware, onPhase = {}, onProgress = onProgress)
- // ── 1. START_DFU + image sizes on Packet, then response ─────────────
- writeControlPoint(byteArrayOf(LegacyDfuOpcode.START_DFU, LegacyDfuImageType.APPLICATION))
- writePacket(legacyImageSizesPayload(appSize = firmware.size))
- handleStartResponse(awaitResponse(START_RESPONSE_TIMEOUT))
+ @Suppress("LongMethod")
+ override suspend fun transferFirmware(
+ firmware: ByteArray,
+ onPhase: suspend (DfuUploadPhase) -> Unit,
+ onProgress: suspend (Float) -> Unit,
+ ): Result = safeCatching {
+ val initPacket =
+ pendingInitPacket
+ ?: throw DfuException.TransferFailed("transferInitPacket must be called before transferFirmware")
+ Logger.i { "Legacy DFU: Starting upload (init=${initPacket.size}B, firmware=${firmware.size}B)..." }
- // ── 2. INIT_PARAMS_START → init bytes on Packet → INIT_PARAMS_COMPLETE → response ──
- writeControlPoint(byteArrayOf(LegacyDfuOpcode.INIT_DFU_PARAMS, LegacyDfuOpcode.INIT_PARAMS_START))
- writePacketChunked(initPacket)
- writeControlPoint(byteArrayOf(LegacyDfuOpcode.INIT_DFU_PARAMS, LegacyDfuOpcode.INIT_PARAMS_COMPLETE))
- requireSuccess(LegacyDfuOpcode.INIT_DFU_PARAMS, awaitResponse(COMMAND_TIMEOUT))
+ // ── 1. START_DFU + image sizes on Packet, then response ─────────────
+ // The bootloader erases the application region before it answers; without lazy erase that is tens of
+ // seconds of silence, so tell the UI what the wait is.
+ onPhase(DfuUploadPhase.PREPARING)
+ writeControlPoint(byteArrayOf(LegacyDfuOpcode.START_DFU, LegacyDfuImageType.APPLICATION))
+ writePacket(legacyImageSizesPayload(appSize = firmware.size))
+ handleStartResponse(awaitResponse(START_RESPONSE_TIMEOUT))
- // Bump the BLE link to high-throughput mode (~7.5 ms interval) before streaming.
- // Default Android intervals (~30-50 ms) starve the link during sustained DFU and trigger LSTO. Mirrors
- // Nordic LegacyDfuImpl.java requestConnectionPriority(CONNECTION_PRIORITY_HIGH).
- val highPriorityRequested = bleConnection.requestHighConnectionPriority()
- Logger.i { "Legacy DFU: requestHighConnectionPriority -> $highPriorityRequested" }
+ // ── 2. INIT_PARAMS_START → init bytes on Packet → INIT_PARAMS_COMPLETE → response ──
+ writeControlPoint(byteArrayOf(LegacyDfuOpcode.INIT_DFU_PARAMS, LegacyDfuOpcode.INIT_PARAMS_START))
+ writePacketChunked(initPacket)
+ writeControlPoint(byteArrayOf(LegacyDfuOpcode.INIT_DFU_PARAMS, LegacyDfuOpcode.INIT_PARAMS_COMPLETE))
+ requireSuccess(LegacyDfuOpcode.INIT_DFU_PARAMS, awaitResponse(COMMAND_TIMEOUT))
- // ── 3. PRN setup ────────────────────────────────────────────────────
- writeControlPoint(legacyPrnRequestPayload(streamProfile.prnIntervalPackets))
+ // Bump the BLE link to high-throughput mode (~7.5 ms interval) before streaming.
+ // Default Android intervals (~30-50 ms) starve the link during sustained DFU and trigger LSTO. Mirrors
+ // Nordic LegacyDfuImpl.java requestConnectionPriority(CONNECTION_PRIORITY_HIGH).
+ val highPriorityRequested = bleConnection.requestHighConnectionPriority()
+ Logger.i { "Legacy DFU: requestHighConnectionPriority -> $highPriorityRequested" }
- // ── 4. RECEIVE_FIRMWARE_IMAGE ──────────────────────────────────────
- writeControlPoint(byteArrayOf(LegacyDfuOpcode.RECEIVE_FIRMWARE_IMAGE))
+ // ── 3. PRN setup ────────────────────────────────────────────────────
+ writeControlPoint(legacyPrnRequestPayload(streamProfile.prnIntervalPackets))
- // ── 5. Stream firmware ─────────────────────────────────────────────
- streamFirmware(firmware, onProgress)
+ // ── 4. RECEIVE_FIRMWARE_IMAGE ──────────────────────────────────────
+ writeControlPoint(byteArrayOf(LegacyDfuOpcode.RECEIVE_FIRMWARE_IMAGE))
- // ── 6. Final RECEIVE_FIRMWARE_IMAGE response ────────────────────────
- requireSuccess(LegacyDfuOpcode.RECEIVE_FIRMWARE_IMAGE, awaitResponse(VALIDATE_TIMEOUT))
+ // ── 5. Stream firmware ─────────────────────────────────────────────
+ onPhase(DfuUploadPhase.STREAMING)
+ streamFirmware(firmware, onProgress)
- // ── 7. VALIDATE ────────────────────────────────────────────────────
- writeControlPoint(byteArrayOf(LegacyDfuOpcode.VALIDATE))
- requireSuccess(LegacyDfuOpcode.VALIDATE, awaitResponse(VALIDATE_TIMEOUT))
+ // ── 6. Final RECEIVE_FIRMWARE_IMAGE response ────────────────────────
+ requireSuccess(LegacyDfuOpcode.RECEIVE_FIRMWARE_IMAGE, awaitResponse(VALIDATE_TIMEOUT))
- // ── 8. ACTIVATE_AND_RESET ──────────────────────────────────────────
- // The device may reset before the GATT write ACK lands; an ordinary disconnect/write Exception is expected
- // because of that reset — safeCatching treats it as success. Structured cancellation and Error subtypes
- // still propagate.
- Logger.i { "Legacy DFU: Sending ACTIVATE_AND_RESET (disconnect during write is expected)" }
- safeCatching { writeControlPoint(byteArrayOf(LegacyDfuOpcode.ACTIVATE_AND_RESET)) }
- .onFailure { Logger.i(it) { "Legacy DFU: ACTIVATE write reported failure (expected on reset)" } }
+ // ── 7. VALIDATE ────────────────────────────────────────────────────
+ writeControlPoint(byteArrayOf(LegacyDfuOpcode.VALIDATE))
+ requireSuccess(LegacyDfuOpcode.VALIDATE, awaitResponse(VALIDATE_TIMEOUT))
- onProgress(1f)
- Logger.i { "Legacy DFU: Upload complete, device rebooting into new firmware." }
- }
+ // ── 8. ACTIVATE_AND_RESET ──────────────────────────────────────────
+ // The device may reset before the GATT write ACK lands; an ordinary disconnect/write Exception is expected
+ // because of that reset — safeCatching treats it as success. Structured cancellation and Error subtypes
+ // still propagate.
+ Logger.i { "Legacy DFU: Sending ACTIVATE_AND_RESET (disconnect during write is expected)" }
+ safeCatching { writeControlPoint(byteArrayOf(LegacyDfuOpcode.ACTIVATE_AND_RESET)) }
+ .onFailure { Logger.i(it) { "Legacy DFU: ACTIVATE write reported failure (expected on reset)" } }
+
+ onProgress(1f)
+ Logger.i { "Legacy DFU: Upload complete, device rebooting into new firmware." }
+ }
/**
* Low-speed when the bootloader did not negotiate a larger MTU, leaving us on the 20-byte packet floor. Valid once
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 1e35ed46ea..cc8ce67b23 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
@@ -39,6 +39,8 @@ 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_preparing_flash
+import org.meshtastic.core.resources.firmware_update_retrying_upload
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
@@ -252,6 +254,19 @@ private fun DfuProtocolKind.serviceUuid(): Uuid = when (this) {
DfuProtocolKind.SECURE -> SecureDfuUuids.SERVICE
}
+/**
+ * UI state for a [DfuUploadPhase] reported by the transport during the firmware transfer. PREPARING is a silent wait
+ * the user would otherwise read as a hang, so it is a [FirmwareUpdateState.Processing] with its own copy rather than an
+ * "Uploading 0%" that never moves.
+ */
+internal fun dfuUploadPhaseState(phase: DfuUploadPhase, uploadMsg: UiText, slowHint: UiText?): FirmwareUpdateState =
+ when (phase) {
+ DfuUploadPhase.PREPARING ->
+ FirmwareUpdateState.Processing(ProgressState(UiText.Resource(Res.string.firmware_update_preparing_flash)))
+
+ DfuUploadPhase.STREAMING -> FirmwareUpdateState.Updating(ProgressState(uploadMsg, 0f, hint = slowHint))
+ }
+
/**
* Drives the bounded Legacy/Secure DFU upload retry loop. Extracted from [SecureDfuHandler.runDfuUploadWithRetry] so
* the retry policy can be unit-tested without bringing up the full BLE stack — callers supply a [runUploadSession]
@@ -283,6 +298,7 @@ internal suspend fun runDfuRetryLoop(
runUploadSession: suspend (LegacyDfuStreamProfile) -> DfuUploadResult,
resetStaleBootloader: suspend () -> Unit,
interAttemptDelay: suspend () -> Unit,
+ onRetryScheduled: (nextAttempt: Int, totalAttempts: Int) -> Unit = { _, _ -> },
): DfuUploadResult {
var uploadAttempts = 0
var staleResets = 0
@@ -375,7 +391,10 @@ internal suspend fun runDfuRetryLoop(
}
}
- if (uploadAttempts < activeAttempts) interAttemptDelay()
+ if (uploadAttempts < activeAttempts) {
+ onRetryScheduled(uploadAttempts + 1, activeAttempts)
+ interAttemptDelay()
+ }
}
}
}
@@ -638,6 +657,13 @@ class SecureDfuHandler(
runUploadSession = { profile -> runUploadSession(protocol, target, pkg, profile, updateState) },
resetStaleBootloader = { resetStaleBootloader(protocol, target) },
interAttemptDelay = { delay(SESSION_RETRY_DELAY_MS) },
+ onRetryScheduled = { next, total ->
+ updateState(
+ FirmwareUpdateState.Processing(
+ ProgressState(UiText.Resource(Res.string.firmware_update_retrying_upload, next, total)),
+ ),
+ )
+ },
)
private fun createTransport(
@@ -714,7 +740,10 @@ class SecureDfuHandler(
val firmwareSize = pkg.firmware.size
val throughputTracker = ThroughputTracker()
transport
- .transferFirmware(pkg.firmware) { progress ->
+ .transferFirmware(
+ pkg.firmware,
+ onPhase = { phase -> updateState(dfuUploadPhaseState(phase, uploadMsg, slowHint)) },
+ ) { progress ->
val bytesSent = (progress * firmwareSize).toLong()
throughputTracker.record(bytesSent)
val details = formatTransferProgress(progress, firmwareSize, throughputTracker.bytesPerSecond())
diff --git a/feature/firmware/src/commonTest/kotlin/org/meshtastic/feature/firmware/ota/dfu/DfuUploadPhaseStateTest.kt b/feature/firmware/src/commonTest/kotlin/org/meshtastic/feature/firmware/ota/dfu/DfuUploadPhaseStateTest.kt
new file mode 100644
index 0000000000..1029db159c
--- /dev/null
+++ b/feature/firmware/src/commonTest/kotlin/org/meshtastic/feature/firmware/ota/dfu/DfuUploadPhaseStateTest.kt
@@ -0,0 +1,43 @@
+/*
+ * Copyright (c) 2026 Meshtastic LLC
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program. If not, see .
+ */
+package org.meshtastic.feature.firmware.ota.dfu
+
+import org.meshtastic.core.resources.UiText
+import org.meshtastic.feature.firmware.FirmwareUpdateState
+import kotlin.test.Test
+import kotlin.test.assertEquals
+import kotlin.test.assertIs
+
+class DfuUploadPhaseStateTest {
+ private val uploadMsg = UiText.DynamicString("Uploading firmware...")
+ private val hint = UiText.DynamicString("slow bootloader")
+
+ @Test
+ fun `PREPARING is a Processing state rather than Uploading at zero`() {
+ val state = dfuUploadPhaseState(DfuUploadPhase.PREPARING, uploadMsg, hint)
+ assertIs(state)
+ }
+
+ @Test
+ fun `STREAMING returns to Uploading at zero with the slow hint preserved`() {
+ val state = dfuUploadPhaseState(DfuUploadPhase.STREAMING, uploadMsg, hint)
+ assertIs(state)
+ assertEquals(uploadMsg, state.progressState.message)
+ assertEquals(0f, state.progressState.progress)
+ assertEquals(hint, state.progressState.hint)
+ }
+}
diff --git a/feature/firmware/src/commonTest/kotlin/org/meshtastic/feature/firmware/ota/dfu/LegacyDfuRetryPolicyTest.kt b/feature/firmware/src/commonTest/kotlin/org/meshtastic/feature/firmware/ota/dfu/LegacyDfuRetryPolicyTest.kt
index b1f142ae42..f4772d756c 100644
--- a/feature/firmware/src/commonTest/kotlin/org/meshtastic/feature/firmware/ota/dfu/LegacyDfuRetryPolicyTest.kt
+++ b/feature/firmware/src/commonTest/kotlin/org/meshtastic/feature/firmware/ota/dfu/LegacyDfuRetryPolicyTest.kt
@@ -195,6 +195,65 @@ class LegacyDfuRetryPolicyTest {
assertEquals(0, staleResets, "ordinary failures must NOT trigger stale cleanup")
}
+ /**
+ * Every retry that is actually going to happen is announced (next attempt number, total) before the inter-attempt
+ * delay, and the exhausted last attempt is not — the UI must never promise a retry that will not come.
+ */
+ @Test
+ fun `retries are announced before the delay and not after the last attempt`() = runTest {
+ val announced = mutableListOf>()
+ val order = mutableListOf()
+
+ val outcome =
+ runDfuRetryLoop(
+ protocol = DfuProtocolKind.LEGACY,
+ budget = DfuAttemptBudget(LEGACY_SESSION_ATTEMPTS, LEGACY_SESSION_ATTEMPTS),
+ maxStaleResets = MAX_LEGACY_STALE_RESETS,
+ runUploadSession = {
+ order.add("upload")
+ DfuUploadResult.Failure(DfuException.TransferFailed("ordinary failure"), false)
+ },
+ resetStaleBootloader = {},
+ interAttemptDelay = { order.add("delay") },
+ onRetryScheduled = { next, total ->
+ order.add("announce")
+ announced.add(next to total)
+ },
+ )
+
+ assertIs(outcome)
+ assertEquals(listOf(2 to 3, 3 to 3), announced)
+ assertEquals(listOf("upload", "announce", "delay", "upload", "announce", "delay", "upload"), order)
+ }
+
+ /** A stale-session cleanup is not a retry of the upload and must not be announced as one. */
+ @Test
+ fun `stale cleanup is not announced as a retry`() = runTest {
+ var announcements = 0
+ var sessions = 0
+
+ val outcome =
+ runDfuRetryLoop(
+ protocol = DfuProtocolKind.LEGACY,
+ budget = DfuAttemptBudget(LEGACY_SESSION_ATTEMPTS, LEGACY_SESSION_ATTEMPTS),
+ maxStaleResets = MAX_LEGACY_STALE_RESETS,
+ runUploadSession = {
+ sessions++
+ if (sessions == 1) {
+ DfuUploadResult.Failure(LegacyDfuException.StaleSessionReset(), true)
+ } else {
+ DfuUploadResult.Success
+ }
+ },
+ resetStaleBootloader = {},
+ interAttemptDelay = {},
+ onRetryScheduled = { _, _ -> announcements++ },
+ )
+
+ assertEquals(DfuUploadResult.Success, outcome)
+ assertEquals(0, announcements)
+ }
+
/**
* A StaleSessionReset must NOT consume an upload attempt — the cleanup cycle never tried to upload. After the stale
* cleanup, the same upload-attempt budget remains, and a subsequent successful attempt succeeds.