Make OpenOCD/PyOCD flash config optional in archive. #514
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
added a commit
to oxidecomputer/hubris
that referenced
this pull request
Oct 10, 2024
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
The support for forcing flashing with openocd winds up producing a bunch of extra support code in the build system, for a feature we basically never use anymore. The support for PyOCD has, apparently, never been finished, and does not work. So I'm hoping to remove them from the Hubris build system, but the first step toward that is teaching Humility to behave gracefully if they're omitted from the img/flash.ron file. This change makes the fields optional, and Humility now only requires their presence if the user explicitly asks for the old --force-openocd switch.
cbiffle
force-pushed
the
cbiffle/openocd-optional
branch
from
d5cfc9b
to
45753c9
Compare
cbiffle
added a commit
to oxidecomputer/hubris
that referenced
this pull request
Oct 10, 2024
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
mkeeter
approved these changes
Oct 11, 2024
cbiffle
added a commit
to oxidecomputer/hubris
that referenced
this pull request
Oct 11, 2024
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
cbiffle
added a commit
to oxidecomputer/hubris
that referenced
this pull request
Oct 11, 2024
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
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 support for forcing flashing with openocd winds up producing a bunch of extra support code in the build system, for a feature we basically never use anymore.
The support for PyOCD has, apparently, never been finished, and does not work.
So I'm hoping to remove them from the Hubris build system, but the first step toward that is teaching Humility to behave gracefully if they're omitted from the img/flash.ron file.
This change makes the fields optional, and Humility now only requires their presence if the user explicitly asks for the old --force-openocd switch.