| Age | Commit message (Collapse) | Author |
|---|
|
The modules were 21% comment, much of it review-cycle argument rather than
guidance -- _kill_kids stated 'descendant by construction, no argv check needed'
twice, eight lines apart. Deleting such comments outright makes maintenance
worse: the next reader simplifies the thing the comment was defending. So the
history moves to the 2026-07-30 fleet-wedge design doc, which gains a trim
addendum recording what was removed, what was deliberately kept, and the rule
that decided each -- the CI ceiling bounds how long a run burns, and does nothing
about state that outlives it.
One comment was not merely long but WRONG: the report wipe carried 'The unlink is
DEFERRED to inside the pool try/except below', which is the opposite of what the
code does -- it sits before Manager() with its own comment explaining why. That
is the failure mode this pass is about, so it is deleted rather than reworded.
Kept everywhere: citations that refute a plausible wrong reading. That
usb_lock_device_interruptible is why the readers are killable, that usblp_mutex
is driver-global, that rawmidi honours O_NONBLOCK where usblp does not.
Two follow-ups are retired with them: pr3803-hil-blindness-reporting.md (there
is no blindness to report any more) and pr3803-usbtest-recovery-reserve.md (the
reserve is derived now). Kept: pr3803-flasher-recover.md, which PR #3832
implements, plus pr3803-pci-rebind-stranding.md and pr3803-hil-iar-rerun-spec.md,
both independent of this work.
|
|
Two layers whose cost was a contract to reason about rather than an outcome.
SYSFS_UNKNOWN was a three-valued return five files had to keep apart, and
misreading unknown as absence was silent: a healthy board reported as a firmware
regression. What it guarded is real -- `serial` is served by usb_string_attr,
which takes usb_lock_device_interruptible (v6.12.96 sysfs.c:141-143), the same
lock a wedged usbfs ioctl holds -- so the BOUND stays, on every caller by
default. usb_scan reads `serial` on every device matching the VID, and hil_lock's
controller_of does that on essentially every board, so one wedged DUT would
otherwise stall every worker, not one. What goes is the third value.
read_sysfs now returns str or None, and the question the third value existed to
answer is asked directly instead, by two predicates that say which question they
answer: sysfs_stranded() is process-wide and sticky, for hil_pool_check's footer
("could anything here be the tool losing sight of healthy hardware?"), and
path_stranded(path) is per-device, which is what usbtest needs to tell a DUT
whose `serial` is held under device_lock from one that genuinely left the bus --
that difference decides whether it performs driver-registry writes that take the
uninterruptible device_lock.
Gone: _SysfsUnknown, SYSFS_UNKNOWN, sysfs_blind, sysfs_blind_note,
note_sysfs_strand, the cross-process blindness publishing and its report banner,
usb_scan's (list, bool) return, usbtest's inconclusive abort, _blind_note's slot
in the result tuple, and bounded_open, whose last caller went in the previous
commit.
The strand memo is rewritten around the one invariant that makes it safe to
reuse: it is keyed by the path's kernfs inode, captured BEFORE the read. A
busport does not change when a board returns to the same physical port, so a
path-only blacklist outlives the wedge and hil_pool_check's own recovery flow --
reset, reflash, wait_device polling that busport -- would never look at the
board again. A re-enumeration destroys the kernfs node and makes a new one, so a
changed inode is the all-clear. Two ceilings bound different things: per path
(_PATH_STRAND_MAX) for a board that flaps while still wedged, and per process
(_STRAND_MAX) as a backstop against RLIMIT_NOFILE, counted per PATH rather than
per reader because hil_pool_check runs four poll threads over one bus.
A board the pool guard never reached is now reported as run-aborted rather than
pool-timed-out, and outranks a stale board-locked cell for the same reason the
pool-timeout cell does.
Both predicates answer conservatively where they are consulted before something
irreversible. path_stranded() covers the paths read_sysfs answered None for
WITHOUT reading -- past _STRAND_MAX it declines to start another reader, and
vouching for a path nobody looked at hands usbtest's fail-CLOSED guard a
fabricated all-clear, running remove_id/unbind against a wedged device. usbtest's
startup lookup carries the same caveat hil_test's absent arm already did, because
its stderr is relayed verbatim into the report cell.
strand_note() survives the removal for the same reason master had it: every
caller that can say "not found" needs the same sentence, and the one site left to
re-invent it got missed -- a wedged-but-enumerated printer was reported as an
enumeration failure, sending a maintainer after firmware.
The two predicates are not interchangeable, and usbtest needs both. Its per-case
verdict is per-DUT -- a peer that stranded at case 2 must not make our board
report wedged at case 29 -- but the finally block's cleanup is process-wide:
remove_id plus an unbind of EVERY interface under the driver, including that
peer's, each taking the uninterruptible device_lock. So the verdict uses
path_stranded() and the global cleanup stays gated on sysfs_stranded().
USBTEST_RECOVERY_BUDGET was an independent 250s that could not actually contain
the ladder it reserved for, which is why usbtest.py carried a _time_left() gate
re-deciding before every step -- with a bare '- 35' for costs paid downstream
that nobody could re-derive. Between them the two produced a recovery that
skipped its own steps for most real hangs.
The reserve is now derived from the bounds usbtest itself declares, per flasher
and per target: a probe reset, a reflash, and the Rescue-DP POR plus retry a
wedged RP DAP needs, plus the settles and hil_util.REAP_GRACE for each bounded
step. The Rescue-DP legs are openocd-only and gated on the RP target cfg, and
a stub reset is screened out, so the reserve tracks each board's real ladder
instead of one fleet number: 390s for the two RP boards -- whose ladder the old
250 could not contain, which is exactly why the gates skipped their steps --
190s for the other seventeen probe-reset boards, and 150s for esptool and
lm4flash, whose reset is a no-op. Changing a bound in usbtest moves the
reserve with it, and a unit test asserts it covers the ladder.
With the room actually reserved, the child runs the ladder straight through:
recovery_steps, _time_left, the three per-step gates and the parsed-but-unused
--outer-timeout are gone. What stays is what decides outcomes -- the convoy_safe
gate, reset-before-reflash, the no_op screen so a stub that resets nothing is not
claimed, and wedged_pids() as the arbiter, because a clean flash only proves the
probe wrote the MCU.
hil_util.py 616 -> 514 lines.
|
|
support (#3853)
Promote SEGGER RTT from an inline debugging technique to a standalone skill
backed by one stdlib-only implementation in tools/rtt.py: a CLI and importable
module for console/capture over J-Link (RTTTelnetPort) and OpenOCD (rtt server)
probes, with probe selection by serial or VID:PID, control-block address via
--elf or --addr, bidirectional console, post-mortem ring dump, and
--reset-before-attach for boot-time capture. The HIL harness reads a board's
console over RTT when its probe has no VCOM ("logger": "rtt" plus a LOGGER=rtt
variant define), covering device_info, pool-check aliveness, and CI wiring.
Validated on 22 boards across both backends; 26 unit tests run in pre-commit.
|
|
hil_test.py cannot run on Windows and never could: it imports helper.hil_lock,
whose module-level `import fcntl` is POSIX-only, so the harness fails at import
before a line of it executes. Past that it reads /sys/bus/usb, /dev/bus/usb,
/dev/serial/by-id and /proc, kills by process group, and takes flock board
locks -- none of which Windows has.
So the guards were protecting a platform the code cannot reach:
- run_cmd branched three ways on os.name to decide whether to set
start_new_session and whether to killpg. The non-POSIX arm called p.kill()
instead, which kills only the direct child -- exactly the semantics the whole
containment design rejects, since a flasher run through a shell reparents out
of reach. Dead code that documented the wrong answer.
- hil_test picked multiprocessing's default context on Windows "so it still
IMPORTS there". It does not import there.
- test_device_audio_test_freertos returned 'skipped' on nt before touching
ALSA, in a function only ever reached from a worker that cannot start there.
- Seven @unittest.skipIf(os.name == 'nt') decorators across the two suites.
These were the only ones with a real effect -- the unit tests DO import and
run on Windows, because they stub pyserial and mostly exercise pure logic --
but what they buy is a partially-green suite for a harness that cannot run,
and nothing verifies the set is correct: the hil-test hook only ever runs on
ubuntu-latest, so a missing guard fails silently until someone tries.
Removing them makes the POSIX assumption single and explicit rather than
scattered and half-honoured. Nothing changes on Linux: every removed branch was
the one already taken there.
Removing the run_cmd guards also removes their `else: p.kill()` arms. Those were
the Windows branches, and p.kill() reaches only the direct child -- a flasher run
through a shell keeps grandchildren it cannot touch, which is the semantics this
containment design rejects. RunCmdCleanupShape pins what is left: both cleanup
paths killpg, no try carries an else whose body would run when the kill
SUCCEEDED, and the BaseException path still re-raises. Structural rather than
behavioural because driving a real SIGINT into a blocked communicate() is
timing-dependent, and what actually breaks this block is an edit that rebinds a
branch -- which is a shape.
|
|
(#3852)
hid_generic_inout was the last unbounded blocking IO in the file. hidapi's
hidraw backend reads manufacturer/product via udev for each device reaching
create_device_info_for_device, both usb_string_attr served under the device
lock a wedged usbfs ioctl holds — and every DUT here is VID cafe, so a wedged
sibling stalls the walk.
A thread cannot bound it: cython-hidapi calls hid_open and hid_close bare
(0.15.0 hid.pyx), so they hold the GIL and the waiter can never resume.
Measured — a 1.0s bound never returned. run_cmd's killpg reaches a child
regardless; it gains an argv form for the -c body.
Filters on both ids: hidapi only runs the free uevent pre-check when ids are
passed (linux/hid.c:962), so an unfiltered walk sends every device straight to
the locked reads. Tests stall via ctypes.PyDLL, which unlike CDLL holds the
GIL — the shape a thread bound cannot cover.
|
|
Three readability changes with no behaviour change on the healthy path —
every pre-existing test passes untouched.
main() was 368 lines with try/finally three deep, its two abort paths
near-identical 40-line blocks; _abort_report holds that shape once, and the
controller-hint cache and pool construction move to their own helpers.
368 -> 279, test_board 180 -> 151, test_device_usbtest 164 -> 125.
test_hil_bounded.py cost 78s on every commit under test/hil/, mostly one 3s
post-flash settle paid by ten tests against a fake rig. Now 37s.
Fixes two pre-existing defects the extraction exposed: _write_failed_spec was
unguarded inside the abort path, so an OSError there replaced the caller's
RuntimeError and no report was written at all; _save_controller_hints overlaid
a startup snapshot onto the re-read cache, clobbering a concurrent job's newer
values. Also five comments that stated the opposite of the code, and both table
renderers measuring width with len() against two-column status marks.
|
|
hil_report.json and hil_report.md were written independently. Four writers
produced the markdown and three wrote no JSON at all -- and those three are the
paths where a run died, so a JSON consumer saw nothing exactly when it mattered:
the per-board verdicts an agent hands back reported the whole fleet as "no
report row" while a human read the real story from the markdown.
Every writer now goes through render_report(), so a table can never contain
something the JSON does not. The document gains `scope` (a three-board PR run
and a full run that lost 24 boards were indistinguishable) and `caveat` (how the
run ended). `banner` carries rig health across an --accumulate retry; `caveat`
records how a run ended and must not -- conflating them made a clean retry
publish an abandonment that never happened.
helper/hil_report.py owns the document end to end, dissolving the import cycle
that forced write_timeout_report to compose its own markdown and removing a
duplicate cell classifier kept in sync by hand. hil_summary.py is deleted; its
CLI moves there. hil_ci.sh uploads the sidecar so a remote --accumulate has a
merge bas
|
|
The roster had two ways to pass a cmake -D to a board's build: `build.args`,
applied to every variant, and `variant[].defines`, applied to one. They did the
same thing, and only metro_m4_express used the first - for MAX3421_HOST=1, which
is what makes it the one rig board that compiles hcd_max3421.c.
A board whose define is always on now carries a single variant named after itself,
which is exactly the shape `board.get('variant') or [{'name': name, 'flags': ''}]`
already synthesises everywhere - so the build dir, the HIL report row and the
variant-boundary handling are unchanged. raspberry_pi_pico has used that shape
for its flags all along.
Removes the BuildCfg type and the parallel code path from all four consumers:
hil_test.build_board, hil_pool_check's two builders, hil_ci_set_matrix and
ci_select.board_options.
Verified: the hil-build matrix entry is byte-identical
(`-b metro_m4_express -DMAX3421_HOST=1`), hil_test's build command is unchanged,
ci_select still selects the board for a max3421 diff with MAX3421_HOST in its
options, and a real build of dual/host_info_to_device_cdc and host/cdc_msc_hid on
that board still compiles hcd_max3421.c.
|
|
Every PR built all 74 legs (2494 example builds on GHA cmake alone) and flashed
all 30 rig boards, whatever it touched. One classifier now walks the PR diff twice
and answers three questions: which families to build, which examples per family,
and which boards run which tests. Fail-open throughout - anything no rule
classifies, any exception, any unusable output falls back to the full matrix, and
a master push always builds everything.
test/hil/helper/hil_select.py moves to tools/ci_select.py: it is no longer HIL-only,
and tools/ is where the build side can import it. test_hil_select.py follows it as
test_ci_select.py.
Rules (docs/superpowers/specs/2026-08-19-ci-build-family-filter-design.md holds the
full table): a port selects the families whose family.cmake references it, and its
role - a dcd change skips host examples and vice versa; a class selects only the
examples whose tusb_config.h enables its CFG_TU[DH]_ macro, following cross-class
includes; an example selects itself; hw/bsp selects its family or board; hw/mcu and
lib select whoever references them. CMake is the reference for all of it - make
follows whatever cmake decides, family.mk is never scanned.
Empty means empty (maintainer ruling): a rule that classifies a path to nothing
selects nothing. Ports no family references, classes no config enables, libs no
example builds and hw/mcu paths that resolve nowhere are all real - nothing
compiles them, so nothing can validate them, and the master-push build is the net.
Structural tests pin each such case with an explicit allowlist, so the day one
stops being empty it fails pre-commit instead of silently narrowing CI.
Per-example builds: build.py grows a repeatable -e, resolved against the targets
CMake actually registered and batched into one `cmake --build --target a b c`.
build_utils mirrors CMake's family_filter (the whole FAMILY_MCUS list, ${...} and
string(TOUPPER ...) resolved) for the cmake side, while the make side keeps
master's algorithm verbatim - the two build systems answer differently and a shared
answer breaks lpc54's make link. hil-build gains this even on a full selection:
1702 example builds become 515.
Transport: the selection travels as a file, never an argv or env var - a mass-sweep
diff selects 261 KB against a 128 KiB exec limit, and E2BIG would fail the step
before its own fallback could run. CircleCI carries the example map inside the
generated config (pipeline parameters cap at 512 chars), swapped into the parameter
defaults by sentinel match, and drops the scoping wholesale if that rewrite fails.
Every PR-derived value written to $GITHUB_ENV/$GITHUB_OUTPUT is character-screened.
Code metrics follow the scoping: metrics.py emits per-example totals, and
metrics_pair_compare compares the (board, example) pairs present on both sides
instead of a scoped run against a full-matrix average.
The selector's own suite gates it in both providers: a selector that exits 0 with
valid-but-wrong JSON is the one failure fail-open cannot catch, so a red suite
means the full matrix.
|
|
~/code/pico/pico-sdk and $HOME/code/esp-idf/export.sh are the ci rig's private layout;
written into instructions they silently stop being true on tusb, a dev PC, or any
future rig. The docs now use the variables the vendors define -- PICO_SDK_PATH for the
Pico SDK and IDF_PATH for ESP-IDF, activated explicitly as `. "$IDF_PATH/export.sh"` --
and leave where the checkouts live to each host's profile.
The variables are only useful if the shells that agents actually get can see them, and
`ssh <rig> 'cmd'` is non-interactive AND non-login: it reads no profile, and Debian's
sshd-sourced ~/.bashrc returns at the interactive guard before most of the file. The ci
rig already keeps its exports in the section ABOVE that early-return; IDF_PATH now sits
there beside PICO_SDK_PATH, and the whole chain is verified from a plain non-interactive
ssh: both variables visible, `. "$IDF_PATH/export.sh"` activates ESP-IDF v5.5.3 with
idf.py on PATH -- no login shell, no alias, no hard-coded path. hil-pool-check documents
that placement so the next rig is set up the same way.
|
|
script directly
A probe whose firmware has wedged reports flash-failed with the probe present and
"probe toggle unconfirmed". The tool's own recovery cannot fix that: an authorized
toggle re-enumerates but never removes power, and hil_pool_check.py:304 already notes
that ST-Link, WCH-Link, CP210x and picoprobe keep their sysfs kobject across one. So
the check correctly gives up, and the operator was left to invent the next rung.
Write it down, as what this rig's hardware actually does rather than what uhubctl
advertises: the Renesas cards list their root hubs as ppps-capable but do not
implement it (owner-confirmed - VBUS never drops, only D+/D-), so a root-port cycle
is a harder forced re-enumeration that a wedged probe can ride out, worth exactly one
attempt; and the AMD 0000:02:00.0, where the WCH-Links live, has no port-power
switching at all - nothing to cycle, straight to a physical replug. Which card a
probe hangs off decides which case applies, so the procedure starts from readlink.
The ordering rules encode the shared-rig protocol: let the full run finish (a bounce
re-enumerates siblings and corrupts checks still in flight), hold --all with this
host's --config before the cycle (hil_lock.py hold validates nothing against the
roster and nothing maps a sysfs busport to a board name, so a narrower hand-listed
hold reserves nothing while reporting success - and --all defaults to tinyusb.json,
which on the tusb rig would reserve 27 boards that do not exist there), release
BEFORE the re-check (hil_pool_check.py self-locks every board it checks, so a hold
still in place makes the verification report locked against your own hold and verify
nothing), and drive the cycle through usb_recover.sh root-cycle by its full in-repo
path - it is on no PATH and sudo's secure_path excludes the checkout. Never a bare
`uhubctl -a cycle`: without -S it writes sysfs disable, whose disable_store takes the
root hub's lock uninterruptibly and then usb_disconnect()s the wedged child - the one
input that turns a probe wedge into a bus-wide wedge. Give the script the wedged
probe's own busport, not the hub path: the serial guard and the success check both
read the path you pass, and the hub's inode always changes when its own port cycles.
Reporting asks for both passes: a final table showing every board healthy hides that
a probe needed power-cycling to get there, which is the signal that it will recur.
Also: the ESP-IDF env hints name `. $HOME/code/esp-idf/export.sh` instead of the
`get-idf` alias, which lives only in interactive shells and fails from scripts.
|
|
hil-validate ran one hil-operator per board. That parallelizes at the wrong layer:
hil_test.py already schedules boards across host controllers and budgets concurrent
flashes and usbtest batteries per controller (FLASH_PARALLEL/USBTEST_PARALLEL), and
those permits live in one process - N parallel runs multiply the budget onto the same
uPD720201 cards for no wall-clock gain over one run that already parallelizes. The
workflow now spawns ONE operator with every board as repeated -b.
The operator no longer retypes the report table. Four consecutive max-effort review
rounds found ~15 defects in this file and every one was in reconstructing board
identity from transcribed prose: report rows are named per VARIANT (nanoch32v203 only
ever produces -fsdev/-usbfs rows), a variant need not start with its board's name,
lock contention is a `board-locked` cell rather than a phrase, and each fix introduced
the next round's bug - including a fake-green test that asserted an invariant with the
one input shape that could not break it. The new helper test/hil/helper/hil_summary.py
does the join where the roster lives and emits one machine verdict per board
({board, ran, pass, locked, detail}); the operator returns that JSON verbatim plus
`wedged`, the only field it authors, and the workflow reads fields, never parses a
string. Its cell classifier mirrors hil_test.py's own tally exactly: failures are
always marked ('fail' or a ❌ prefix, TestFail's contract), everything unmarked is a
pass - a passing test may return a plain metric cell like '13443 KB/s', and the
mirrored rule is what keeps a green table from becoming a red verdict.
hil_ci.sh kept only the LAST -b, so multi-board remote runs staged one board's
binaries and every other board died on the rig after its lock and flash slot were
spent. It now parses every -b spelling argparse accepts (with the -bt arms ordered
first, longest-match, so the <config>.failed retry form is never read as a board named
"t..."), pre-flights roster membership and build dirs for ALL boards before anything
is wiped or staged, warns per declared variant with no build dir (which hil_test.py
would silently green-skip), forwards HIL_* knobs as export lines in one %q word the
remote evals ('; '-joined so it round-trips under dash - an authorized
HIL_NO_BOARD_LOCK force must not silently no-op), keeps HIL_REPORT_DIR local because
the copy-backs look in REMOTE_DIR, and copies hil_report.json and the .failed re-run
spec back beside the markdown, deleting stale local copies first so a green run cannot
leave last run's spec looking current.
Retries preserve the fleet: the documented path is the <config>.failed spec, which
already begins with --accumulate; a fresh scoped re-run would unlink the report and
collapse the whole-fleet table to the retried boards alone.
The risky logic is executable, not argued about: .claude/workflows/test-hil-validate.mjs
pins the lookup/verdict helpers and runs in pre-commit (hil-validate-logic); nine
staging tests drive hil_ci.sh through an ssh stub that models the real thing (argv
joined into one string the remote re-splits, heredoc on stdin - the naive echo-stub
passed while the feature was broken); and deliberate mutations of the verdict logic
are all caught.
Validated on the rig: a 2-board run (usbtest 30/30 on both; the pre-fix classifier,
replayed against that run's real report, fails the fully-green stm32f723disco on its
two passing '13443 KB/s' cells), the .failed --accumulate retry (merged report kept
every earlier row), and a 10-run soak over random subsets of a 22-board pool - 43
board-slots, every failure signature matched pre-existing CI state or known flake,
zero tooling failures, no locks left behind.
|
|
A wedged USB device used to take the whole HIL run with it. Every worker that
touched the poisoned node blocked uninterruptibly, the pool could not be joined,
map_async discarded every board's result, and the job ran to the GitHub ceiling
with no report at all -- while the self-hosted runner's single job slot stayed
occupied and every queued job waited behind it.
Bound the calls a worker makes itself. read_sysfs, bounded_open and run_cmd all
answer within a wall clock; read_sysfs distinguishes "absent" from "unknown",
because a blocked read is not evidence of absence, and caps stranded readers at
four (each costs a thread and an fd for the life of the process) after which the
worker declares itself blind. mtype, the gio unmount, the libmtp session and the
arecord/iperf reaps go through those bounds; the MTP session runs in a disposable
subprocess, since libmtp's ctypes calls block unkillably in D state.
Bound the run. A pool guard (HIL_POOL_TIMEOUT, 60 min) fires before any job
ceiling and still writes a report. When the pool will not shut down, the sweep
kills what the workers spawned -- descendants, not just direct children, since
flashers run in their own session -- confirms each kill actually landed, and
exits early so the runner is freed. Whatever survived is named in the report.
Deliberately shallow past that point. We do not re-scan process groups, prove
pid ownership, or escalate through sudo: a root-owned survivor is reported, not
force-killed, because signalling a pid we cannot prove is ours is the worse
failure, and the job ceiling backstops whatever this misses. A D-state holder
was never killable anyway.
Recover instead of reporting a wedge. A HUNG usbtest case reflashes its own DUT
through its roster flasher, but only where the flasher can reach its probe past
a poisoned node -- openocd pinned to a validated vid_pid, or esptool. Where it
cannot, the run says so rather than reserving budget for a path that cannot fire.
Raise the CI ceilings above the pool guard so the guard fires first and still
writes its report, and pin --retry 1 on every HIL leg: the guard is a flat
constant and does not scale with max_retry, so argparse's default of 3 would
triple the serialized usbtest tail against an unchanged guard.
Split the module: execution in hil_test/hil_flash/usbtest, infrastructure in
helper/ (locking, health, selection, shared bounded IO), and the two matrix
generators into .github/scripts/ -- ci_set_matrix.py sat in workflows/, where
GitHub treats every file as a workflow definition. 193 tests cover the bounded
paths, the kill ladder, the guard and the selector against synthetic /proc trees
and PATH-injected fakes; a real wedge cannot be manufactured on demand.
|
