Refactor and improve the firmware update path - #45
Merged
Merged
Conversation
The update command now renders progress through Spectre.Console rather than writing a progress bar directly to the console, so progress is also reported correctly when standard output is redirected. Progress reports are applied on the thread that calls Report instead of being posted to the thread pool, so a report can no longer be applied out of order or concurrently with another. A completed update prints Firmware updated.
The update command now uses firmware routines carried by the toolkit. Bootloader, DeviceFirmware and FirmwareMetadata are copied from bonsai-rx/harp at e38b422 into a single ATxmega folder, since the paged hex image and the file naming convention are both specific to that bootloader. The copy keeps the released behavior of rethrowing the original exception when the device does not answer an unforced update, instead of wrapping it, so a missing or denied port is still reported by name rather than as an unhandled error. Upload retry and the post-dispose wait have been ported, so a failed upload is now retried up to three times and the serial port is released before the bootloader is opened.
Remove the nullable disable pragmas from the ported firmware code and annotate it. The optional progress reporter, the equality members and the metadata TryParse result are all declared nullable to match what the bodies already did, and TryParse carries NotNullWhen so Parse can return its result directly. The comparison operators on FirmwareMetadata now take nullable operands, so comparing metadata against null no longer warns at the call site.
Port the firmware update test and its hardware helper from bonsai-rx/harp at e38b422. One test, skipped automatically when no port is configured, so dotnet test runs the ordinary suite without a device and without any flag. It repeats the update, classifies each failure by the stage it reached, waits for the device to respond before starting the next one, and forces an update to recover a device left in bootloader mode. When a device does not respond within the ceiling, the test stops polling and waits quietly instead, since polling holds a device in bootloader mode, and it abandons the run. The port, the firmware image, the reset consent and the iteration count are read from the HARP_TOOLKIT_TEST_PORT, _FIRMWARE, _ALLOW_RESET and _ITERATIONS environment variables. The solution also drops the build configuration left behind by the device project template, which was split into its own repository.
The update command now reports a bad firmware file name, a malformed hex image, an incompatible device and every error the bootloader reports as a message and exit code 1, where each previously printed an unhandled stack trace. The firmware file is read inside the error handler so a failure reading it can be reported, and DeviceFirmware takes the parsed metadata, so it is parsed only once. The compatibility check now names both sides, so a refused update says which device and hardware version the file is for and which the device reports.
An update that fails after the device has been reset now reports the percentage it reached and that the device may be left in bootloader mode, rather than the message describing the port. A lost connection, a device that stops responding and an error reported by the device are each named, and any other failure is reported with its exception type instead of an unhandled stack trace. The upload also retries when the serial port reports it is no longer open, which is the first error from a device unplugged mid update, before the reopen fails. A device that does not answer before the reset now also reports that it may still be restarting, or may be in bootloader mode and need a forced update, where it previously said only that it did not respond in time.
A bootloader reply is now accepted only when it carries the protocol header, echoes the opcode that was sent and reports a known error code, where previously a correct checksum was the only requirement and an unrecognised error code counted as success. The page size is also rejected unless it is a power of two within the range the cores use.
The update command now probes the bootloader protocol when a device does not answer a Harp command, and reports that the device is in bootloader mode and needs --force only when a bootloader answers. The command also accepts --timeout, which now binds the Harp reads during an update in place of a fixed 500 ms.
Await Task.Delay rather than an Rx timer for the settle delays, which removes the only use of System.Reactive from the firmware folder, and prefer reading a nullable through GetValueOrDefault.
The update command now removes the progress bar from the terminal when it stops. The percentage reached is reported in the failure message.
Add an article covering the naming convention followed by firmware images, the options, and how to recover a device left in bootloader mode by an interrupted update. The landing page gains a new section beside the ones for code generation and verification, and the quickstart drops its update step, leaving every step there read-only. An image that is not an Intel HEX file is now refused with a message naming the cores the update supports. The description of the --force option also covers both of its intended uses, since the flag is the only way to recover a device in bootloader mode.
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.
The firmware update routines move from
Bonsai.Harpinto the toolkit, atsrc/Harp.Toolkit/Firmware/ATxmega/. The update path is then reworked. Failures report as messages instead of stack traces, bootloader mode is detected before a forced update is advised, and the Harp read timeout becomes an option.Firmware routines
Bootloader,DeviceFirmwareandFirmwareMetadataare copied from bonsai-rx/harp@e38b422. That revision includes two fixes the pinned 3.6.1 release does not. A failed upload is retried up to three times, on both the forced and unforced paths. The serial port is released before the bootloader is opened.The copy keeps the released behavior of rethrowing the original exception when a device does not answer an unforced update.
The update now validates bootloader replies. It accepts a reply only if the reply contains the protocol header, echoes the opcode that was sent, reports a known error code, and states a page size that is a power of two in the range the cores use. A correct checksum was previously the only requirement.
Error reporting
The following now report a message and exit code 1. Each previously printed an unhandled stack trace.
The update probes a device that does not answer a Harp command, one time, with a bootloader frame. It advises
--forceonly when a bootloader answers. When nothing answers, the message states that the device can be restarting or on a different port.Options
--timeoutnow applies toupdate. It bounds the Harp reads, which used a fixed 500 ms. The default is 2000, and-1waits indefinitely.The
--forcedescription now covers both of its uses.Measurements
100 updates against a Behavior board, hardware 2.0, over an FTDI bridge. Each iteration starts from a device that has answered. The first column is the upstream baseline for the same test against the unfixed library.
The five retried uploads are the five slowest, 5503 to 5847 ms against a 3337 ms median. The next slowest is 3381 ms. On 3.6.1 each of the five would have failed.
A device answers Harp again a median of 2148 ms after an update, with a maximum of 10479 ms. Any tool that reconnects immediately can fail.
Hardware tests
A test project repeats the update and classifies each failure by the stage it reached. It skips itself when no port is configured, so
dotnet testruns without a device and without a flag.Documentation
docs/articles/update.mddescribes the naming convention for firmware images, the options, and how to recover a device left in bootloader mode. The landing page gains a section for it. The quickstart drops its update step.Not included
Devices built on the Pico core. Their update process is different, so the article states that these devices are not currently supported. An image that is not Intel HEX is refused with a message that names the supported cores.