mirror of
https://github.com/hathach/tinyusb.git
synced 2026-08-18 11:02:16 +00:00
docs: hand off follow-up work as per-PR plans
Records the convention in CLAUDE.md -- deferred work is a SEPARATE scope that deserves its own PR, written by another session, so it is handed off as a writing-plans doc in docs/superpowers/followup/pr<NNN>-<topic>.md rather than accumulated in the PR that found it. Five handoffs from #3803: flasher_recover (convoy-safe recovery for J-Link boards, seven validated on the rig), the blindness reporting gaps, the usbtest recovery reserve, the IAR re-run spec, and the pci-rebind stranding question. Each carries what is already established with its citations and measurements, what remains, and why it was split out. One doc per follow-up, not one per PR: a per-PR file invites unrelated work into the same document and rots as a unit.
This commit is contained in:
@ -20,6 +20,7 @@ Bias toward caution over speed. For trivial tasks, use judgment.
|
||||
- **Safety:** no dynamic allocation; defer ISR work to task context; use `TU_ASSERT()` for error checks; always check return values; include order: C stdlib → tusb common → drivers → classes.
|
||||
- **Layout:** `src/` core, `hw/{mcu,bsp}/` MCU+BSP, `examples/{device,host,dual}/`, `test/{unit-test,fuzz,hil}/`, `docs/`, `tools/`.
|
||||
- **Commits/PRs:** imperative mood, scoped changes, link issues, include test/build evidence. After opening a PR, drive it to green: address automated review comments (Copilot/Codex/Claude) and fix failing CI, pushing follow-ups until checks pass and threads resolve. Useful: `gh pr checks <num> --watch`, `gh pr view <num> --comments`.
|
||||
- **Deferred work:** work that is worth doing but is a *separate scope* from the current PR — it deserves its own PR, written by a different session. Write it as a **handoff** with the `superpowers:writing-plans` skill, one doc per follow-up, in `docs/superpowers/followup/pr<NNN>-<topic>.md` (the PR it was split out of, so the origin stays traceable). Say what is already established (with citations/measurements), what remains, and why it was split out. Delete the doc when its PR lands. Never bundle unrelated follow-ups into one file.
|
||||
- **Formatting/lint:** `clang-format` (`.clang-format`), `codespell` (`.codespellrc`); run `pre-commit run --all-files` before submitting.
|
||||
|
||||
## Bootstrap
|
||||
|
||||
280
docs/superpowers/followup/pr3803-flasher-recover.md
Normal file
280
docs/superpowers/followup/pr3803-flasher-recover.md
Normal file
@ -0,0 +1,280 @@
|
||||
# `flasher_recover` Implementation Plan
|
||||
|
||||
> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking.
|
||||
|
||||
**Goal:** Give the 15 HIL boards whose flasher cannot reach its probe past a poisoned usbfs
|
||||
node a second, convoy-safe flasher used only for recovery.
|
||||
|
||||
**Architecture:** An optional roster key `flasher_recover` beside `flasher`.
|
||||
`hil_flash.recover_flasher(board)` picks it when present; `hil_test` substitutes it into the
|
||||
`--recover-board` JSON so `usbtest.py` never learns a second entry exists. Delivery over
|
||||
openocd's jlink driver is convoy-safe by construction, but the flash command form must
|
||||
differ from the one `flash_openocd` uses, so the recovery gets its own flasher name.
|
||||
|
||||
**Tech Stack:** Python 3.13 stdlib, openocd 0.12.0+dev (build 0ce743125 on ci.lan),
|
||||
libjaylink, J-Link probes.
|
||||
|
||||
## Global Constraints
|
||||
|
||||
- Roster JSON: `test/hil/tinyusb.json`. `flasher_recover` is OPTIONAL; absent means today's
|
||||
behaviour (`recover_flasher` returns the primary).
|
||||
- Never change the shape of `board['flasher']` — it is read as a dict in `hil_flash`,
|
||||
`hil_test`, `usbtest`, `hil_pool_check`, `hil_select` and the roster lint, and is shipped
|
||||
as JSON to a subprocess.
|
||||
- Flasher dispatch is by name: `getattr(hil_flash, f'flash_{name}')` / `reset_{name}`.
|
||||
- `RECOVER_FLASH_TIMEOUT = 90`, `RECOVER_RESET_TIMEOUT = 30` (`usbtest.py`). Any board whose
|
||||
flash cannot finish inside 90 s is not a candidate.
|
||||
- Tests run offline: `cd test/hil && python3 test/test_hil_select.py`.
|
||||
|
||||
## What is already established
|
||||
|
||||
**Landed on PR #3803 and inert without roster entries:** `hil_flash.recover_flasher()`,
|
||||
`convoy_safe()` accepting openocd-over-jlink, `hil_test` substituting the recovery flasher
|
||||
into `--recover-board`, and `test_hil_select.FlasherRecoverEntry` (4 tests).
|
||||
|
||||
**Verified in source:**
|
||||
- openocd's jlink driver ignores `adapter usb vid_pid` — `jlink.c` never reads
|
||||
`adapter_usb_get_vids/pids`; selection is `adapter serial` / USB address / usb location.
|
||||
Do NOT lint a jlink recovery entry for `vid_pid`.
|
||||
- It is convoy-safe anyway: libjaylink `discovery_usb.c` returns early unless
|
||||
`idVendor == 0x1366` and the PID is in its table, and only THEN calls `libusb_open`. A
|
||||
wedged `cafe:4010` DUT is never opened.
|
||||
- CMSIS-DAP stays pin-gated: `cmsis_dap_usb_bulk.c:107` skips before `libusb_open`, and
|
||||
`id_filter` is only `vids[0] || pids[0]`.
|
||||
|
||||
**Measured on ci.lan 2026-08-17**, base args
|
||||
`-f interface/jlink.cfg -c "transport select swd" -c "adapter speed 4000" -f target/<cfg>`:
|
||||
|
||||
| Board | target cfg | flash | reset |
|
||||
|--------------------------|--------------|-------|-------|
|
||||
| stm32f407disco | stm32f4x | OK | OK |
|
||||
| stm32f072disco | stm32f0x | OK | OK |
|
||||
| stm32f723disco | stm32f7x | OK | OK |
|
||||
| stm32l476disco | stm32l4x | OK | OK |
|
||||
| feather_nrf52840_express | nrf52 | OK | OK |
|
||||
| metro_m4_express | atsame5x | OK | OK |
|
||||
| frdm_k64f | k60 | OK | OK |
|
||||
|
||||
`frdm_k64f` is host-only (`tests.device == false`) — verify its reset over UART
|
||||
(`/dev/serial/by-id/usb-SEGGER_J-Link_000621000000-if00`), never by USB disconnect.
|
||||
|
||||
**Excluded, with reasons:** `lpcxpresso11u37` — 118 s for 24 KB at 1 MHz with a verify
|
||||
mismatch, versus 0.277 s via JLinkExe; cannot fit `RECOVER_FLASH_TIMEOUT`.
|
||||
`mimxrt1064_evk`, `ra4m1_ek`, `nrf54lm20dk` — no target config exists in this openocd
|
||||
build, so they cannot be covered at all. **The board that wedges most (mimxrt1064_evk) is
|
||||
therefore still uncovered by this work.**
|
||||
|
||||
**The blocker this plan solves:** `flash_openocd` issues `program <fw> verify reset exit`,
|
||||
which fails over the jlink transport on BOTH families tried (`stm32f4x`, `stm32f0x`) with
|
||||
`Examination failed` → `auto_probe failed`, with or without a preceding `init; reset halt`.
|
||||
Every successful flash above used the explicit sequence in Task 1.
|
||||
|
||||
**Why this is a separate PR:** it adds a roster capability and a new flasher backend, which
|
||||
is a different scope from containing a wedge; and it needs bench time on seven boards.
|
||||
|
||||
## File Structure
|
||||
|
||||
- `test/hil/hil_flash.py` — add `flash_openocd_seq` / `reset_openocd_seq`; extend
|
||||
`convoy_safe` to accept the new name. This is the only file that learns the command form.
|
||||
- `test/hil/tinyusb.json` — seven `flasher_recover` entries.
|
||||
- `test/hil/test/test_hil_select.py` — extend `FlasherRecoverEntry`; add a roster lint.
|
||||
|
||||
---
|
||||
|
||||
### Task 1: `openocd_seq` flasher backend
|
||||
|
||||
**Files:**
|
||||
- Modify: `test/hil/hil_flash.py` (beside `flash_openocd`, ~line 100)
|
||||
- Test: `test/hil/test/test_hil_select.py`
|
||||
|
||||
**Interfaces:**
|
||||
- Consumes: `_openocd_cmd_base(flasher)`, `hil_util.run_cmd`.
|
||||
- Produces: `flash_openocd_seq(board, firmware, timeout=None)`,
|
||||
`reset_openocd_seq(board, timeout=None)`, both returning
|
||||
`subprocess.CompletedProcess`; `convoy_safe()` returns True for
|
||||
`{'name': 'openocd_seq', 'args': '...interface/jlink.cfg...'}`.
|
||||
|
||||
- [ ] **Step 1: Write the failing test**
|
||||
|
||||
```python
|
||||
def test_openocd_seq_is_convoy_safe_over_jlink(self):
|
||||
self.assertTrue(hil_flash.convoy_safe(
|
||||
{'name': 'openocd_seq', 'args': '-f interface/jlink.cfg -f target/stm32f4x.cfg'}))
|
||||
|
||||
def test_openocd_seq_uses_explicit_flash_commands_not_program(self):
|
||||
"""`program` fails over the jlink transport: Examination failed -> auto_probe
|
||||
failed, measured on stm32f4x and stm32f0x."""
|
||||
seen = {}
|
||||
real = hil_util.run_cmd
|
||||
hil_util.run_cmd = lambda cmd, **k: seen.setdefault('cmd', cmd) or real('true')
|
||||
try:
|
||||
hil_flash.flash_openocd_seq(
|
||||
{'flasher': {'name': 'openocd_seq', 'uid': 'X', 'args': '-f interface/jlink.cfg'}},
|
||||
'/tmp/fw.elf', timeout=5)
|
||||
finally:
|
||||
hil_util.run_cmd = real
|
||||
self.assertIn('flash write_image erase /tmp/fw.elf', seen['cmd'])
|
||||
self.assertIn('verify_image /tmp/fw.elf', seen['cmd'])
|
||||
self.assertNotIn('program ', seen['cmd'])
|
||||
```
|
||||
|
||||
- [ ] **Step 2: Run test to verify it fails**
|
||||
|
||||
Run: `cd test/hil && python3 test/test_hil_select.py FlasherRecoverEntry -v`
|
||||
Expected: FAIL — `module 'hil_flash' has no attribute 'flash_openocd_seq'`
|
||||
|
||||
- [ ] **Step 3: Write minimal implementation**
|
||||
|
||||
```python
|
||||
def flash_openocd_seq(board, firmware, timeout=None):
|
||||
# Explicit commands, NOT `program`: over the jlink transport `program` fails at the
|
||||
# flash bank probe ("Examination failed" -> "auto_probe failed"), measured on
|
||||
# stm32f4x and stm32f0x, with or without a preceding reset halt. This sequence
|
||||
# succeeded on all seven candidate boards.
|
||||
flasher = board['flasher']
|
||||
verify = f' -c "verify_image {firmware}"' if flasher.get('verify', True) else ''
|
||||
return hil_util.run_cmd(
|
||||
f'{_openocd_cmd_base(flasher)} -c "init" -c "reset halt" '
|
||||
f'-c "flash write_image erase {firmware}"{verify} -c "reset run" -c "shutdown"',
|
||||
timeout=timeout)
|
||||
|
||||
|
||||
def reset_openocd_seq(board, timeout=None):
|
||||
flasher = board['flasher']
|
||||
return hil_util.run_cmd(
|
||||
f'{_openocd_cmd_base(flasher)} -c "init" -c "reset run" -c "shutdown"',
|
||||
timeout=timeout)
|
||||
```
|
||||
|
||||
In `convoy_safe`, replace `if name != 'openocd':` with:
|
||||
|
||||
```python
|
||||
if name not in ('openocd', 'openocd_seq'):
|
||||
return False
|
||||
```
|
||||
|
||||
- [ ] **Step 4: Run test to verify it passes**
|
||||
|
||||
Run: `cd test/hil && python3 test/test_hil_select.py FlasherRecoverEntry -v`
|
||||
Expected: PASS
|
||||
|
||||
- [ ] **Step 5: Commit**
|
||||
|
||||
```bash
|
||||
git add test/hil/hil_flash.py test/hil/test/test_hil_select.py
|
||||
git commit -m "hil: add openocd_seq flasher for convoy-safe recovery delivery"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
### Task 2: Roster entries for the seven validated boards
|
||||
|
||||
**Files:**
|
||||
- Modify: `test/hil/tinyusb.json`
|
||||
- Test: `test/hil/test/test_hil_select.py`
|
||||
|
||||
**Interfaces:**
|
||||
- Consumes: `flash_openocd_seq` / `reset_openocd_seq` from Task 1.
|
||||
- Produces: seven boards for which `hil_flash.convoy_safe(hil_flash.recover_flasher(b))`
|
||||
is True.
|
||||
|
||||
- [ ] **Step 1: Write the failing test**
|
||||
|
||||
```python
|
||||
def test_roster_recover_entries_are_convoy_safe_and_named_openocd_seq(self):
|
||||
import json, pathlib
|
||||
roster = json.loads((pathlib.Path(__file__).parent.parent / 'tinyusb.json').read_text())
|
||||
recover = [b for b in roster['boards'] if 'flasher_recover' in b]
|
||||
self.assertGreaterEqual(len(recover), 7)
|
||||
for b in recover:
|
||||
f = b['flasher_recover']
|
||||
self.assertEqual(f['name'], 'openocd_seq', b['name'])
|
||||
self.assertIn('interface/jlink.cfg', f['args'], b['name'])
|
||||
self.assertIn('adapter speed', f['args'], b['name']) # required; see below
|
||||
self.assertTrue(hil_flash.convoy_safe(f), b['name'])
|
||||
```
|
||||
|
||||
- [ ] **Step 2: Run test to verify it fails**
|
||||
|
||||
Run: `cd test/hil && python3 test/test_hil_select.py FlasherRecoverEntry -v`
|
||||
Expected: FAIL — `0 >= 7`
|
||||
|
||||
- [ ] **Step 3: Add the entries**
|
||||
|
||||
`adapter speed` is REQUIRED: without it examination fails outright on the jlink driver.
|
||||
Add to each board below, using the SAME `uid` as its primary jlink entry:
|
||||
|
||||
```json
|
||||
"flasher_recover": {
|
||||
"name": "openocd_seq",
|
||||
"uid": "<same probe serial as flasher.uid>",
|
||||
"args": "-f interface/jlink.cfg -c \"transport select swd\" -c \"adapter speed 4000\" -f target/<cfg>.cfg"
|
||||
}
|
||||
```
|
||||
|
||||
| Board | `uid` | `<cfg>` |
|
||||
|--------------------------|----------------|-----------|
|
||||
| stm32f407disco | 000773661813 | stm32f4x |
|
||||
| stm32f072disco | 779541626 | stm32f0x |
|
||||
| stm32f723disco | 000776606156 | stm32f7x |
|
||||
| stm32l476disco | 777632258 | stm32l4x |
|
||||
| feather_nrf52840_express | 681295394 | nrf52 |
|
||||
| metro_m4_express | 123456 | atsame5x |
|
||||
| frdm_k64f | 000621000000 | k60 |
|
||||
|
||||
- [ ] **Step 4: Run test to verify it passes**
|
||||
|
||||
Run: `cd test/hil && python3 test/test_hil_select.py -v`
|
||||
Expected: PASS, and no other selector test regresses.
|
||||
|
||||
- [ ] **Step 5: Commit**
|
||||
|
||||
```bash
|
||||
git add test/hil/tinyusb.json test/hil/test/test_hil_select.py
|
||||
git commit -m "hil: give seven J-Link boards a convoy-safe recovery flasher"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
### Task 3: Bench validation on the rig
|
||||
|
||||
**Files:** none — this task produces evidence, not code.
|
||||
|
||||
- [ ] **Step 1: Confirm the rig is idle and take the locks**
|
||||
|
||||
```bash
|
||||
ssh hathach@ci.lan 'if pgrep -f "[h]il_test.py" >/dev/null; then echo BUSY; exit 1; fi'
|
||||
ssh hathach@ci.lan 'cd ~/actions-runner/_work/tinyusb/tinyusb && \
|
||||
nohup timeout 900 python3 test/hil/helper/hil_lock.py hold <boards...> --reason "flasher_recover validation" &'
|
||||
```
|
||||
|
||||
Guard with `if`, never `cmd && echo || echo` — that form only gates the echo and will take
|
||||
locks during a live CI run.
|
||||
|
||||
- [ ] **Step 2: For each board, flash then reset through the recovery entry**
|
||||
|
||||
```bash
|
||||
python3 test/hil/hil_test.py -b <board> test/hil/tinyusb.json # normal path still works
|
||||
```
|
||||
|
||||
Then force the recovery path by running usbtest with the recovery flags and a firmware that
|
||||
hangs a case, or drive `hil_flash.flash_openocd_seq` / `reset_openocd_seq` directly.
|
||||
|
||||
- [ ] **Step 3: Verify**
|
||||
|
||||
Device boards: `sudo dmesg` shows `USB disconnect` then a fresh enumeration.
|
||||
`frdm_k64f`: UART shows the boot banner (see above).
|
||||
Every flash must finish well inside `RECOVER_FLASH_TIMEOUT` (90 s).
|
||||
|
||||
- [ ] **Step 4: Release locks and record the results in the PR body**
|
||||
|
||||
---
|
||||
|
||||
## Out of scope, and why
|
||||
|
||||
- **`mimxrt1064_evk`** needs an i.MX RT target config that this openocd build does not
|
||||
have. Sourcing or writing one is its own investigation; until then the board with the
|
||||
most wedges has no automated recovery.
|
||||
- **Changing `flash_openocd`** to the explicit form would cover these boards without a new
|
||||
name, but `program` is what nine pinned CMSIS-DAP boards use in CI daily and no CMSIS-DAP
|
||||
image could be built in the originating worktree (no pico-sdk) to re-validate it.
|
||||
185
docs/superpowers/followup/pr3803-hil-blindness-reporting.md
Normal file
185
docs/superpowers/followup/pr3803-hil-blindness-reporting.md
Normal file
@ -0,0 +1,185 @@
|
||||
# Blindness Reporting Gaps Implementation Plan
|
||||
|
||||
> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking.
|
||||
|
||||
**Goal:** Make a HIL worker's sysfs blindness reach the report in the two cases where it
|
||||
currently does not — an untested producer, and a board that raises.
|
||||
|
||||
**Architecture:** A worker returns `hil_util.sysfs_blind()` as the last field of its result
|
||||
tuple; `_blind_note()` turns that into a report banner. Two holes: nothing tests the
|
||||
producer, and a board that raises returns no tuple at all, so its blindness is lost.
|
||||
|
||||
**Tech Stack:** Python 3.13 stdlib, multiprocessing Pool with `maxtasksperchild=1`.
|
||||
|
||||
## Global Constraints
|
||||
|
||||
- A blind worker answers `SYSFS_UNKNOWN` for every attribute, so its "device not found"
|
||||
means "could not tell". The report must say so or a red cell reads as a broken board.
|
||||
- `maxtasksperchild=1`: one worker per board, so the flag is per-board and must not be
|
||||
smeared across boards.
|
||||
- Tests: `cd test/hil && python3 test/test_hil_bounded.py`.
|
||||
|
||||
## What is already established
|
||||
|
||||
- `hil_test.test_board` returns `(..., hil_util.sysfs_blind(), stray)`; `_blind_note(mret)`
|
||||
renders the banner; wired into all three report paths.
|
||||
- **The producer is provably untested**: replacing `hil_util.sysfs_blind()` with `False` in
|
||||
the return leaves all tests green. Nothing drives `test_board` — it needs a board dict, a
|
||||
real flock, a flasher and `test_example` per test.
|
||||
- Blindness fired for real on ci.lan: four workers went blind in one run, and cells failed
|
||||
*because* of it (`Printer device not found ... (this worker is blind)`).
|
||||
|
||||
**Why this is a separate PR:** closing it means making `test_board` testable, which is a
|
||||
refactor of the harness's orchestration layer — a different scope from the containment
|
||||
work, and the reason the gap was accepted rather than papered over.
|
||||
|
||||
## File Structure
|
||||
|
||||
- `test/hil/hil_test.py` — extract the result-tuple assembly from `test_board` so it can be
|
||||
built and asserted without running a board; carry blindness out of the raise path.
|
||||
- `test/hil/test/test_hil_bounded.py` — tests for both.
|
||||
|
||||
---
|
||||
|
||||
### Task 1: Make the result tuple assembly testable
|
||||
|
||||
**Files:**
|
||||
- Modify: `test/hil/hil_test.py` (`test_board`, the `return (name, err_count, ...)` at the
|
||||
end of the try block)
|
||||
- Test: `test/hil/test/test_hil_bounded.py`
|
||||
|
||||
**Interfaces:**
|
||||
- Produces: `_board_result(name, err_count, failed_tests, rows, t_total, board_wide_fail)`
|
||||
returning the 7-tuple `(name, err_count, failed, rows, t_total, blind, stray)`, reading
|
||||
`hil_util.sysfs_blind()` and `hil_health.kill_own_children()` itself.
|
||||
|
||||
- [ ] **Step 1: Write the failing test**
|
||||
|
||||
```python
|
||||
class BoardResultCarriesBlindness(unittest.TestCase):
|
||||
def test_a_blind_worker_reports_it(self):
|
||||
from helper import hil_util, hil_health
|
||||
self.addCleanup(setattr, hil_util, 'sysfs_blind', hil_util.sysfs_blind)
|
||||
self.addCleanup(setattr, hil_health, 'kill_own_children', hil_health.kill_own_children)
|
||||
hil_util.sysfs_blind = lambda: True
|
||||
hil_health.kill_own_children = lambda: 0
|
||||
row = hil_test._board_result('b', 0, [], [], 1.0, False)
|
||||
self.assertTrue(row[5], 'blindness did not reach the result tuple')
|
||||
self.assertIn('b', hil_test._blind_note([row]))
|
||||
|
||||
def test_a_sighted_worker_does_not(self):
|
||||
from helper import hil_util, hil_health
|
||||
self.addCleanup(setattr, hil_util, 'sysfs_blind', hil_util.sysfs_blind)
|
||||
self.addCleanup(setattr, hil_health, 'kill_own_children', hil_health.kill_own_children)
|
||||
hil_util.sysfs_blind = lambda: False
|
||||
hil_health.kill_own_children = lambda: 0
|
||||
row = hil_test._board_result('b', 0, [], [], 1.0, False)
|
||||
self.assertFalse(row[5])
|
||||
self.assertEqual(hil_test._blind_note([row]), '')
|
||||
```
|
||||
|
||||
- [ ] **Step 2: Run test to verify it fails**
|
||||
|
||||
Run: `cd test/hil && python3 test/test_hil_bounded.py BoardResultCarriesBlindness -v`
|
||||
Expected: FAIL — `module 'hil_test' has no attribute '_board_result'`
|
||||
|
||||
- [ ] **Step 3: Write minimal implementation**
|
||||
|
||||
```python
|
||||
def _board_result(name, err_count, failed_tests, rows, t_total, board_wide_fail):
|
||||
"""Assemble a worker's result tuple. Separate from test_board so the two fields only
|
||||
the WORKER can answer -- its process-global blindness latch and what it could not kill
|
||||
-- are testable without running a board."""
|
||||
stray = hil_health.kill_own_children()
|
||||
return (name, err_count, [] if board_wide_fail else sorted(set(failed_tests)),
|
||||
rows, t_total, hil_util.sysfs_blind(), stray)
|
||||
```
|
||||
|
||||
Replace the tail of `test_board` with:
|
||||
|
||||
```python
|
||||
return _board_result(name, err_count, failed_tests, rows, t_total, board_wide_fail)
|
||||
```
|
||||
|
||||
- [ ] **Step 4: Run test to verify it passes**
|
||||
|
||||
Run: `cd test/hil && python3 test/test_hil_bounded.py -v`
|
||||
Expected: PASS, and the existing `BlindWorkerReachesTheReport` tests still pass.
|
||||
|
||||
- [ ] **Step 5: Verify the mutation is now caught**
|
||||
|
||||
Replace `hil_util.sysfs_blind()` with `False` inside `_board_result` and re-run; the suite
|
||||
MUST fail. Restore it.
|
||||
|
||||
- [ ] **Step 6: Commit**
|
||||
|
||||
```bash
|
||||
git add test/hil/hil_test.py test/hil/test/test_hil_bounded.py
|
||||
git commit -m "test/hil: make the worker result tuple testable, covering blindness"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
### Task 2: Carry blindness out of the worker-raise path
|
||||
|
||||
**Files:**
|
||||
- Modify: `test/hil/hil_test.py` (`test_board`'s except/finally, and `main`'s worker-raise
|
||||
handler that builds synthetic rows)
|
||||
- Test: `test/hil/test/test_hil_bounded.py`
|
||||
|
||||
**Interfaces:**
|
||||
- Consumes: `_board_result` from Task 1.
|
||||
- Produces: a board that raises still contributes a row whose blindness field is accurate.
|
||||
|
||||
- [ ] **Step 1: Write the failing test**
|
||||
|
||||
```python
|
||||
def test_a_board_that_raises_still_reports_blindness(self):
|
||||
"""The result tuple is returned inside a try whose finally only releases the lock,
|
||||
so a board that dies by exception contributed nothing -- and its blindness, the
|
||||
thing that most explains its failure, was lost with it."""
|
||||
from helper import hil_util
|
||||
self.addCleanup(setattr, hil_util, 'sysfs_blind', hil_util.sysfs_blind)
|
||||
hil_util.sysfs_blind = lambda: True
|
||||
row = hil_test._board_result_on_error('b', RuntimeError('boom'))
|
||||
self.assertTrue(row[5])
|
||||
self.assertIn('b', hil_test._blind_note([row]))
|
||||
```
|
||||
|
||||
- [ ] **Step 2: Run test to verify it fails**
|
||||
|
||||
Run: `cd test/hil && python3 test/test_hil_bounded.py BoardResultCarriesBlindness -v`
|
||||
Expected: FAIL — no `_board_result_on_error`
|
||||
|
||||
- [ ] **Step 3: Write minimal implementation**
|
||||
|
||||
```python
|
||||
def _board_result_on_error(name, exc):
|
||||
"""A row for a board that died by exception. err_count 1, no per-test detail, but the
|
||||
blindness and stray fields are still accurate -- they explain the failure more often
|
||||
than the exception text does."""
|
||||
rows = [(name, {BOUNDARY_CELL: f'{REPORT_CELL["fail"]} {type(exc).__name__}'}, None)]
|
||||
return _board_result(name, 1, [], rows, 0.0, True)
|
||||
```
|
||||
|
||||
Wrap the body of `test_board` so the exception path returns it instead of propagating.
|
||||
|
||||
- [ ] **Step 4: Run test to verify it passes**
|
||||
|
||||
Run: `cd test/hil && python3 test/test_hil_bounded.py -v`
|
||||
Expected: PASS
|
||||
|
||||
- [ ] **Step 5: Commit**
|
||||
|
||||
```bash
|
||||
git add test/hil/hil_test.py test/hil/test/test_hil_bounded.py
|
||||
git commit -m "test/hil: keep a raising board's blindness in the report"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Caution
|
||||
|
||||
`test_board`'s `finally` releases the board flock. Any restructuring MUST keep that
|
||||
release on every path, including the new error path — a leaked flock locks the board until
|
||||
the host reboots.
|
||||
118
docs/superpowers/followup/pr3803-hil-iar-rerun-spec.md
Normal file
118
docs/superpowers/followup/pr3803-hil-iar-rerun-spec.md
Normal file
@ -0,0 +1,118 @@
|
||||
# IAR HIL Leg Re-run Spec Implementation Plan
|
||||
|
||||
> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking.
|
||||
|
||||
**Goal:** Let the `hil-hfp-iar` CI leg re-run only its failed boards, as the other two HIL
|
||||
legs already do.
|
||||
|
||||
**Architecture:** `hil_test.py` writes a `<config>.failed` spec into `HIL_REPORT_DIR`; a
|
||||
workflow step reads it on the next attempt and passes the boards back as arguments. The IAR
|
||||
leg passes `--retry 1` like the others but sets no `HIL_REPORT_DIR` and has no read-back
|
||||
step, so its spec is written into the workspace and never read.
|
||||
|
||||
**Tech Stack:** GitHub Actions YAML, self-hosted runner.
|
||||
|
||||
## Global Constraints
|
||||
|
||||
- `.github/workflows/build.yml`. The two working legs are `hil-tinyusb` (matrix) — see its
|
||||
`Set HIL report dir (per run+job; persists across run attempts)` and `Get re-run spec from
|
||||
previous attempt` steps — and they are the pattern to copy.
|
||||
- The report dir must be keyed by run id AND job so a matrix leg does not collide with
|
||||
another, and must survive across run attempts (that is the whole point).
|
||||
- The IAR leg is the only HIL job that BUILDS inline; its `Build` step is bounded at
|
||||
`timeout-minutes: 30` under a 120-minute job ceiling. Do not disturb that.
|
||||
|
||||
## What is already established
|
||||
|
||||
- Verified by reading the workflow: `hil-hfp-iar` has neither `HIL_REPORT_DIR` nor a
|
||||
`Get re-run spec` step, while passing `--retry 1`.
|
||||
- Consequence: a GitHub re-run of that job re-tests its whole matrix. **This is not a
|
||||
regression** — that leg never had the mechanism — and the unread spec costs only a file.
|
||||
- The report artifact upload for that leg is named `hil-report-hfp-iar`.
|
||||
|
||||
**Why this is a separate PR:** it is CI plumbing with no code change, it needs a real
|
||||
re-run on the self-hosted runner to prove, and it duplicates ~15 lines of workflow that
|
||||
would be better factored — a decision worth making on its own.
|
||||
|
||||
## File Structure
|
||||
|
||||
- `.github/workflows/build.yml` — the `hil-hfp-iar` job only.
|
||||
|
||||
---
|
||||
|
||||
### Task 1: Give the IAR leg a persistent report dir and a re-run spec
|
||||
|
||||
**Files:**
|
||||
- Modify: `.github/workflows/build.yml` (job `hil-hfp-iar`)
|
||||
|
||||
**Interfaces:**
|
||||
- Consumes: `hil_test.py`'s existing `--report-dir` / `.failed` behaviour — no code change.
|
||||
- Produces: `env.HIL_REPORT_DIR` for the job, and `$RERUN_ARGS` for the test step.
|
||||
|
||||
- [ ] **Step 1: Copy the two steps from `hil-tinyusb`, before the Build step**
|
||||
|
||||
```yaml
|
||||
- name: Set HIL report dir (per run+job; persists across run attempts)
|
||||
run: |
|
||||
BASE=$HOME/hil-reports
|
||||
echo "HIL_REPORT_DIR=$BASE/${GITHUB_RUN_ID}-hfp-iar" >> "$GITHUB_ENV"
|
||||
|
||||
- name: Get re-run spec from previous attempt
|
||||
run: |
|
||||
SPEC="$HIL_REPORT_DIR/hfp.json.failed"
|
||||
if [ -f "$SPEC" ]; then
|
||||
echo "RERUN_ARGS=$(cat "$SPEC")" >> "$GITHUB_ENV"
|
||||
echo "re-running only: $(cat "$SPEC")"
|
||||
fi
|
||||
```
|
||||
|
||||
Match the exact spec filename `hil_test.py` writes for this leg's config — read
|
||||
`_write_failed_spec` and the `failed_fname` construction rather than assuming.
|
||||
|
||||
- [ ] **Step 2: Pass the spec to the test step**
|
||||
|
||||
```yaml
|
||||
python3 test/hil/hil_test.py --retry 1 $SEL_ARGS hfp.json $RERUN_ARGS
|
||||
```
|
||||
|
||||
`--retry 1` stays FIRST so argparse's last-wins keeps any explicit override working.
|
||||
|
||||
- [ ] **Step 3: Point the artifact upload at the report dir**
|
||||
|
||||
```yaml
|
||||
path: ${{ env.HIL_REPORT_DIR }}/hil_report.md
|
||||
```
|
||||
|
||||
- [ ] **Step 4: Validate the YAML**
|
||||
|
||||
Run: `python3 -c "import yaml,sys; d=yaml.safe_load(open('.github/workflows/build.yml')); j=d['jobs']['hil-hfp-iar']; print(j['timeout-minutes'], [s.get('name') for s in j['steps']])"`
|
||||
Expected: the ceiling is still 120, the Build step still carries `timeout-minutes: 30`, and
|
||||
the two new steps appear before Build.
|
||||
|
||||
- [ ] **Step 5: Commit**
|
||||
|
||||
```bash
|
||||
git add .github/workflows/build.yml
|
||||
git commit -m "ci: let the IAR HIL leg re-run only its failed boards"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
### Task 2: Prove it on a real re-run
|
||||
|
||||
**Files:** none — evidence only.
|
||||
|
||||
- [ ] **Step 1:** Push and let `hil-hfp-iar` run to a failure (or force one).
|
||||
- [ ] **Step 2:** Confirm `$HIL_REPORT_DIR/hfp.json.failed` exists on the runner after the
|
||||
job.
|
||||
- [ ] **Step 3:** Use GitHub's "Re-run failed jobs" and confirm the log line
|
||||
`re-running only: ...` and that only those boards are tested.
|
||||
- [ ] **Step 4:** Record the run URL in the PR body.
|
||||
|
||||
---
|
||||
|
||||
## Consider first
|
||||
|
||||
Three jobs would then carry the same ~15 lines. Factoring them into a composite action, or
|
||||
computing the report dir inside `hil_test.py` from `GITHUB_RUN_ID`, may be the better
|
||||
change — decide that before copying the block a third time.
|
||||
157
docs/superpowers/followup/pr3803-pci-rebind-stranding.md
Normal file
157
docs/superpowers/followup/pr3803-pci-rebind-stranding.md
Normal file
@ -0,0 +1,157 @@
|
||||
# `pci-rebind` Stranding Investigation Plan
|
||||
|
||||
> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking.
|
||||
|
||||
**Goal:** Settle when a PCI unbind/rebind of an xHCI controller strands it driverless, so
|
||||
the `usb-kernel-recover` skill can state a rule instead of a hypothesis.
|
||||
|
||||
**Architecture:** No product code. This is a controlled reproduction against the rig's
|
||||
kernel, ending in a documentation change and — if the boundary turns out to be
|
||||
detectable — a guard in `usb_recover.sh`.
|
||||
|
||||
**Tech Stack:** Linux 6.12.96 (ci.lan), Renesas uPD720201 xHCI, `usb_recover.sh`.
|
||||
|
||||
## Global Constraints
|
||||
|
||||
- ci.lan is a live CI rig. Take every affected board's lock first
|
||||
(`hil_lock.py hold --all --reason ...`) and confirm no `hil_test.py` is running, with an
|
||||
`if`, not an `&&` chain.
|
||||
- A stranded controller takes every fixture on it offline; recovery is
|
||||
`usb_recover.sh pci-bind <addr>` or, failing that, a PVE **host** power cycle — an
|
||||
operator action. Do not start this without being able to reach the host.
|
||||
- The rig has two Renesas controllers plus an AMD one; pick the controller with the fewest
|
||||
fixtures for the experiment.
|
||||
|
||||
## What is already established
|
||||
|
||||
**The skill claimed, unconditionally, that `pci-rebind`'s re-bind hangs on the D-state URB
|
||||
and leaves the controller with no driver.** That claim was generalised from ONE observation
|
||||
and was used to delete `pci-rebind` and `pci-bind` from `usb_recover.sh` entirely.
|
||||
|
||||
**It was refuted in the field on 2026-08-17.** After `hub-cycle 17-2.7` failed to clear a
|
||||
wedge, `pci-rebind 0000:05:00.0` recovered the controller in about one second:
|
||||
|
||||
```
|
||||
02:34:41 remove, state 4 / USB bus 18 deregistered
|
||||
02:34:41 remove, state 1 / USB bus 17 deregistered
|
||||
02:34:42 xHCI Host Controller / new USB bus registered, assigned bus number 1
|
||||
02:34:42 new USB bus registered, assigned bus number 2
|
||||
```
|
||||
|
||||
Both actions were restored, with the guidance scoped to failure mode: **dead controller →
|
||||
use it; device-lock convoy → do not**. Buses renumbered 17/18 → 1/2, which is why rig-wide
|
||||
operations need every board's lock.
|
||||
|
||||
**What is NOT known:** why the earlier attempt stranded and this one did not. The leading
|
||||
hypothesis is that it turns on whether a live D-state URB exists **on that controller** at
|
||||
the moment of the re-bind — but in the 02:34 incident the wedged board (17-2.7) was on that
|
||||
very controller, which weakens it. An alternative is that `hub-cycle` had already cleared
|
||||
the holder, leaving only a dead controller.
|
||||
|
||||
**Why this is a separate PR:** it is an experiment that risks taking the rig offline, and
|
||||
its output is a documentation change plus possibly a guard — a different scope from any
|
||||
code change.
|
||||
|
||||
## File Structure
|
||||
|
||||
- `.claude/skills/usb-kernel-recover/SKILL.md` — replace the hypothesis in section 3b and
|
||||
the Common-mistakes entry with whatever the experiment establishes.
|
||||
- `.claude/skills/usb-kernel-recover/scripts/usb_recover.sh` — only if the boundary is
|
||||
detectable from userspace.
|
||||
|
||||
---
|
||||
|
||||
### Task 1: Reproduce a controller-scoped D-state wedge
|
||||
|
||||
**Files:** none.
|
||||
|
||||
- [ ] **Step 1: Establish the safety net**
|
||||
|
||||
```bash
|
||||
ssh hathach@ci.lan 'if pgrep -f "[h]il_test.py" >/dev/null; then echo BUSY; exit 1; fi'
|
||||
# hold ALL boards on the target controller
|
||||
```
|
||||
|
||||
Confirm host access to pve.lan before continuing.
|
||||
|
||||
- [ ] **Step 2: Create a wedge deliberately**
|
||||
|
||||
Run `usbtest.py` against a board known to hang (`mimxrt1064_evk` has wedged eight times,
|
||||
TEST 9/10/24/27), or drive `testusb` directly until a case does not return.
|
||||
|
||||
- [ ] **Step 3: Confirm the holder and its controller**
|
||||
|
||||
```bash
|
||||
ps -eo pid,stat,etimes,wchan:22,args | awk '$2 ~ /D/'
|
||||
sudo cat /proc/<pid>/stack # usbdev_ioctl + [usbtest] = the owner
|
||||
readlink -f /sys/bus/usb/devices/usb<N> # bus -> PCI addr
|
||||
```
|
||||
|
||||
Record whether the holder is on the SAME controller you will rebind.
|
||||
|
||||
---
|
||||
|
||||
### Task 2: Rebind and record the outcome
|
||||
|
||||
**Files:** none.
|
||||
|
||||
- [ ] **Step 1: Rebind, with a bounded observer**
|
||||
|
||||
```bash
|
||||
timeout 120 sudo usb_recover.sh pci-rebind <addr>; echo "rc=$?"
|
||||
```
|
||||
|
||||
- [ ] **Step 2: Record which of the three outcomes occurred**
|
||||
|
||||
1. Re-bind completes, controller recovers (as on 2026-08-17).
|
||||
2. Re-bind hangs; `/sys/bus/pci/devices/<addr>/driver` is gone → **stranded**.
|
||||
3. Re-bind completes but the wedge persists.
|
||||
|
||||
Capture `sudo journalctl -k --since ...` around the attempt either way.
|
||||
|
||||
- [ ] **Step 3: If stranded, recover**
|
||||
|
||||
```bash
|
||||
sudo usb_recover.sh pci-bind <addr>
|
||||
```
|
||||
|
||||
If that hangs too, the only remaining step is a PVE host power cycle — an operator action.
|
||||
|
||||
- [ ] **Step 4: Repeat at least three times**
|
||||
|
||||
One observation is what produced the wrong rule in the first place. Vary whether a D-state
|
||||
holder is live on that controller at rebind time; that is the hypothesis under test.
|
||||
|
||||
---
|
||||
|
||||
### Task 3: Write down what was learned
|
||||
|
||||
**Files:**
|
||||
- Modify: `.claude/skills/usb-kernel-recover/SKILL.md`
|
||||
|
||||
- [ ] **Step 1: Replace section 3b's scoping with the measured rule**
|
||||
|
||||
State the condition under which stranding occurs, with the journal lines. If the experiment
|
||||
does NOT reproduce stranding, say that too, with the attempt count — "not reproduced in N
|
||||
attempts" is a better record than an unexplained warning.
|
||||
|
||||
- [ ] **Step 2: If the boundary is detectable, guard the script**
|
||||
|
||||
For example, refuse `pci-rebind` when a D-state holder exists on that controller, since the
|
||||
holder is enumerable from `/proc` and the controller from `readlink`. Only add this if the
|
||||
experiment shows it predicts the outcome.
|
||||
|
||||
- [ ] **Step 3: Commit**
|
||||
|
||||
```bash
|
||||
git add .claude/skills/usb-kernel-recover/
|
||||
git commit -m "skills: replace the pci-rebind stranding hypothesis with measurement"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Abort criteria
|
||||
|
||||
Stop and hand back to the operator if: a rebind strands the controller and `pci-bind` does
|
||||
not recover it; `uhubctl` starts hanging (the convoy has spread to the hub locks); or a CI
|
||||
run starts while the rig is in a broken state.
|
||||
175
docs/superpowers/followup/pr3803-usbtest-recovery-reserve.md
Normal file
175
docs/superpowers/followup/pr3803-usbtest-recovery-reserve.md
Normal file
@ -0,0 +1,175 @@
|
||||
# usbtest Recovery Reserve Implementation Plan
|
||||
|
||||
> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking.
|
||||
|
||||
**Goal:** Make the post-hang recovery reserve a derived, asserted property instead of an
|
||||
accident of four independently-set constants.
|
||||
|
||||
**Architecture:** `hil_test` passes `--budget` and `--outer-timeout` to `usbtest.py`, which
|
||||
decides at runtime whether a recovery still fits. Today the reserve survives only because
|
||||
the four numbers happen to line up; nothing ties them together or fails when they stop.
|
||||
|
||||
**Tech Stack:** Python 3.13 stdlib.
|
||||
|
||||
## Global Constraints
|
||||
|
||||
- `usbtest.py`: `RECOVER_FLASH_TIMEOUT = 90`, `RECOVER_RESET_TIMEOUT = 30`.
|
||||
- `hil_test.py`: `USBTEST_BATTERY_BUDGET = 260`, `USBTEST_RECOVERY_BUDGET = 250`,
|
||||
`USBTEST_OVERSHOOT = 120`; `outer = BATTERY_BUDGET + (RECOVERY_BUDGET if recovery else
|
||||
OVERSHOOT)`, used for both the child's `--outer-timeout` and the parent's `run_cmd` bound.
|
||||
- All five are env-overridable via `hil_util.pos_int_env`, so a rig can change them.
|
||||
- Tests: `cd test/hil && python3 test/test_hil_health.py` and `test_hil_bounded.py`.
|
||||
|
||||
## What is already established
|
||||
|
||||
The reserve holds at the shipped values, checked by hand:
|
||||
|
||||
- The battery checks its budget BEFORE dispatching a case, so it can overshoot by one
|
||||
case — worst case `260 + 60 + 5 = 325 s`.
|
||||
- Recovery is gated on `_time_left() >= RECOVER_RESET_TIMEOUT`, where
|
||||
`_time_left() = outer_timeout - elapsed - 35`; with `outer = 510` that allows recovery
|
||||
until `elapsed = 445 s`, and the reflash until `385 s`.
|
||||
- So ~60 s of margin survives, and recovery does fire.
|
||||
|
||||
**The defect is structural, not arithmetic:** lower `--outer-timeout`, raise `--timeout`, or
|
||||
raise `USBTEST_BATTERY_BUDGET` via the env and the reserve silently disappears. The failure
|
||||
mode is a skipped reflash that leaves the D-state holder for the next job — the exact thing
|
||||
the containment exists to prevent — with no error anywhere.
|
||||
|
||||
**Why this is a separate PR:** it changes the timing contract between `hil_test` and
|
||||
`usbtest.py`, which affects every board's run duration, so it wants its own review and a
|
||||
full rig run.
|
||||
|
||||
## File Structure
|
||||
|
||||
- `test/hil/usbtest.py` — a `reserve_ok()` predicate plus a startup assertion.
|
||||
- `test/hil/hil_test.py` — derive the battery budget from the outer bound rather than
|
||||
setting both independently.
|
||||
- `test/hil/test/test_hil_health.py` — tests.
|
||||
|
||||
---
|
||||
|
||||
### Task 1: Assert the reserve at startup
|
||||
|
||||
**Files:**
|
||||
- Modify: `test/hil/usbtest.py` (constants block, and `main()` after argparse)
|
||||
- Test: `test/hil/test/test_hil_health.py`
|
||||
|
||||
**Interfaces:**
|
||||
- Produces: `usbtest.reserve_ok(budget, outer, case_timeout)` returning bool.
|
||||
|
||||
- [ ] **Step 1: Write the failing test**
|
||||
|
||||
```python
|
||||
class RecoveryReserveIsChecked(unittest.TestCase):
|
||||
"""The battery may overshoot its budget by ONE already-started case, so the outer bound
|
||||
must leave room for that overshoot AND a bounded recovery afterwards."""
|
||||
|
||||
def setUp(self):
|
||||
import usbtest
|
||||
self.u = usbtest
|
||||
|
||||
def test_the_shipped_numbers_leave_room(self):
|
||||
self.assertTrue(self.u.reserve_ok(budget=260, outer=510, case_timeout=60))
|
||||
|
||||
def test_a_tighter_outer_bound_is_rejected(self):
|
||||
self.assertFalse(self.u.reserve_ok(budget=260, outer=380, case_timeout=60))
|
||||
|
||||
def test_a_longer_case_timeout_is_rejected(self):
|
||||
self.assertFalse(self.u.reserve_ok(budget=260, outer=510, case_timeout=200))
|
||||
```
|
||||
|
||||
- [ ] **Step 2: Run test to verify it fails**
|
||||
|
||||
Run: `cd test/hil && python3 test/test_hil_health.py RecoveryReserveIsChecked -v`
|
||||
Expected: FAIL — `module 'usbtest' has no attribute 'reserve_ok'`
|
||||
|
||||
- [ ] **Step 3: Write minimal implementation**
|
||||
|
||||
```python
|
||||
def reserve_ok(budget: int, outer: int, case_timeout: int) -> bool:
|
||||
"""Does `outer` leave room for the battery's worst case AND a bounded recovery?
|
||||
|
||||
The budget is checked BEFORE dispatch, so the battery can run to
|
||||
`budget + case_timeout + 5` (the +5 is run_case's reap). _time_left() subtracts a
|
||||
further 35 s of fixed tail. A reflash needs RECOVER_FLASH_TIMEOUT beyond that.
|
||||
"""
|
||||
worst_case_end = budget + case_timeout + 5
|
||||
return outer - worst_case_end - 35 >= RECOVER_FLASH_TIMEOUT
|
||||
```
|
||||
|
||||
In `main()`, after parsing args:
|
||||
|
||||
```python
|
||||
if args.budget and args.outer_timeout and not reserve_ok(
|
||||
args.budget, args.outer_timeout, args.timeout):
|
||||
print(f'warning: --outer-timeout {args.outer_timeout} leaves no room for a bounded '
|
||||
f'recovery after a --budget {args.budget} battery with --timeout '
|
||||
f'{args.timeout} cases; a HUNG board will be left wedged', file=sys.stderr)
|
||||
```
|
||||
|
||||
Warn, do not exit: a caller that deliberately runs without recovery is legitimate.
|
||||
|
||||
- [ ] **Step 4: Run test to verify it passes**
|
||||
|
||||
Run: `cd test/hil && python3 test/test_hil_health.py RecoveryReserveIsChecked -v`
|
||||
Expected: PASS
|
||||
|
||||
- [ ] **Step 5: Commit**
|
||||
|
||||
```bash
|
||||
git add test/hil/usbtest.py test/hil/test/test_hil_health.py
|
||||
git commit -m "usbtest: check the recovery reserve instead of assuming it"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
### Task 2: Derive the outer bound from one place
|
||||
|
||||
**Files:**
|
||||
- Modify: `test/hil/hil_test.py` (constants block ~line 227, and `test_device_usbtest`)
|
||||
- Test: `test/hil/test/test_hil_bounded.py`
|
||||
|
||||
**Interfaces:**
|
||||
- Consumes: `usbtest.reserve_ok` semantics (duplicate the arithmetic, do not import
|
||||
usbtest — `hil_test` must not import it).
|
||||
- Produces: an assertion at module import that the shipped constants satisfy the reserve.
|
||||
|
||||
- [ ] **Step 1: Write the failing test**
|
||||
|
||||
```python
|
||||
def test_the_shipped_constants_satisfy_the_reserve(self):
|
||||
"""Whatever the env overrides, the pair hil_test computes must leave recovery room:
|
||||
outer - (budget + case_timeout + 5) - 35 >= 90."""
|
||||
outer = hil_test.USBTEST_BATTERY_BUDGET + hil_test.USBTEST_RECOVERY_BUDGET
|
||||
self.assertGreaterEqual(outer - (hil_test.USBTEST_BATTERY_BUDGET + 60 + 5) - 35, 90)
|
||||
```
|
||||
|
||||
- [ ] **Step 2: Run test to verify it fails**
|
||||
|
||||
Temporarily set `HIL_USBTEST_RECOVERY_BUDGET=100` and run; expect FAIL. Unset.
|
||||
|
||||
- [ ] **Step 3: Add the guard**
|
||||
|
||||
```python
|
||||
# The recovery reserve is a PROPERTY of these two, not a coincidence: the battery may
|
||||
# overshoot its budget by one already-started case (checked before dispatch), and a bounded
|
||||
# reflash needs 90 s after a 35 s fixed tail. Env overrides make this checkable at import
|
||||
# rather than discoverable when a wedge is left unrecovered.
|
||||
if USBTEST_RECOVERY_BUDGET - 60 - 5 - 35 < 90:
|
||||
print(f'warning: HIL_USBTEST_RECOVERY_BUDGET={USBTEST_RECOVERY_BUDGET} leaves no room '
|
||||
f'for a bounded reflash after a one-case overshoot; HUNG boards will stay wedged',
|
||||
file=sys.stderr)
|
||||
```
|
||||
|
||||
- [ ] **Step 4: Run tests to verify they pass**
|
||||
|
||||
Run: `cd test/hil && python3 test/test_hil_bounded.py -v`
|
||||
Expected: PASS
|
||||
|
||||
- [ ] **Step 5: Commit**
|
||||
|
||||
```bash
|
||||
git add test/hil/hil_test.py test/hil/test/test_hil_bounded.py
|
||||
git commit -m "hil: warn when the timeout constants leave no recovery reserve"
|
||||
```
|
||||
Reference in New Issue
Block a user