fix: classify bare metal from the flash descriptor when no ROM-only alt is listed - #198
fix: classify bare metal from the flash descriptor when no ROM-only alt is listed#198boringethan wants to merge 2 commits into
Conversation
…lt is listed A unit whose `dfu-util -l` output carried only the `@Internal Flash` alt was reported as BootMode.UNKNOWN, so FirmwareUpdater.update() refused it with "could not tell whether this device has the bootloader installed". Consoles running firmware older than OW_CMD_BOOT_INFO have no other way to be classified, which left them unflashable. parse_boot_mode() recognised bare metal only via the ROM-only alt settings (@option Bytes / @otp Memory / @device Feature). Those are still checked first, but they are no longer required: failing that, the Internal Flash descriptor at 0x08000000 now decides, from its DfuSe access letters. That is positive evidence, not a guess. Both bootloaders hard-code a descriptor that marks sector 0 and the reserved/config region read-only, because that clamp is the security property they exist to enforce: openmotion-bl 01*128Ka,04*128Kg,11*128Ka open-motion-console-bl 01*128Ka,08*128Kg,07*128Ka So a descriptor with no read-only run cannot be a bootloader, and the ROM loader is the only other thing it can be. A descriptor with no parseable runs, or one at some other address, still reports UNKNOWN. This intentionally overturns test_single_fully_writable_alt_is_unknown_not_ bootloader, which asserted the conservative UNKNOWN before the invariant above was verified against both bootloader sources; it now asserts BARE_METAL with the reasoning recorded. The existing update-flow tests stub detect_boot_mode() outright and so could never have caught this, so the new coverage drives a real listing through the genuine detect_boot_mode -> parse_boot_mode chain — the path the apps use. Refs #197 Co-Authored-By: Claude Opus 5 <[email protected]>
Hardware verification on a genuine pre-boot-info console — and an important limitation
Confirmed it was a genuine old unit: Then drove the real production path — Console came back on 1.8.1-rc.2, and The limitation — this run does NOT exercise the new code pathThe listing detection actually classified carried both alts: So it was classified by the pre-existing ROM-only marker, not the new descriptor fallback. Running both parsers over that captured listing:
This bench console does not reproduce #197. Its ROM loader always reports the Option Bytes alt, so DFU detection worked here before this change and still does. What that means for review
|
Attempted reproduction through the test app — the reported symptom does NOT reproduceStaged the exact reported scenario end to end:
App log: The update completed normally on the unfixed SDK. No That matches the parser comparison posted earlier: for this hardware's listing, old and new agree on What this means
RecommendationJudge this PR purely as defensive hardening of Reopening the question of what actually caused the original refusal. The things that would pin it: the verbatim error text, the |
FirmwareUpdater.update() raised "could not tell whether this device has the bootloader installed; refusing to flash, because the wrong address would brick it" whenever detect_boot_mode() came back UNKNOWN. That is the error reported from the field, and it strands the device: there is no way to flash it at all, so the unit is stuck on whatever firmware it already has. Fall back to bare metal instead. This is safe because the hardware enforces it, not because we are confident in the guess: both bootloaders clamp their DFU write window to the application slot and mark sector 0 read-only openmotion-bl 01*128Ka,04*128Kg,11*128Ka open-motion-console-bl 01*128Ka,08*128Kg,07*128Ka so a bare-metal write at 0x08000000 against a bootloader unit is rejected by the bootloader and fails loudly rather than bricking. The opposite default has no such backstop: a signed image at 0x08020000 writes happily into the middle of a bare-metal device's flash and leaves it unbootable. last_boot_mode still reports what was actually detected (UNKNOWN), so neither callers nor the apps' lock indicator can mistake the fallback for an observation, and the fallback is logged as a warning. install_bootloader() is deliberately unchanged and still aborts on UNKNOWN. Converting a device is irreversible over USB, so an assumption is not acceptable there. Refs #197 Co-Authored-By: Claude Opus 5 <[email protected]>
The reported error is confirmed, and this PR now actually addresses itThe verbatim message from the failing unit is this SDK's own
So The stale SDK is not the causeThe failing unit runs test-app New commit: fall back to bare metal instead of refusingRefusing strands the device — there is no way to flash it at all, so it stays on whatever firmware it has. This is safe because the hardware enforces it, not because the guess is confident. Both bootloaders clamp the DFU write window to the application slot and mark sector 0 read-only, so a bare-metal write at Guard rails kept:
Scope of the PR nowTwo layers, in order:
Full software suite: 771 passed, 207 deselected. Still openWhat made that machine's listing unclassifiable is not yet known — the fallback makes it flashable either way, but the underlying cause is worth finding. |
Root cause CONFIRMED: a USB driver problem on the failing hostConfirmed by the reporter. The console was in DFU, but
The code path that turned an unreadable device into a brick warning
if self.vidpid and self.vidpid.lower() in out.lower():
return True
Demonstrated against the real functions:
Only the last row produces the correct "DFU device did not appear" error. The first three are the trap. What this means for the two commits hereNeither commit fixes the reported failure, and this should not be merged as though it does:
Both remain defensible on their own merits. The actual fix for this class of failure is separate: require a parseable |
Problem
Flashing a console running firmware older than
OW_CMD_BOOT_INFO(console FW <= 1.8.0) could fail with:The unit is an ordinary bare-metal device, but it cannot be updated at all. Old firmware has no other way to be classified — the boot-info command is the only non-DFU route, and it does not exist there (
0x0Bis unassigned in the 1.8.0 command enum, which jumps0x0A -> 0x0D).Root cause
parse_boot_mode()recognised bare metal only via the ROM-only alt settings (@Option Bytes,@OTP Memory,@Device Feature). Whendfu-util -lreports just the@Internal Flashalt, none of those markers are present and classification falls through toUNKNOWN:Deterministic, not a race. How many alts the ROM loader reports varies — a bench console showed two, not the four the module documented.
Fix
The ROM-only alts are still checked first, but are no longer required. Failing that, the
@Internal Flashalt at0x08000000is classified from its DfuSe access letters:a) runBOOTLOADERBOOTLOADER(unchanged)UNKNOWNBARE_METALUNKNOWNUNKNOWN(unchanged)0x08000000UNKNOWNUNKNOWN(unchanged)Why this is evidence, not a guess
Both bootloaders hard-code their DFU flash descriptor, and both always mark sector 0 (the bootloader itself) and the reserved/config region read-only — that clamp is the security property they exist to enforce:
FLASH_DESC_STRopenmotion-bl(sensor)@Internal Flash/0x08000000/01*128Ka,04*128Kg,11*128Kaopen-motion-console-bl@Internal Flash/0x08000000/01*128Ka,08*128Kg,07*128KaA descriptor at
0x08000000with no read-only run therefore cannot be an openmotion bootloader, and the ROM loader is the only other thing it can be. Both signals are positive evidence; neither infers a mode from the absence of the other, so the "never guess an address" posture is preserved.Behaviour change to review carefully
This intentionally overturns
test_single_fully_writable_alt_is_unknown_not_bootloader, which asserted the conservativeUNKNOWN. That test predates the verification above — the invariant makes the case decidable, so it now assertsBARE_METALwith the reasoning recorded in the docstring. This is the one judgement call in the PR and the thing most worth a second opinion.Test coverage
The existing update-flow tests stub
detect_boot_mode()outright, so they could never have caught a classification bug. Added coverage drives a real listing through the genuinedetect_boot_mode -> parse_boot_modechain — the path the apps actually take:test_update_flashes_bare_metal_when_only_the_flash_alt_is_listed— the regressiontest_update_still_routes_a_converted_console_to_the_signed_slot— the other half: relaxing bare-metal detection must not send a bootloader unit a bare-metal image at the base of flashBoth new tests were confirmed to fail with the fix reverted. Full software suite: 769 passed, 207 deselected.
Test app
No change needed.
openmotion-test-appresolvesomotionfrom disk viaPYTHONPATHand delegates all boot-mode policy to the SDK ("All of the policy — which asset suits which boot mode, and what address it goes to — lives in the SDK; this app only supplies buttons"), so its flash thread picks this up throughFirmwareUpdater.update(). The second test above covers that path directly.Hardware verification
Bench console (HWID
23004c00065133333735383300000000) on FW 1.8.0:0x08000000withmotion-console-fw-baremetal.binget_boot_mode()returnsBARE_METALfromOW_CMD_BOOT_INFO(vtor0x08000000)The DFU-derived answer and the firmware's own report agree, which is the cross-check this fix relies on. Note 1.8.1-rc.2 does implement
OW_CMD_BOOT_INFO, so that console no longer needs the fallback — but units still on <= 1.8.0 do.Refs #197