From da9d36fa4cef193abf12b581ac02e1986ea215a7 Mon Sep 17 00:00:00 2001 From: hathach Date: Tue, 28 Apr 2026 22:18:21 +0700 Subject: update AGENTS.md and claude hil skill --- .claude/commands/hil.md | 58 ------ .claude/skills/hil/SKILL.md | 75 +++++++ AGENTS.md | 479 ++++++++++++-------------------------------- 3 files changed, 199 insertions(+), 413 deletions(-) delete mode 100644 .claude/commands/hil.md create mode 100644 .claude/skills/hil/SKILL.md diff --git a/.claude/commands/hil.md b/.claude/commands/hil.md deleted file mode 100644 index 07e1a865b..000000000 --- a/.claude/commands/hil.md +++ /dev/null @@ -1,58 +0,0 @@ -# hil - -Run Hardware-in-the-Loop (HIL) tests on physical boards. - -## Arguments -- $ARGUMENTS: Optional flags (e.g. board name, extra args). If empty, runs all boards with default config. - -## Instructions - -1. Parse $ARGUMENTS: - - If $ARGUMENTS contains `-b BOARD_NAME`, run for that specific board only. - - If $ARGUMENTS is empty or has no `-b`, run for all boards in the config. - - Pass through any other flags (e.g. `-v` for verbose, `-r N` for retry count) directly to the command. - -2. Determine whether to run **locally** or **remotely via SSH**: - - **Local**: boards are attached to this machine (default when `local.json` is used) - - **Remote (`ssh ci.lan`)**: boards are attached to the CI machine (when `tinyusb.json` is used) - -3. **Local execution** (boards attached to this machine): - ```bash - python test/hil/hil_test.py -b BOARD_NAME -B examples $HIL_CONFIG $EXTRA_ARGS - ``` - -4. **Remote execution** (boards attached to `ci.lan`): - Only copy the minimal files needed (firmware binaries + test script + config), then run remotely. - - ```bash - REMOTE=ci.lan - REMOTE_DIR=/tmp/tinyusb-hil - - # Create remote working directory - ssh $REMOTE "rm -rf $REMOTE_DIR && mkdir -p $REMOTE_DIR/test/hil" - - # Copy HIL test script and its dependency - scp test/hil/hil_test.py test/hil/pymtp.py test/hil/tinyusb.json $REMOTE:$REMOTE_DIR/test/hil/ - - # Copy only the firmware binaries for the target board(s) - # For a specific board: - scp -r examples/cmake-build-$BOARD_NAME $REMOTE:$REMOTE_DIR/examples/ - - # Or for all boards that have been built: - # for dir in examples/cmake-build-*/; do scp -r "$dir" $REMOTE:$REMOTE_DIR/examples/; done - - # Run the test remotely - ssh $REMOTE "cd $REMOTE_DIR && python3 test/hil/hil_test.py -b $BOARD_NAME -B examples tinyusb.json $EXTRA_ARGS" - ``` - - Note: The remote machine (`ci.lan`) must have: - - Python 3 with `pyserial` installed (`pip install pyserial`) - - Flasher tools: `JLinkExe`, `openocd`, etc. as needed by the board - - USB access to the boards (udev rules configured) - -5. Use a timeout of at least 20 minutes (600000ms). HIL tests take 2-5 minutes. NEVER cancel early. - -6. After the test completes: - - Show the test output to the user. - - Summarize pass/fail results per board. - - If there are failures, suggest re-running with `-v` flag for verbose output to help debug. diff --git a/.claude/skills/hil/SKILL.md b/.claude/skills/hil/SKILL.md new file mode 100644 index 000000000..638b34b2d --- /dev/null +++ b/.claude/skills/hil/SKILL.md @@ -0,0 +1,75 @@ +--- +name: hil +description: Use when running TinyUSB Hardware-in-the-Loop (HIL) tests on physical boards, debugging HIL failures, or copying firmware to the ci.lan test rig. Covers local execution and remote execution over SSH, config selection, and debugging tips. +--- + +# Hardware-in-the-Loop (HIL) Testing + +Run TinyUSB HIL tests against real boards. Two execution modes — **local** (boards attached to this machine) and **remote** (boards attached to `ci.lan`, reached over SSH). Default to **local** unless the user specifies `remote`. Do not auto-detect. + +## Prerequisites + +- Examples must already be built for the target board(s). See AGENTS.md "Build" section, Option 2 (all examples for a board), which produces `examples/cmake-build-BOARD_NAME/`. +- `-B examples` tells `hil_test.py` that `examples/` is the parent folder containing the per-board build outputs. + +## Choosing arguments + +Infer from the user's request: + +- **Mode:** `local` (default) or `remote`. Only switch to `remote` if the user explicitly says so or names `ci.lan`. +- **Board:** if the user names a specific board, pass `-b BOARD_NAME`. Otherwise run all boards in the config. +- **Pass-through flags:** `-v` (verbose), `-r N` (retry count), etc. — pass through unchanged. + +Config file follows from mode: +- **Local** → `local.json` +- **Remote** → `tinyusb.json` + +## Local execution + +Boards attached to this machine: + +```bash +python test/hil/hil_test.py -b BOARD_NAME -B examples local.json $EXTRA_ARGS +# or for all boards in the config: +python test/hil/hil_test.py -B examples local.json $EXTRA_ARGS +``` + +## Remote execution (ci.lan) + +Copy only the minimal files needed (firmware binaries + test script + config), then run remotely: + +```bash +REMOTE=ci.lan +REMOTE_DIR=/tmp/tinyusb-hil + +# Create remote working directory +ssh $REMOTE "rm -rf $REMOTE_DIR && mkdir -p $REMOTE_DIR/test/hil" + +# Copy HIL test script and its dependency +scp test/hil/hil_test.py test/hil/pymtp.py test/hil/tinyusb.json $REMOTE:$REMOTE_DIR/test/hil/ + +# Copy firmware binaries +# Specific board: +scp -r examples/cmake-build-$BOARD_NAME $REMOTE:$REMOTE_DIR/examples/ +# Or all built boards: +# for dir in examples/cmake-build-*/; do scp -r "$dir" $REMOTE:$REMOTE_DIR/examples/; done + +# Run the test remotely +ssh $REMOTE "cd $REMOTE_DIR && python3 test/hil/hil_test.py -b $BOARD_NAME -B examples tinyusb.json $EXTRA_ARGS" +``` + +The remote machine (`ci.lan`) must have: +- Python 3 with `pyserial` installed (`pip install pyserial`) +- Flasher tools: `JLinkExe`, `openocd`, etc. as needed by the board +- USB access to the boards (udev rules configured) + +## Timing + +HIL runs take 2-5 minutes. Use a timeout of at least 20 minutes (600000 ms). NEVER cancel early. + +## Reporting results + +After the test completes: +- Show the test output to the user. +- Summarize pass/fail per board. +- On failure, suggest re-running with `-v` for verbose output. If `-v` isn't enough, temporarily add debug prints to `test/hil/hil_test.py` to pinpoint the issue. diff --git a/AGENTS.md b/AGENTS.md index eb6b737c3..13e5af66d 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -1,428 +1,197 @@ # TinyUSB Agent Instructions -TinyUSB is an open-source cross-platform USB Host/Device stack for embedded systems, designed to be memory-safe with no -dynamic allocation and thread-safe with all interrupt events deferred to non-ISR task functions. +TinyUSB is a cross-platform USB Host/Device stack for embedded systems: memory-safe (no dynamic allocation) and thread-safe (ISR events deferred to task context). -Always reference these instructions first and fallback to search or bash commands only when you encounter unexpected -information that does not match the info here. +Reference these instructions first; fall back to search/bash only when reality diverges. -## Shared Ground Rules -- Keep TinyUSB memory-safe: avoid dynamic allocation, defer ISR work to task context, and follow C99 with two-space indentation/no tabs. -- Match file organization: core stack under `src`, MCU/BSP support in `hw/{mcu,bsp}`, examples under `examples/{device,host,dual}`, docs in `docs`, tests under `test/{unit-test,fuzz,hil}`. -- Use descriptive snake_case for helpers, reserve `tud_`/`tuh_` for public APIs, `TU_` for macros, and keep headers self-contained with `#if CFG_TUSB_MCU` guards where needed. -- Prefer `.clang-format` for C/C++ formatting, run `pre-commit run --all-files` before submitting, and document board/HIL coverage when applicable. -- Commit in imperative mood, keep changes scoped, and supply PRs with linked issues plus test/build evidence. +## Behavioral Guidelines +Bias toward caution over speed. For trivial tasks, use judgment. -## Bootstrap and Build Setup +- **Think first** — state assumptions; ask if unclear; present alternatives instead of picking silently. +- **Simplicity** — no features, abstractions, flexibility, or error handling beyond what was asked. If 200 lines could be 50, rewrite. +- **Surgical changes** — touch only what the task requires; match existing style; don't refactor working code; mention unrelated dead code rather than deleting it. Remove only orphans *your* changes created. +- **Goal-driven** — turn tasks into verifiable goals ("write failing test, make it pass"). For multi-step work, state a brief `step → verify` plan. -- Install ARM GCC toolchain: `sudo apt-get update && sudo apt-get install -y gcc-arm-none-eabi` -- Fetch core dependencies: `python3 tools/get_deps.py` -- takes <1 second. NEVER CANCEL. -- For specific board families: `python3 tools/get_deps.py FAMILY_NAME` (e.g., rp2040, stm32f4), or - `python3 tools/get_deps.py -b BOARD_NAME` -- Dependencies are cached in `lib/` and `hw/mcu/` directories -- For **Espressif** boards, initialize the ESP-IDF environment before any build/flash/monitor command: - `. $HOME/code/esp-idf/export.sh` +## Ground Rules -## Build Examples +- **Language/style:** C99, 2-space indent (no tabs), snake_case helpers, `UPPER_CASE` macros. Public APIs use `tud_`/`tuh_`; macros use `TU_`. Headers self-contained with `#if CFG_TUSB_MCU` guards. +- **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. +- **Formatting/lint:** `clang-format` (`.clang-format`), `codespell` (`.codespellrc`), run `pre-commit run --all-files` before submitting. -Choose ONE of these approaches: -**Option 1: Individual Example with CMake and Ninja (RECOMMENDED)** +## Bootstrap ```bash -cd examples/device/cdc_msc -mkdir -p build && cd build -cmake -DBOARD=raspberry_pi_pico -G Ninja -DCMAKE_BUILD_TYPE=MinSizeRel .. -cmake --build . +sudo apt-get install -y gcc-arm-none-eabi # ARM toolchain (2-5 min, one-time) +python3 tools/get_deps.py [FAMILY|-b BOARD] # fetch deps into lib/, hw/mcu/ (<1 s) +. $HOME/code/esp-idf/export.sh # Espressif only: before any build/flash/monitor ``` --- takes 1-2 seconds. NEVER CANCEL. Set timeout to 5+ minutes. - -**Option 2: All Examples for a Board** - -different folder than Option 1 +## Build +Single example (CMake+Ninja, recommended, 1-3 s): ```bash -cd examples/ -mkdir -p build && cd build +cd examples/device/cdc_msc && mkdir -p build && cd build cmake -DBOARD=raspberry_pi_pico -G Ninja -DCMAKE_BUILD_TYPE=MinSizeRel .. cmake --build . ``` --- takes 15-20 seconds, may have some objcopy failures that are non-critical. NEVER CANCEL. Set timeout to 30+ minutes. - -**Option 3: Individual Example with Make** - +All examples for a board (15-20 s; some objcopy failures are non-critical): ```bash -cd examples/device/cdc_msc -make BOARD=raspberry_pi_pico all +cd examples && mkdir -p build && cd build +cmake -DBOARD=raspberry_pi_pico -G Ninja -DCMAKE_BUILD_TYPE=MinSizeRel .. && cmake --build . ``` --- takes 2-3 seconds. NEVER CANCEL. Set timeout to 5+ minutes. - -**Option 4: Espressif Example with ESP-IDF** - -Only ESP-IDF-enabled examples are supported for Espressif boards. Use FreeRTOS examples such as `examples/device/cdc_msc_freertos` -that contain `idf_component_register()` support. +Single example with Make: +```bash +cd examples/device/cdc_msc && make BOARD=raspberry_pi_pico all +``` +Espressif (only ESP-IDF examples like `cdc_msc_freertos`): ```bash . $HOME/code/esp-idf/export.sh cd examples/device/cdc_msc_freertos idf.py -DBOARD=espressif_s3_devkitc build ``` -Use `-DBOARD=...` with any supported board under `hw/bsp/espressif/boards/`. NEVER CANCEL. Set timeout to 10+ minutes. - - -## Build Options - -- **Debug build**: - - CMake: `-DCMAKE_BUILD_TYPE=Debug` - - Make: `DEBUG=1` -- **With logging**: - - CMake: `-DLOG=2` - - Make: `LOG=2` -- **With RTT logger**: - - CMake: `-DLOG=2 -DLOGGER=rtt` - - Make: `LOG=2 LOGGER=rtt` -- **RootHub port selection**: - - CMake: `-DRHPORT_DEVICE=1` - - Make: `RHPORT_DEVICE=1` -- **Port speed**: - - CMake: `-DRHPORT_DEVICE_SPEED=OPT_MODE_FULL_SPEED` - - Make: `RHPORT_DEVICE_SPEED=OPT_MODE_FULL_SPEED` - -## Flashing and Deployment - -- **Flash with JLink**: - - CMake: `ninja cdc_msc-jlink` - - Make: `make BOARD=raspberry_pi_pico flash-jlink` -- **Flash with OpenOCD**: - - CMake: `ninja cdc_msc-openocd` - - Make: `make BOARD=raspberry_pi_pico flash-openocd` -- **Generate UF2**: - - CMake: `ninja cdc_msc-uf2` - - Make: `make BOARD=raspberry_pi_pico all uf2` -- **List all targets** (CMake/Ninja): `ninja -t targets` -- **Espressif flash**: - - Run `. $HOME/code/esp-idf/export.sh` - - `cd examples/device/cdc_msc_freertos` - - `idf.py -DBOARD=espressif_s3_devkitc flash` -- **Espressif serial monitor / chip log output**: - - Run `. $HOME/code/esp-idf/export.sh` - - `cd examples/device/cdc_msc_freertos` - - `idf.py -DBOARD=espressif_s3_devkitc monitor` +**Build options** (CMake `-D…` / Make `…=…`): +- Debug: `CMAKE_BUILD_TYPE=Debug` / `DEBUG=1` +- Logging: `LOG=2` (add `LOGGER=rtt` for RTT) +- Root hub port: `RHPORT_DEVICE=1` +- Speed: `RHPORT_DEVICE_SPEED=OPT_MODE_FULL_SPEED` + +## Flash + +```bash +ninja cdc_msc-jlink | make BOARD=… flash-jlink # JLink +ninja cdc_msc-openocd | make BOARD=… flash-openocd # OpenOCD +ninja cdc_msc-uf2 | make BOARD=… all uf2 # UF2 +ninja -t targets # list CMake targets +idf.py -DBOARD=… flash|monitor # Espressif (after export.sh) +``` ## GDB Debugging -Look up the board's `JLINK_DEVICE` and `OPENOCD_OPTION` from `hw/bsp/*/boards/*/board.cmake` (or `board.mk`). +Look up `JLINK_DEVICE` / `OPENOCD_OPTION` in `hw/bsp/*/boards/*/board.cmake`. -### JLinkGDBServer +**JLink — Terminal 1:** +```bash +JLinkGDBServer -device stm32h743xi -if SWD -speed 4000 -port 2331 -swoport 2332 -telnetport 2333 -nogui +``` -**Terminal 1 – start the GDB server:** +**OpenOCD — Terminal 1:** ```bash -JLinkGDBServer -device stm32h743xi -if SWD -speed 4000 \ - -port 2331 -swoport 2332 -telnetport 2333 -nogui +openocd -f interface/stlink.cfg -f target/stm32h7x.cfg # or interface/jlink.cfg +# rp2040/rp2350 via CMSIS-DAP: +openocd -f interface/cmsis-dap.cfg -f target/rp2040.cfg -c "adapter speed 5000" ``` -**Terminal 2 – connect GDB:** +**Terminal 2 — connect GDB** (JLink :2331, OpenOCD :3333): ```bash arm-none-eabi-gdb /tmp/build/firmware.elf (gdb) target remote :2331 (gdb) monitor reset halt (gdb) load +(gdb) break main # optional, to stop at entry (gdb) continue ``` -To break on entry instead of running immediately: -```bash -(gdb) monitor reset halt -(gdb) load -(gdb) break main -(gdb) continue -``` +**RTT logging:** build with `LOG=2 LOGGER=rtt`, flash, then run JLinkGDBServer with `-RTTTelnetPort 19021`, and in another terminal `JLinkRTTClient` (pipe to `tee rtt.log` or use `timeout 20s JLinkRTTClient > rtt.log` for non-interactive capture). -### OpenOCD +## Testing -**Terminal 1 – start the GDB server:** +**Unit (Ceedling, Unity+CMock, ~4 s):** ```bash -openocd -f interface/stlink.cfg -f target/stm32h7x.cfg -# or with J-Link probe: -openocd -f interface/jlink.cfg -f target/stm32h7x.cfg +sudo gem install ceedling +cd test/unit-test && ceedling test:all # or ceedling test:test_fifo ``` -For **rp2040/rp2350** with a CMSIS-DAP probe (e.g. Picoprobe, debugprobe): -```bash -openocd -f interface/cmsis-dap.cfg -f target/rp2040.cfg -c "adapter speed 5000" -# or for rp2350: -openocd -f interface/cmsis-dap.cfg -f target/rp2350.cfg -c "adapter speed 5000" -``` +**HIL (2-5 min):** invoke the `hil` skill (`.claude/skills/hil/SKILL.md`) for the full procedure (local vs remote mode, config selection, SSH copy steps, debugging tips). Requires pre-built examples (Build Option 2). -For boards that define `OPENOCD_OPTION` in `board.cmake`, use those options directly: -```bash -openocd $(cat hw/bsp/FAMILY/boards/BOARD/board.cmake | grep OPENOCD_OPTION | ...) -``` +## Documentation -**Terminal 2 – connect GDB (OpenOCD default port is 3333):** ```bash -arm-none-eabi-gdb /tmp/build/firmware.elf -(gdb) target remote :3333 -(gdb) monitor reset halt -(gdb) load -(gdb) continue +pip install -r docs/requirements.txt +cd docs && sphinx-build -b html . _build # ~2.5 s ``` -### RTT Logging with JLinkGDBServer - -- Build with RTT logging enabled (example): - `cd examples/device/cdc_msc && make BOARD=stm32h743eval LOG=2 LOGGER=rtt all` -- Flash with J-Link: - `cd examples/device/cdc_msc && make BOARD=stm32h743eval LOG=2 LOGGER=rtt flash-jlink` -- Launch GDB server with RTT port (keep this running in terminal 1): - `JLinkGDBServer -device stm32h743xi -if SWD -speed 4000 -port 2331 -swoport 2332 -telnetport 2333 -RTTTelnetPort 19021 -nogui` -- Read RTT output (terminal 2): - `JLinkRTTClient` -- Capture RTT to file (optional): - `JLinkRTTClient | tee rtt.log` -- For non-interactive capture: - `timeout 20s JLinkRTTClient > rtt.log` - -## Unit Testing - -- Install Ceedling: `sudo gem install ceedling` -- Run all unit tests: `cd test/unit-test && ceedling` or `cd test/unit-test && ceedling test:all` -- takes 4 seconds. - NEVER CANCEL. Set timeout to 10+ minutes. -- Run specific test: `cd test/unit-test && ceedling test:test_fifo` -- Tests use Unity framework with CMock for mocking - -## Hardware-in-the-Loop (HIL) Testing - -- `-B examples` means `examples` is the parent folder that contains multi-board build outputs such as `examples/cmake-build-BOARD_NAME/...` -- Select config file before running HIL tests: - - if GitHub Actions self-hosted runner service is running, use `tinyusb.json` - - otherwise use `local.json` - - example: - `HIL_CONFIG=$( (systemctl list-units --type=service --state=running 2>/dev/null; systemctl --user list-units --type=service --state=running 2>/dev/null) | grep -q 'actions\.runner' && echo tinyusb.json || echo local.json )` -- Run tests on actual hardware, one of following ways: - - test a specific board `python test/hil/hil_test.py -b BOARD_NAME -B examples $HIL_CONFIG` - - test all boards in config `python test/hil/hil_test.py -B examples $HIL_CONFIG` -- In case of error, enabled verbose mode with `-v` flag for detailed logs. Also try to observe script output, and try to - modify hil_test.py (temporarily) to add more debug prints to pinpoint the issue. -- Requires pre-built (all) examples for target boards (see Build Examples section 2) - -take 2-5 minutes. NEVER CANCEL. Set timeout to 20+ minutes. - -## Documentation - -- Install requirements: `pip install -r docs/requirements.txt` -- Build docs: `cd docs && sphinx-build -b html . _build` -- takes 2-3 seconds. NEVER CANCEL. Set timeout to 10+ minutes. - ## Code Size Metrics -Generate and compare code size metrics to evaluate the impact of changes. This is the most common workflow -when making code changes — use it to verify size impact before committing. - -**Quick single-board metrics (preferred for iterative development):** +Verify size impact before committing. +**Single-board (iterative, ~30 s):** ```bash rm -rf cmake-build python3 tools/build.py -b raspberry_pi_pico --target all --target tinyusb_metrics python3 tools/metrics.py combine -j -m -f tinyusb/src cmake-build/cmake-build-*/metrics.json ``` -This builds all examples for one board and produces `metrics.json` + `metrics.md`. Takes ~30 seconds. -NEVER CANCEL. Set timeout to 10+ minutes. - -**Comparing with master (before/after workflow):** - -1. On master: build and save baseline - ```bash - rm -rf cmake-build - python3 tools/build.py -b raspberry_pi_pico --target all --target tinyusb_metrics - python3 tools/metrics.py combine -j -m -f tinyusb/src cmake-build/cmake-build-*/metrics.json - mv metrics.json metrics_master.json - ``` -2. Switch to your branch: rebuild - ```bash - rm -rf cmake-build - python3 tools/build.py -b raspberry_pi_pico --target all --target tinyusb_metrics - python3 tools/metrics.py combine -j -m -f tinyusb/src cmake-build/cmake-build-*/metrics.json - ``` -3. Compare: `python3 tools/metrics.py compare -m -f tinyusb/src metrics_master.json metrics.json` - Produces `metrics_compare.md` showing size differences. - -**Full CI metrics (all arm-gcc families, for thorough validation):** +**Compare vs master:** run the above on master, `mv metrics.json metrics_master.json`, switch branch, rebuild, then: +```bash +python3 tools/metrics.py compare -m -f tinyusb/src metrics_master.json metrics.json +``` +**Full CI (all arm-gcc families, 2-4 min):** ```bash rm -rf cmake-build -FAMILIES=$(python3 .github/workflows/ci_set_matrix.py | python3 -c "import sys,json; d=json.load(sys.stdin); print(' '.join(d.get('arm-gcc',[])))") +FAMILIES=$(python3 .github/workflows/ci_set_matrix.py | python3 -c "import sys,json;d=json.load(sys.stdin);print(' '.join(d.get('arm-gcc',[])))") python3 tools/build.py --one-first --target all --target tinyusb_metrics $FAMILIES python3 tools/metrics.py combine -j -m -f tinyusb/src cmake-build/cmake-build-*/metrics.json ``` -Builds the first board of each family. Takes 2-4 minutes. NEVER CANCEL. Set timeout to 10+ minutes. - -## Code Quality and Validation - -- Format code: `clang-format -i path/to/file.c` (uses `.clang-format` config) -- Check spelling: `pip install codespell && codespell` (uses `.codespellrc` config) -- Pre-commit hooks validate unit tests and code quality automatically - -## Static Analysis with PVS-Studio - -- **Analyze whole project**: - ```bash - pvs-studio-analyzer analyze -f examples/cmake-build-raspberry_pi_pico/compile_commands.json -R .PVS-Studio/.pvsconfig -o pvs-report.log -j12 --dump-files --misra-cpp-version 2008 --misra-c-version 2023 --use-old-parser - ``` -- **Analyze specific source files**: - ```bash - pvs-studio-analyzer analyze -f examples/cmake-build-raspberry_pi_pico/compile_commands.json -R .PVS-Studio/.pvsconfig -S path/to/file.c -o pvs-report.log -j12 --dump-files --misra-cpp-version 2008 --misra-c-version 2023 --use-old-parser - ``` -- **Multiple specific files**: - ```bash - pvs-studio-analyzer analyze -f examples/cmake-build-raspberry_pi_pico/compile_commands.json -R .PVS-Studio/.pvsconfig -S src/file1.c -S src/file2.c -o pvs-report.log -j12 --dump-files --misra-cpp-version 2008 --misra-c-version 2023 --use-old-parser - ``` -- Requires `compile_commands.json` in the build directory (generated by CMake with `-DCMAKE_EXPORT_COMPILE_COMMANDS=ON`) -- Use `-f` option to specify path to `compile_commands.json` -- Use `-R .PVS-Studio/.pvsconfig` to specify rule configuration file -- Use `-j12` for parallel analysis with 12 threads -- `--dump-files` saves preprocessed files for debugging -- `--misra-c-version 2023` enables MISRA C:2023 checks -- `--misra-cpp-version 2008` enables MISRA C++:2008 checks -- `--use-old-parser` uses legacy parser for compatibility -- Analysis takes ~10-30 seconds depending on project size. Set timeout to 5+ minutes. -- View results: `plog-converter -a GA:1,2 -t errorfile pvs-report.log` or open in PVS-Studio GUI - -## Validation Checklist - -### ALWAYS Run These After Making Changes - -1. **Pre-commit validation** (RECOMMENDED): `pre-commit run --all-files` - - Install pre-commit: `pip install pre-commit && pre-commit install` - - Runs all quality checks, unit tests, spell checking, and formatting - - Takes 10-15 seconds. NEVER CANCEL. Set timeout to 15+ minutes. -2. **Build validation**: Build at least one board with all example that exercises your changes, see Build Examples - section (option 2) -3. Run unit tests relevant to touched modules; add fuzz/HIL coverage when modifying parsers or protocol state machines. - -### Manual Testing Scenarios -- **Device examples**: Cannot be fully tested without real hardware, but must build successfully -- **Unit tests**: Exercise core stack functionality - ALL tests must pass -- **Build system**: Must be able to build examples for multiple board families - -### Board Selection for Testing -- **STM32F4**: `stm32f407disco` - no external SDK required, good for testing -- **RP2040**: `raspberry_pi_pico` - requires Pico SDK, commonly used -- **Other families**: Check `hw/bsp/FAMILY/boards/` for available boards - -## Release Instructions - -**DO NOT commit files automatically - only modify files and let the maintainer review before committing.** - -1. Bump the release version variable at the top of `tools/make_release.py`. -2. Execute `python3 tools/make_release.py` to refresh: - - `src/tusb_option.h` (version defines) - - `repository.yml` (version mapping) - - `library.json` (PlatformIO version) - - `sonar-project.properties` (SonarQube version) - - `docs/reference/boards.rst` (generated board documentation) - - `hw/bsp/BoardPresets.json` (CMake presets) -3. Generate release notes for `docs/info/changelog.rst`: - - Get commit list: `git log ..HEAD --oneline` - - **Visit GitHub PRs** for merged pull requests to understand context and gather details - - Use GitHub tools to search/read PRs: `github-mcp-server-list_pull_requests`, `github-mcp-server-pull_request_read` - - Extract key changes, API modifications, bug fixes, and new features from PR descriptions - - Add new changelog entry following the existing format: - - Version heading with equals underline (e.g., `0.20.0` followed by `======`) - - Release date in italics (e.g., `*November 19, 2024*`) - - Major sections: General, API Changes, Controller Driver (DCD & HCD), Device Stack, Host Stack, Testing - - Use bullet lists with descriptive categorization - - Reference function names, config macros, and file paths using RST inline code (double backticks) - - Include meaningful descriptions, not just commit messages -4. **Validation before commit**: - - Run unit tests: `cd test/unit-test && ceedling test:all` - - Build at least one example: `cd examples/device/cdc_msc && make BOARD=stm32f407disco all` - - Verify changed files look correct: `git diff --stat` -5. **Leave files unstaged** for maintainer to review, modify if needed, and commit with message: `Bump version to X.Y.Z` -6. **After maintainer commits**: Create annotated tag with `git tag -a vX.Y.Z -m "Release X.Y.Z"` -7. Push commit and tag: `git push origin && git push origin vX.Y.Z` -8. Create GitHub release from the tag with changelog content - -## Repository Structure Quick Reference -``` -├── src/ # Core TinyUSB stack -│ ├── class/ # USB device classes (CDC, HID, MSC, Audio, etc.) -│ ├── portable/ # MCU-specific drivers (organized by vendor) -│ ├── device/ # USB device stack core -│ ├── host/ # USB host stack core -│ └── common/ # Shared utilities (FIFO, etc.) -├── examples/ # Example applications -│ ├── device/ # Device examples (cdc_msc, hid_generic, etc.) -│ ├── host/ # Host examples -│ └── dual/ # Dual-role examples -├── hw/bsp/ # Board Support Packages -│ └── FAMILY/boards/ # Board-specific configurations -├── test/unit-test/ # Unit tests using Ceedling -├── tools/ # Build and utility scripts -└── docs/ # Sphinx documentation +## Static Analysis (PVS-Studio) + +Requires `compile_commands.json` (CMake `-DCMAKE_EXPORT_COMPILE_COMMANDS=ON`). + +```bash +pvs-studio-analyzer analyze \ + -f examples/cmake-build-raspberry_pi_pico/compile_commands.json \ + -R .PVS-Studio/.pvsconfig [-S path/to/file.c ...] \ + -o pvs-report.log -j12 --dump-files \ + --misra-c-version 2023 --misra-cpp-version 2008 --use-old-parser +plog-converter -a GA:1,2 -t errorfile pvs-report.log # view results ``` -#### Build Time Reference -- **Dependency fetch**: <1 second -- **Single example build**: 1-3 seconds -- **Unit tests**: ~4 seconds -- **Documentation build**: ~2.5 seconds -- **Full board examples**: 15-20 seconds -- **Toolchain installation**: 2-5 minutes (one-time) - -#### Key Files to Know -- `tools/get_deps.py`: Manages dependencies for MCU families -- `tools/build.py`: Builds multiple examples, supports make/cmake -- `src/tusb.h`: Main TinyUSB header file -- `src/tusb_config.h`: Configuration template -- `examples/device/cdc_msc/`: Most commonly used example for testing -- `test/unit-test/project.yml`: Ceedling test configuration - -#### MCU Reference Manuals and Datasheets -- Look in `$HOME/Documents/Calibre Library` for all MCU reference manuals, datasheets and board schematics. - -#### Debugging Build Issues -- **Missing compiler**: Install `gcc-arm-none-eabi` package -- **Missing dependencies**: Run `python3 tools/get_deps.py FAMILY` -- **Board not found**: Check `hw/bsp/FAMILY/boards/` for valid board names -- **objcopy errors**: Often non-critical in full builds, try individual example builds - -#### Working with USB Device Classes -- **CDC (Serial)**: `src/class/cdc/` - Virtual serial port -- **HID**: `src/class/hid/` - Human Interface Device (keyboard, mouse, etc.) -- **MSC**: `src/class/msc/` - Mass Storage Class (USB drive) -- **Audio**: `src/class/audio/` - USB Audio Class -- Each class has device (`*_device.c`) and host (`*_host.c`) implementations - -#### MCU Family Support -- **STM32**: Largest support (F0, F1, F2, F3, F4, F7, G0, G4, H7, L4, U5, etc.) -- **Raspberry Pi**: RP2040, RP2350 with PIO-USB host support -- **NXP**: iMXRT, Kinetis, LPC families -- **Microchip**: SAM D/E/G/L families -- Check `hw/bsp/` for complete list and `docs/reference/boards.rst` for details - -### Code Style Guidelines - -#### General Coding Standards -- Use C99 standard -- Memory-safe: no dynamic allocation -- Thread-safe: defer all interrupt events to non-ISR task functions -- 2-space indentation, no tabs -- Use snake_case for variables/functions -- Use UPPER_CASE for macros and constants -- Follow existing variable naming patterns in files you're modifying -- Include proper header comments with MIT license -- Add descriptive comments for non-obvious functions - -#### Best Practices -- When including headers, group in order: C stdlib, tusb common, drivers, classes -- Always check return values from functions that can fail -- Use TU_ASSERT() for error checking with return statements -- Follow the existing code patterns in the files you're modifying - -Remember: TinyUSB is designed for embedded systems - builds are fast, tests are focused, and the codebase is optimized for resource-constrained environments. +Add `-S ` (repeatable) to restrict to specific sources. ~10-30 s. + +## Validation After Changes + +1. `pre-commit run --all-files` — format, spell, unit tests (10-15 s). +2. Build at least one board's full example set (Build Option 2) for modules you touched. +3. Run relevant unit tests; add fuzz/HIL coverage for parsers or protocol state machines. + +**Boards good for local testing:** +- `stm32f407disco` — no external SDK +- `raspberry_pi_pico` — Pico SDK required +- Others: see `hw/bsp/FAMILY/boards/` + +Device examples need real hardware to validate runtime behavior; must at least build. + +## Release + +**Do not commit automatically — leave changes for maintainer review.** + +1. Bump version at top of `tools/make_release.py`. +2. Run `python3 tools/make_release.py` to refresh: `src/tusb_option.h`, `repository.yml`, `library.json`, `sonar-project.properties`, `docs/reference/boards.rst`, `hw/bsp/BoardPresets.json`. +3. Changelog `docs/info/changelog.rst`: + - `git log ..HEAD --oneline` for commit list. + - Read merged PRs for context (`gh pr view`, or github MCP tools). + - Follow existing format: version + `======` underline, italic date, sections (General, API Changes, DCD & HCD, Device Stack, Host Stack, Testing), RST inline code for symbols. +4. Validate: `ceedling test:all`, build `cdc_msc` for `stm32f407disco`, review `git diff --stat`. +5. Leave unstaged. Maintainer commits `Bump version to X.Y.Z`, then: `git tag -a vX.Y.Z -m "Release X.Y.Z" && git push origin vX.Y.Z`. Create GitHub release from tag. + +## References + +- MCU reference manuals, datasheets, schematics: `$HOME/Documents/Calibre Library`. +- Supported MCUs/boards: `hw/bsp/` and `docs/reference/boards.rst`. +- USB classes: `src/class/{cdc,hid,msc,audio,…}/` — each has `*_device.c` and `*_host.c`. +- Key files: `src/tusb.h`, `src/tusb_config.h`, `tools/get_deps.py`, `tools/build.py`, `test/unit-test/project.yml`. + +## Common Build Issues + +- Missing compiler → install `gcc-arm-none-eabi`. +- Missing deps → `python3 tools/get_deps.py FAMILY`. +- Unknown board → check `hw/bsp/FAMILY/boards/`. +- `objcopy` errors in full builds are often non-critical; retry the single example. -- cgit v1.3.1 From 47f2228cedfb216411c1ac50c4f10a30907cdb51 Mon Sep 17 00:00:00 2001 From: hathach Date: Wed, 29 Apr 2026 11:09:48 +0700 Subject: address review feedback for AGENTS.md and hil skill AGENTS.md: - fix build dir to cmake-build- (matches hil_test.py expectation) - reformat flash section to avoid shell-pipe ambiguity, use - mention board.mk for Make-based builds - complete OpenOCD jlink interface example - update stale "Build Option 2" references to "All examples for a board" - split PVS-Studio command so it is copy-pasteable .claude/skills/hil/SKILL.md: - clarify local.json is user-supplied, not tracked in repo - use python3 consistently - add all-boards variant for remote execution - delegate remote execution to test/hil/hil_ci.sh test/hil/hil_ci.sh: - portable shebang (/usr/bin/env bash) - set -euo pipefail - env overrides for REMOTE, REMOTE_DIR, CONFIG, ROOT_DIR - --prune-empty-dirs on rsync to skip empty subdirs - fail-fast sanity check on repo layout Co-Authored-By: Claude Opus 4.7 (1M context) --- .claude/skills/hil/SKILL.md | 47 +++++++++++++++++------------------------ AGENTS.md | 51 +++++++++++++++++++++++++++++++++------------ test/hil/hil_ci.sh | 37 +++++++++++++++++++------------- 3 files changed, 80 insertions(+), 55 deletions(-) diff --git a/.claude/skills/hil/SKILL.md b/.claude/skills/hil/SKILL.md index 638b34b2d..1f3d7d072 100644 --- a/.claude/skills/hil/SKILL.md +++ b/.claude/skills/hil/SKILL.md @@ -9,7 +9,7 @@ Run TinyUSB HIL tests against real boards. Two execution modes — **local** (bo ## Prerequisites -- Examples must already be built for the target board(s). See AGENTS.md "Build" section, Option 2 (all examples for a board), which produces `examples/cmake-build-BOARD_NAME/`. +- Examples must already be built for the target board(s). See AGENTS.md "Build" → "All examples for a board", which produces `examples/cmake-build-/`. - `-B examples` tells `hil_test.py` that `examples/` is the parent folder containing the per-board build outputs. ## Choosing arguments @@ -17,51 +17,42 @@ Run TinyUSB HIL tests against real boards. Two execution modes — **local** (bo Infer from the user's request: - **Mode:** `local` (default) or `remote`. Only switch to `remote` if the user explicitly says so or names `ci.lan`. -- **Board:** if the user names a specific board, pass `-b BOARD_NAME`. Otherwise run all boards in the config. +- **Board:** if the user names a specific board, pass `-b BOARD_NAME`. Otherwise omit `-b` to run all boards in the config. - **Pass-through flags:** `-v` (verbose), `-r N` (retry count), etc. — pass through unchanged. Config file follows from mode: -- **Local** → `local.json` -- **Remote** → `tinyusb.json` +- **Local** → `test/hil/local.json` (user-supplied; not tracked in repo — describes boards attached locally) +- **Remote** → `test/hil/tinyusb.json` (tracked; describes the `ci.lan` test rig) + +If `local.json` is missing, fall back to `tinyusb.json` only when explicitly told to; otherwise stop and ask the user to supply one. ## Local execution Boards attached to this machine: ```bash -python test/hil/hil_test.py -b BOARD_NAME -B examples local.json $EXTRA_ARGS -# or for all boards in the config: -python test/hil/hil_test.py -B examples local.json $EXTRA_ARGS +# Specific board: +python3 test/hil/hil_test.py -b BOARD_NAME -B examples test/hil/local.json $EXTRA_ARGS +# All boards in the config (no -b): +python3 test/hil/hil_test.py -B examples test/hil/local.json $EXTRA_ARGS ``` ## Remote execution (ci.lan) -Copy only the minimal files needed (firmware binaries + test script + config), then run remotely: +Use `test/hil/hil_ci.sh` — it handles dir setup, scp of test scripts, rsync of firmware artifacts (`.elf` / `.bin` / `.hex` only), and running `hil_test.py` on `ci.lan`: ```bash -REMOTE=ci.lan -REMOTE_DIR=/tmp/tinyusb-hil - -# Create remote working directory -ssh $REMOTE "rm -rf $REMOTE_DIR && mkdir -p $REMOTE_DIR/test/hil" - -# Copy HIL test script and its dependency -scp test/hil/hil_test.py test/hil/pymtp.py test/hil/tinyusb.json $REMOTE:$REMOTE_DIR/test/hil/ - -# Copy firmware binaries # Specific board: -scp -r examples/cmake-build-$BOARD_NAME $REMOTE:$REMOTE_DIR/examples/ -# Or all built boards: -# for dir in examples/cmake-build-*/; do scp -r "$dir" $REMOTE:$REMOTE_DIR/examples/; done - -# Run the test remotely -ssh $REMOTE "cd $REMOTE_DIR && python3 test/hil/hil_test.py -b $BOARD_NAME -B examples tinyusb.json $EXTRA_ARGS" +bash test/hil/hil_ci.sh -b raspberry_pi_pico2 +# All boards in tinyusb.json: +bash test/hil/hil_ci.sh +# Pass-through extra args (any non -b flag is forwarded to hil_test.py): +bash test/hil/hil_ci.sh -b raspberry_pi_pico2 -t host/cdc_msc_hid -r 1 ``` -The remote machine (`ci.lan`) must have: -- Python 3 with `pyserial` installed (`pip install pyserial`) -- Flasher tools: `JLinkExe`, `openocd`, etc. as needed by the board -- USB access to the boards (udev rules configured) +Overrides via env vars: `REMOTE=ci.lan`, `REMOTE_DIR=/tmp/tinyusb-hil`, `CONFIG=test/hil/tinyusb.json`. + +The script fails fast if the build dir or repo layout is missing. ## Timing diff --git a/AGENTS.md b/AGENTS.md index 13e5af66d..37fac2b05 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -38,10 +38,11 @@ cmake -DBOARD=raspberry_pi_pico -G Ninja -DCMAKE_BUILD_TYPE=MinSizeRel .. cmake --build . ``` -All examples for a board (15-20 s; some objcopy failures are non-critical): +All examples for a board (15-20 s; some objcopy failures are non-critical). Use `cmake-build-` as the build dir — HIL tests expect that exact name: ```bash -cd examples && mkdir -p build && cd build -cmake -DBOARD=raspberry_pi_pico -G Ninja -DCMAKE_BUILD_TYPE=MinSizeRel .. && cmake --build . +cd examples +cmake -B cmake-build-raspberry_pi_pico -DBOARD=raspberry_pi_pico -G Ninja -DCMAKE_BUILD_TYPE=MinSizeRel . +cmake --build cmake-build-raspberry_pi_pico ``` Single example with Make: @@ -65,16 +66,28 @@ idf.py -DBOARD=espressif_s3_devkitc build ## Flash ```bash -ninja cdc_msc-jlink | make BOARD=… flash-jlink # JLink -ninja cdc_msc-openocd | make BOARD=… flash-openocd # OpenOCD -ninja cdc_msc-uf2 | make BOARD=… all uf2 # UF2 +# JLink +ninja cdc_msc-jlink # CMake +make BOARD= flash-jlink # Make + +# OpenOCD +ninja cdc_msc-openocd # CMake +make BOARD= flash-openocd # Make + +# UF2 +ninja cdc_msc-uf2 # CMake +make BOARD= all uf2 # Make + ninja -t targets # list CMake targets -idf.py -DBOARD=… flash|monitor # Espressif (after export.sh) + +# Espressif (after . $HOME/code/esp-idf/export.sh) +idf.py -DBOARD= flash +idf.py -DBOARD= monitor ``` ## GDB Debugging -Look up `JLINK_DEVICE` / `OPENOCD_OPTION` in `hw/bsp/*/boards/*/board.cmake`. +Look up `JLINK_DEVICE` / `OPENOCD_OPTION` in `hw/bsp/*/boards/*/board.cmake` (CMake builds) or `board.mk` (Make builds). **JLink — Terminal 1:** ```bash @@ -83,7 +96,9 @@ JLinkGDBServer -device stm32h743xi -if SWD -speed 4000 -port 2331 -swoport 2332 **OpenOCD — Terminal 1:** ```bash -openocd -f interface/stlink.cfg -f target/stm32h7x.cfg # or interface/jlink.cfg +openocd -f interface/stlink.cfg -f target/stm32h7x.cfg +# or with a J-Link interface: +openocd -f interface/jlink.cfg -f target/stm32h7x.cfg # rp2040/rp2350 via CMSIS-DAP: openocd -f interface/cmsis-dap.cfg -f target/rp2040.cfg -c "adapter speed 5000" ``` @@ -108,7 +123,7 @@ sudo gem install ceedling cd test/unit-test && ceedling test:all # or ceedling test:test_fifo ``` -**HIL (2-5 min):** invoke the `hil` skill (`.claude/skills/hil/SKILL.md`) for the full procedure (local vs remote mode, config selection, SSH copy steps, debugging tips). Requires pre-built examples (Build Option 2). +**HIL (2-5 min):** invoke the `hil` skill (`.claude/skills/hil/SKILL.md`) for the full procedure (local vs remote mode, config selection, SSH copy steps, debugging tips). Requires pre-built examples — see Build → "All examples for a board". ## Documentation @@ -146,20 +161,30 @@ python3 tools/metrics.py combine -j -m -f tinyusb/src cmake-build/cmake-build-*/ Requires `compile_commands.json` (CMake `-DCMAKE_EXPORT_COMPILE_COMMANDS=ON`). ```bash +# Whole project: +pvs-studio-analyzer analyze \ + -f examples/cmake-build-raspberry_pi_pico/compile_commands.json \ + -R .PVS-Studio/.pvsconfig \ + -o pvs-report.log -j12 --dump-files \ + --misra-c-version 2023 --misra-cpp-version 2008 --use-old-parser + +# Specific files (add one or more `-S `): pvs-studio-analyzer analyze \ -f examples/cmake-build-raspberry_pi_pico/compile_commands.json \ - -R .PVS-Studio/.pvsconfig [-S path/to/file.c ...] \ + -R .PVS-Studio/.pvsconfig \ + -S src/foo.c -S src/bar.c \ -o pvs-report.log -j12 --dump-files \ --misra-c-version 2023 --misra-cpp-version 2008 --use-old-parser + plog-converter -a GA:1,2 -t errorfile pvs-report.log # view results ``` -Add `-S ` (repeatable) to restrict to specific sources. ~10-30 s. +Takes ~10-30 s. ## Validation After Changes 1. `pre-commit run --all-files` — format, spell, unit tests (10-15 s). -2. Build at least one board's full example set (Build Option 2) for modules you touched. +2. Build at least one board's full example set (Build → "All examples for a board") for modules you touched. 3. Run relevant unit tests; add fuzz/HIL coverage for parsers or protocol state machines. **Boards good for local testing:** diff --git a/test/hil/hil_ci.sh b/test/hil/hil_ci.sh index fa8bb0245..d1b5f7def 100644 --- a/test/hil/hil_ci.sh +++ b/test/hil/hil_ci.sh @@ -1,15 +1,24 @@ -#!/bin/bash +#!/usr/bin/env bash # Run HIL test remotely on ci.lan # Usage: test/hil/hil_ci.sh [-b BOARD] [-t TEST] [extra hil_test.py args...] # Example: # test/hil/hil_ci.sh -b stm32f723disco # test/hil/hil_ci.sh -b stm32f723disco -t host/cdc_msc_hid -r 1 +# +# Env overrides: REMOTE, REMOTE_DIR, CONFIG (path to HIL config json), +# ROOT_DIR (tinyusb checkout to test; defaults to the script's own checkout). -set -e +set -euo pipefail -REMOTE=ci.lan -REMOTE_DIR=/tmp/tinyusb-hil -SCRIPT_DIR="$(cd "$(dirname "$0")/../.." && pwd)" +REMOTE=${REMOTE:-ci.lan} +REMOTE_DIR=${REMOTE_DIR:-/tmp/tinyusb-hil} +ROOT_DIR=${ROOT_DIR:-$(cd "$(dirname "$0")/../.." && pwd)} +CONFIG=${CONFIG:-$ROOT_DIR/test/hil/tinyusb.json} + +[[ -f "$ROOT_DIR/test/hil/hil_test.py" && -d "$ROOT_DIR/examples" ]] || { + echo "error: $ROOT_DIR does not look like a tinyusb checkout" >&2 + exit 1 +} # Parse -b BOARD from arguments to know which build to copy BOARD="" @@ -34,22 +43,21 @@ ssh "$REMOTE" "rm -rf $REMOTE_DIR && mkdir -p $REMOTE_DIR/test/hil $REMOTE_DIR/e # Copy HIL test script and config echo "==> Copying test scripts" -scp -q "$SCRIPT_DIR/test/hil/hil_test.py" \ - "$SCRIPT_DIR/test/hil/pymtp.py" \ - "$SCRIPT_DIR/test/hil/tinyusb.json" \ +scp -q "$ROOT_DIR/test/hil/hil_test.py" \ + "$ROOT_DIR/test/hil/pymtp.py" \ + "$CONFIG" \ "$REMOTE:$REMOTE_DIR/test/hil/" # Copy only firmware binaries (elf/bin/hex), preserving directory structure copy_board_binaries() { local src="$1" - local board_name - board_name=$(basename "$src") - rsync -a --include='*/' --include='*.elf' --include='*.bin' --include='*.hex' --exclude='*' \ + rsync -a --prune-empty-dirs \ + --include='*/' --include='*.elf' --include='*.bin' --include='*.hex' --exclude='*' \ "$src" "$REMOTE:$REMOTE_DIR/examples/" } if [ -n "$BOARD" ]; then - BUILD_DIR="$SCRIPT_DIR/examples/cmake-build-$BOARD" + BUILD_DIR="$ROOT_DIR/examples/cmake-build-$BOARD" if [ ! -d "$BUILD_DIR" ]; then echo "Error: build directory not found: $BUILD_DIR" echo "Build first with: cd examples && cmake -DBOARD=$BOARD -G Ninja -B cmake-build-$BOARD .. && cmake --build cmake-build-$BOARD" @@ -59,11 +67,12 @@ if [ -n "$BOARD" ]; then copy_board_binaries "$BUILD_DIR" else echo "==> Copying all built binaries" - for dir in "$SCRIPT_DIR"/examples/cmake-build-*/; do + for dir in "$ROOT_DIR"/examples/cmake-build-*/; do [ -d "$dir" ] && copy_board_binaries "$dir" done fi # Run test +CONFIG_BASENAME="$(basename "$CONFIG")" echo "==> Running HIL test on $REMOTE" -ssh -t "$REMOTE" "cd $REMOTE_DIR && python3 -u test/hil/hil_test.py -B examples ${ARGS[*]} tinyusb.json" +ssh -t "$REMOTE" "cd $REMOTE_DIR && python3 -u test/hil/hil_test.py -B examples ${ARGS[*]} test/hil/$CONFIG_BASENAME" -- cgit v1.3.1 From fd715afcc52b27127de4e7a6a89a7782fdef5676 Mon Sep 17 00:00:00 2001 From: hathach Date: Wed, 29 Apr 2026 11:46:34 +0700 Subject: Add `code-size` skill and integrate `metrics_compare_base.py` tool - Introduced a `code-size` skill under `.claude/skills` for evaluating TinyUSB code size changes between the base branch and current branch. - Added `metrics_compare_base.py`, automating code size comparison with granular options for examples, boards, and CI-wide runs. - Updated `AGENTS.md` to include quick references and usage guidance for the new feature. --- .claude/skills/code-size/SKILL.md | 76 ++++++++++++ .gitignore | 1 + AGENTS.md | 27 ++-- tools/metrics_compare_base.py | 252 ++++++++++++++++++++++++++++++++++++++ 4 files changed, 340 insertions(+), 16 deletions(-) create mode 100644 .claude/skills/code-size/SKILL.md create mode 100644 tools/metrics_compare_base.py diff --git a/.claude/skills/code-size/SKILL.md b/.claude/skills/code-size/SKILL.md new file mode 100644 index 000000000..f10380374 --- /dev/null +++ b/.claude/skills/code-size/SKILL.md @@ -0,0 +1,76 @@ +--- +name: code-size +description: Use when comparing TinyUSB code size between a base ref (master by default) and the current branch to evaluate the size impact of changes. Three granularities — single example on one board (with optional bloaty), all examples on one board, or all examples across CI families combined. +--- + +# Code Size Comparison + +Compare TinyUSB code size between a base ref (default `master`) and the current branch using `tools/metrics_compare_base.py`. Three granularities — pick the narrowest one that exercises your change: + +| Granularity | When to use | Command | +|---|---|---| +| **single example, one board** | Focused change touching one feature | `-b BOARD -e device/cdc_msc` | +| **all examples, one board** | Per-board regression sweep | `-b BOARD` | +| **all examples, all CI families (combined)** | Pre-merge full check | `--ci` | + +The script handles the full base-vs-branch dance: +1. Creates a temporary git worktree of the base ref under `cmake-metrics/_worktree/`. +2. Builds the base in `cmake-metrics//base/`. +3. Builds the current tree in `cmake-metrics//build/`. +4. Runs `tools/metrics.py compare` and writes `cmake-metrics//metrics_compare.md`. +5. Removes the worktree on exit. + +`--combined` (auto-set by `--ci`) also produces `cmake-metrics/_combined/metrics_compare.md` aggregating across all boards. + +## Choosing arguments + +Infer from the user's request: + +- **Board(s):** named board → `-b BOARD` (repeatable). "All boards" / "CI" / "full sweep" → `--ci` (first board of each arm-gcc family). Default to a fast board (`raspberry_pi_pico`) if unspecified for an iterative check. +- **Example:** named example → `-e /` (e.g. `-e device/cdc_msc`). "All examples" → omit `-e`. +- **Bloaty:** only with `-e`. Use when the user wants a section/symbol-level breakdown for a single binary. +- **Base ref:** default `master`. Override with `--base-branch ` (tag or commit also works). +- **Filter:** default `tinyusb/src` (only counts TinyUSB stack code, not example/BSP). Change only if asked. + +## Common invocations + +```bash +# Single example, one board (linkermap, fastest): +python3 tools/metrics_compare_base.py -b raspberry_pi_pico -e device/cdc_msc + +# Same with bloaty for section/symbol breakdown: +python3 tools/metrics_compare_base.py -b raspberry_pi_pico -e device/cdc_msc --bloaty + +# All examples for one board: +python3 tools/metrics_compare_base.py -b raspberry_pi_pico + +# Multiple boards, one combined report: +python3 tools/metrics_compare_base.py -b raspberry_pi_pico -b raspberry_pi_pico2 --combined + +# Full CI sweep (first board per arm-gcc family, combined): +python3 tools/metrics_compare_base.py --ci + +# Compare against a tag/commit instead of master: +python3 tools/metrics_compare_base.py -b raspberry_pi_pico --base-branch v0.18.0 +``` + +## Outputs + +- **Per-board:** `cmake-metrics//metrics_compare.md` (and `_.md` when `-e` is set) +- **Combined (with `--combined`/`--ci`):** `cmake-metrics/_combined/metrics_compare.md` +- **Bloaty:** printed to stdout as section + symbol diffs + +## Timing + +- Single example, single board: ~30 s +- All examples, single board: ~60-90 s +- `--ci` (all arm-gcc families, first board each): 4-8 minutes (parallel build) + +Use timeouts ≥ 10 minutes (600000 ms) for `--ci`. + +## Reporting results + +After running: +- Show the markdown report's summary table to the user. +- Highlight any rows with non-zero diff in `tinyusb/src` paths — those are the actual stack-size deltas. +- If the diff is unexpected, follow up with a single-example `--bloaty` run to localize. diff --git a/.gitignore b/.gitignore index b833191f8..e324916a4 100644 --- a/.gitignore +++ b/.gitignore @@ -56,3 +56,4 @@ BrowseInfo .cmake_build README_processed.rst .worktrees +cmake-metrics/ diff --git a/AGENTS.md b/AGENTS.md index 37fac2b05..eefe9dde1 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -134,28 +134,23 @@ cd docs && sphinx-build -b html . _build # ~2.5 s ## Code Size Metrics -Verify size impact before committing. +Verify size impact before committing. Invoke the `code-size` skill (`.claude/skills/code-size/SKILL.md`) — it wraps `tools/metrics_compare_base.py` to handle the base-vs-branch worktree + build + compare flow. -**Single-board (iterative, ~30 s):** +Quick reference: ```bash -rm -rf cmake-build -python3 tools/build.py -b raspberry_pi_pico --target all --target tinyusb_metrics -python3 tools/metrics.py combine -j -m -f tinyusb/src cmake-build/cmake-build-*/metrics.json -``` +# Single example, one board: +python3 tools/metrics_compare_base.py -b raspberry_pi_pico -e device/cdc_msc +# Add --bloaty for section/symbol breakdown. -**Compare vs master:** run the above on master, `mv metrics.json metrics_master.json`, switch branch, rebuild, then: -```bash -python3 tools/metrics.py compare -m -f tinyusb/src metrics_master.json metrics.json -``` +# All examples, one board: +python3 tools/metrics_compare_base.py -b raspberry_pi_pico -**Full CI (all arm-gcc families, 2-4 min):** -```bash -rm -rf cmake-build -FAMILIES=$(python3 .github/workflows/ci_set_matrix.py | python3 -c "import sys,json;d=json.load(sys.stdin);print(' '.join(d.get('arm-gcc',[])))") -python3 tools/build.py --one-first --target all --target tinyusb_metrics $FAMILIES -python3 tools/metrics.py combine -j -m -f tinyusb/src cmake-build/cmake-build-*/metrics.json +# All arm-gcc CI families combined (pre-merge sweep, 4-8 min): +python3 tools/metrics_compare_base.py --ci ``` +Reports land in `cmake-metrics//metrics_compare.md` (per-board) and `cmake-metrics/_combined/metrics_compare.md` (with `--combined`/`--ci`). + ## Static Analysis (PVS-Studio) Requires `compile_commands.json` (CMake `-DCMAKE_EXPORT_COMPILE_COMMANDS=ON`). diff --git a/tools/metrics_compare_base.py b/tools/metrics_compare_base.py new file mode 100644 index 000000000..a189e3143 --- /dev/null +++ b/tools/metrics_compare_base.py @@ -0,0 +1,252 @@ +#!/usr/bin/env python3 +"""Build base branch (master) and current tree, then compare code size metrics. + +Creates cmake-metrics//{base,build} directories for each board. +With --combined, also writes cmake-metrics/_combined/metrics_compare.md aggregating +all boards into a single comparison. + +Usage: + python tools/metrics_compare_base.py -b raspberry_pi_pico + python tools/metrics_compare_base.py -b raspberry_pi_pico -b raspberry_pi_pico2 + python tools/metrics_compare_base.py -b raspberry_pi_pico -f portable/raspberrypi + python tools/metrics_compare_base.py -b raspberry_pi_pico -e device/cdc_msc + python tools/metrics_compare_base.py -b raspberry_pi_pico -e device/cdc_msc --bloaty + python tools/metrics_compare_base.py --ci # first board of each arm-gcc family, combined + python tools/metrics_compare_base.py -b pico -b pico2 --combined # aggregate listed boards +""" +import argparse +import glob +import json +import os +import subprocess +import sys + +TINYUSB_ROOT = os.path.dirname(os.path.dirname(os.path.abspath(__file__))) +METRICS_DIR = os.path.join(TINYUSB_ROOT, 'cmake-metrics') + +verbose = False + + +def run(cmd, **kwargs): + if verbose: + print(f' $ {cmd}') + return subprocess.run(cmd, shell=True, capture_output=True, text=True, **kwargs) + + +def ci_first_boards(): + """Return the first board (alphabetical) of each arm-gcc CI family.""" + matrix_py = os.path.join(TINYUSB_ROOT, '.github', 'workflows', 'ci_set_matrix.py') + if not os.path.isfile(matrix_py): + return [] + ret = run(f'{sys.executable} {matrix_py}') + if ret.returncode != 0: + return [] + try: + data = json.loads(ret.stdout) + except json.JSONDecodeError: + return [] + families = data.get('arm-gcc', []) + boards = [] + bsp_root = os.path.join(TINYUSB_ROOT, 'hw', 'bsp') + for family in families: + family_boards = sorted( + d for d in os.listdir(os.path.join(bsp_root, family, 'boards')) + if os.path.isdir(os.path.join(bsp_root, family, 'boards', d)) + ) if os.path.isdir(os.path.join(bsp_root, family, 'boards')) else [] + if family_boards: + boards.append(family_boards[0]) + return boards + + +def build_board(src_dir, build_dir, board, example=None): + """Configure and build examples for a board. Returns True on success.""" + os.makedirs(build_dir, exist_ok=True) + ret = run(f'cmake -B {build_dir} -G Ninja -DBOARD={board} -DCMAKE_BUILD_TYPE=MinSizeRel ' + f'{os.path.join(src_dir, "examples")}') + if ret.returncode != 0: + print(f' Error configuring {board}: {ret.stderr}') + return False + target = f'--target {os.path.basename(example)}' if example else '' + ret = run(f'cmake --build {build_dir} {target}', timeout=600) + if ret.returncode != 0: + print(f' Error building {board}: {ret.stderr}') + return False + return True + + +def generate_metrics(build_dir, out_basename, filter_str, example=None): + """Run metrics.py combine on .map.json files. Returns metrics json path or None.""" + if example: + patterns = glob.glob(f'{build_dir}/{example}/*.map.json') + else: + patterns = glob.glob(f'{build_dir}/**/*.map.json', recursive=True) + if not patterns: + print(f' Error: no .map.json files in {build_dir}' + (f' for {example}' if example else '')) + return None + + metrics_py = os.path.join(TINYUSB_ROOT, 'tools', 'metrics.py') + ret = run(f'{sys.executable} {metrics_py} combine -f {filter_str} -j -q ' + f'-o {out_basename} {" ".join(patterns)}') + if ret.returncode != 0: + print(f' Error: {ret.stderr}') + return None + return f'{out_basename}.json' + + +def main(): + global verbose + + parser = argparse.ArgumentParser(description='Compare code size metrics with base branch') + parser.add_argument('-b', '--board', action='append', default=[], + help='Board name (repeatable). Required unless --ci is given.') + parser.add_argument('-f', '--filter', default='tinyusb/src', + help='Path filter for metrics (default: tinyusb/src)') + parser.add_argument('--base-branch', default='master', + help='Base branch to compare against (default: master)') + parser.add_argument('-e', '--example', action='append', default=None, + help='Compare specific example (repeatable, e.g. -e device/cdc_msc -e host/cdc_msc_hid)') + parser.add_argument('--bloaty', action='store_true', + help='Use bloaty for detailed section/symbol diff (requires -e)') + parser.add_argument('--ci', action='store_true', + help='Add the first board of every arm-gcc CI family. Implies --combined.') + parser.add_argument('--combined', action='store_true', + help='Aggregate map.json files across all boards into one comparison ' + '(in cmake-metrics/_combined/), instead of (or in addition to) per-board.') + parser.add_argument('-v', '--verbose', action='store_true', + help='Print build commands') + args = parser.parse_args() + verbose = args.verbose + + if args.bloaty and not args.example: + parser.error('--bloaty requires -e/--example') + + if args.ci: + args.combined = True + ci_boards = ci_first_boards() + if not ci_boards: + parser.error('--ci: failed to derive boards from .github/workflows/ci_set_matrix.py') + # Append, dedup, preserve order + seen = set(args.board) + for b in ci_boards: + if b not in seen: + args.board.append(b) + seen.add(b) + + if not args.board: + parser.error('at least one -b BOARD is required (or pass --ci)') + + metrics_py = os.path.join(TINYUSB_ROOT, 'tools', 'metrics.py') + linkermap_dir = os.path.join(TINYUSB_ROOT, 'tools', 'linkermap') + worktree_dir = os.path.join(METRICS_DIR, '_worktree') + + # Step 1: Create worktree for base branch + print(f'[1/5] Setting up {args.base_branch} worktree...') + if os.path.isdir(worktree_dir): + run(f'git -C {TINYUSB_ROOT} worktree remove --force {worktree_dir}') + ret = run(f'git -C {TINYUSB_ROOT} worktree add {worktree_dir} {args.base_branch}') + if ret.returncode != 0: + print(f'Error creating worktree: {ret.stderr}') + sys.exit(1) + + # Ensure linkermap is available + wt_linkermap = os.path.join(worktree_dir, 'tools', 'linkermap') + if not os.path.exists(wt_linkermap) and os.path.exists(linkermap_dir): + os.symlink(linkermap_dir, wt_linkermap) + + try: + examples = args.example or [None] + # For --combined: track every (base_build, cur_build) pair so we can aggregate at the end. + built_pairs = [] + + for board in args.board: + print(f'\n=== {board} ===') + board_dir = os.path.join(METRICS_DIR, board) + base_build = os.path.join(board_dir, 'base') + cur_build = os.path.join(board_dir, 'build') + + # Step 2: Build base (all examples, cmake will skip already-built) + print(f'[2/5] Building {args.base_branch} for {board}...') + if not build_board(worktree_dir, base_build, board): + continue + + # Step 3: Build current + print(f'[3/5] Building current for {board}...') + if not build_board(TINYUSB_ROOT, cur_build, board): + continue + + built_pairs.append((board, base_build, cur_build)) + base_filter = args.filter.replace('tinyusb/', '', 1) if args.filter.startswith('tinyusb/') else args.filter + + for example in examples: + suffix = f'_{example.replace("/", "_")}' if example else '' + label = f' ({example})' if example else '' + + # Step 4: Generate metrics + print(f'[4/5] Generating metrics for {board}{label}...') + base_json = generate_metrics(base_build, os.path.join(board_dir, f'base_metrics{suffix}'), + base_filter, example) + cur_json = generate_metrics(cur_build, os.path.join(board_dir, f'build_metrics{suffix}'), + args.filter, example) + if not base_json or not cur_json: + continue + + # Step 5: Compare + out_base = os.path.join(board_dir, f'metrics_compare{suffix}') + print(f'[5/5] Comparing {board}{label}...') + ret = run(f'{sys.executable} {metrics_py} compare -m -o {out_base} {base_json} {cur_json}') + print(ret.stdout) + + # Optional: bloaty diff + if args.bloaty and example: + elf_name = os.path.basename(example) + base_elf = os.path.join(base_build, example, f'{elf_name}.elf') + cur_elf = os.path.join(cur_build, example, f'{elf_name}.elf') + if os.path.exists(base_elf) and os.path.exists(cur_elf): + src_filter = f'--source-filter={args.filter}' if args.filter else '' + print(f'--- bloaty sections ---') + ret = run(f'bloaty --domain=vm -d compileunits,sections {src_filter} {cur_elf} -- {base_elf}') + print(ret.stdout) + print(f'--- bloaty symbols ---') + ret = run(f'bloaty --domain=vm -d compileunits,symbols -s vm {src_filter} {cur_elf} -- {base_elf}') + print(ret.stdout) + else: + print(f' bloaty: ELF not found') + + # Optional combined comparison across all boards + if args.combined and built_pairs: + combined_dir = os.path.join(METRICS_DIR, '_combined') + os.makedirs(combined_dir, exist_ok=True) + base_filter = args.filter.replace('tinyusb/', '', 1) if args.filter.startswith('tinyusb/') else args.filter + base_maps = [] + cur_maps = [] + for _board, base_build, cur_build in built_pairs: + base_maps += glob.glob(f'{base_build}/**/*.map.json', recursive=True) + cur_maps += glob.glob(f'{cur_build}/**/*.map.json', recursive=True) + if not base_maps or not cur_maps: + print(' combined: no map.json files collected, skipping') + else: + print(f'\n=== combined ({len(args.board)} boards) ===') + base_out = os.path.join(combined_dir, 'base_metrics') + cur_out = os.path.join(combined_dir, 'build_metrics') + ret = run(f'{sys.executable} {metrics_py} combine -f {base_filter} -j -q ' + f'-o {base_out} {" ".join(base_maps)}') + if ret.returncode != 0: + print(f' combined base error: {ret.stderr}') + else: + ret = run(f'{sys.executable} {metrics_py} combine -f {args.filter} -j -q ' + f'-o {cur_out} {" ".join(cur_maps)}') + if ret.returncode != 0: + print(f' combined current error: {ret.stderr}') + else: + out_combined = os.path.join(combined_dir, 'metrics_compare') + ret = run(f'{sys.executable} {metrics_py} compare -m ' + f'-o {out_combined} {base_out}.json {cur_out}.json') + print(ret.stdout) + print(f' combined report: {out_combined}.md') + finally: + print(f'\nCleaning up worktree...') + run(f'git -C {TINYUSB_ROOT} worktree remove --force {worktree_dir}') + + +if __name__ == '__main__': + main() -- cgit v1.3.1 From f5d6c6ba91e7176ddf5965608c361ccf5d515bde Mon Sep 17 00:00:00 2001 From: hathach Date: Wed, 29 Apr 2026 11:56:37 +0700 Subject: Improve remote execution in `hil_ci.sh` --- .claude/skills/code-size/SKILL.md | 2 +- AGENTS.md | 4 +- test/hil/hil_ci.sh | 21 ++++- tools/metrics_compare_base.py | 192 +++++++++++++++++++++++++++----------- 4 files changed, 158 insertions(+), 61 deletions(-) diff --git a/.claude/skills/code-size/SKILL.md b/.claude/skills/code-size/SKILL.md index f10380374..e12a30d86 100644 --- a/.claude/skills/code-size/SKILL.md +++ b/.claude/skills/code-size/SKILL.md @@ -64,7 +64,7 @@ python3 tools/metrics_compare_base.py -b raspberry_pi_pico --base-branch v0.18.0 - Single example, single board: ~30 s - All examples, single board: ~60-90 s -- `--ci` (all arm-gcc families, first board each): 4-8 minutes (parallel build) +- `--ci` (all arm-gcc families, first board each): 4-8 minutes — sequential sweep across boards (Ninja parallelizes within each board, not across) Use timeouts ≥ 10 minutes (600000 ms) for `--ci`. diff --git a/AGENTS.md b/AGENTS.md index eefe9dde1..5c9908d19 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -103,10 +103,10 @@ openocd -f interface/jlink.cfg -f target/stm32h7x.cfg openocd -f interface/cmsis-dap.cfg -f target/rp2040.cfg -c "adapter speed 5000" ``` -**Terminal 2 — connect GDB** (JLink :2331, OpenOCD :3333): +**Terminal 2 — connect GDB** (replace `` with `2331` for JLinkGDBServer or `3333` for OpenOCD): ```bash arm-none-eabi-gdb /tmp/build/firmware.elf -(gdb) target remote :2331 +(gdb) target remote : (gdb) monitor reset halt (gdb) load (gdb) break main # optional, to stop at entry diff --git a/test/hil/hil_ci.sh b/test/hil/hil_ci.sh index d1b5f7def..96872e2e1 100644 --- a/test/hil/hil_ci.sh +++ b/test/hil/hil_ci.sh @@ -26,6 +26,7 @@ ARGS=() while [[ $# -gt 0 ]]; do case "$1" in -b) + [[ $# -ge 2 ]] || { echo "error: -b requires a BOARD argument" >&2; exit 1; } BOARD="$2" ARGS+=("$1" "$2") shift 2 @@ -37,9 +38,14 @@ while [[ $# -gt 0 ]]; do esac done -# Setup remote directory +# Setup remote directory. Use `bash -s` + heredoc so REMOTE_DIR (user-overridable) +# is passed as a positional parameter and never reinterpreted by the remote shell. echo "==> Setting up remote $REMOTE:$REMOTE_DIR" -ssh "$REMOTE" "rm -rf $REMOTE_DIR && mkdir -p $REMOTE_DIR/test/hil $REMOTE_DIR/examples" +ssh "$REMOTE" bash -s -- "$REMOTE_DIR" <<'REMOTE' +set -e +rm -rf -- "$1" +mkdir -p -- "$1/test/hil" "$1/examples" +REMOTE # Copy HIL test script and config echo "==> Copying test scripts" @@ -60,7 +66,7 @@ if [ -n "$BOARD" ]; then BUILD_DIR="$ROOT_DIR/examples/cmake-build-$BOARD" if [ ! -d "$BUILD_DIR" ]; then echo "Error: build directory not found: $BUILD_DIR" - echo "Build first with: cd examples && cmake -DBOARD=$BOARD -G Ninja -B cmake-build-$BOARD .. && cmake --build cmake-build-$BOARD" + echo "Build first with: cd examples && cmake -DBOARD=$BOARD -G Ninja -B cmake-build-$BOARD . && cmake --build cmake-build-$BOARD" exit 1 fi echo "==> Copying binaries for $BOARD" @@ -72,7 +78,12 @@ else done fi -# Run test +# Run test. Use `bash -s` so REMOTE_DIR + ARGS reach the remote shell as positional +# parameters; quoting and metacharacters in args are preserved. CONFIG_BASENAME="$(basename "$CONFIG")" echo "==> Running HIL test on $REMOTE" -ssh -t "$REMOTE" "cd $REMOTE_DIR && python3 -u test/hil/hil_test.py -B examples ${ARGS[*]} test/hil/$CONFIG_BASENAME" +ssh "$REMOTE" bash -s -- "$REMOTE_DIR" "${ARGS[@]}" "test/hil/$CONFIG_BASENAME" <<'REMOTE' +cd -- "$1" +shift +exec python3 -u test/hil/hil_test.py -B examples "$@" +REMOTE diff --git a/tools/metrics_compare_base.py b/tools/metrics_compare_base.py index a189e3143..0fb767bb7 100644 --- a/tools/metrics_compare_base.py +++ b/tools/metrics_compare_base.py @@ -18,19 +18,57 @@ import argparse import glob import json import os +import re +import shlex import subprocess import sys TINYUSB_ROOT = os.path.dirname(os.path.dirname(os.path.abspath(__file__))) METRICS_DIR = os.path.join(TINYUSB_ROOT, 'cmake-metrics') +def tinyusb_src_filter(checkout_dir): + """Return a path-substring filter that uniquely matches TinyUSB stack source files + in `checkout_dir`. The substring is the absolute path to the checkout's `src/` + dir — collision-free with vendored deps (pico-sdk, lwip, FreeRTOS, etc.) which + live at unrelated paths.""" + return os.path.realpath(os.path.join(checkout_dir, 'src')) + os.sep + verbose = False def run(cmd, **kwargs): + """Run a command. cmd must be a list (no shell=True).""" + if not isinstance(cmd, list): + raise TypeError('run() requires a list, got str — fix the caller') if verbose: - print(f' $ {cmd}') - return subprocess.run(cmd, shell=True, capture_output=True, text=True, **kwargs) + print(f' $ {" ".join(shlex.quote(str(c)) for c in cmd)}') + return subprocess.run(cmd, capture_output=True, text=True, **kwargs) + + +def symlink_deps(main_root, worktree_dir): + """Symlink dependency directories (fetched by tools/get_deps.py) from the main + checkout into the temporary worktree. Without this, the base build fails because + the worktree doesn't have the untracked deps.""" + def link_subdirs(rel_parent): + src_parent = os.path.join(main_root, rel_parent) + dst_parent = os.path.join(worktree_dir, rel_parent) + if not os.path.isdir(src_parent): + return + os.makedirs(dst_parent, exist_ok=True) + for entry in os.listdir(src_parent): + src = os.path.join(src_parent, entry) + dst = os.path.join(dst_parent, entry) + if os.path.isdir(src) and not os.path.exists(dst): + os.symlink(src, dst) + + # lib/* and tools/* deps (e.g. lib/lwip, tools/linkermap) + link_subdirs('lib') + link_subdirs('tools') + # hw/mcu// (e.g. hw/mcu/raspberry_pi/Pico-PIO-USB) + hw_mcu = os.path.join(main_root, 'hw', 'mcu') + if os.path.isdir(hw_mcu): + for vendor in os.listdir(hw_mcu): + link_subdirs(os.path.join('hw', 'mcu', vendor)) def ci_first_boards(): @@ -38,7 +76,7 @@ def ci_first_boards(): matrix_py = os.path.join(TINYUSB_ROOT, '.github', 'workflows', 'ci_set_matrix.py') if not os.path.isfile(matrix_py): return [] - ret = run(f'{sys.executable} {matrix_py}') + ret = run([sys.executable, matrix_py]) if ret.returncode != 0: return [] try: @@ -59,23 +97,34 @@ def ci_first_boards(): def build_board(src_dir, build_dir, board, example=None): - """Configure and build examples for a board. Returns True on success.""" + """Configure and build examples for a board. Returns True on success. + + When `example` is given, only that target is built (`cmake --build --target NAME`), + keeping single-example workflows fast. + """ os.makedirs(build_dir, exist_ok=True) - ret = run(f'cmake -B {build_dir} -G Ninja -DBOARD={board} -DCMAKE_BUILD_TYPE=MinSizeRel ' - f'{os.path.join(src_dir, "examples")}') + ret = run(['cmake', '-B', build_dir, '-G', 'Ninja', + f'-DBOARD={board}', '-DCMAKE_BUILD_TYPE=MinSizeRel', + os.path.join(src_dir, 'examples')]) if ret.returncode != 0: print(f' Error configuring {board}: {ret.stderr}') return False - target = f'--target {os.path.basename(example)}' if example else '' - ret = run(f'cmake --build {build_dir} {target}', timeout=600) + cmd = ['cmake', '--build', build_dir] + if example: + cmd += ['--target', os.path.basename(example)] + ret = run(cmd, timeout=600) if ret.returncode != 0: print(f' Error building {board}: {ret.stderr}') return False return True -def generate_metrics(build_dir, out_basename, filter_str, example=None): - """Run metrics.py combine on .map.json files. Returns metrics json path or None.""" +def generate_metrics(build_dir, out_basename, filters, example=None): + """Run metrics.py combine on .map.json files. Returns metrics json path or None. + + `filters` is a list of substrings; metrics.py keeps a compile unit if its path + contains any of them. + """ if example: patterns = glob.glob(f'{build_dir}/{example}/*.map.json') else: @@ -85,8 +134,11 @@ def generate_metrics(build_dir, out_basename, filter_str, example=None): return None metrics_py = os.path.join(TINYUSB_ROOT, 'tools', 'metrics.py') - ret = run(f'{sys.executable} {metrics_py} combine -f {filter_str} -j -q ' - f'-o {out_basename} {" ".join(patterns)}') + cmd = [sys.executable, metrics_py, 'combine'] + for f in filters: + cmd += ['-f', f] + cmd += ['-j', '-q', '-o', out_basename, *patterns] + ret = run(cmd) if ret.returncode != 0: print(f' Error: {ret.stderr}') return None @@ -99,8 +151,12 @@ def main(): parser = argparse.ArgumentParser(description='Compare code size metrics with base branch') parser.add_argument('-b', '--board', action='append', default=[], help='Board name (repeatable). Required unless --ci is given.') - parser.add_argument('-f', '--filter', default='tinyusb/src', - help='Path filter for metrics (default: tinyusb/src)') + parser.add_argument('-f', '--filter', action='append', default=None, + help='Path-substring filter (repeatable). When given, ' + 'overrides the default and is applied to BOTH base and ' + 'current builds. Default: each side\'s own absolute ' + '/src/ path, which uniquely matches TinyUSB ' + 'stack code without colliding with vendored deps.') parser.add_argument('--base-branch', default='master', help='Base branch to compare against (default: master)') parser.add_argument('-e', '--example', action='append', default=None, @@ -136,22 +192,28 @@ def main(): parser.error('at least one -b BOARD is required (or pass --ci)') metrics_py = os.path.join(TINYUSB_ROOT, 'tools', 'metrics.py') - linkermap_dir = os.path.join(TINYUSB_ROOT, 'tools', 'linkermap') worktree_dir = os.path.join(METRICS_DIR, '_worktree') + # Per-side filters: when no override is given, each build uses its own + # absolute /src/ path so we only match TinyUSB stack code from that + # checkout (and never vendored-dep `src/` like pico-sdk/src/...). + if args.filter: + base_filters = cur_filters = list(args.filter) + else: + base_filters = [tinyusb_src_filter(worktree_dir)] + cur_filters = [tinyusb_src_filter(TINYUSB_ROOT)] + # Step 1: Create worktree for base branch print(f'[1/5] Setting up {args.base_branch} worktree...') if os.path.isdir(worktree_dir): - run(f'git -C {TINYUSB_ROOT} worktree remove --force {worktree_dir}') - ret = run(f'git -C {TINYUSB_ROOT} worktree add {worktree_dir} {args.base_branch}') + run(['git', '-C', TINYUSB_ROOT, 'worktree', 'remove', '--force', worktree_dir]) + ret = run(['git', '-C', TINYUSB_ROOT, 'worktree', 'add', worktree_dir, args.base_branch]) if ret.returncode != 0: print(f'Error creating worktree: {ret.stderr}') sys.exit(1) - # Ensure linkermap is available - wt_linkermap = os.path.join(worktree_dir, 'tools', 'linkermap') - if not os.path.exists(wt_linkermap) and os.path.exists(linkermap_dir): - os.symlink(linkermap_dir, wt_linkermap) + # Symlink dependency dirs (lib/*, hw/mcu/*/*, tools/*) so the worktree builds. + symlink_deps(TINYUSB_ROOT, worktree_dir) try: examples = args.example or [None] @@ -164,18 +226,23 @@ def main(): base_build = os.path.join(board_dir, 'base') cur_build = os.path.join(board_dir, 'build') - # Step 2: Build base (all examples, cmake will skip already-built) - print(f'[2/5] Building {args.base_branch} for {board}...') - if not build_board(worktree_dir, base_build, board): - continue - - # Step 3: Build current - print(f'[3/5] Building current for {board}...') - if not build_board(TINYUSB_ROOT, cur_build, board): + # Build only the requested examples (or all if -e not given). Single-example + # mode used to build everything and filter at metric time — that was wasted work. + board_failed = False + for example in examples: + build_label = f' --target {os.path.basename(example)}' if example else '' + print(f'[2/5] Building {args.base_branch} for {board}{build_label}...') + if not build_board(worktree_dir, base_build, board, example): + board_failed = True + break + print(f'[3/5] Building current for {board}{build_label}...') + if not build_board(TINYUSB_ROOT, cur_build, board, example): + board_failed = True + break + if board_failed: continue built_pairs.append((board, base_build, cur_build)) - base_filter = args.filter.replace('tinyusb/', '', 1) if args.filter.startswith('tinyusb/') else args.filter for example in examples: suffix = f'_{example.replace("/", "_")}' if example else '' @@ -184,16 +251,16 @@ def main(): # Step 4: Generate metrics print(f'[4/5] Generating metrics for {board}{label}...') base_json = generate_metrics(base_build, os.path.join(board_dir, f'base_metrics{suffix}'), - base_filter, example) + base_filters, example) cur_json = generate_metrics(cur_build, os.path.join(board_dir, f'build_metrics{suffix}'), - args.filter, example) + cur_filters, example) if not base_json or not cur_json: continue # Step 5: Compare out_base = os.path.join(board_dir, f'metrics_compare{suffix}') print(f'[5/5] Comparing {board}{label}...') - ret = run(f'{sys.executable} {metrics_py} compare -m -o {out_base} {base_json} {cur_json}') + ret = run([sys.executable, metrics_py, 'compare', '-m', '-o', out_base, base_json, cur_json]) print(ret.stdout) # Optional: bloaty diff @@ -202,50 +269,69 @@ def main(): base_elf = os.path.join(base_build, example, f'{elf_name}.elf') cur_elf = os.path.join(cur_build, example, f'{elf_name}.elf') if os.path.exists(base_elf) and os.path.exists(cur_elf): - src_filter = f'--source-filter={args.filter}' if args.filter else '' + # Bloaty expects one regex; OR-join all filters (current side + # for the new ELF, base side for the base ELF). + bloaty_regex = '(' + '|'.join( + re.escape(f) for f in (cur_filters + base_filters) + ) + ')' + bloaty_common = ['bloaty', '--domain=vm', f'--source-filter={bloaty_regex}'] print(f'--- bloaty sections ---') - ret = run(f'bloaty --domain=vm -d compileunits,sections {src_filter} {cur_elf} -- {base_elf}') + ret = run(bloaty_common + ['-d', 'compileunits,sections', cur_elf, '--', base_elf]) print(ret.stdout) print(f'--- bloaty symbols ---') - ret = run(f'bloaty --domain=vm -d compileunits,symbols -s vm {src_filter} {cur_elf} -- {base_elf}') + ret = run(bloaty_common + ['-d', 'compileunits,symbols', '-s', 'vm', + cur_elf, '--', base_elf]) print(ret.stdout) else: print(f' bloaty: ELF not found') - # Optional combined comparison across all boards + # Optional combined comparison across all boards. + # Aggregates the per-board metrics JSONs (not raw map.json globs) so the argv + # stays small even with --ci spanning many boards. if args.combined and built_pairs: combined_dir = os.path.join(METRICS_DIR, '_combined') os.makedirs(combined_dir, exist_ok=True) - base_filter = args.filter.replace('tinyusb/', '', 1) if args.filter.startswith('tinyusb/') else args.filter - base_maps = [] - cur_maps = [] - for _board, base_build, cur_build in built_pairs: - base_maps += glob.glob(f'{base_build}/**/*.map.json', recursive=True) - cur_maps += glob.glob(f'{cur_build}/**/*.map.json', recursive=True) - if not base_maps or not cur_maps: - print(' combined: no map.json files collected, skipping') + + # Use the no-suffix per-board JSONs (whole-board metrics). Combined mode + # is meant for board-level sweeps; -e/--example combinations skip combined. + base_jsons, cur_jsons = [], [] + for board, _, _ in built_pairs: + bj = os.path.join(METRICS_DIR, board, 'base_metrics.json') + cj = os.path.join(METRICS_DIR, board, 'build_metrics.json') + if os.path.isfile(bj) and os.path.isfile(cj): + base_jsons.append(bj) + cur_jsons.append(cj) + + if not base_jsons or not cur_jsons: + print(' combined: no per-board metrics found (did you pass -e? skip --combined with -e)') else: - print(f'\n=== combined ({len(args.board)} boards) ===') + print(f'\n=== combined ({len(base_jsons)} boards) ===') base_out = os.path.join(combined_dir, 'base_metrics') cur_out = os.path.join(combined_dir, 'build_metrics') - ret = run(f'{sys.executable} {metrics_py} combine -f {base_filter} -j -q ' - f'-o {base_out} {" ".join(base_maps)}') + + # Per-board JSONs are already filtered to TinyUSB-only files; combine + # without re-filtering so we don't accidentally drop entries. + def _combine(out_basename, inputs): + cmd = [sys.executable, metrics_py, 'combine', + '-j', '-q', '-o', out_basename, *inputs] + return run(cmd) + + ret = _combine(base_out, base_jsons) if ret.returncode != 0: print(f' combined base error: {ret.stderr}') else: - ret = run(f'{sys.executable} {metrics_py} combine -f {args.filter} -j -q ' - f'-o {cur_out} {" ".join(cur_maps)}') + ret = _combine(cur_out, cur_jsons) if ret.returncode != 0: print(f' combined current error: {ret.stderr}') else: out_combined = os.path.join(combined_dir, 'metrics_compare') - ret = run(f'{sys.executable} {metrics_py} compare -m ' - f'-o {out_combined} {base_out}.json {cur_out}.json') + ret = run([sys.executable, metrics_py, 'compare', '-m', + '-o', out_combined, f'{base_out}.json', f'{cur_out}.json']) print(ret.stdout) print(f' combined report: {out_combined}.md') finally: print(f'\nCleaning up worktree...') - run(f'git -C {TINYUSB_ROOT} worktree remove --force {worktree_dir}') + run(['git', '-C', TINYUSB_ROOT, 'worktree', 'remove', '--force', worktree_dir]) if __name__ == '__main__': -- cgit v1.3.1 From 6ba8aeff1603ae54e0fcf2309b0f19e335a16cdc Mon Sep 17 00:00:00 2001 From: hathach Date: Wed, 29 Apr 2026 12:45:55 +0700 Subject: metrics_compare_base: catch TimeoutExpired; fix code-size skill docs - run() now catches subprocess.TimeoutExpired (only triggered by `cmake --build`'s timeout=600) and returns CompletedProcess(rc=124) so the caller falls through to error reporting and worktree cleanup instead of crashing with a traceback. - code-size SKILL.md: document the actual default filter (per-side absolute /src/ path, not the old `tinyusb/src` substring) and adjust the reporting guidance to match what the report rows actually contain. Co-Authored-By: Claude Opus 4.7 (1M context) --- .claude/skills/code-size/SKILL.md | 4 ++-- tools/metrics_compare_base.py | 12 ++++++++++-- 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/.claude/skills/code-size/SKILL.md b/.claude/skills/code-size/SKILL.md index e12a30d86..f3c51ccfa 100644 --- a/.claude/skills/code-size/SKILL.md +++ b/.claude/skills/code-size/SKILL.md @@ -30,7 +30,7 @@ Infer from the user's request: - **Example:** named example → `-e /` (e.g. `-e device/cdc_msc`). "All examples" → omit `-e`. - **Bloaty:** only with `-e`. Use when the user wants a section/symbol-level breakdown for a single binary. - **Base ref:** default `master`. Override with `--base-branch ` (tag or commit also works). -- **Filter:** default `tinyusb/src` (only counts TinyUSB stack code, not example/BSP). Change only if asked. +- **Filter:** default is the absolute path of each side's `/src/` directory, which uniquely identifies TinyUSB stack code without matching vendored deps that also have a `src/` (e.g. `pico-sdk/src/`). Override with one or more `-f SUBSTRING` flags to use repo-relative substrings instead. Change only if asked. ## Common invocations @@ -72,5 +72,5 @@ Use timeouts ≥ 10 minutes (600000 ms) for `--ci`. After running: - Show the markdown report's summary table to the user. -- Highlight any rows with non-zero diff in `tinyusb/src` paths — those are the actual stack-size deltas. +- Highlight any rows with non-zero `% diff` — under the default filter every row is a TinyUSB stack source file (e.g. `usbd.c`, `cdc_device.c`, `dcd_.c`), so any non-zero delta is a real stack-size impact. - If the diff is unexpected, follow up with a single-example `--bloaty` run to localize. diff --git a/tools/metrics_compare_base.py b/tools/metrics_compare_base.py index 0fb767bb7..a541dae79 100644 --- a/tools/metrics_compare_base.py +++ b/tools/metrics_compare_base.py @@ -37,12 +37,20 @@ verbose = False def run(cmd, **kwargs): - """Run a command. cmd must be a list (no shell=True).""" + """Run a command. cmd must be a list (no shell=True). On `timeout=`-induced + TimeoutExpired, return a CompletedProcess with rc=124 instead of letting the + exception propagate, so the caller can fall through to error reporting and + worktree cleanup rather than crashing with a traceback.""" if not isinstance(cmd, list): raise TypeError('run() requires a list, got str — fix the caller') if verbose: print(f' $ {" ".join(shlex.quote(str(c)) for c in cmd)}') - return subprocess.run(cmd, capture_output=True, text=True, **kwargs) + try: + return subprocess.run(cmd, capture_output=True, text=True, **kwargs) + except subprocess.TimeoutExpired as e: + msg = f'Command timed out after {e.timeout}s: {" ".join(shlex.quote(str(c)) for c in cmd)}' + stderr = (e.stderr or '') + ('\n' if e.stderr else '') + msg + return subprocess.CompletedProcess(cmd, 124, stdout=(e.stdout or ''), stderr=stderr) def symlink_deps(main_root, worktree_dir): -- cgit v1.3.1 From 17572a960a53e27ffa07d7d7fda3486bfcc95a2d Mon Sep 17 00:00:00 2001 From: hathach Date: Wed, 29 Apr 2026 12:59:40 +0700 Subject: metrics_compare_base: use git worktree add --detach `git worktree add ` fails if is already checked out elsewhere (main repo, another worktree). --detach checks out the ref at a detached HEAD instead of claiming the branch, making the script work regardless of what is currently checked out. Co-Authored-By: Claude Opus 4.7 (1M context) --- tools/metrics_compare_base.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/tools/metrics_compare_base.py b/tools/metrics_compare_base.py index a541dae79..799a96800 100644 --- a/tools/metrics_compare_base.py +++ b/tools/metrics_compare_base.py @@ -215,7 +215,11 @@ def main(): print(f'[1/5] Setting up {args.base_branch} worktree...') if os.path.isdir(worktree_dir): run(['git', '-C', TINYUSB_ROOT, 'worktree', 'remove', '--force', worktree_dir]) - ret = run(['git', '-C', TINYUSB_ROOT, 'worktree', 'add', worktree_dir, args.base_branch]) + # --detach: check out the ref at a detached HEAD instead of trying to claim the + # branch. Lets us add a worktree of `master` even if master is already checked + # out elsewhere (main repo, another worktree). + ret = run(['git', '-C', TINYUSB_ROOT, 'worktree', 'add', '--detach', + worktree_dir, args.base_branch]) if ret.returncode != 0: print(f'Error creating worktree: {ret.stderr}') sys.exit(1) -- cgit v1.3.1