fix(firmware): batch of P3 OTA/DFU cleanups from the #5915 audit#5916
Merged
Conversation
Seven independent low-priority issues surfaced during the firmware-update audit + hardware testing that produced #5915. Each is a minimal, self-contained fix; no behavior change on the validated happy paths. - ESP32 BLE OTA handshake completes on an explicit OK / tolerates ERASING instead of counting fragments (latent low-MTU hang). - Route ESP32 OTA BLE-vs-WiFi on a strict MAC match so an IPv6 literal is no longer misclassified as a BLE address. - Show a proactive "unsupported on this connection" state for Serial+ESP32 / TCP+nRF52 instead of an Update button that only fails on press. - Verify downloaded ESP32 firmware against the manifest md5/bytes. - Warn (and document) that combined multi-image DFU zips flash only the primary image. - Fail immediately on empty firmware instead of waiting out the 10s verification timeout. - Downgrade the buttonless-DFU success-path WARN to debug and move the Forget+Re-pair guidance to the DFU-mode scan-failure path. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Low-priority (P3) follow-ups surfaced during the firmware-update audit and hardware testing that produced #5915 (which fixed the higher-severity issues). Each item below is independent and was implemented as the smallest self-contained change; the hardware-validated OTA/DFU happy paths are unchanged.
🐛 Bug Fixes
BleOtaTransport.startOtadrove handshake completion offresponsesReceived >= packetsSent(the same fragment-count model fix(firmware): harden ESP32 OTA + nRF DFU update paths (hardware-validated) #5915 removed fromstreamFirmware). Safe at the negotiated MTU 512 but latent at very low MTU. It now completes only on an explicitOKand tolerates interimERASING, exactly like the fixed streaming loop.Esp32OtaUpdateHandlersplit BLE-vs-WiFi ontarget.contains(":"), which misclassifies an IPv6 literal as a BLE MAC. Routing now uses a strict 6-octet MAC match (isBleMacAddress); IPv6/hostname targets correctly route to WiFi.FirmwareRetriever.resolveFromManifestlogged but never verified the downloaded.binagainst the manifest. It now checks size, then md5 (FirmwareHashUtil.calculateMd5Hex, pure-Okio so it stays incommonMain); a mismatch falls back to the filename heuristics. This surfaces a bad download before the disruptive reboot-into-OTA dance rather than failing device-side mid-flash.VERIFICATION_TIMEOUTbefore failing.streamFirmwarenow fails immediately withTransferFailed("Firmware is empty").🌟 UX
getHandlerthrows), yet the screen still rendered a working "Update" button (method =Unknown) that only failed on press. The ViewModel now maps TCP+non-ESP32 toUnknown, and the Ready screen shows a proactive "firmware updates are not supported over this connection" message instead of an actionable button. New stringfirmware_update_unsupported_transport.🧹 Chores / Hardening
DfuZipParsertransfers only the primary image (app > softdevice_bootloader > bootloader > softdevice). This is correct for app-only Meshtastic OTA zips, but a combined app+SD+BL package would silently flash only one. The limitation is now documented and guarded: the parser logs aWARNwhen a package declares more than one image. (Multi-image sequencing is intentionally out of scope.)SecureDfuTransport.triggerButtonlesslogged aWARNabout a stale bond on the normal success path (the WITH_RESPONSE write times out because the device reboots before the ATT ACK — expected and handled). Downgraded todebugand reworded; the Forget+Re-pair guidance moved toconnectToDfuMode's scan-failure error, where it actually indicates a problem.Testing Performed
Baseline:
./gradlew spotlessApply spotlessCheck detekt :feature:firmware:allTests— green (429 tests).Added/updated
commonTestcoverage:BleOtaTransportTest—streamFirmwarefails immediately on empty firmware (existingstartOtaOK/ERASING tests already cover the handshake change).Esp32OtaRoutingTest(new) —isBleMacAddressaccepts MACs and rejects IPv4, IPv6 literals, and hostnames.CommonFirmwareRetrieverTest— manifest artifact accepted on matching md5+size; rejected (and falls back) on md5 mismatch and on size mismatch.DfuZipParserTest— priority image is chosen when multiple images are present.FirmwareUpdateViewModelTest— TCP + nRF52 resolves toUnknown.