Move probe-rs chip name into boards/*.toml, remove scary matches #1903
Conversation
This file contains 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
cbiffle
force-pushed
the
cbiffle/improved-board-config
branch
from
a5633bf
to
2c7bc3a
Compare
lzrd
approved these changes
Oct 10, 2024
cbiffle
force-pushed
the
cbiffle/improved-board-config
branch
from
2c7bc3a
to
e5afc05
Compare
Humility has historically had support for a --force-openocd flag on the flash subcommand, as a hedge against probe-rs having bugs or limited chip support. In practice, as far as I can tell, we never use it. This support required one of the three giant merge-conflict-prone match statements on which I declared war in #1886. The second such match statement was providing similar support for PyOCD, which Humility appears to have never actually implemented! So this change removes both. Archives built at or after this change can only be flashed (by Humility) using probe-rs. One giant scary match statement remains, but it's in my sights. I have left the openocd.cfg file in the archive because it's useful for debugging, even if we don't use it for _flashing._ (My workflow if I need gdb is typically to flash with Humility via probe-rs, and _then_ fire up openocd.) I've bumped the archive version here so that using such an archive with an older Humility gets you a cogent error. (Otherwise, you get a weird crash about loading flash.ron, no matter what you're trying to do.) Note that this requires a corresponding change in Humility, both to tolerate the absence of the removed fields, and to handle the new archive version. oxidecomputer/humility#514
Historically, the build system's flash support has contained a big scary match mapping board names to probe-rs chip names. This is super error prone, as I ranted about at length over in #1886. I recently introduced per-board configuration files as a way to (1) check that board names used in apps are not mispeld, and (2) store configuration like this. This change switches the build system to look in the toml files for the chip name, removing the final big scary match statement. Note that this information is _more specific_ than the information in chip.toml, because probe-rs is weirdly fanatical about specifying chip type (up to and including the package!), whereas Hubris's peripheral maps don't care. Fixes #1886.
cbiffle
force-pushed
the
cbiffle/improved-board-config
branch
from
e5afc05
to
3ca5d91
Compare
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 commit messages have a lot more detail if you're curious, but this eliminates all of the matches that I've been mad about, and should make maintenance a lot easier.
Anticipated question: why not put the chip name in the chip.toml that the app already references? It's because probe-rs is weeeeeirdly specific about chip names, and Hubris is not. For instance, probe-rs distinguishes the same chip in different packages for reasons I cannot fathom. So boards need to be more precise than chip.toml.
I'd like, later, to also move the chip.toml name reference out of app.toml and into the board definition, but I have not done that here.