HIL: replace build.flags_on with named build variants#3687
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates the HIL configuration and tooling to replace the implicit build.flags_on mechanism with explicit, named variant build entries, making build directories and report rows deterministic and human-readable.
Changes:
- Introduces a
variantschema in HIL config (name+ rawflags) and migrates existingflags_onboards to the new format. - Updates HIL tooling to build and test per-variant (matrix generation, build invocation, firmware discovery/report row labeling).
- Extends
tools/build.py(andget_deps.pypassthrough) with--build-nameand repeatable--cflagsupport.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tools/get_deps.py | Accepts/ignores new build CLI args so CI arg passthrough doesn’t fail. |
| tools/build.py | Adds --build-name + --cflag to support named build output dirs and raw CFLAGS injection. |
| test/hil/tinyusb.json | Migrates HIL config from build.flags_on to explicit variant entries. |
| test/hil/hil_test.py | Iterates build variants for build/test execution and report row naming. |
| test/hil/hil_ci.sh | Copies per-board build outputs, attempting to include variant build dirs as well. |
| test/hil/hil_ci_set_matrix.py | Emits one CI build arg set per variant (named dir + cflags tokens). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
314
to
+315
| # build all boards | ||
| result = build_boards_list(all_boards, build_defines, build_system, build_flags_on, build_targets) | ||
| result = build_boards_list(all_boards, build_defines, build_system, build_name, build_cflags, build_targets) |
Owner
Author
There was a problem hiding this comment.
Fixed in d2f037b: build.py now errors out when --build-name is combined with families or more than one -b board, since the renamed build dir is global to the invocation.
Comment on lines
+69
to
+77
| # Copy the board's build dir plus any variant dirs (cmake-build-<BOARD>-<variant>). | ||
| # Collect only dirs that actually exist (the bare cmake-build-<BOARD> is a literal, | ||
| # not a glob, so it would otherwise stay in the list even when missing). | ||
| shopt -s nullglob | ||
| BUILD_DIRS=() | ||
| for d in "$ROOT_DIR"/examples/cmake-build-"$BOARD" "$ROOT_DIR"/examples/cmake-build-"$BOARD"-*; do | ||
| [ -d "$d" ] && BUILD_DIRS+=("$d") | ||
| done | ||
| shopt -u nullglob |
Owner
Author
There was a problem hiding this comment.
Fixed in d2f037b: hil_ci.sh -b now reads the variant names from $CONFIG and copies cmake-build-<variant.name> for each (deduplicated), keeping the cmake-build--* glob as a fallback for ad-hoc local builds.
Contributor
Code reviewNo issues found. Checked for bugs and CLAUDE.md compliance. |
Size Difference ReportBecause TinyUSB code size varies by port and configuration, the metrics below represent the averaged totals across all example builds. Note: If there is no change, only one value is shown. Changes >1% in sizeNo entries. Changes <1% in sizeNo entries. No changes
|
Hardware-in-the-loop (HIL) Test Reporthfp-iar
Legend: ✅ pass · ❌ fail · ⚪ skipped · blank not run hfp.json
Legend: ✅ pass · ❌ fail · ⚪ skipped · blank not run tinyusb.json
Legend: ✅ pass · ❌ fail · ⚪ skipped · blank not run |
|
| target | .text | .rodata | .data | .bss | total | % diff |
|---|---|---|---|---|---|---|
| frdm_kl25z/board_test | 308 → 304 (-4) | — | — | — | 308 → 304 (-4) | -1.3% |
| stm32l412nucleo/board_test | 240 → 232 (-8) | — | — | — | 648 → 640 (-8) | -1.2% |
| metro_m0_express/board_test | 376 → 372 (-4) | — | — | — | 376 → 372 (-4) | -1.1% |
| stm32g0b1nucleo/board_test | 216 → 212 (-4) | — | — | — | 408 → 404 (-4) | -1.0% |
| cynthion_d11/board_test | 412 → 408 (-4) | — | — | — | 412 → 408 (-4) | -1.0% |
| stm32l052dap52/board_test | 216 → 212 (-4) | — | — | — | 412 → 408 (-4) | -1.0% |
| stm32c071nucleo/board_test | 232 → 228 (-4) | — | — | — | 428 → 424 (-4) | -0.9% |
| b_u585i_iot2a/board_test | 288 → 280 (-8) | — | — | — | 864 → 856 (-8) | -0.9% |
| stm32u083cdk/board_test | 244 → 240 (-4) | — | — | — | 436 → 432 (-4) | -0.9% |
| lpcxpresso51u68/board_test | 440 → 436 (-4) | — | — | — | 440 → 436 (-4) | -0.9% |
hathach
force-pushed
the
claude/hil-variants
branch
2 times, most recently
from
3dc8416 to
85775f3
Compare
Owner
Author
|
@HiFiPhile I think I mess up your vm, at first I notice the stm32f7 jlink become stlink. So I guess the fw got reverted somehow or it go into bootloader mode. I try to reboot your VM, and now the jlink for lpcxpresso43 is also missing. I guess I have to ping you for the roll back or something. |
Collaborator
I'll try to put debuggers in a separate hub in a few days. |
Owner
Author
|
I think that is Ok. I have them all on the same hub aswell. I guess it is some weird things that got stlink reverted back to its stock firmware. Why reboot make it worse is out of my understanding. |
Collaborator
|
I just unplug and replugged. For audio test it seems a delay is needed before launching arecord. |
Boards declare build variants as `variant: [{name, flags}]` instead of
`build.flags_on`. The variant `name` is the build dir (cmake-build-<name>) and
the HIL report row; `flags` is the raw CFLAGS string (-D...=1) injected via
CFLAGS_CLI. No `variant` => a single build named after the board.
- build.py: --build-name <name> (dir) + --cflag=<token> (raw CFLAGS, repeatable,
=form survives the matrix's shell word-splitting); drop -f1/CFLAGS wrapping.
- hil_ci_set_matrix.py: emit one build arg per variant.
- hil_test.py: iterate variants; report row + build dir = variant name.
- hil_ci.sh: copy all cmake-build-<board>* dirs for -b runs.
- get_deps.py: accept (ignore) --build-name/--cflag from matrix args.
- tinyusb.json: migrate all 6 flags_on boards to variant.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The variant-aware -b path put the bare cmake-build-<BOARD> literal into the array unconditionally (nullglob only drops globs, not literals), so the empty-check never fired and a missing build silently produced an all-"no binary" run instead of erroring. Collect only dirs that actually exist, then error out if none. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Every DWC2 DMA variant now enables both -DCFG_TUD_DWC2_DMA_ENABLE=1 and -DCFG_TUH_DWC2_DMA_ENABLE=1 (stm32f723disco-DMA, stm32h743nucleo-DMA, stm32f769disco-DMA previously had only one of the two). Also add a stm32f746disco-DMA variant to hfp.json with both flags. stm32l412nucleo is left untouched (STM32L4 USB FS, not DWC2). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Each variant carries its own --build-name/--cflag, which are global to a single build.py invocation — joining all hfp.json matrix entries leaked a variant's DMA flags onto every board (e.g. stm32l412nucleo, which isn't DWC2) and collided their build dirs. Iterate the matrix entries and build each separately, matching the per-entry gcc/esp-idf matrix builds. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
hathach
force-pushed
the
claude/hil-variants
branch
from
d723e2d to
3445924
Compare
- tools/build.py: reject --build-name with families or more than one board; all boards would otherwise clobber the same renamed build dir - hil_ci.sh -b: read variant names from $CONFIG when collecting build dirs to copy, since variant names are not required to be prefixed with the board name; keep the glob as fallback and deduplicate Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
CFG_TUD_DWC2_SLAVE_ENABLE defaults to !CFG_TUD_DWC2_DMA_ENABLE, so building the variant with TUD DMA compiled slave mode out — but the f723disco device port (port0) is the slave-only FS core, leaving the device with no working transfer mode: every device test failed with no enumeration and the retries blew the 3000s HIL pool timeout. The DMA variant on this board is meant to exercise the HS host port (port1), so only enable CFG_TUH_DWC2_DMA_ENABLE. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
hil-hfp-iar hung for 26+ minutes (until the 30-min job timeout) in printer_to_cdc on stm32l412nucleo: ser.write() blocks forever when the device stops draining CDC OUT, since serial.Serial was opened with a read timeout only. With write_timeout the wedge becomes a normal test failure that goes through retry instead of stalling the whole pool. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The wfe park from #3690 bricked SWD on mimxrt1064_evk and max32666fthr: sleeping gates the debug clocks on these MCUs, and since the parked image is the boot image the board comes back unflashable — attach and connect-under-reset both fail and the state survives power cycles. Verified on the ci.lan rig: J-Link reads garbage (CPUID 0xA05F0001) on the 1064, openocd gets "stalled AP operation" on the max32666, after a full rig reboot. Both boards need one-time manual recovery. A busy spin keeps the debug port clocked on every MCU; the park goal (no USB activity) is unchanged. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Report cell now reads "✅ CDC 25.2M/20.4M MSC 35.7M/32.9M" instead of the terser "C 25.2M/20.4M M 35.7M/32.9M", so pass/fail and which numbers belong to which class are clear at a glance. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Restructure: the CI park build gets a dedicated minimal main() at the top of the file instead of ifdef blocks woven through the normal blink/echo implementation. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
IAR builds with --warnings_are_errors and flags the return after the infinite park loop: Error[Pe111] statement is unreachable (broke stm32f3/stm32wba IAR CI builds). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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
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.
Summary
Replaces the HIL config's
build.flags_onwith an explicit, namedvariantschema. Each variant declares its ownname(used as the build dircmake-build-<name>and the HIL report row) and rawflags(CFLAGS, e.g.-DCFG_TUD_DWC2_DMA_ENABLE=1). A board with novariantbuilds a single target named after the board.This makes variant build dirs and report rows explicit instead of the derived
-f1_<flags>suffix.Changes
--build-name <name>(build dir) +--cflag=<token>(raw CFLAGS intoCFLAGS_CLI, repeatable; the=-form keeps-D…=1tokens space-free so they survive the CI matrix's shell word-splitting). Dropped-f1.-bruns copy the board's dir plus anycmake-build-<board>-*variant dirs (and still fail fast when none exist).--build-name/--cflag(matrix args are passed through it).flags_onboards tovariant.Test evidence
build.pybuilds bothstm32f723discovariants; DMA getsCFLAGS_CLI=-DCFG_TUH_DWC2_DMA_ENABLE=1, base none.stm32f723discovariants flashed and tested; report shows separatestm32f723disco/stm32f723disco-DMArows, 0 failed.🤖 Generated with Claude Code