HIL: boot every board against every config it can be flashed with - #379
Draft
mullinmax wants to merge 4 commits into
Draft
HIL: boot every board against every config it can be flashed with#379mullinmax wants to merge 4 commits into
mullinmax wants to merge 4 commits into
Conversation
Implements DESIGN.md's G3. Per board: build and flash that target once, then loop over every config in src/<target>/config/ - write gamename into the FRAM configuration record over the REPL, reset, wait for the ready marker, and ask the board what it loaded. The game name is the assertion that earns its keep. A config that fails to apply does not fault or crash from outside: GameDefsLoad.go falls back to safe_defaults and the board serves the generic definition for its hardware, healthy in every other respect. /api/game/active_config does not catch that either - it reads the gamename field back out of FRAM, not what actually loaded. Comparing /api/game/name against GameInfo.GameName in the source JSON is what separates "loaded my config" from "silently fell back", and it cross-checks the on-board bundle against the repo while it is there. HDWR01 is fatal here even though flash_and_check.py only warns about it: it sends main.py down the safe_mode path where the config is never read, so every downstream assertion would pass or fail for reasons unrelated to the config under test. Shared plumbing moves to bench.py verbatim - inventory, resolve, build, flash, reset, wait-for-boot, USB request - so both harnesses agree on how a board is brought up and neither has to be edited to add a check. flash_and_check.py keeps its own assertions and behaves exactly as before. Two findings from writing this, both pre-existing and neither fixed here: - HarleyDavidson_L3 and GilliganIsland_L9 are 17 characters and the FRAM gamename field is 16, so struct.pack truncates them and the two games can never be selected on a real board - the UI offers them, the write is accepted, and the next boot comes up on safe defaults with CONF01. The harness catches this before spending a boot cycle, and a unit test catches any new offender in ordinary CI. - /api/adjustments/status 500s for the 14 configs that declare no Adjustments section, because gdata is not merged with safe_defaults. Warned about rather than failed, so a firmware gap the harness cannot fix does not bury the signal it exists for. Hardware-free parts are unit tested: config discovery, selection and ordering, and each assertion with the board faked out - including the silent-fallback case the whole harness is built around. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013c2GwCiEu9rRxznR5vfkuU
Contributor
|
Developer build links: Sys11 (Tiny) WPC EM WhiteStar DataEast Classic |
…n a dead board First bench run: sys11 passed all 39 configs at ~21s each, which is the harness working end to end. WPC wedged - silent console, no reply to Ctrl-C - and then the run spent 63 minutes writing the same 60s timeout 63 times before dying in teardown, taking the summary with it and never touching data_east. Three separate faults, fixed separately. 1. The wedge. The Pico has one CDC endpoint, so using mpremote to write the gamename meant closing our connection, letting a second process open the port, and reopening after - once per config. sys11 (MicroPython 1.24.1) survived that 39 times; WPC (1.26.0-preview) did not survive it once. A running board printing into a CDC endpoint nothing is draining is the difference between them. The REPL is now driven directly over the connection the harness already holds (bench.Repl), so nothing else ever opens the port and the port is closed only while the board is mid-reset. run_matrix became a pipeline - boot, assert, set the next config, reboot - carrying one connection per boot instead of churning three. Repl owns its read buffer, which is the point of it being a class: raw REPL is a sequence of markers and a read that syncs on one almost always pulls in bytes belonging to the next. The first version dropped them and desynchronised; there is a test for it now. 2. 63 timeouts. Two consecutive setup failures abandon the board, after one cheap recovery attempt that drains its console and sends Ctrl-C. Boot timeout for the matrix drops to 90s - a healthy boot answers in 12-16s, so 150s only bought a wedged board more time to waste. A dead board now costs about three minutes instead of an hour, and the boards after it still run. 3. Teardown taking down the run. restore_default caught CheckFailure but not TimeoutExpired, so it raised through main and lost results already in hand. Teardown is best-effort now, and a board that fails outright is one board's failure rather than the run's. Also: stty raw -echo before dumping a console in both HIL workflows. A tty reverts to ECHO-on once every handle closes, so `cat /dev/ttyACM*` was echoing each board's output back into it - the logs show all three boards parsing their own log lines as USB API requests. Simulated end to end against a fake board for the healthy, unstorable-name and wedged cases; the wedge that cost 63 minutes now costs two boots. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013c2GwCiEu9rRxznR5vfkuU
Run 2 died in inventory, before any of the config-matrix fixes could run:
`mpremote connect /dev/ttyACM1` timed out at 30s. The WPC board is still
deadlocked from run 1, so the bench is down until something revives it.
TrenchCoat confirms the mechanism in its own source (src/ray.py,
send_command): "if nothing ever drains the board's output, the USB CDC buffers
fill up, MicroPython blocks writing to stdout, and the board deadlocks
mid-script". Its enter_bootloader_mode() goes through machine.bootloader() on
the REPL, which is no use once a board is in that state - hence this.
dev/hil/recover.py escalates cheapest-first and stops the moment the board
answers:
1. drain read whatever is queued and send Ctrl-C. If it is blocked on a
full CDC buffer, reading is the remedy. Zero bytes drained is
itself a diagnosis: it is stuck on something else.
2. usb reset USBDEVFS_RESET on the device node, to re-enumerate and reset
TinyUSB's endpoint state.
3. power uhubctl on the board's hub port. The board is USB bus powered
(Trench-Coat-Install-Guide.md), so this is a genuine cold boot.
4. reflash 1200 baud touch into the ROM bootloader, then a MicroPython
UF2 onto the RPI-RP2 drive. The touch is a CDC line-coding
change handled in USB interrupt context, so a blocked Python VM
does not stop it - which is exactly why it can work when the
REPL cannot.
Two guards on step 4, because it is the one that can make things worse. It
refuses to touch a board into BOOTSEL unless something here can actually mount
the drive afterwards - a wedged board is at least still a serial device, while
a BOOTSEL board that cannot be flashed needs a physical replug. And the UF2 is
downloaded and checksummed *before* the touch, so a bad fetch cannot strand the
board either. UF2s come from warped-pinball/trench-coat pinned by commit, with
sha256 per file, rather than being vendored.
A dead board cannot say which system it is, so the target is deduced by
elimination: ask the boards that do answer, and whatever VECTOR_HIL_BOARD_MAP
still expects is the one on the floor. With one board down that is exact; with
two it refuses to guess, because flashing the wrong system's UF2 is worse than
leaving a board dead.
The run opens with a capability report - whether the USB node is writable,
whether uhubctl is installed, whether anything can mount an RPI-RP2 drive - so
even a failed recovery says which one-time bit of runner setup would make the
bench self-healing. Expect steps 2 and 3 to be unavailable today: the runner
user is in dialout, which covers serial but not raw USB.
22 tests cover the ladder, the elimination logic, both step-4 guards and the
sysfs path parsing.
Co-Authored-By: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013c2GwCiEu9rRxznR5vfkuU
The recovery run hung. Fourteen minutes into a step that should take one, with no output, until the job timeout killed it - and the cause is our side of the same failure the board has. `serial.Serial(timeout=...)` sets the READ timeout only. Every connection the harness opened had no `write_timeout`, so a write to a board that has stopped draining its USB OUT endpoint blocks forever. That is exactly the state a wedged board is in, and exactly the board the recovery tool has to write to: it drained the console fine, then blocked on the Ctrl-C it sends afterwards. `flush()` is worse. It is termios tcdrain, it waits for the kernel's output buffer to reach the device, and it takes no timeout at all - so it hangs on the same board even where the write did not. Every call site is gone: handing bytes to the kernel is enough, and every exchange here is already synchronised by a read with a deadline. Both are now impossible to reintroduce by accident. open_serial() is the only way a port gets opened and always sets both timeouts; serial_write() turns a deaf board into a CheckFailure naming the cause; a test asserts bench.py contains no .flush() at all. This was not only the recovery tool's problem - config_matrix's nudge() would have hung the matrix the same way, on the same board. Added bench.time_limit as a backstop: a SIGALRM ceiling around each recovery step (180s) and each config in the matrix (240s). Every call in here is meant to be bounded, but a board in a bad enough state can block a syscall no library timeout covers, and one board must never hang a bench job again. SIGALRM interrupts the syscall, so it catches what the individual timeouts miss. Also removed the config matrix's push trigger for now. It shares the hil-bench concurrency group with the recovery workflow, and GitHub keeps only one pending run per group, so a push touching both would race and silently cancel one. While the bench needs recovering, recovery gets the queue; the workflow says how to put it back. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013c2GwCiEu9rRxznR5vfkuU
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
2 participants
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.
Description
Implements DESIGN.md's G3 — every available config can be parsed and boot, as
dev/hil/config_matrix.pyplus.github/workflows/hil-config-matrix.yml.Per board: build and flash that target once, so the config bundle under test is the one this checkout produces, then loop over every config in
src/<target>/config/:gamenameinto the FRAMconfigurationrecord over the REPL, and read it backmachine.reset(), wait for the ready marker on the consoleCONF00/CONF01, and noHDWR01either/api/game/active_configis the config we set/api/game/namematchesGameInfo.GameNamefrom the source JSON in this repo/api/leadersand/api/adjustments/statusreturn 200 — the definition is usable, not merely loadableEach board's bundle is also compared against the source directory once before the loop (same config names, same game names), which localises a packing bug to one boot instead of one boot per affected config. Every board is put back on its generic config when its matrix finishes, including after a failure.
Why the game name is the load-bearing assertion. A config that fails to apply raises nothing an outside observer can see:
GameDefsLoad.gofalls back tosafe_defaultsand the board serves the generic definition for its hardware, healthy in every other respect./api/game/active_configdoes not catch it either — that route reads thegamenamefield back out of FRAM, not what actually loaded. The game name is what separates "loaded my config" from "silently fell back".HDWR01is fatal here even thoughflash_and_check.pyonly warns about it: it sendsmain.pydown thesafe_modepath where the config is never read at all, so every downstream assertion would pass or fail for reasons unrelated to the config under test.Refactor
Shared plumbing moves out of
flash_and_check.pyintodev/hil/bench.pyverbatim — inventory, resolve, build, flash, reset, wait-for-boot, USB request — so both harnesses agree on how a board is brought up.bench.pyasserts nothing about firmware behaviour; assertions stay in the harness that imports it.flash_and_check.pykeeps its own checks and behaves exactly as before.Related Issues
None filed. Two pre-existing defects the harness found are described below and are not fixed here.
Motivation and Context
Requested: configure each board with every configuration it can be flashed as, and confirm it boots as that game rather than falling back to the generic config for that hardware. WPC is the interesting case at 63 configs.
Findings — pre-existing, not fixed in this PR
1. Two WPC configs can never be selected on a real board.
configuration.gamenameis a 16-byte fixed-width field (SPI_DataStore.py) andstruct.packtruncates silently.HarleyDavidson_L3andGilliganIsland_L9are 17 characters, so the web UI offers them,/api/settings/...accepts the write, the name is truncated on the way into FRAM, and the next boot matches nothing and comes up on safe defaults withCONF01. Fixing it means shortening the two filenames or widening the field — the latter is aMapVersionchange, since the 96-byte record is fully used. Happy to do either in a follow-up; it needs a call on whether renaming a config filename is acceptable.Meanwhile: the harness reports it before spending a boot cycle, and
test_no_new_config_name_exceeds_the_gamename_fieldfails an ordinary PR that adds a new over-long name. The two that exist are listed inKNOWN_UNREACHABLE_CONFIGS.2.
/api/adjustments/statusreturns 500 for the 14 configs with noAdjustmentssection.GameDefsLoadassigns the parsed config straight toSharedState.gdatawithout mergingsafe_defaultsinto it, so the key is absent andAdjustments._get_range_from_gamedefraisesKeyError, whichroute_wrapperturns into a 500. Warned about rather than failed, so a firmware gap the harness cannot fix does not bury the signal it exists for.Limits, recorded in DESIGN.md
GameName(the fourAddamsFam_*) are not distinguished from each other — no route exposes anything else fromgdata. A pass there means "parsed and loaded without faulting", not "this exact ROM revision's definition is in memory".gc.mem_free()over the REPL means interrupting the firmware whose boot is being measured. It needs a small firmware-side route first.Testing
dev/tests/test_hil_config_matrix.py— 27 tests, all passing. Covers config discovery from source JSON, selection/ordering (--configs,--limit,--changed-since), the FRAM field-width rule against every shipped config, and each assertion with the board faked out: the silent-fallback case, a board running a different config, the EMactive_configspecial case, a name that cannot be stored, every fault class, the adjustments warn-vs-fail split, and the job summary.flake8(repo settings) clean on every file touched; new files areblack/isortclean.bench.pyis left byte-identical to the code it came from so the move is reviewable as a move, which means it inheritsflash_and_check.py's existingblackdrift.--target wpc --limit 5.Types of Changes
Checklist
Additional Notes
The workflow is
workflow_dispatchonly, plus apushtrigger on this branch so it can be validated before merging (same pattern as the existing HIL workflows — drop thepushtrigger once it is onmain). It shares thehil-benchconcurrency group, so it queues behind the other HIL jobs rather than interleaving with them. Dispatch inputs go through the environment rather than${{ }}interpolation into the shell.Generated by Claude Code