diff options
| author | hathach <[email protected]> | 2026-08-11 17:08:05 +0700 |
|---|---|---|
| committer | hathach <[email protected]> | 2026-09-04 04:21:57 +0700 |
| commit | dcb3fcfd30ac03b810d6de82674c008d5163400f (patch) | |
| tree | acdbcca6edb35c292f8ac0fb60c320616dca71d0 | |
| parent | 7474173b075ae656ed16dea5dcb3f830349f3450 (diff) | |
docs: sysview v2 and HIL-report specs and plans, skill-vs-technique criteria
The v2 post-mortem design and plan, the HIL performance-report design and
plan, and the promotion criteria for when a debugging technique earns its
own skill (added to the agents-workflows design).
5 files changed, 2030 insertions, 2 deletions
diff --git a/docs/superpowers/plans/2026-07-25-sysview-v2.md b/docs/superpowers/plans/2026-07-25-sysview-v2.md new file mode 100644 index 000000000..c1cf369d3 --- /dev/null +++ b/docs/superpowers/plans/2026-07-25-sysview-v2.md @@ -0,0 +1,772 @@ +# sysview v2 Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Make SEGGER SystemView a first-class TinyUSB analysis feature — leveled in-tree instrumentation (`CFG_TUD_SYSVIEW`/`CFG_TUH_SYSVIEW` = 0–4, like `CFG_TUSB_DEBUG`) for USB task/ISR/per-function timing plus heap/stack metrics, a pinned `get_deps` dependency, machine-readable reports, post-mortem crash capture, and an OpenOCD capture route. + +**Architecture:** Target-side instrumentation moves from skill scripts into `src/common/tusb_sysview.[ch]` behind `CFG_TUD_SYSVIEW`/`CFG_TUH_SYSVIEW` levels (default 0 = off, zero cost; 1 = USB ISR, 2 = +usbd/usbh, 3 = +dcd/hcd API at the usbd/usbh call sites, 4 = +class drivers), hooked at `tusb_rhport_init` (init) and `tusb_int_handler` (ISR) so **no application edits are needed** — `cmake -DSYSVIEW=<level>` is the entire instrumentation step. Host side keeps the validated v1 recorder (GUI live recorder under Xvfb) and gains a post-mortem dump script and an OpenOCD raw-capture recipe, both feeding the existing `--from-raw` decode. The reporter grows function/stack/heap/marker/blocked-time tables and `--json`. + +**Tech Stack:** C99 (TinyUSB style), CMake (`hw/bsp/family_support.cmake`), SEGGER SystemView target sources V4.12.0 (github.com/SEGGERMicro/SystemView) + vendored `lib/SEGGER_RTT`, Python 3 host scripts, same54_xplained + J-Trace `jtrace` on htpc for validation, stm32h743nucleo (OpenOCD/ST-Link, ci.lan rig) for the OpenOCD route. + +## Global Constraints + +- Branch: work on `claude/add-systemview-debug` in worktree `.worktrees/add-systemview-debug` (LOCAL — never push). Symlink deps per CLAUDE.md worktree note. +- Commits: imperative mood, **no Co-Authored-By/Claude-Session trailers** (authorship rule). `pre-commit run --files <touched>` before every commit. +- C99, 2-space indent, snake_case, `TU_`/`tusb_` prefixes; headers self-contained. +- `CFG_TUD_SYSVIEW=0`/`CFG_TUH_SYSVIEW=0` (default) must add **zero** code/data — gated by a code-size compare (Task 8). +- Never modify `lib/FreeRTOS-Kernel` or `hw/mcu/*` (vendor trees). `lib/SystemView` is a fetched dep — never edit its contents. +- Hardware: one probe, one client (kill stray `systemview`/`Xvfb`/`JLink*` first); captures to scratchpad or `/tmp/sysview-*` (never `/tmp/sv-*` — SystemView deletes those); pristine reflash + revert any temporary validation edits before finishing a task that flashed hardware. +- Validation board `same54_xplained`: J-Link device `ATSAME54P20`, probe nickname `jtrace`, TTY `/dev/serial/by-id/usb-TinyUSB_TinyUSB_Device_*-if00`. Traffic one-liner (referenced by tasks as “CDC load”): + `stty -F $TTY raw -echo; (timeout 7 cat $TTY >/dev/null &); for i in $(seq 65); do head -c 512 /dev/zero > $TTY; sleep 0.1; done` +- Worktree paths below are relative to the worktree root. + +--- + +### Task 1: `lib/SystemView` as a pinned optional dependency + +**Files:** +- Modify: `tools/get_deps.py` (deps_optional dict, alphabetical position after `lib/lwip`) + +**Interfaces:** +- Produces: `lib/SystemView/` populated at V4.12.0 (`SYSVIEW/SEGGER_SYSVIEW.c`, `Sample/FreeRTOSV11/…`, `Config/…`). Task 3's CMake requires exactly this path. + +- [ ] **Step 1: Add the dependency entry** + +In `tools/get_deps.py`, `deps_optional`, after the `'lib/lwip'` entry: + +```python + 'lib/SystemView': ['https://github.com/SEGGERMicro/SystemView.git', + '92ca7a810c5765ba64911919acd511c61b6b083f', + 'all'], +``` + +(That commit is the `V4.12.0` tag.) + +- [ ] **Step 2: Verify the fetch** + +Run: `python3 tools/get_deps.py samd5x_e5x` (from the worktree root) +Expected: clones into `lib/SystemView`; then +`ls lib/SystemView/SYSVIEW/SEGGER_SYSVIEW.c lib/SystemView/Sample/FreeRTOSV11/SEGGER_SYSVIEW_FreeRTOS.c` → both exist. + +- [ ] **Step 3: Commit** + +```bash +git add tools/get_deps.py +git commit -m "add SEGGER SystemView target sources as optional dependency (V4.12.0)" +``` + +--- + +### Task 2: In-tree core instrumentation (`tusb_sysview.[ch]` + hooks) + +**Files:** +- Create: `src/common/tusb_sysview.h`, `src/common/tusb_sysview.c` +- Modify: `src/tusb_option.h` (add `CFG_TUD_SYSVIEW`/`CFG_TUH_SYSVIEW` defaults), `src/tusb.c:65` (`tusb_rhport_init` — init hook), `src/tusb.c:130` (`tusb_int_handler` — ISR wrap) + +**Interfaces:** +- Produces: `tusb_sysview_init(void)`; `TU_SYSVIEW_ISR_ENTER()/TU_SYSVIEW_ISR_EXIT()`; `TUD_SYSVIEW_CALL/RET(level, id)` + `TUH_SYSVIEW_CALL/RET(level, id)` with `tu_sysview_id_t` enum and `CFG_TUSB_SYSVIEW_LEVEL_{ISR,USB,PORT,CLASS}` category levels; `tusb_sysview_stack_report(void)`; `tusb_sysview_heap_alloc(void*, unsigned)` / `tusb_sysview_heap_free(void*)`. All compile to nothing when both config levels are 0; a site compiles only when its level ≤ the configured level (TU_LOG-style token paste). Tasks 3–5 consume these exact names. + +- [ ] **Step 1: `tusb_option.h` defaults** + +Next to the other `CFG_TUSB_*` defaults. Leveled like `CFG_TUSB_DEBUG` +(0 = off; a site is compiled only when its level ≤ the configured level): + +```c +// SEGGER SystemView instrumentation level (like CFG_TUSB_DEBUG): +// 0=off 1=USB ISR 2=+usbd/usbh functions 3=+dcd/hcd API 4=+class driver API +// Requires lib/SystemView (python3 tools/get_deps.py) and a SYSVIEW=<level> build. +#ifndef CFG_TUD_SYSVIEW + #define CFG_TUD_SYSVIEW 0 +#endif +#ifndef CFG_TUH_SYSVIEW + #define CFG_TUH_SYSVIEW 0 +#endif +``` + +- [ ] **Step 2: Write `src/common/tusb_sysview.h`** + +```c +#ifndef TUSB_SYSVIEW_H_ +#define TUSB_SYSVIEW_H_ + +#include "tusb_option.h" +#include "common/tusb_compiler.h" // TU_XSTRCAT + +#define TU_SYSVIEW_ENABLED (CFG_TUD_SYSVIEW || CFG_TUH_SYSVIEW) + +#if TU_SYSVIEW_ENABLED +#include "SEGGER_SYSVIEW.h" + +// Category levels — overridable, must expand to a literal 1..4 +#ifndef CFG_TUSB_SYSVIEW_LEVEL_ISR + #define CFG_TUSB_SYSVIEW_LEVEL_ISR 1 // USB interrupt enter/exit +#endif +#ifndef CFG_TUSB_SYSVIEW_LEVEL_USB + #define CFG_TUSB_SYSVIEW_LEVEL_USB 2 // usbd/usbh core functions +#endif +#ifndef CFG_TUSB_SYSVIEW_LEVEL_PORT + #define CFG_TUSB_SYSVIEW_LEVEL_PORT 3 // dcd/hcd API (wrapped at usbd/usbh call sites) +#endif +#ifndef CFG_TUSB_SYSVIEW_LEVEL_CLASS + #define CFG_TUSB_SYSVIEW_LEVEL_CLASS 4 // class driver API +#endif + +// Function-timing event ids: index into the module description string in +// tusb_sysview.c — the two MUST stay in the same order. +typedef enum { + TU_SV_ID_TUD_TASK = 0, // one usbd event processed (level USB) + TU_SV_ID_USBD_XFER, // usbd_edpt_xfer (level USB) + TU_SV_ID_DCD_XFER, // dcd_edpt_xfer call (level PORT) + TU_SV_ID_CDC_FLUSH, // tud_cdc_n_write_flush (level CLASS) + TU_SV_ID_CDC_READ, // tud_cdc_n_read (level CLASS) + TU_SV_ID_MSC_XFER, // mscd_xfer_cb (level CLASS) + TU_SV_ID_TUH_TASK, // one usbh event processed (level USB) + TU_SV_ID_HCD_XFER, // hcd_edpt_xfer call (level PORT) + TU_SV_ID_COUNT +} tu_sysview_id_t; + +extern SEGGER_SYSVIEW_MODULE tusb_sysview_module; + +void tusb_sysview_init(void); +void tusb_sysview_stack_report(void); +void tusb_sysview_heap_alloc(void* ptr, unsigned size); +void tusb_sysview_heap_free(void* ptr); + +// Per-level backends: _TUD_SV_CALL_<n> is live iff CFG_TUD_SYSVIEW >= n. +// TU_LOG-style: TUD_SYSVIEW_CALL(level, id) token-pastes to the backend, so a +// site whose level exceeds the config expands to nothing. The level argument +// is one of the CFG_TUSB_SYSVIEW_LEVEL_* macros (expands to 1..4 first). +#define _TU_SV_RECORD(_id) SEGGER_SYSVIEW_RecordVoid(tusb_sysview_module.EventOffset + (_id)) +#define _TU_SV_END(_id) SEGGER_SYSVIEW_RecordEndCall(tusb_sysview_module.EventOffset + (_id)) + +#if CFG_TUD_SYSVIEW >= 1 + #define _TUD_SV_CALL_1(_id) _TU_SV_RECORD(_id) + #define _TUD_SV_RET_1(_id) _TU_SV_END(_id) +#else + #define _TUD_SV_CALL_1(_id) + #define _TUD_SV_RET_1(_id) +#endif +#if CFG_TUD_SYSVIEW >= 2 + #define _TUD_SV_CALL_2(_id) _TU_SV_RECORD(_id) + #define _TUD_SV_RET_2(_id) _TU_SV_END(_id) +#else + #define _TUD_SV_CALL_2(_id) + #define _TUD_SV_RET_2(_id) +#endif +#if CFG_TUD_SYSVIEW >= 3 + #define _TUD_SV_CALL_3(_id) _TU_SV_RECORD(_id) + #define _TUD_SV_RET_3(_id) _TU_SV_END(_id) +#else + #define _TUD_SV_CALL_3(_id) + #define _TUD_SV_RET_3(_id) +#endif +#if CFG_TUD_SYSVIEW >= 4 + #define _TUD_SV_CALL_4(_id) _TU_SV_RECORD(_id) + #define _TUD_SV_RET_4(_id) _TU_SV_END(_id) +#else + #define _TUD_SV_CALL_4(_id) + #define _TUD_SV_RET_4(_id) +#endif +// same 4-block ladder for _TUH_SV_CALL_/RET_ gated on CFG_TUH_SYSVIEW +// (write it out — 16 more lines, identical shape) + +#define TUD_SYSVIEW_CALL(_level, _id) TU_XSTRCAT(_TUD_SV_CALL_, _level)(_id) +#define TUD_SYSVIEW_RET(_level, _id) TU_XSTRCAT(_TUD_SV_RET_, _level)(_id) +#define TUH_SYSVIEW_CALL(_level, _id) TU_XSTRCAT(_TUH_SV_CALL_, _level)(_id) +#define TUH_SYSVIEW_RET(_level, _id) TU_XSTRCAT(_TUH_SV_RET_, _level)(_id) + +// ISR wrap serves the shared tusb_int_handler entry (device and/or host) +#if (CFG_TUD_SYSVIEW >= CFG_TUSB_SYSVIEW_LEVEL_ISR) || (CFG_TUH_SYSVIEW >= CFG_TUSB_SYSVIEW_LEVEL_ISR) + #define TU_SYSVIEW_ISR_ENTER() SEGGER_SYSVIEW_RecordEnterISR() + #define TU_SYSVIEW_ISR_EXIT() SEGGER_SYSVIEW_RecordExitISR() +#else + #define TU_SYSVIEW_ISR_ENTER() + #define TU_SYSVIEW_ISR_EXIT() +#endif + +#else // !TU_SYSVIEW_ENABLED + +#define TU_SYSVIEW_ISR_ENTER() +#define TU_SYSVIEW_ISR_EXIT() +#define TUD_SYSVIEW_CALL(_level, _id) +#define TUD_SYSVIEW_RET(_level, _id) +#define TUH_SYSVIEW_CALL(_level, _id) +#define TUH_SYSVIEW_RET(_level, _id) +#define tusb_sysview_init() +#define tusb_sysview_stack_report() + +#endif // TU_SYSVIEW_ENABLED + +#endif // TUSB_SYSVIEW_H_ +``` + +- [ ] **Step 3: Write `src/common/tusb_sysview.c`** + +Port of the validated v1 `SEGGER_SYSVIEW_Config_TinyUSB.c` plus module + +heap/stack helpers. The whole file is inside `#if CFG_TUD_SYSVIEW || CFG_TUH_SYSVIEW`: + +```c +#include "tusb_option.h" + +#if CFG_TUD_SYSVIEW || CFG_TUH_SYSVIEW + +#include "common/tusb_sysview.h" + +#if CFG_TUSB_OS == OPT_OS_FREERTOS +#include "FreeRTOS.h" +#include "task.h" +extern const SEGGER_SYSVIEW_OS_API SYSVIEW_X_OS_TraceAPI; +#define SYSVIEW_OS_API (&SYSVIEW_X_OS_TraceAPI) +#define SYSVIEW_OS_DESC ",O=FreeRTOS" +#else +#define SYSVIEW_OS_API 0 +#define SYSVIEW_OS_DESC "" +#endif + +#ifndef SYSVIEW_APP_NAME +#define SYSVIEW_APP_NAME "TinyUSB" +#endif +#ifndef SYSVIEW_DEVICE_NAME +#define SYSVIEW_DEVICE_NAME "Cortex-M" +#endif +#ifndef SYSVIEW_RAM_BASE +#define SYSVIEW_RAM_BASE (0x20000000) +#endif + +extern unsigned int SystemCoreClock; + +// Event names, same order as tu_sysview_id_t +SEGGER_SYSVIEW_MODULE tusb_sysview_module = { + "M=TinyUSB," + "0 tud_task," + "1 usbd_edpt_xfer," + "2 dcd_edpt_xfer," + "3 tud_cdc_write_flush," + "4 tud_cdc_read," + "5 mscd_xfer_cb," + "6 tuh_task," + "7 hcd_edpt_xfer", + TU_SV_ID_COUNT, 0, NULL, NULL +}; + +static void send_sys_desc(void) { + SEGGER_SYSVIEW_SendSysDesc("N=" SYSVIEW_APP_NAME ",D=" SYSVIEW_DEVICE_NAME SYSVIEW_OS_DESC); + SEGGER_SYSVIEW_SendSysDesc("I#15=SysTick"); +} + +void tusb_sysview_init(void) { + static bool inited = false; + if (inited) { return; } + inited = true; + /* DWT cycle counter drives timestamps; enable on-target so recording + * never depends on a debugger-side enable (also needed post-mortem). */ + (*(volatile unsigned int*) 0xE000EDFC) |= (1u << 24); /* DEMCR.TRCENA */ + (*(volatile unsigned int*) 0xE0001000) |= 1u; /* DWT_CTRL.CYCCNTENA */ + SEGGER_SYSVIEW_Init(SystemCoreClock, SystemCoreClock, SYSVIEW_OS_API, send_sys_desc); + SEGGER_SYSVIEW_SetRAMBase(SYSVIEW_RAM_BASE); + SEGGER_SYSVIEW_RegisterModule(&tusb_sysview_module); + SEGGER_SYSVIEW_Start(); /* self-start: host recorders only drain */ +} + +void tusb_sysview_heap_alloc(void* ptr, unsigned size) { + static bool heap_defined = false; + if (!heap_defined) { + heap_defined = true; + SEGGER_SYSVIEW_HeapDefine(ptr, ptr, size, 0); /* base unknowable here; events still carry ptr+size */ + } + SEGGER_SYSVIEW_HeapAlloc(ptr, ptr, size); +} + +void tusb_sysview_heap_free(void* ptr) { + SEGGER_SYSVIEW_HeapFree(ptr, ptr); +} + +void tusb_sysview_stack_report(void) { +#if CFG_TUSB_OS == OPT_OS_FREERTOS && (configUSE_TRACE_FACILITY == 1) + TaskStatus_t status[8]; + UBaseType_t n = uxTaskGetSystemState(status, 8, NULL); + for (UBaseType_t i = 0; i < n; i++) { + SEGGER_SYSVIEW_TASKINFO info = {0}; + info.TaskID = (U32)(uintptr_t) status[i].xHandle; + info.sName = status[i].pcTaskName; + info.Prio = status[i].uxCurrentPriority; + info.StackBase = (U32)(uintptr_t) status[i].pxStackBase; + info.StackUsage = status[i].usStackHighWaterMark * sizeof(StackType_t); + SEGGER_SYSVIEW_SendTaskInfo(&info); + } +#endif +} + +#endif /* CFG_TUD_SYSVIEW || CFG_TUH_SYSVIEW */ +``` + +Note: `SEGGER_SYSVIEW_TASKINFO` field names must be checked against +`lib/SystemView/SYSVIEW/SEGGER_SYSVIEW.h` when compiling (V4.12 has +`TaskID/sName/Prio/StackBase/StackSize/StackUsage` — use whichever of +StackSize/StackUsage exist; `SendTaskInfo` also emits the STACK_INFO event). + +- [ ] **Step 4: Hook init and ISR in `src/tusb.c`** + +Top of file, with the other includes: `#include "common/tusb_sysview.h"`. + +In `tusb_rhport_init()` (line ~65), first statement of the function body: + +```c + tusb_sysview_init(); // no-op unless a SYSVIEW level is set +``` + +In `tusb_int_handler()` (line ~130), wrap the existing body: + +```c +void tusb_int_handler(uint8_t rhport, bool in_isr) { + TU_SYSVIEW_ISR_ENTER(); + ... existing body unchanged ... + TU_SYSVIEW_ISR_EXIT(); +} +``` + +(If the body has early `return`s, restructure to a single exit so +`TU_SYSVIEW_ISR_EXIT()` always runs — check the actual body first.) + +- [ ] **Step 5: Verify disabled build is untouched** + +Run: `cd examples/device/cdc_msc_freertos && rm -rf build-off && cmake -B build-off -DBOARD=same54_xplained -G Ninja -DCMAKE_BUILD_TYPE=MinSizeRel -DJLINK_OPTION="-USB jtrace" . && cmake --build build-off` +Expected: builds green with NO sysview sources compiled (`grep -c SYSVIEW build-off/.ninja_log` → 0). + +- [ ] **Step 6: Commit** + +```bash +git add src/common/tusb_sysview.h src/common/tusb_sysview.c src/tusb_option.h src/tusb.c +git commit -m "add leveled CFG_TUD/TUH_SYSVIEW SystemView instrumentation core" +``` + +--- + +### Task 3: Build wiring (`SYSVIEW=<level>`) in family_support.cmake + hardware parity run + +**Files:** +- Modify: `hw/bsp/family_support.cmake` (inside `family_configure_common`, adjacent to the `LOGGER` rtt block at ~line 480) +- Delete: `.claude/skills/sysview/scripts/sysview.cmake`, `.claude/skills/sysview/scripts/SEGGER_SYSVIEW_Config_TinyUSB.c` (superseded — delete in the SAME commit that lands the replacement) + +**Interfaces:** +- Consumes: Task 1 dep path, Task 2 sources/defines. +- Produces: `cmake -DSYSVIEW=<1..4|ON>` as the complete instrumentation build switch (Tasks 4–8 build with it). CMake cache vars honored: `SYSVIEW_BUFFER_SIZE` (default 16384), `SYSVIEW_RAM_BASE` (default 0x20000000). + +- [ ] **Step 1: Add the SYSVIEW block** + +In `family_configure_common`, after the LOGGER handling: + +```cmake + if (SYSVIEW) + if (SYSVIEW STREQUAL "ON") + set(SYSVIEW 4) # bare -DSYSVIEW=ON = full instrumentation + endif () + if (NOT SYSVIEW MATCHES "^[1-4]$") + message(FATAL_ERROR "SYSVIEW must be 1..4 (1=ISR 2=+usbd/usbh 3=+dcd/hcd 4=+class)") + endif () + set(SYSVIEW_SRC ${TOP}/lib/SystemView) + if (NOT EXISTS ${SYSVIEW_SRC}/SYSVIEW/SEGGER_SYSVIEW.c) + message(FATAL_ERROR "SYSVIEW needs lib/SystemView: run python3 tools/get_deps.py <family>") + endif () + if (NOT DEFINED SYSVIEW_BUFFER_SIZE) + set(SYSVIEW_BUFFER_SIZE 16384) # 4 KB overflows under bulk USB load + endif () + if (NOT DEFINED SYSVIEW_RAM_BASE) + set(SYSVIEW_RAM_BASE 0x20000000) + endif () + # vendored lib/SEGGER_RTT predates SEGGER_RTT_ConfDefaults.h; bridge it + file(WRITE ${CMAKE_BINARY_DIR}/sysview_shim/SEGGER_RTT_ConfDefaults.h "#include \"SEGGER_RTT_Conf.h\"\n") + target_sources(${TARGET} PUBLIC + ${SYSVIEW_SRC}/SYSVIEW/SEGGER_SYSVIEW.c + ${TOP}/src/common/tusb_sysview.c + ${TOP}/lib/SEGGER_RTT/RTT/SEGGER_RTT_ASM_ARMv7M.S) + if (NOT LOGGER STREQUAL "rtt") + target_sources(${TARGET} PUBLIC ${TOP}/lib/SEGGER_RTT/RTT/SEGGER_RTT.c) + endif () + target_include_directories(${TARGET} PUBLIC + ${SYSVIEW_SRC}/SEGGER ${SYSVIEW_SRC}/SYSVIEW ${SYSVIEW_SRC}/Config + ${CMAKE_BINARY_DIR}/sysview_shim + ${TOP}/lib/SEGGER_RTT/RTT ${TOP}/lib/SEGGER_RTT/Config) + # vendor sources are not expected to pass TinyUSB's -Werror set + set_source_files_properties( + ${SYSVIEW_SRC}/SYSVIEW/SEGGER_SYSVIEW.c + ${SYSVIEW_SRC}/Sample/FreeRTOSV11/SEGGER_SYSVIEW_FreeRTOS.c + ${TOP}/lib/SEGGER_RTT/RTT/SEGGER_RTT.c + TARGET_DIRECTORY ${TARGET} PROPERTIES COMPILE_OPTIONS "-w") + target_compile_definitions(${TARGET} PUBLIC + CFG_TUD_SYSVIEW=${SYSVIEW} + CFG_TUH_SYSVIEW=${SYSVIEW} + SYSVIEW_APP_NAME="${TARGET}" + SYSVIEW_DEVICE_NAME="${BOARD}" + SYSVIEW_RAM_BASE=${SYSVIEW_RAM_BASE} + SEGGER_SYSVIEW_RTT_BUFFER_SIZE=${SYSVIEW_BUFFER_SIZE} + SYSVIEW_FREERTOS_MAX_NOF_TASKS=16) + if (TARGET freertos_kernel) + target_sources(${TARGET} PUBLIC ${SYSVIEW_SRC}/Sample/FreeRTOSV11/SEGGER_SYSVIEW_FreeRTOS.c) + # freertos_config is the INTERFACE target the kernel and app both see: + # defines + include dirs there make the FreeRTOSConfig.h guarded block work + target_compile_definitions(freertos_config INTERFACE CFG_TUD_SYSVIEW=${SYSVIEW} CFG_TUH_SYSVIEW=${SYSVIEW} SYSVIEW_FREERTOS_MAX_NOF_TASKS=16) + target_include_directories(freertos_config INTERFACE + ${SYSVIEW_SRC}/SEGGER ${SYSVIEW_SRC}/SYSVIEW ${SYSVIEW_SRC}/Config + ${SYSVIEW_SRC}/Sample/FreeRTOSV11 + ${CMAKE_BINARY_DIR}/sysview_shim + ${TOP}/lib/SEGGER_RTT/RTT ${TOP}/lib/SEGGER_RTT/Config) + endif () + message(STATUS "SYSVIEW instrumentation enabled (buffer=${SYSVIEW_BUFFER_SIZE})") + endif () +``` + +NOTE: until Task 4 adds the FreeRTOSConfig.h guarded include, task events +come only after Task 4 — this task's parity run therefore ALSO adds Task 4's +samd5x_e5x FreeRTOSConfig block if built before Task 4… to avoid that +ordering trap, **run Task 4's Step 1 edit before this task's hardware step** +(tasks 3 and 4 land as separate commits but validate together). + +- [ ] **Step 2: Build instrumented** + +Run: `python3 tools/get_deps.py samd5x_e5x` (if not yet), then +`cd examples/device/cdc_msc_freertos && rm -rf build-sv && cmake -B build-sv -DBOARD=same54_xplained -G Ninja -DCMAKE_BUILD_TYPE=MinSizeRel -DSYSVIEW=4 -DJLINK_OPTION="-USB jtrace" . && cmake --build build-sv` +Expected: green; `arm-none-eabi-nm build-sv/cdc_msc_freertos.elf | grep -c SEGGER_SYSVIEW` ≥ 20. + +- [ ] **Step 3: Hardware parity run (the v1 regression test)** + +Flash + record + report exactly as the sysview SKILL documents (`ninja -C build-sv cdc_msc_freertos-jlink`, then `sysview_record.py --device ATSAME54P20 --probe jtrace --elf build-sv/cdc_msc_freertos.elf --duration-ms 8000 --out /tmp/sysview-v2 --traffic-cmd "<CDC load>"`, then `sysview_report.py /tmp/sysview-v2`). +Expected: report parity with v1 numbers — cdc ~3–4 % CPU, usbd task present, USB `ISR 96/98/99` ~34 µs p50, overflow 0 — with **zero source edits** (the win this task exists for). + +- [ ] **Step 4: Commit (including script deletions)** + +```bash +git rm .claude/skills/sysview/scripts/sysview.cmake .claude/skills/sysview/scripts/SEGGER_SYSVIEW_Config_TinyUSB.c +git add hw/bsp/family_support.cmake +git commit -m "wire leveled SYSVIEW build option; retire skill-local cmake injection" +``` + +--- + +### Task 4: FreeRTOS stack + heap metrics + +**Files:** +- Modify: `hw/bsp/samd5x_e5x/FreeRTOSConfig/FreeRTOSConfig.h` (guarded block at end of file, before the final `#endif`) +- Modify: `src/device/usbd.c` (periodic `tusb_sysview_stack_report()` from the task loop) +- Modify: `.claude/skills/sysview/scripts/sysview_report.py` (stack + heap + blocked-time output) + +**Interfaces:** +- Consumes: Task 2's `tusb_sysview_stack_report/heap_alloc/heap_free`, Task 3's `freertos_config` defines. +- Produces: `Stack Information` populated in contexts.csv; `Heap Alloc/Free` rows in events.txt (dynamic builds); reporter prints `stack` table (task, stack high-water bytes) + `heap` line (allocs/frees/net bytes) + `blocked_ms` column. + +- [ ] **Step 1: FreeRTOSConfig guarded block (samd5x_e5x)** + +At the END of the file, immediately before the closing `#endif`: + +```c +/* SEGGER SystemView instrumentation (SYSVIEW=<level> build) — must be last */ +#if (defined(CFG_TUD_SYSVIEW) && CFG_TUD_SYSVIEW) || (defined(CFG_TUH_SYSVIEW) && CFG_TUH_SYSVIEW) + #undef INCLUDE_uxTaskPriorityGet + #define INCLUDE_uxTaskPriorityGet 1 + #undef INCLUDE_xTaskGetIdleTaskHandle + #define INCLUDE_xTaskGetIdleTaskHandle 1 + extern void tusb_sysview_heap_alloc(void* ptr, unsigned size); + extern void tusb_sysview_heap_free(void* ptr); + #define traceMALLOC(pvAddress, uiSize) tusb_sysview_heap_alloc(pvAddress, uiSize) + #define traceFREE(pvAddress, uiSize) tusb_sysview_heap_free(pvAddress) + #include "SEGGER_SYSVIEW_FreeRTOS.h" +#endif +``` + +- [ ] **Step 2: Periodic stack report from the usbd task** + +In `src/device/usbd.c`, inside the `tud_task_ext` event loop (after an event +is dequeued and processed): + +```c +#if CFG_TUD_SYSVIEW + { static uint16_t sv_cnt = 0; + if (0 == (++sv_cnt & 0x3FFu)) { tusb_sysview_stack_report(); } } +#endif +``` + +(`#include "common/tusb_sysview.h"` with usbd.c's other includes.) + +- [ ] **Step 3: Reporter — stack, heap, blocked time** + +In `sysview_report.py`: (a) contexts table gains `blk_ms` column from the +existing `Total Blocked Time` CSV field (`parse_time_s`, already imported); +(b) after the ISR table, if events.txt present, parse stack/heap rows. +Format discovery is part of this step: record once (Step 4), inspect the +literal `event` names in events.txt (`Stack Info`, `Heap Alloc`/`Heap +Free` are expected but MUST be read from the actual export), then lock the +parser to what is observed. Output shape: + +``` +stack high-water bytes_used +usbd 372 +cdc 288 +... +heap: allocs=N frees=M net_bytes=K (or "heap: no events (static allocation build)") +``` + +- [ ] **Step 4: Hardware validation (stack; heap absent)** + +Rebuild `build-sv`, flash, record 8 s under CDC load, report. +Expected: stack table lists blinky/usbd/cdc/IDLE with plausible high-water +bytes; heap line prints the static-allocation notice; `blk_ms` populated +(cdc mostly blocked); overflow 0; task/ISR numbers unchanged from Task 3. + +- [ ] **Step 5: Heap-mapping validation (temporary dynamic build)** + +Temporarily set `configSUPPORT_DYNAMIC_ALLOCATION 1` in the samd5x_e5x +FreeRTOSConfig.h, rebuild, flash, record boot (queues/semaphores allocate +at init), report. Expected: `heap: allocs>0`. **Revert the temporary edit** +(`git checkout hw/bsp/samd5x_e5x/FreeRTOSConfig/FreeRTOSConfig.h` would +also revert Step 1 — instead flip the single line back by editing), rebuild, +reflash pristine-instrumented. + +- [ ] **Step 6: Commit** + +```bash +git add hw/bsp/samd5x_e5x/FreeRTOSConfig/FreeRTOSConfig.h src/device/usbd.c .claude/skills/sysview/scripts/sysview_report.py +git commit -m "sysview: FreeRTOS stack high-water + heap trace mapping + reporter columns" +``` + +--- + +### Task 5: Per-function timing (hot functions) + markers/Printf/DisableEvents recipes + +**Files:** +- Modify: `src/device/usbd.c` (`TUD_SYSVIEW_CALL/RET` around event processing + `usbd_edpt_xfer`), `src/class/cdc/cdc_device.c` (`tud_cdc_n_write_flush`, `tud_cdc_n_read`), `src/class/msc/msc_device.c` (`mscd_xfer_cb`), `src/host/usbh.c` (`tuh_task_ext` event processing, guarded `#if CFG_TUH_ENABLED`) +- Modify: `.claude/skills/sysview/scripts/sysview_report.py` (function-timing table + marker table) + +**Interfaces:** +- Consumes: Task 2 macros/ids. +- Produces: reporter `function` table: name, n, p50/p99/max µs; `marker` table (same shape, from `Mark Start/Stop` pairs). + +- [ ] **Step 1: Instrument call sites (leveled)** + +Pattern (every site pairs CALL with RET on ALL exit paths — restructure to +single-exit where needed; example for `usbd_edpt_xfer` in usbd.c, level USB, +with the dcd boundary inside it at level PORT): + +```c +bool usbd_edpt_xfer(...) { + TUD_SYSVIEW_CALL(CFG_TUSB_SYSVIEW_LEVEL_USB, TU_SV_ID_USBD_XFER); + ... + TUD_SYSVIEW_CALL(CFG_TUSB_SYSVIEW_LEVEL_PORT, TU_SV_ID_DCD_XFER); + bool ok = dcd_edpt_xfer(rhport, ep_addr, buffer, total_bytes); + TUD_SYSVIEW_RET(CFG_TUSB_SYSVIEW_LEVEL_PORT, TU_SV_ID_DCD_XFER); + ... + TUD_SYSVIEW_RET(CFG_TUSB_SYSVIEW_LEVEL_USB, TU_SV_ID_USBD_XFER); + return ret; +} +``` + +Sites and their levels (dcd/hcd instrumentation lives at the usbd/usbh call +sites — portable drivers are never edited): + +| site | macro/level | id | +|---|---|---| +| `tud_task_ext` per-event processing (around the event switch, NOT the blocking queue wait) | `TUD` / `LEVEL_USB` | `TU_SV_ID_TUD_TASK` | +| `usbd_edpt_xfer` body | `TUD` / `LEVEL_USB` | `TU_SV_ID_USBD_XFER` | +| the `dcd_edpt_xfer(...)` call inside it | `TUD` / `LEVEL_PORT` | `TU_SV_ID_DCD_XFER` | +| `tud_cdc_n_write_flush` | `TUD` / `LEVEL_CLASS` | `TU_SV_ID_CDC_FLUSH` | +| `tud_cdc_n_read` | `TUD` / `LEVEL_CLASS` | `TU_SV_ID_CDC_READ` | +| `mscd_xfer_cb` | `TUD` / `LEVEL_CLASS` | `TU_SV_ID_MSC_XFER` | +| `tuh_task_ext` per-event processing | `TUH` / `LEVEL_USB` | `TU_SV_ID_TUH_TASK` | +| the `hcd_edpt_xfer(...)` call in usbh.c | `TUH` / `LEVEL_PORT` | `TU_SV_ID_HCD_XFER` | + +Each file adds `#include "common/tusb_sysview.h"`. + +- [ ] **Step 1b: Level-gating compile check** + +Build the example twice: `-DSYSVIEW=1` and `-DSYSVIEW=4`. +Expected: both green; the level-1 ELF contains NO `_TU_SV_RECORD`-emitting +sites (`arm-none-eabi-nm build-sv1/*.elf | grep -c RecordVoid` → 0 uses via +module events; simplest observable: level-1 events.txt from a short record +shows ISR events but no `tud_task` module events, level-4 shows both). + +- [ ] **Step 2: Reporter — function + marker tables** + +Discovery step (same protocol as Task 4 Step 3): record once, read how +module events and `RecordEndCall` render in events.txt (named per the module +description, e.g. `tud_task` + a return/end event), then implement pairing: +per id, duration = end_ts − start_ts within the same context; emit +n/p50/p99/max µs. Markers: pair `Mark Start`/`Mark Stop` per marker id the +same way. If module events export as raw ids (decode risk noted in spec), +fall back to mapping `eventint` (= module EventOffset + id, offset visible +in the `Module Description` row) → name table copied from +`tu_sysview_id_t`. + +- [ ] **Step 3: Hardware validation** + +Rebuild, flash, record 8 s under CDC load + msc mount, report. +Expected: function table shows `tud_task`, `usbd_edpt_xfer`, +`tud_cdc_write_flush`, `tud_cdc_read` with n>100 and p50 in single-digit to +tens of µs; overflow 0 at 16 KB (if nonzero, that is Task 5's data for the +DisableEvents recipe — record the working mask). + +- [ ] **Step 4: Overflow A/B + DisableEvents mask (deterministic, 4 KB)** + +Build with `-DSYSVIEW_BUFFER_SIZE=4096`; record under CDC load → +expected: overflow > 0 (reliable at 4 KB, per v1). Then add a temporary +`SEGGER_SYSVIEW_DisableEvents(<mask>)` after init in `tusb_sysview_init`, +determining `<mask>` empirically (start: disable API/module+PLOT classes, +keep task+ISR — masks in `SEGGER_SYSVIEW.h` `SYSVIEW_EVTMASK_*`) until the +same 4 KB run reports overflow 0. Record the final mask as a documented +`SYSVIEW_EVENT_MASK`-style define in tusb_sysview.c (compile-time optional: +`#ifdef SYSVIEW_DISABLE_EVENTS ... #endif`), remove the temporary line. + +- [ ] **Step 5: Markers + PrintfHost proof** + +Temporarily add to `examples/device/cdc_msc_freertos/src/main.c` cdc_task: +`SEGGER_SYSVIEW_MarkStart(0)/MarkStop(0)` around the echo loop body and one +`SEGGER_SYSVIEW_PrintfHost("cdc rx %u", count);` — rebuild, record with +`--export-terminal`, report. Expected: marker table row with n>0; +terminal.csv contains the printf lines. **Revert the example edit.** + +- [ ] **Step 6: Commit** + +```bash +git add src/device/usbd.c src/class/cdc/cdc_device.c src/class/msc/msc_device.c src/host/usbh.c src/common/tusb_sysview.c .claude/skills/sysview/scripts/sysview_report.py +git commit -m "sysview: per-function timing instrumentation + marker/function report tables" +``` + +--- + +### Task 6: Post-mortem capture (`SYSVIEW_POST_MORTEM=1` + `sysview_dump.py`) + +**Files:** +- Modify: `hw/bsp/family_support.cmake` (extend Task 3 block) +- Create: `.claude/skills/sysview/scripts/sysview_dump.py` + +**Interfaces:** +- Consumes: Task 3 build block; v1 `sysview_record.py --from-raw` (unchanged). +- Produces: `sysview_dump.py --device X --probe Y --elf Z --out DIR [--resume]` → `DIR/capture.SVDat` (raw, decodable via `--from-raw`). + +- [ ] **Step 1: CMake option** + +Inside the Task 3 `if (SYSVIEW)` block: + +```cmake + if (SYSVIEW_POST_MORTEM) + target_compile_definitions(${TARGET} PUBLIC SEGGER_SYSVIEW_POST_MORTEM_MODE=1) + # SEGGER_SYSVIEW_SYNC_PERIOD_SHIFT stays at its source default (8) + endif () +``` + +- [ ] **Step 2: Write `sysview_dump.py`** + +Flow (reuse `resolve_probe`/`rtt_cb_from_elf` patterns from +`sysview_record.py` — copy the two helpers, do not import): +1. Resolve probe serial; RTT control block address from `--elf`. +2. One JLinkExe session (`-CommandFile`, NO reset commands — attach, `h`): + `mem32 <cb+0x18+aUp[1] offsets>` to read the channel-1 descriptor + (pBuffer, SizeOfBuffer, WrOff, RdOff — offsets: aUp[] starts at cb+0x18, + each SEGGER_RTT_BUFFER_UP is 6 words: sName,pBuffer,SizeOfBuffer,WrOff, + RdOff,Flags → channel 1 descriptor at cb+0x18+24), verify magic + "SEGGER RTT" at cb first (abort if absent), then + `savebin <tmp>/ring.bin, <pBuffer>, <SizeOfBuffer>`. +3. Linearize BOTH candidates (spec's open question): + A = ring[WrOff:] + ring[:WrOff]; B = ring[WrOff:] + ring[:RdOff]. + Write A as `capture.SVDat`, B as `capture_rdoff.SVDat` when RdOff differs. +4. Default leaves the core halted (mid-autopsy); `--resume` sends `g`. +Print WrOff/RdOff/size and which files were written. + +- [ ] **Step 3: Hardware validation (split-point discovery)** + +Build `-DSYSVIEW=4 -DSYSVIEW_POST_MORTEM=1`, flash, run CDC load ~10 s, +run `sysview_dump.py` mid-load (its halt = the simulated crash), decode +candidate A via `sysview_record.py --from-raw … --no-events`, report. +Expected: contexts table with cdc/usbd/ISRs and sane numbers (leading +pre-sync garbage tolerated). If A fails/garbles wholesale, decode B. +**Record the winner in the dump script (drop the loser) and in the SKILL +(Task 8).** Reflash pristine (non-post-mortem) build after. + +- [ ] **Step 4: Commit** + +```bash +git add hw/bsp/family_support.cmake .claude/skills/sysview/scripts/sysview_dump.py +git commit -m "sysview: post-mortem mode build option + halted-target dump script" +``` + +--- + +### Task 7: OpenOCD capture route (non-SEGGER probes) + +**Files:** +- No code — validation + recipe evidence for Task 8's SKILL text. + +**Interfaces:** +- Consumes: existing OpenOCD rtt recipes (target-debug SKILL), `--from-raw`. +- Produces: verified command sequence for the SKILL's OpenOCD section. + +- [ ] **Step 1: Build for stm32h743nucleo** + +`python3 tools/get_deps.py stm32h7`; build `cdc_msc_freertos` +`-DBOARD=stm32h743nucleo -DSYSVIEW=4` (add the Task 4 guarded block to +`hw/bsp/stm32h7/FreeRTOSConfig/FreeRTOSConfig.h` — same text, own commit). + +- [ ] **Step 2: Rig validation (ci.lan, lock held)** + +Hold `board_lock.py hold stm32h743nucleo --reason "sysview openocd validation"`. +Flash via the board's openocd target. Capture raw channel 1: + +```bash +openocd -f interface/stlink-dap.cfg -c 'adapter serial <uid from tinyusb.json>' -f target/stm32h7x.cfg \ + -c init -c 'rtt setup 0x24000000 0x80000 "SEGGER RTT"' -c 'rtt start' \ + -c 'rtt server start 19333 1' & +timeout 10 nc localhost 19333 > /tmp/sysview-h7/capture.SVDat # drive CDC load meanwhile +``` + +(RAM base/size from the h743 linker script; rtt re-`start` after reflash.) +Decode: `sysview_record.py --from-raw /tmp/sysview-h7/capture.SVDat --out /tmp/sysview-h7 --no-events` (decode runs on htpc — copy the file over) → report. +Expected: contexts/ISR tables populate; note overflow count (OpenOCD polls — +nonzero overflow under burst is the documented caveat, not a failure). +Release the lock; reflash pristine. + +- [ ] **Step 3: Commit (the h7 config block from Step 1)** + +```bash +git add hw/bsp/stm32h7/FreeRTOSConfig/FreeRTOSConfig.h +git commit -m "sysview: enable FreeRTOS trace hooks for stm32h7 family" +``` + +--- + +### Task 8: `--json` report, SKILL.md rewrite, zero-cost gate, GREEN verification + +**Files:** +- Modify: `.claude/skills/sysview/scripts/sysview_report.py` (`--json`) +- Modify: `.claude/skills/sysview/SKILL.md` (build = `-DSYSVIEW=<level>` with the level table, no source edits, function/stack/heap tables, post-mortem, OpenOCD route, DisableEvents mask, updated warnings) +- Modify: `.claude/skills/target-debug/SKILL.md` + `.claude/agents/target-debugger.md` (one-line row updates: sysview needs no source edits now; post-mortem answers "what ran before the crash") + +**Interfaces:** +- Consumes: everything above. +- Produces: `sysview_report.py <dir> --json` → single JSON object `{contexts:[{name,activations,cpu_pct,total_ms,min_us,avg_us,max_us,blocked_ms}], isr:[…], ready_run:[…], functions:[…], markers:[…], stack:[…], heap:{allocs,frees,net_bytes}|null, overflow:N}` printed to stdout (tables suppressed) — the future HIL/PR-comment payload. + +- [ ] **Step 1: Implement `--json`** (serialize the already-computed tables; `json.dumps(..., indent=1)`; every numeric field a number, not a formatted string). + +- [ ] **Step 2: Zero-cost gate** — code-size skill single-example compare: +`python3 tools/metrics_compare_base.py -e device/cdc_msc_freertos -b same54_xplained` (per code-size SKILL) comparing branch (SYSVIEW **off**) vs master. +Expected: 0 flash / 0 RAM delta. Nonzero = a hook leaked outside `#if` — fix before proceeding. + +- [ ] **Step 3: Build sweep for touched families** — full example set for +`same54_xplained` and `stm32h743nucleo`, both WITHOUT `SYSVIEW` (default path unchanged) and WITH `-DSYSVIEW=4`. Expected: all green. Plus `pre-commit run --all-files`. + +- [ ] **Step 4: Rewrite SKILL.md** — Iron-Law order: the doc is rewritten only now, AFTER every recipe above was hardware-proven; carry over v1's still-true gotchas (recorder internals, `/tmp/sv-*`, license dialog), replace the instrument section ("build with `-DSYSVIEW=<level>` — no source edits; level table 1=ISR 2=+usbd/usbh 3=+dcd/hcd 4=+class"), add function/stack/heap reading guide, post-mortem + OpenOCD sections with the exact validated commands, `--json`. + +- [ ] **Step 5: GREEN verification (writing-skills)** — fresh subagent, dry-run scenario: "find the hot TinyUSB functions and check task stacks on same54_xplained during CDC jitter, headless; also: what would you do after a hardfault, and on an ST-Link-only board?" Expected: correct `-DSYSVIEW=<level>` flow with zero source edits (picking level 3+ to see dcd timing), report/`--json` for numbers, post-mortem dump for the crash, OpenOCD raw route for ST-Link. REFACTOR SKILL.md on any gap and re-verify. + +- [ ] **Step 6: Final commit + bookkeeping** + +```bash +git add .claude/skills/sysview .claude/skills/target-debug/SKILL.md .claude/agents/target-debugger.md +git commit -m "sysview v2: json report, first-class build docs, post-mortem + openocd sections" +``` +Update the session memory (`sysview_skill.md`): v2 landed on branch, still unpushed. + +--- + +## Self-review notes + +- Spec coverage: dependency (T1), in-tree first-class + zero-edit (T2/T3), hot functions (T5), stack+heap (T4), reportable/json (T4/T5/T8), post-mortem + split-point discovery (T6), OpenOCD answer validated (T7), quick wins markers/Printf/DisableEvents deterministic A/B (T5), out-of-scope list untouched. +- Known discovery points are explicit steps with acceptance criteria (events.txt naming for module/stack/heap rows; post-mortem split point; DisableEvents mask) — not placeholders. +- Type/name consistency: `tu_sysview_id_t`, `tusb_sysview_module`, `TUD/TUH_SYSVIEW_CALL/RET(level, id)`, `tusb_sysview_stack_report`, `tusb_sysview_heap_alloc/free` used identically across T2/T4/T5. diff --git a/docs/superpowers/plans/2026-07-29-sysview-hil-report.md b/docs/superpowers/plans/2026-07-29-sysview-hil-report.md new file mode 100644 index 000000000..e9b349e62 --- /dev/null +++ b/docs/superpowers/plans/2026-07-29-sysview-hil-report.md @@ -0,0 +1,793 @@ +# SystemView HIL Performance Report Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Per-PR performance report from HIL hardware — SystemView captures on sysview-flagged boards, delta vs a base-branch baseline artifact, posted into the existing HIL sticky PR comment with mermaid charts and a legend. Spec: `docs/superpowers/specs/2026-07-29-sysview-hil-report-design.md`. + +**Architecture:** Two reporter bug-fixes in `sysview_report.py` (data-quality prerequisites), then a new standalone `test/hil/sysview_ci.py` with a pure `report` subcommand (TDD, fixture JSONs → markdown) and a rig-side `capture` subcommand (reuses `hil_flash.py` / `hil_lock.py`), then workflow wiring: capture+artifact in `build.yml` HIL jobs, an ubuntu compare job, and one append in `pr_comment.yml`'s existing hil-comment job. + +**Tech Stack:** Python 3.11 stdlib (`json`, `csv`, `re`, `subprocess`, `unittest`), GitHub Actions YAML, OpenOCD RTT, mermaid `xychart-beta`. + +## Global Constraints + +- Validity gates, exactly: a capture with `metrics.overflow > 0` withholds **every** duration metric from it (`– ⚠︎ overflow N`); a metric with `n < 50` is withheld (`– ⚠︎ n=<n>`); Δ only when both sides pass; `max` is **never rendered anywhere**. +- Function rows and chart bars sorted by CPU occupancy = `n × p50_us`, computed from the PR side, descending; chart capped at **8 bars**. +- RTT capture always uses `rtt polling_interval 1`; a WCH (`openocd_wch`-flashed) capture session must **never** issue `reset run`. +- Capture is non-blocking in CI: the workflow step uses `continue-on-error: true`; per-board failures land in the JSON `error` field, never as a nonzero exit for the whole step. +- Artifact names: `sysview-<display>` (per HIL rig), `sysview-comment` (rendered markdown). Baseline = the same `sysview-<display>` artifacts from the base branch via `dawidd6/action-download-artifact@v11`. +- Comment header text: `## ⚡ SystemView performance — HIL`. Legend appears exactly once, after the last board section, wording per spec §5. +- Commit messages: imperative mood, **no** `Co-Authored-By`/`Claude-Session` trailers (hathach is sole author). +- Run `pre-commit run --files <changed>` before each commit (it runs from the hook anyway; fix anything it flags). +- Python tests live in `test/hil/`, runnable as `python3 test/hil/<file>.py` (pattern: `test_hil_select.py`). They must not import hardware modules at module scope. + +## File Structure + +- Modify: `.claude/skills/sysview/scripts/sysview_report.py` (pairing guard, live window, 2 new `--json` fields) +- Modify: `.claude/skills/sysview/SKILL.md` (schema block only) +- Create: `test/hil/test_sysview_report.py` (fixture-driven tests for the reporter fixes) +- Create: `test/hil/sysview_ci.py` (`capture` + `report` subcommands) +- Create: `test/hil/test_sysview_ci.py` (report-generator tests, capture arg/selection tests) +- Modify: `test/hil/tinyusb.json` (add `"sysview"` blocks: `stm32f407disco`, `raspberry_pi_pico`) +- Modify: `.github/workflows/build.yml` (capture step + artifact in HIL matrix job; new `sysview-report` job) +- Modify: `.github/workflows/pr_comment.yml` (hil-comment job appends `sysview-comment`) + +--- + +### Task 1: Reporter fix — discard spliced CALL/RET pairs + +`sysview_report.py` currently trusts every `"Returns after X us"` duration. When a record is lost, SystemView's exporter pairs one invocation's CALL with a later invocation's RET (measured: 134 ms "max" on a function whose p99 is 10 µs). Track pairing state per function id and count discards in a new `dropped_pairs` field. + +**Files:** +- Modify: `.claude/skills/sysview/scripts/sysview_report.py` +- Create: `test/hil/test_sysview_report.py` + +**Interfaces:** +- Produces: `--json` object gains top-level `"dropped_pairs": <int>`. Task 5's wrapper embeds it verbatim; Task 3's generator does not read it (gates use `overflow`/`n`), but it must survive round-trip. + +- [ ] **Step 1: Write the failing test** + +`test/hil/test_sysview_report.py`. The helper fabricates a minimal SystemView export dir (the two files `sysview_report.py` reads: `contexts.csv`, `events.txt`) and runs the reporter as a subprocess with `--json` — same interface CI uses, no imports of skill code. + +```python +#!/usr/bin/env python3 +"""Tests for sysview_report.py's data-quality guards (spliced pairs, live window).""" +import json, os, subprocess, sys, tempfile, unittest + +REPORT = os.path.join(os.path.dirname(__file__), '..', '..', + '.claude', 'skills', 'sysview', 'scripts', 'sysview_report.py') + +EV_HEADER = ("sequencenum,timestamp,context,event,detail,timestampint," + "contextinint,contextint,contextoutint,eventint,eventoffset,eventsize,eventdata\n") +CTX_HEADER = "Name,Type,Activations,CPU Load,Total Run Time,Total Blocked Time,Min Run Time,Avg Run Time,Max Run Time\n" + +def ev(seq, ts, event, detail=""): + return f'{seq},0.0,"ctx","{event}","{detail}",{ts},0x0,0x0,0x0,0,0,0,\n' + +INIT = ev(0, 0, "Init", "Cycle Freq.: 1000000, CPU Freq.: 48000000, ID Base: 0x20000000, ID Shift: 0") + +def run_report(events_rows, contexts_rows=""): + d = tempfile.mkdtemp() + with open(os.path.join(d, 'events.txt'), 'w') as f: + f.write(EV_HEADER + INIT + "".join(events_rows)) + with open(os.path.join(d, 'contexts.csv'), 'w') as f: + f.write(CTX_HEADER + contexts_rows) + r = subprocess.run([sys.executable, REPORT, d, '--json'], + capture_output=True, text=True) + assert r.returncode == 0, r.stderr + return json.loads(r.stdout) + +class SplicedPairs(unittest.TestCase): + def test_clean_pairs_kept(self): + rows = [] + t = 1000 + for i in range(60): # 60 clean call/ret pairs, 10 us each + rows.append(ev(len(rows)+1, t, "Function #512")) + rows.append(ev(len(rows)+1, t+10, "Function #512", "Returns after 10.000 us")) + t += 1000 + j = run_report(rows) + fn = {f['name']: f for f in j['functions']} + self.assertEqual(fn['tud_task']['n'], 60) + self.assertEqual(j['dropped_pairs'], 0) + + def test_ret_after_overflow_dropped(self): + rows = [ev(1, 1000, "Function #512")] # call + rows.append(ev(2, 1500, "*** Overflow ***")) # loss marker + rows.append(ev(3, 135000, "Function #512", "Returns after 134000.000 us")) # spliced ret + j = run_report(rows) + self.assertEqual(j.get('functions', []), []) # no bogus 134 ms sample + self.assertEqual(j['dropped_pairs'], 2) # invalidated call + orphan ret + + def test_double_call_drops_first(self): + rows = [ev(1, 1000, "Function #512"), # call, ret lost + ev(2, 2000, "Function #512"), # next call + ev(3, 2010, "Function #512", "Returns after 10.000 us")] + j = run_report(rows) + fn = {f['name']: f for f in j['functions']} + self.assertEqual(fn['tud_task']['n'], 1) # only the clean pair + self.assertEqual(j['dropped_pairs'], 1) + +if __name__ == '__main__': + unittest.main() +``` + +- [ ] **Step 2: Run it, confirm it fails** + +Run: `python3 test/hil/test_sysview_report.py -v` +Expected: FAIL — `dropped_pairs` KeyError (field doesn't exist), and the 134 ms sample appears in `functions`. + +*(If the fixture instead fails on CSV column names: adjust `CTX_HEADER`/`EV_HEADER` to whatever `sysview_report.py` actually reads — check its `csv.DictReader` usage — then re-run. The fixture serves the reporter, not vice versa.)* + +- [ ] **Step 3: Implement the pairing guard** + +In `sysview_report.py`'s event loop (the `for row in csv.DictReader(...)` block): add before the loop `open_calls = {}` and `dropped_pairs = 0`. Change the two branches: + +```python + elif event == "*** Overflow ***": + overflow += 1 + # any in-flight CALL may have lost its RET (or vice versa) across the + # gap -- the exporter would splice it with a later invocation + dropped_pairs += len(open_calls) + open_calls.clear() + elif event.startswith("Function #"): + fm = func_re.match(event) + if fm: + fid = int(fm.group(1)) - TU_SV_EVENT_BASE + dm = returns_re.search(detail) + if dm is None: # a CALL + if open_calls.pop(fid, None): # previous call never returned + dropped_pairs += 1 + open_calls[fid] = True + elif open_calls.pop(fid, None) is None: + dropped_pairs += 1 # RET without its CALL: spliced + else: + func_durs.setdefault(fid, []).append( + float(dm.group(1)) * (1e-6 if dm.group(2) == "us" else 1e-3)) +``` + +Add `"dropped_pairs": dropped_pairs` to the `--json` output object and a `dropped_pairs: N` line to the text output (next to the overflow line). + +- [ ] **Step 4: Run tests, confirm pass** + +Run: `python3 test/hil/test_sysview_report.py -v` → 3 passing. + +- [ ] **Step 5: Commit** + +```bash +git add .claude/skills/sysview/scripts/sysview_report.py test/hil/test_sysview_report.py +git commit -m "sysview: discard spliced CALL/RET pairs instead of reporting bogus durations" +``` + +--- + +### Task 2: Reporter fix — statistics over the live window only + +The export contains boot-time records still in the RTT ring; a quiet capture's stats average over dead air (measured: 111 s span, 98 s idle). Split at the **last inter-event gap > 2 s**: everything after it is the live window. Percentile samples (ISR / ready→run / functions / markers) are filtered to the live window — exact. Context `cpu_pct` is rescaled as `total_ms / live_window_ms` — the numerator still includes boot activity (contexts.csv is whole-recording), a small overstatement that is identical on both sides of a delta; documented in the code comment. + +**Files:** +- Modify: `.claude/skills/sysview/scripts/sysview_report.py` +- Modify: `.claude/skills/sysview/SKILL.md` (add `dropped_pairs` + `live_window_s` to the `--json` schema block) +- Test: extend `test/hil/test_sysview_report.py` + +**Interfaces:** +- Produces: `--json` gains `"live_window_s": <float>`; `cpu_pct` semantics change to live-window share. Task 5 embeds; Task 3 renders `live_window_s` into the section header line. + +- [ ] **Step 1: Write the failing tests** (append to `test_sysview_report.py`) + +```python +class LiveWindow(unittest.TestCase): + def _mixed_rows(self): + rows = [] + t = 1000 + for i in range(55): # stale boot burst at 0-1s + rows.append(ev(len(rows)+1, t, "ISR Enter", "Runs for 100.000 us")); t += 15 + t = 99_000_000 # 98 s hole, then live window + for i in range(60): + rows.append(ev(len(rows)+1, t, "ISR Enter", "Runs for 5.000 us")); t += 100_000 + return rows + + def test_stale_samples_excluded(self): + j = run_report(self._mixed_rows()) + isr = j['isr'][0] + self.assertEqual(isr['n'], 60) # only live-window samples + self.assertEqual(isr['p50_us'], 5.0) # not polluted by the 100 us boot ISRs + + def test_live_window_reported(self): + j = run_report(self._mixed_rows()) + self.assertAlmostEqual(j['live_window_s'], 5.9, delta=0.2) + + def test_no_gap_means_full_span(self): + rows = [ev(1, 1000, "ISR Enter", "Runs for 5.000 us"), + ev(2, 500_000, "ISR Enter", "Runs for 5.000 us")] + j = run_report(rows) + self.assertAlmostEqual(j['live_window_s'], 0.5, delta=0.01) +``` + +- [ ] **Step 2: Run, confirm fail** — `python3 test/hil/test_sysview_report.py -v` (n=115, no `live_window_s`). + +- [ ] **Step 3: Implement** + +Restructure the event loop to two passes: `rows = list(csv.DictReader(...))` first; parse the timestamp frequency from the Init row's detail (`re.search(r"Cycle Freq\.: (\d+)", ...)`, default 1_000_000 if absent); compute `ts = [int(r["timestampint"]) for r in rows]`; find the last index `i` where `ts[i+1]-ts[i] > 2*freq`; `live_start = ts[i+1]` (or `ts[0]` when no such gap). `live_window_s = (ts[-1]-live_start)/freq`. In the existing sample-collection branches, `continue` for rows with timestamp `< live_start` **except** the overflow counter (count overflow over the whole stream — a pre-window overflow still voids trust). Rescale each context row: `cpu_pct = total_ms / (live_window_s*1000) * 100` when `live_window_s > 0`. Emit `"live_window_s": round(live_window_s, 2)` in `--json` and in the text header. Update the SKILL.md `--json` schema block with both new fields. + +- [ ] **Step 4: Run all reporter tests** — 6 passing. + +- [ ] **Step 5: Commit** + +```bash +git add .claude/skills/sysview/scripts/sysview_report.py .claude/skills/sysview/SKILL.md test/hil/test_sysview_report.py +git commit -m "sysview: compute statistics over the live window, report live_window_s" +``` + +--- + +### Task 3: `sysview_ci.py report` — pure markdown generator (TDD) + +**Files:** +- Create: `test/hil/sysview_ci.py` (this task: shared helpers + `report`; Task 5 adds `capture`) +- Create: `test/hil/test_sysview_ci.py` + +**Interfaces:** +- Consumes: per-board JSON files named `sysview-<board>.json`, schema per spec §3 (`board, commit, example, workload, duration_s, capture{route,poll_ms,n_events,live_window_s}, metrics{...sysview_report --json...}, error`). +- Produces: `report(base_dir, pr_dir) -> str` (empty string = post nothing) and CLI `sysview_ci.py report BASE_DIR PR_DIR -o OUT.md`. Task 6 calls the CLI. + +- [ ] **Step 1: Write the failing tests** + +`test/hil/test_sysview_ci.py` — fixtures built by helpers, properties asserted (not golden files): + +```python +#!/usr/bin/env python3 +import json, os, sys, tempfile, unittest +sys.path.insert(0, os.path.dirname(__file__)) +import sysview_ci + +def metrics(overflow=0, funcs=None, isr=None, contexts=None, stack=None): + return {"contexts": contexts or [], "isr": isr or [], "ready_run": [], + "functions": funcs or [], "markers": [], "stack": stack or [], + "heap": None, "overflow": overflow, "dropped_pairs": 0, + "live_window_s": 14.2} + +def board_json(board="stm32f407disco", err=None, **mk): + return {"board": board, "commit": "abc1234", "example": "device/cdc_msc", + "workload": "cdc_burst", "duration_s": 15, + "capture": {"route": "openocd-rtt", "poll_ms": 1, + "n_events": 100000, "live_window_s": 14.2}, + "metrics": None if err else metrics(**mk), "error": err} + +def write_set(d, *objs): + os.makedirs(d, exist_ok=True) + for o in objs: + with open(os.path.join(d, f"sysview-{o['board']}.json"), 'w') as f: + json.dump(o, f) + +F = lambda name, n, p50: {"name": name, "n": n, "p50_us": p50, "p99_us": p50*2, "max_us": p50*99} +I = lambda name, n, p50: {"name": name, "n": n, "p50_us": p50, "p99_us": p50+3, "max_us": p50*9} + +class Report(unittest.TestCase): + def setUp(self): + self.tmp = tempfile.mkdtemp() + self.base, self.pr = os.path.join(self.tmp,'b'), os.path.join(self.tmp,'p') + + def go(self, base_objs, pr_objs): + write_set(self.base, *base_objs); write_set(self.pr, *pr_objs) + return sysview_ci.report(self.base, self.pr) + + def test_empty_intersection_returns_empty(self): + self.assertEqual(self.go([board_json(board="a")], [board_json(board="b")]), "") + self.assertEqual(sysview_ci.report(self.base, os.path.join(self.tmp,'nope')), "") + + def test_delta_and_occupancy_order(self): + b = board_json(funcs=[F("dcd_edpt_xfer",200,5.9), F("tud_task",22000,7.4)], + isr=[I("ISR 83",24000,6.7)]) + p = board_json(funcs=[F("dcd_edpt_xfer",200,6.1), F("tud_task",22000,7.4)], + isr=[I("ISR 83",24000,6.8)]) + md = self.go([b],[p]) + self.assertIn("## ⚡ SystemView performance — HIL", md) + self.assertIn("+3.4%", md) # dcd 5.9 -> 6.1 + self.assertLess(md.index("tud_task"), md.index("dcd_edpt_xfer")) # occupancy order + self.assertIn("```mermaid", md) + self.assertEqual(md.count("Legend"), 1) + self.assertNotIn("max", md.split("Legend")[0]) # max never rendered + + def test_overflow_gates_all_durations(self): + p = board_json(overflow=3, funcs=[F("tud_task",22000,7.4)]) + md = self.go([board_json(funcs=[F("tud_task",22000,7.4)])],[p]) + self.assertIn("⚠︎ overflow 3", md) + self.assertNotIn("+", md.split("tud_task")[1].split("\n")[0]) # no delta on gated row + + def test_low_n_gates_metric(self): + p = board_json(funcs=[F("mscd_xfer_cb",9,13.0)]) + md = self.go([board_json(funcs=[F("mscd_xfer_cb",9,13.0)])],[p]) + self.assertIn("⚠︎ n=9", md) + + def test_capture_failed_board(self): + md = self.go([board_json()],[board_json(err="flash failed: rc=1")]) + self.assertIn("capture failed: flash failed: rc=1", md) + + def test_missing_baseline_absolute(self): + md = sysview_ci.report(os.path.join(self.tmp,'nobase'), self.pr) or \ + self.go([], [board_json(funcs=[F("tud_task",22000,7.4)])]) + self.assertIn("new", md) # Δ column shows new + self.assertIn("7.4", md) + + def test_chart_capped_at_8(self): + funcs=[F(f"fn{i}", 1000-i, 5.0+i) for i in range(10)] + md = self.go([board_json(funcs=funcs)],[board_json(funcs=funcs)]) + chart = md.split("```mermaid")[1].split("```")[0] + self.assertLessEqual(len(chart.split("x-axis")[1].split("]")[0].split(",")), 8) + +if __name__ == '__main__': + unittest.main() +``` + +- [ ] **Step 2: Run, confirm fail** — `python3 test/hil/test_sysview_ci.py -v` (ImportError: no sysview_ci). + +- [ ] **Step 3: Implement `report` in `test/hil/sysview_ci.py`** + +```python +#!/usr/bin/env python3 +"""SystemView CI: per-board capture on the HIL rig + PR performance report. + +report: pure -- two directories of sysview-<board>.json in, markdown out. +capture (Task 5): flash SYSVIEW build, drive workload, RTT-capture, decode. +Spec: docs/superpowers/specs/2026-07-29-sysview-hil-report-design.md +""" +import argparse, glob, json, os, re, sys + +GATE_MIN_N = 50 +CHART_MAX_BARS = 8 +HEADER = "## ⚡ SystemView performance — HIL" +LEGEND = ("<sub>**Legend** — **p50/p99**: median / 99th-percentile duration over all calls " + "in the capture window (µs; p50 = typical cost, p99 = tail latency). **Δ**: " + "change vs base branch; **−** is faster/better. **pt**: percentage points. " + "**CPU load**: context's share of the live capture window. **stack high-water**: " + "peak bytes of stack used. Function rows/bars are ordered by CPU occupancy " + "(calls × p50) in the PR capture, hottest first. **– ⚠︎**: metric withheld — RTT " + "ring overflowed (`overflow N`) or too few samples (`n<50`); withheld beats wrong. " + "`max` is never shown: under overflow it splices two invocations into one bogus " + "duration. Capture: OpenOCD RTT @1 ms poll — p50/p99 match the J-Link recorder " + "within ~1%.</sub>") + +def load_set(d): + out = {} + for p in glob.glob(os.path.join(d, "sysview-*.json")): + try: + j = json.load(open(p)) + out[j["board"]] = j + except (OSError, ValueError, KeyError): + continue + return out + +def gate(side, name_metrics): + """None if usable, else the withhold reason string.""" + if side is None: + return "new" + if side["metrics"] is None: + return "failed" + if side["metrics"].get("overflow", 0) > 0: + return f"overflow {side['metrics']['overflow']}" + if name_metrics is not None and name_metrics.get("n", 0) < GATE_MIN_N: + return f"n={name_metrics.get('n', 0)}" + return None + +def by_name(mlist): + return {m["name"]: m for m in (mlist or [])} + +def fmt_delta(b, p): + if b is None or p is None or b == 0: + return "new" if b is None else "—" + d = (p - b) / b * 100 + if abs(d) < 1.0: + return "—" + mark = " ✅" if d < 0 else "" + bold = ("**", "**") if abs(d) >= 5 else ("", "") + return f"{bold[0]}{d:+.1f}%{bold[1]}{mark}" + +def metric_cell(side_json, m, fmt): + reason = gate(side_json, m) + if reason in (None,): + return fmt(m), None + if reason == "new": + return None, "new" + if reason == "failed": + return None, "failed" + return f"– ⚠︎ {reason}", reason + +def row(label, bmap, pmap, name, base_j, pr_j, key="p50_us", unit=" µs"): + bm, pm = bmap.get(name), pmap.get(name) + if pm is None and bm is None: + return None + fmt = lambda m: f"{m[key]:.1f}{unit}" if m else "—" + bcell, bgate = metric_cell(base_j, bm, fmt) if bm else ("—", None) + pcell, pgate = metric_cell(pr_j, pm, fmt) if pm else ("—", None) + if bgate or pgate: + delta = "—" + bcell, pcell = bcell or "—", pcell or "—" + else: + delta = fmt_delta(bm and bm[key], pm and pm[key]) + return f"| {label} | {bcell} | {pcell} | {delta} |" + +def board_section(name, base_j, pr_j): + lines = [f"### {name}", ""] + if pr_j.get("error"): + return "\n".join(lines + [f"capture failed: {pr_j['error']}", ""]) + pm_all = pr_j["metrics"] + bm_all = (base_j or {}).get("metrics") or {} + lines += ["| metric | base | PR | Δ |", "|---|---:|---:|---:|"] + bisr, pisr = by_name(bm_all.get("isr")), by_name(pm_all.get("isr")) + for iname, pm in pisr.items(): + bm = bisr.get(iname) + preason = gate(pr_j, pm) + breason = "new" if bm is None else gate(base_j, bm) + def _cell(m, reason): + if reason is None: + return f"{m['p50_us']:.1f} / {m['p99_us']:.1f} µs" + return "—" if reason == "new" else f"– ⚠︎ {reason}" + if preason or (breason not in (None, "new")): + delta = "—" + elif breason == "new": + delta = "new" + else: + delta = (f"{fmt_delta(bm['p50_us'], pm['p50_us'])} / " + f"{fmt_delta(bm['p99_us'], pm['p99_us'])}") + lines.append(f"| {iname} p50 / p99 | {_cell(bm, breason)} | " + f"{_cell(pm, preason)} | {delta} |") + bfn, pfn = by_name(bm_all.get("functions")), by_name(pm_all.get("functions")) + order = sorted(pfn, key=lambda k: pfn[k]["n"] * pfn[k]["p50_us"], reverse=True) + for fname in order: + r = row(f"`{fname}` p50", bfn, pfn, fname, base_j, pr_j) + if r: lines.append(r) + bctx, pctx = by_name(bm_all.get("contexts")), by_name(pm_all.get("contexts")) + for cname, c in pctx.items(): + if cname.lower() in ("usbd", "usbh"): + b = bctx.get(cname) + d = "new" if base_j is None else ( + f"{c['cpu_pct']-b['cpu_pct']:+.1f} pt" if b and abs(c['cpu_pct']-b['cpu_pct']) >= 0.1 else "—") + lines.append(f"| CPU load ({cname} ctx) | " + f"{b['cpu_pct']:.1f} % | {c['cpu_pct']:.1f} % | {d} |" if b else + f"| CPU load ({cname} ctx) | — | {c['cpu_pct']:.1f} % | {d} |") + bst, pst = by_name(bm_all.get("stack")), by_name(pm_all.get("stack")) + for sname, sm in pst.items(): + b = bst.get(sname) + d = "—" if (b and b["bytes_used"] == sm["bytes_used"]) else ("new" if not b else + f"{sm['bytes_used']-b['bytes_used']:+d} B") + lines.append(f"| `{sname}` stack high-water | " + f"{b['bytes_used'] if b else '—'} B | {sm['bytes_used']} B | {d} |") + # chart: occupancy order, capped, only ungated + if order and gate(pr_j, None) is None: + chart = order[:CHART_MAX_BARS] + short = [re.sub(r'^(tud_|tuh_|dcd_|hcd_)', '', c) for c in chart] + pv = [f"{pfn[c]['p50_us']:.1f}" for c in chart] + bv = [f"{bfn[c]['p50_us']:.1f}" if c in bfn else "0" for c in chart] + ymax = max(float(v) for v in pv + bv) * 1.3 + lines += ["", "```mermaid", "xychart-beta", + ' title "hot functions p50 µs (base vs PR)"', + f" x-axis [{', '.join(short)}]", + f' y-axis "µs" 0 --> {ymax:.0f}', + f" bar [{', '.join(bv)}]", + f" bar [{', '.join(pv)}]", "```"] + return "\n".join(lines) + "\n" + +def report(base_dir, pr_dir): + pr = load_set(pr_dir) + if not pr: + return "" + base = load_set(base_dir) + boards = sorted(pr) + any_pr = next(iter(pr.values())) + head = (f"*{any_pr['example']} `SYSVIEW=4`, workload `{any_pr['workload']}` " + f"{any_pr['duration_s']} s, OpenOCD rtt @1 ms · " + f"base `{next(iter(base.values()))['commit'] if base else '(none)'}` → " + f"PR `{any_pr['commit']}`*") + parts = [HEADER, "", head, ""] + for b in boards: + parts.append(board_section(b, base.get(b), pr[b])) + parts.append(LEGEND) + return "\n".join(parts) + "\n" + +def main(): + ap = argparse.ArgumentParser() + sub = ap.add_subparsers(dest="cmd", required=True) + rp = sub.add_parser("report") + rp.add_argument("base_dir"); rp.add_argument("pr_dir") + rp.add_argument("-o", "--out", default="sysview_report.md") + args = ap.parse_args() + if args.cmd == "report": + md = report(args.base_dir, args.pr_dir) + with open(args.out, "w") as f: + f.write(md) + print(f"{'empty (no captures)' if not md else args.out}") + +if __name__ == "__main__": + main() +``` + +- [ ] **Step 4: Run, iterate until green** — `python3 test/hil/test_sysview_ci.py -v`. Adjust test/implementation mismatches by fixing the *implementation* unless the test contradicts the spec. + +- [ ] **Step 5: Eyeball one render** — `python3 - <<'EOF'` (build the delta fixture, print `report()`), paste output into any markdown previewer; check the mermaid block parses (mermaid.live). + +- [ ] **Step 6: Commit** + +```bash +git add test/hil/sysview_ci.py test/hil/test_sysview_ci.py +git commit -m "hil: sysview report generator - gated deltas, occupancy-sorted chart, legend" +``` + +--- + +### Task 4: Flag two boards in `tinyusb.json` + +**Files:** Modify: `test/hil/tinyusb.json` + +- [ ] **Step 1: Add the blocks** — to `stm32f407disco` (jlink-flashed, so `ocd_args` required) and `raspberry_pi_pico` (openocd-flashed, flasher args reused): + +```json +"sysview": {"example": "device/cdc_msc", "workload": "cdc_burst", "duration_s": 15, + "ocd_args": "-f interface/jlink.cfg -c \"transport select swd\" -f target/stm32f4x.cfg"} +``` +```json +"sysview": {"example": "device/cdc_msc", "workload": "cdc_burst", "duration_s": 15} +``` + +- [ ] **Step 2: Validate** — `python3 -c "import json; json.load(open('test/hil/tinyusb.json'))"` and `python3 test/hil/test_hil_select.py` (selection must be unaffected by the new key). + +- [ ] **Step 3: Commit** — `git commit -m "hil: flag stm32f407disco and raspberry_pi_pico for sysview capture"` + +--- + +### Task 5: `sysview_ci.py capture` + +**Files:** Modify: `test/hil/sysview_ci.py`, `test/hil/test_sysview_ci.py` + +**Interfaces:** +- Consumes: `hil_flash.flash_<name>(board, firmware)` (firmware = extension-less path), `hil_flash.find_firmware(variant, example)`, `hil_lock.acquire_board_lock` (**check its exact call convention in `hil_test.py` first** — `grep -n acquire_board_lock test/hil/hil_test.py` — and mirror it), skill scripts `sysview_record.py --from-raw` / `sysview_report.py --json`, `rtt_cb_from_elf` imported from `sysview_record.py`. +- Produces: `sysview-<board>.json` files per spec §3 in `--out` dir. Exit code 0 unless *zero* boards were even attempted due to bad args. + +- [ ] **Step 1: Write failing tests for the pure parts** (append to `test_sysview_ci.py`) + +```python +class CaptureSelection(unittest.TestCase): + CFG = {"boards": [ + {"name": "a", "uid": "U1", "flasher": {"name": "openocd", "uid": "P1", + "args": "-f interface/x.cfg -f target/y.cfg"}, + "sysview": {"example": "device/cdc_msc", "workload": "cdc_burst", "duration_s": 15}}, + {"name": "b", "uid": "U2", "flasher": {"name": "jlink", "uid": "P2", "args": "-device X"}, + "sysview": {"example": "device/cdc_msc", "workload": "idle", "duration_s": 10, + "ocd_args": "-f interface/jlink.cfg -f target/z.cfg"}}, + {"name": "c", "uid": "U3", "flasher": {"name": "openocd", "uid": "P3", "args": ""}}]} + + def test_flagged_only(self): + self.assertEqual([b["name"] for b in sysview_ci.select_boards(self.CFG, [])], ["a", "b"]) + + def test_intersection_with_board_args(self): + self.assertEqual([b["name"] for b in sysview_ci.select_boards(self.CFG, ["b", "c"])], ["b"]) + + def test_ocd_args_resolution(self): + a, b = sysview_ci.select_boards(self.CFG, []) + self.assertIn("target/y.cfg", sysview_ci.capture_ocd_args(a)) # falls back to flasher + self.assertIn("target/z.cfg", sysview_ci.capture_ocd_args(b)) # explicit override + with self.assertRaises(ValueError): # jlink flasher, no override + sysview_ci.capture_ocd_args({"flasher": {"name": "jlink", "args": "-device X"}, + "sysview": {}}) + + def test_wrapper_error_shape(self): + j = sysview_ci.board_result(self.CFG["boards"][0], "abc1234", error="flash failed: rc=1") + self.assertEqual(j["error"], "flash failed: rc=1"); self.assertIsNone(j["metrics"]) +``` + +- [ ] **Step 2: Run, confirm fail.** + +- [ ] **Step 3: Implement.** Add to `sysview_ci.py` (below `report`, above `main`), with `capture` kept import-safe (hardware imports inside functions): + +```python +# ---------------------------------------------------------------- capture +OPENOCD_FAMILY = ("openocd", "openocd_wch", "openocd_adi") + +def select_boards(cfg, board_args): + picked = [b for b in cfg["boards"] if "sysview" in b] + if board_args: + picked = [b for b in picked if b["name"] in set(board_args)] + return picked + +def capture_ocd_args(board): + args = board["sysview"].get("ocd_args") or ( + board["flasher"]["args"] if board["flasher"]["name"] in OPENOCD_FAMILY else None) + if not args: + raise ValueError(f"{board.get('name')}: non-openocd flasher and no sysview.ocd_args") + return [a.strip('"') for a in re.findall(r'"[^"]*"|\S+', args)] + +def board_result(board, commit, metrics=None, capture_info=None, error=None): + sv = board["sysview"] + return {"board": board["name"], "commit": commit, + "example": sv["example"], "workload": sv["workload"], + "duration_s": sv["duration_s"], + "capture": capture_info or {}, "metrics": metrics, "error": error} +``` + +Then the hardware path (single function per concern, all subprocess-based, mirroring the session-proven harness): + +```python +def _workload_cdc_burst(node, duration_s): + import serial, time + s = serial.Serial(node, 115200, timeout=0.02) + end = time.monotonic() + duration_s + while time.monotonic() < end: + t = time.monotonic() + while time.monotonic() - t < 0.30 and time.monotonic() < end: + try: + s.write(b"x" * 64); s.read(64) + except Exception: + return + time.sleep(min(1.0, max(0, end - time.monotonic()))) + s.close() + +WORKLOADS = {"cdc_burst": _workload_cdc_burst, + "idle": lambda node, duration_s: __import__("time").sleep(duration_s)} + +def capture_one(board, commit, out_dir, repo_root): + """Build, flash, RTT-capture, decode; returns the wrapper dict. Never raises.""" + import subprocess, time, socket, signal + sv, name = board["sysview"], board["name"] + scripts = os.path.join(repo_root, ".claude", "skills", "sysview", "scripts") + sys.path.insert(0, scripts) + from sysview_record import rtt_cb_from_elf + import hil_flash + try: + ocd_args = capture_ocd_args(board) + exdir = os.path.join(repo_root, "examples", sv["example"]) + bdir = os.path.join(repo_root, "examples", f"cmake-build-sysview-{name}") + buf = [f"-DSYSVIEW_BUFFER_SIZE={sv['buffer']}"] if "buffer" in sv else [] + for cmd in ([ "cmake", "-B", bdir, f"-DBOARD={name}", "-G", "Ninja", + "-DCMAKE_BUILD_TYPE=MinSizeRel", "-DSYSVIEW=4", *buf, "." ], + [ "cmake", "--build", bdir ]): + r = subprocess.run(cmd, cwd=exdir, capture_output=True, text=True, timeout=1800) + if r.returncode: + return board_result(board, commit, error=f"build failed: {r.stderr[-300:]}") + base = os.path.basename(sv["example"]) + fw = os.path.join(bdir, base) # extension-less base path + flash = getattr(hil_flash, f"flash_{board['flasher']['name']}") + r = flash(board, fw) + if r.returncode: + return board_result(board, commit, error=f"flash failed: rc={r.returncode}") + node = f"/dev/serial/by-id/usb-TinyUSB_TinyUSB_Device_{board['uid']}-if00" + for _ in range(25): + if os.path.exists(node): break + time.sleep(1) + cb = int(rtt_cb_from_elf(fw + ".elf"), 16) + is_wch = board["flasher"]["name"] == "openocd_wch" + port = 19500 + ocd = ["openocd", "-c", "tcl_port disabled", "-c", "gdb_port disabled", + "-c", "telnet_port disabled", + "-c", f"adapter serial {board['flasher']['uid']}"] + ocd_args + \ + ["-c", "init"] + ([] if is_wch else ["-c", "reset run", "-c", "sleep 2000"]) + \ + ["-c", f'rtt setup {cb} 0x1000 "SEGGER RTT"', + "-c", "rtt polling_interval 1", "-c", "rtt start", + "-c", f"rtt server start {port} 1"] + raw = os.path.join(out_dir, f"{name}-capture.SVDat") + p = subprocess.Popen(ocd, stdout=subprocess.DEVNULL, stderr=subprocess.PIPE, text=True) + try: + for _ in range(80): + time.sleep(0.2) + try: + socket.create_connection(("localhost", port), 0.3).close(); break + except OSError: + if p.poll() is not None: + return board_result(board, commit, + error=f"openocd: {p.stderr.read()[-200:]}") + else: + return board_result(board, commit, error="rtt server never listened") + with open(raw, "wb") as f: + nc = subprocess.Popen(["nc", "localhost", str(port)], stdout=f) + try: + WORKLOADS[sv["workload"]](node, sv["duration_s"]) + finally: + nc.send_signal(signal.SIGINT) + try: nc.wait(5) + except subprocess.TimeoutExpired: nc.kill() + finally: + p.send_signal(signal.SIGINT) + try: p.wait(8) + except subprocess.TimeoutExpired: p.kill() + dec = os.path.join(out_dir, f"{name}-decoded") + for cmd in ([sys.executable, os.path.join(scripts, "sysview_record.py"), + "--from-raw", raw, "--out", dec],): + r = subprocess.run(cmd, capture_output=True, text=True, timeout=900) + r = subprocess.run([sys.executable, os.path.join(scripts, "sysview_report.py"), + dec, "--json"], capture_output=True, text=True, timeout=600) + if r.returncode: + return board_result(board, commit, error=f"decode failed: {r.stderr[-200:]}") + m = json.loads(r.stdout) + info = {"route": "openocd-rtt", "poll_ms": 1, + "n_events": None, "live_window_s": m.get("live_window_s")} + return board_result(board, commit, metrics=m, capture_info=info) + except Exception as e: + return board_result(board, commit, error=f"{type(e).__name__}: {e}") + finally: + _reflash_pristine(board, repo_root) + +def _reflash_pristine(board, repo_root): + import hil_flash, subprocess + try: + fw = hil_flash.find_firmware(board["name"], board["sysview"]["example"]) + if fw: + getattr(hil_flash, f"flash_{board['flasher']['name']}")(board, str(fw)) + except Exception: + pass # pristine reflash is best-effort +``` + +`capture` CLI in `main()`: `capture CONFIG_JSON [-b BOARD]... [--out DIR]`; loads config, `select_boards`, per board: `with acquire_board_lock(name, reason="sysview capture"):` (convention verified per Interfaces) around `capture_one`, writes `sysview-<board>.json`, prints one status line per board, exits 0. + +- [ ] **Step 4: Run tests** — `python3 test/hil/test_sysview_ci.py -v` (all report + selection tests green). + +- [ ] **Step 5: Commit** — `git commit -m "hil: sysview capture - flash SYSVIEW build, drive workload, RTT-capture, decode"` + +--- + +### Task 6: `build.yml` wiring + +**Files:** Modify: `.github/workflows/build.yml` + +- [ ] **Step 1: Capture step + artifact in the `hil-tinyusb` matrix job**, immediately after the `hil_test.py` step, passing the **identical selection-args expansion** that step uses (copy its `${{ matrix.test_args }} ... $RERUN_ARGS`-style expression, converting to `-b` form only if hil_select emits bare `-b` args — inspect the select step output format and mirror): + +```yaml + - name: SystemView capture + if: always() + continue-on-error: true + run: | + python3 test/hil/sysview_ci.py capture ${{ env.HIL_JSON }} \ + $SYSVIEW_BOARD_ARGS --out sysview-out + - name: Upload SystemView captures + if: always() + continue-on-error: true + uses: actions/upload-artifact@v7 + with: + name: sysview-${{ matrix.display }} + path: sysview-out/sysview-*.json + if-no-files-found: ignore +``` + +where `SYSVIEW_BOARD_ARGS` is derived in the same step from the test step's board selection (empty on push = all flagged boards). + +- [ ] **Step 2: `sysview-report` job** (ubuntu, `needs: hil-tinyusb`, `if: always() && github.event_name == 'pull_request'`): checkout; `download-artifact` pattern `sysview-*` → `pr-sysview/` (merge-multiple); `dawidd6/action-download-artifact@v11` with `workflow: build.yml`, `branch: ${{ github.base_ref }}`, `name: sysview-.*`, `name_is_regexp: true`, `path: base-sysview`, `continue-on-error: true`; flatten both dirs; `python3 test/hil/sysview_ci.py report base-sysview pr-sysview -o sysview_report.md`; upload artifact `sysview-comment` (`if-no-files-found: ignore`, skip upload when the file is empty). + +- [ ] **Step 3: Validate YAML** — `python3 -c "import yaml; yaml.safe_load(open('.github/workflows/build.yml'))"` (PyYAML available on the dev box; if not, `actionlint` or a py `ruamel` fallback). + +- [ ] **Step 4: Commit** — `git commit -m "ci: sysview capture in HIL jobs, compare job producing sysview-comment"` + +--- + +### Task 7: `pr_comment.yml` — append to the HIL sticky comment + +**Files:** Modify: `.github/workflows/pr_comment.yml` + +- [ ] **Step 1:** In the `hil-comment` job, after the `hil-report-*` download, add a `sysview-comment` download (`continue-on-error: true`, same run-id/token pattern). In the combine step, after the rig loop, append: + +```bash + if [ -s sysview-comment/sysview_report.md ]; then + echo >> hil_combined.md + cat sysview-comment/sysview_report.md >> hil_combined.md + fi +``` + +The existing zero-width-space @-mention neutralization runs after combining, so it covers the sysview markdown automatically — keep the append **before** that step. + +- [ ] **Step 2: Validate YAML** (as Task 6 Step 3). Commit — `git commit -m "ci: append sysview performance report to the HIL sticky comment"` + +--- + +### Task 8: Live verification on ci.lan + mermaid render check + +No code. Verifies the hardware path end-to-end before the workflow ever runs it. + +- [ ] **Step 1:** Sync the worktree to the rig mirror (`rsync` `test/hil/`, `src/`, `hw/bsp/`, `.claude/skills/sysview/` → `[email protected]:sysview-v2/`, never touching dep symlinks). +- [ ] **Step 2:** On ci.lan (toolchains + `~/.local/bin` on PATH): `python3 test/hil/sysview_ci.py capture test/hil/tinyusb.json -b stm32f407disco -b raspberry_pi_pico --out /tmp/sv-ci-test` — expect two `sysview-*.json` with `error: null`, `metrics.overflow == 0`, `live_window_s ≈ 14–15`. +- [ ] **Step 3:** `report` with the same dir as both base and PR (`-o /tmp/r.md`) — every Δ must be `—`; then against a copy with one hand-edited p50 — that Δ and only that Δ appears. +- [ ] **Step 4:** Paste `/tmp/r.md` into a scratch GitHub PR comment (or gist) — confirm the mermaid chart renders and the legend reads correctly. Delete the scratch comment. +- [ ] **Step 5:** Confirm both boards are back on pristine firmware (`hil_test.py -b <board>` smoke or check enumeration), release any locks, remove `/tmp/sv-ci-test`. +- [ ] **Step 6:** Commit any fixes found, message prefixed `hil: sysview capture fixes from rig verification -`. + +--- + +## Execution notes + +- Tasks 1→2→3 are strictly ordered (reporter fields feed fixtures). Task 4 is independent after 3. Task 5 depends on 3+4. Tasks 6–7 depend on 5. Task 8 last. +- The rig is shared with live CI: Task 8 must hold board locks and strictly follow the one-instance rule for hardware access. +- If `acquire_board_lock`'s convention differs from the plan's `with` usage, adapt the call site — do not modify `hil_lock.py`. diff --git a/docs/superpowers/specs/2026-07-09-claude-agents-workflows-design.md b/docs/superpowers/specs/2026-07-09-claude-agents-workflows-design.md index 3035723c4..811e22885 100644 --- a/docs/superpowers/specs/2026-07-09-claude-agents-workflows-design.md +++ b/docs/superpowers/specs/2026-07-09-claude-agents-workflows-design.md @@ -14,7 +14,8 @@ workers do the volume. ## Context - Existing process skills: `hil`, `code-size`, `pvs`, `build-doc`, `usbmon`, - `usb-debug`, `usb-recover`, `make-release` (`.claude/skills/`). + `usb-debug`, `usb-recover` (since renamed `usb-kernel-debug`, + `usb-kernel-recover`), `make-release` (`.claude/skills/`). - One prototype workflow exists in the master working tree (untracked): `.claude/workflows/port-audit.js`. This design supersedes it. - No custom agent definitions exist yet (`.claude/agents/` absent). @@ -42,7 +43,7 @@ the agent. | `port-dev` | xhigh | Implement one well-scoped change in one port / file set. Follows repo rules: C99, 2-space indent, snake_case, `TU_ASSERT`, no dynamic allocation, ISR work deferred to task context. Runs `clang-format` (repo `.clang-format`) on touched files before finishing. Cross-checks the MCU datasheet in `$HOME/Documents/calibre-library` when changing dcd/hcd register logic. Verifies with a targeted build of one board using the port. Returns `{item, diffstat, buildOk, notes}`. | | `driver-reviewer` | xhigh | Review one dcd/hcd directory against dimensions: correctness, ISR safety, register use vs. datasheet AND MCU errata (calibre library; missing erratum workarounds are findings), style. Returns structured findings `{file, line, snippet, why, severity, confidence}` — coverage-first (report everything; filtering happens downstream). | | `hil-operator` | default | All rig interaction — the actions-runner service is NEVER stopped; per-board flock locks arbitrate with concurrent CI. `hil_test.py` runs rely on its per-board self-locking; manual hardware work (JLink/GDB, usbtest, serial) is wrapped in `test/hil/board_lock.py hold/release`; rig-wide ops (uhubctl, pci-rebind) require `hold --all`; on wedge `usb_recover.sh` + dmesg. Used strictly serially — never two instances concurrently. | -| `target-debugger` | xhigh | Root-cause one USB misbehavior on one board by instrumenting the device side (TU_LOG/RTT, RAM ring-buffer trace, GDB autopsy, J-Link PC-sampling) with dual-side host+target capture, per `.claude/skills/usb-target-debug/SKILL.md`, plus wire-level capture via the ataradov hardware tap (`.claude/skills/usb-sniffer/SKILL.md`) when the host side can't see or is disputed. Deliberately serial loop under one held board lock (released around `hil_test.py` runs, which self-lock); strictly one instance. Diagnosis standard: evidence shows the mechanism, or a fix flips the ORIGINAL failing case on hardware; stops after two evidence-free cycles with a partial report. Hard rule "fix stays, probe goes, re-verify clean": instrumentation reverted, candidate fix left uncommitted and re-verified on a clean build, pristine firmware reflashed before lock release. Returns `{board, bug, diagnosis, confirmed, ruledOut[], evidence[], fixDiffstat, fixVerified, instrumentationReverted, lockReleased, notes}`. | +| `target-debugger` | xhigh | Root-cause one USB misbehavior on one board — device or host stack — by instrumenting the target with correlated dual-side capture; strictly serial, one instance, one held board lock. The charter (skill routing table: `target-debug`, `usbmon`, `usb-sniffer`, `etm-trace`, `sysview`, ..., diagnosis standard, lock discipline, "fix stays, probe goes" rule, output contract) lives in `.claude/agents/target-debugger.md` — the single source of truth; this row is deliberately a pointer so the two cannot drift. | | `pr-monitor` | default | Triage one GitHub PR via `gh`: check CI status (`gh pr checks`), read failing run logs and classify each failure infra/flake vs real; re-run infra failures (`gh run rerun --failed`); harvest automated review comments (Codex/Copilot/Claude bots — knows their signals: Codex posts a "Didn't find any major issues" issue comment when clean; Copilot drops out of `requested_reviewers` when done; bot logins differ across APIs); adversarially validate each finding against the actual code. Returns structured triage `{ci: {status, infraRerun[], realFailures[]}, findings: [{source, file, line, claim, verdict, fixHint}]}`. Read/triage/re-run/reply only — never edits code. | | `static-analyzer` | low | Run PVS-Studio (SAST + MISRA C:2023/C++:2008) for one board: build with exported `compile_commands.json` (via `run_pvs.sh` solo, or a dedicated `cmake-build-pvs` dir when parallel builders run), analyze against `.PVS-Studio/.pvsconfig`, gate on diagnostics in files changed vs a base ref. Returns `{pass, ga1, ga2, changedFindings[], detail}`; `pass=false` only on GA:1 in changed files or tool failure. Read-only. | @@ -121,6 +122,37 @@ test boards from `hw/bsp` (fallback: `stm32f407disco`, `raspberry_pi_pico`), launch `full-check` with that board list, and summarize the verdict. Markdown carries the judgment; JS carries the orchestration. +### Skill vs technique — promotion criteria + +The debugging playbook (`target-debug`) bundles techniques inline; some +capabilities are standalone skills (`usbmon`, `usb-sniffer`, `etm-trace`, +`sysview`, ...). A capability becomes a standalone skill when it meets **two +or more** of: + +1. **Ships tooling** — scripts/config that need versioning and maintenance + (etm-trace's capture/profile pair, sysview's recorder/reporter, usbmon's + `usbcap.sh`). Recipes over already-installed tools don't count. +2. **Answers its own routed question** — it earns a distinct row in the + capture-channel tables ("what it answers"), with its own trigger + vocabulary for discovery. A technique is a *how* within an existing + question; a skill is a new *question*. +3. **Carries validation state or host setup** — per-board bring-up notes / + validated-hardware matrix, host installs, physical-wiring preconditions. +4. **Long but conditionally relevant** — would add a page+ to target-debug + (always loaded by target-debugger) that most sessions never need; skills + are lazy-loaded via the routing tables. + +Criterion 2 is not necessary: a capability can promote on 1+3+4 while still +answering an existing question — the existing question's row then points at +it as an alternative channel instead of gaining a new row. + +It stays an inline technique in target-debug when it's a sub-screen recipe +over standard rig tools (JLinkExe/GDB/OpenOCD) sharing the +lock→instrument→flash→capture loop. + +Current borderline: **SWO** (exception/data trace) — promote only if/when +capture+decode scripts get built; until then it stays inline. + ## Model & effort policy - Tiered worker models: `port-dev`/`driver-reviewer`/`target-debugger` **opus** diff --git a/docs/superpowers/specs/2026-07-25-sysview-v2-postmortem-design.md b/docs/superpowers/specs/2026-07-25-sysview-v2-postmortem-design.md new file mode 100644 index 000000000..c36f6fd7c --- /dev/null +++ b/docs/superpowers/specs/2026-07-25-sysview-v2-postmortem-design.md @@ -0,0 +1,165 @@ +# sysview skill v2 — post-mortem autopsy + instrumentation quick wins — Design + +Date: 2026-07-25 +Branch: claude/add-systemview-debug (builds on the sysview skill, f198be682) + +## Goal + +Close the gaps between the sysview skill and the SystemView feature set +(UM08027) that matter for USB debugging: crash-context capture (post-mortem +mode) and three small instrumentation recipes (markers, event filtering, +PrintfHost), plus the reporter columns to make them useful without the GUI. + +## Context + +The sysview skill (`.claude/skills/sysview/`) covers continuous J-Link +recording, FreeRTOS task events, manual ISR wrap, contexts/CPU-load analysis, +and CSV export — hardware-validated on same54_xplained + J-Trace. A gap +analysis against UM08027 found the highest-value uncovered features to be +post-mortem mode (§3.15.3) and the DisableEvents/marker/Printf APIs. The +`--export-terminal` flag exists but has never been exercised with real +`PrintfHost` output. + +## Design + +### 1. Post-mortem capture path + +- **Build**: `sysview.cmake` gains `-DSYSVIEW_POST_MORTEM=1`, defining + `SEGGER_SYSVIEW_POST_MORTEM_MODE=1` (UM08027 §4.5.2.4). + `SEGGER_SYSVIEW_SYNC_PERIOD_SHIFT` stays at its target-source default + (8 = sync every 256 events); add an override only if validation shows + resync gaps in decoded dumps. The target then + writes the SysView RTT ring with overwrite — no host reader needed; the + buffer always holds the most recent scheduling history (~last seconds, + buffer-size dependent). The existing config's target-side DWT CYCCNT enable + already satisfies the manual's ENABLE_DWT_CYCCNT requirement (no debugger + attached while recording). +- **Dump**: new script `scripts/sysview_dump.py` — a separate unit from the + live recorder (nothing in common with Xvfb/GUI choreography). Flow: resolve + probe (shared helper pattern), attach **without reset or reflash** (evidence + preservation, same rule as target-debug's fault autopsy), halt, read the + `_SEGGER_RTT.aUp[1]` descriptor (pBuffer/SizeOfBuffer/WrOff/RdOff) via + JLinkExe, dump the buffer memory, and linearize oldest→newest. **Split + point to be determined on hardware**: ring logic says `[WrOff..end) + + [0..WrOff)`, but the manual's literal text (§3.15.3) ends the second chunk + at `RdOff - 1` — with no host reader these may or may not coincide. + Implementation decodes both candidates; the one yielding a sane timeline + wins and gets recorded in the SKILL. Write `capture.SVDat` into `--out`. +- **Decode**: unchanged — `sysview_record.py --from-raw <capture.SVDat>` + already parses a raw stream via SystemView `-load` and exports CSV. +- **Routing**: one cross-ref line in target-debug's vector-catch/fault-autopsy + section: a post-mortem sysview build answers "what was the system doing + right before the fault/wedge". +- **Caveats documented in SKILL.md**: post-mortem and live recording are + mutually exclusive build modes (the live GUI recorder cannot drain an + overwrite-mode ring); halting for the dump kills USB service (host URB + timeouts), which is acceptable post-crash. + +### 2. Instrumentation quick wins (SKILL.md optional edits 3–5) + +- **Markers**: `SEGGER_SYSVIEW_MarkStart(id)/MarkStop(id)` bracketing one code + path (e.g. a transfer, an enumeration phase), with `NameMarker` for the + label. +- **Event filtering**: `SEGGER_SYSVIEW_DisableEvents(<mask>)` after `Conf()` + as the documented overflow fix — keeps task/ISR events, drops the highest- + rate classes. The exact mask combination is determined empirically during + implementation on hardware and recorded in the SKILL (not guessed here). +- **PrintfHost**: `SEGGER_SYSVIEW_PrintfHost()` for log lines correlated with + the timeline; exercised in validation so `--export-terminal` is proven. + +### 3. Reporter additions (`sysview_report.py`) + +- Surface `Total Blocked Time` per context (column already present in + contexts.csv, currently dropped). +- Marker-pair durations from events.txt: per marker id, n/p50/p99/max — same + table shape as the existing ISR/ready→run tables. + +## Error handling + +- `sysview_dump.py`: refuses to run if the RTT magic ("SEGGER RTT") is absent + at the given/ELF-derived address (wrong ELF or corrupted RAM); reports + whether the buffer had wrapped (WrOff vs sync coverage); resumes the core + only if it halted it and `--resume` is passed (default: leave halted — the + user is mid-autopsy). +- Decode of a wrapped ring starting mid-packet is expected to produce leading + garbage until the first sync — the SYNC_PERIOD_SHIFT packets exist for + this; the SKILL documents "leading events before the first sync are + unreliable". + +## Validation (same54_xplained + jtrace, dogfood pattern) + +1. Post-mortem: flash `-DSYSVIEW_POST_MORTEM=1` build, run CDC bulk traffic, + then run `sysview_dump.py` mid-load — its halt IS the simulated crash — + and decode via `--from-raw`. Expect plausible contexts and the traffic + window's tail present in the timeline. +2. Quick wins, deterministic overflow A/B at `-DSYSVIEW_BUFFER_SIZE=4096` + (the size that reliably overflowed in v1 under CDC bulk load with API + tracing): baseline run shows nonzero overflow; same run with the + validated DisableEvents mask shows overflow 0. Same instrumented build + carries a marker pair + PrintfHost — expect the marker table populated + and terminal.csv containing the Printf lines. +3. Both scripts pass `python3 -m py_compile`; pre-commit clean; instrumentation + edits reverted and pristine firmware reflashed afterward (skill's own + rule). + +## Scope revision (2026-07-25, user-directed — supersedes parts of the above) + +- **SystemView becomes a first-class optional dependency**: + `tools/get_deps.py` entry `lib/SystemView` → + github.com/SEGGERMicro/SystemView pinned at the V4.12.0 tag commit + (92ca7a810c5765ba64911919acd511c61b6b083f). `sysview.cmake` consumes it; + the ~/.cache ad-hoc clone goes away. +- **Leveled instrumentation** (2026-07-25 refinement): not a boolean — + `CFG_TUD_SYSVIEW` and `CFG_TUH_SYSVIEW` are levels 0–4 (0 = off), like + `CFG_TUSB_DEBUG`. Site macros `TUD_SYSVIEW_CALL/RET(level, id)` (and + `TUH_`) expand to nothing when the configured level is below the site's + level, via the same token-paste dispatch as `TU_LOG(n, …)`. Category + levels are macro-configurable with defaults: **USB ISR = 1, usbd/usbh + functions = 2, dcd/hcd API = 3, class-driver API = 4**. dcd/hcd + instrumentation wraps the call sites in usbd.c/usbh.c (the port boundary), + never the portable drivers themselves. Build: `-DSYSVIEW=<level>` sets + both sides (ON = 4). +- **Instrumentation moves in-tree as first-class analysis**: + `SEGGER_SYSVIEW_Config_TinyUSB.c` becomes `src/common/tusb_sysview.c/.h` + behind a `CFG_TUSB_SYSVIEW` option (default 0; empty macros, zero code/size + when off — proven with a code-size compare). Adds `TU_SYSVIEW_*` + enter/exit macros placed in usbd/usbh/dcd/hcd/class-driver hot paths + (SEGGER module + RecordVoid/RecordEndCall convention) → **per-function + timing of the USB stack with no trace hardware** — the easy alternative to + etm-trace for hot-function hunting. +- **Metrics focus** (all machine-readable): USB task + ISR timing (have), + per-function stack-path timing (new), FreeRTOS **task stack** high-water + (INCLUDE_* defines + shim include via guarded blocks in family + FreeRTOSConfig.h — in-tree now, the zero-edit constraint no longer + applies; validation families first, sweep later), **heap** events via a + traceMALLOC/traceFREE → SEGGER_SYSVIEW_HeapAlloc/HeapFree mapping (the + V4.12 shim lacks one — verified). Heap events exist only when + `configSUPPORT_DYNAMIC_ALLOCATION=1`; validation families are fully + static, so the reporter handles zero-heap gracefully and the mapping is + validated with a temporary dynamic-alloc build. +- **Reportable everything**: `sysview_report.py --json` emitting + contexts/ISR/latency/functions/stack/heap/overflow — the future hook for + posting a PR comment from HIL runs (the posting itself is NOT wired now). +- **OpenOCD / non-SEGGER probes** (question resolved): the SystemView GUI's + live recorder is J-Link-only (SEGGER-official; community TCP bridges are + experimental). Supported OpenOCD routes here: (a) raw channel-1 capture + via OpenOCD `rtt server` → file → `--from-raw` decode — works because the + target self-starts recording in `Conf()`; (b) the post-mortem dump — + probe-agnostic memory reads. OpenOCD RTT is polled (drop risk under + burst) — the overflow gate detects loss. Validation task on + stm32h743nucleo (rig, ST-Link/OpenOCD). + +## Out of scope + +Each waits for a real pull, per the harness promotion-criteria philosophy: +UART recorder (non-SEGGER-probe boards), multicore (rp2350), data plot +(RegisterData/SampleData), single-shot recording, GUI trigger modes, +HIL/PR-comment posting automation. + +## Rejected alternatives + +- Folding the dump into `sysview_record.py` as a `--post-mortem` flag: one + entry point, but it grows an already-long script and tangles two unrelated + workflows (live capture vs halted autopsy). +- Automating the GUI's Target → Read Recorded Data: more headless dialog + choreography — the most fragile part of v1. diff --git a/docs/superpowers/specs/2026-07-29-sysview-hil-report-design.md b/docs/superpowers/specs/2026-07-29-sysview-hil-report-design.md new file mode 100644 index 000000000..a5ba615cc --- /dev/null +++ b/docs/superpowers/specs/2026-07-29-sysview-hil-report-design.md @@ -0,0 +1,266 @@ +# SystemView HIL Performance Report — Design + +Per-PR performance report from real hardware: SystemView captures taken during +the HIL run, compared against the base branch, posted as a sticky PR comment. +Makes TinyUSB's speed story (ISR cost, task latency, hot-function timing) +visible on every PR, and makes performance regressions reviewable the way code +size already is. + +Decisions fixed during brainstorming: **approach A** (lean headline metrics, +one screen per 2 boards) · sticky PR comment · delta vs base-branch artifact · +boards **and** workload selectable per-board in `tinyusb.json` · mermaid +charts · legend explaining every statistic. + +## 1. Flow + +``` +HIL job (self-hosted rig, only boards both HIL-selected and sysview-flagged) + hil_test.py passes + → sysview_ci.py capture --board <b> # flash SYSVIEW build, run + → sysview-<board>.json # workload, RTT capture, decode, + → reflash pristine build # reflash pristine + → upload artifact sysview-<rig> + +ubuntu report job in build.yml (no hardware; needs: the HIL jobs) + download PR sysview-* artifacts + download baseline artifact from base branch (dawidd6/action-download-artifact, + workflow: build.yml, branch: base_ref, continue-on-error — the exact + code-metrics pattern) + → sysview_ci.py report base/ pr/ → sysview_report.md + → upload artifact sysview-comment + +pr_comment.yml (workflow_run, base-repo context) + the EXISTING hil-comment job — which already combines hil-report-* artifacts + into the "Hardware-in-the-loop (HIL) Test Report" sticky comment — also + downloads sysview-comment and appends it to hil_combined.md + +push to master + same capture step; uploads sysview-* as the new baseline artifact +``` + +The posting split is load-bearing, not cosmetic: on forked PRs build.yml's +token is read-only and cannot comment — pr_comment.yml exists precisely for +this, posts even when the build failed, and already neutralizes @-mentions in +report content (fork-abuse guard, which applies to our markdown too). Riding +the HIL comment also puts the performance section where a reviewer already +looks for hardware results, instead of a third sticky comment. + +Properties: + +- **Non-blocking.** A capture/decode failure never fails the HIL job; the + board's section renders as `capture failed: <reason>` and CI stays green. +- **Missing baseline** (first run after enabling, or a board newly flagged): + absolute values, no Δ column — same degradation code-metrics uses. +- **PR-scoped HIL** (`hil_select.py` prunes boards per PR): the report compares + only boards present in *both* PR and baseline artifact sets. If no flagged + board ran, the comment section is omitted entirely — never a wall of + "missing data". + +## 2. Configuration — `test/hil/tinyusb.json` + +Per-board opt-in block; absence means no capture for that board: + +```json +"sysview": { + "example": "device/cdc_msc", // example to build with -DSYSVIEW=4 + "workload": "cdc_burst", // named workload, see §4 + "duration_s": 15, // capture window + "buffer": 16384, // optional; -DSYSVIEW_BUFFER_SIZE override + "ocd_args": "..." // optional; OpenOCD interface+target args for +} // the RTT capture. Defaults to the flasher's +``` // args — which only works when the flasher is + // openocd-family. jlink/stlink-flashed boards + // must set it (e.g. f407: "-f interface/jlink.cfg + // -c \"transport select swd\" -f target/stm32f4x.cfg") + +Both the board set and the workload are config, not code — changing either is +a `tinyusb.json` edit, no harness change. + +## 3. `test/hil/sysview_ci.py` (new, standalone) + +`hil_test.py` is untouched. Two subcommands with a hard purity split: + +### `capture` — runs on the rig, needs hardware + +Board selection is the intersection of "sysview-flagged in the json" and +"tested by this job": `capture` accepts the same `-b/--board` append syntax as +`hil_test.py`, and the workflow step passes it the identical selection +arguments (`matrix.test_args` + the `hil_select.py` args) that the test step +received. No args = all flagged boards in the json. + +Per selected board: build `example` with `-DSYSVIEW=4` (+ buffer override) → +flash via **`hil_flash.py`**'s per-flasher functions (verified: `flash_jlink` +/ `flash_openocd` / `flash_openocd_wch` / `flash_stlink` …, signature +`(board, firmware)` where `firmware` is the extension-less base path — the +SYSVIEW build lives in its own build dir, so `capture` passes that path +explicitly rather than using `find_firmware`, which only searches +`cmake-build-<variant>`) → wait for enumeration → +start OpenOCD RTT (`rtt setup` from the ELF symbol table, **`rtt +polling_interval 1`**, never `reset run` inside a WCH session) → run the named +workload for `duration_s` → decode via `sysview_record.py --from-raw` + +`sysview_report.py --json` → write `sysview-<board>.json` → **reflash the +pristine build**. Board locks held for the whole sequence via `hil_lock.py`'s importable +`acquire_board_lock()` (no CLI subprocess). + +Output schema (one file per board): + +```json +{ + "board": "stm32f407disco", "commit": "a1b2c3d", + "example": "device/cdc_msc", "workload": "cdc_burst", "duration_s": 15, + "capture": {"route": "openocd-rtt", "poll_ms": 1, "live_window_s": 14.2}, + "metrics": { ... verbatim sysview_report.py --json object ... }, + "error": null // or "flash failed: ...", metrics absent +} +``` + +### `report` — pure, no hardware, unit-testable + +`report <base-dir> <pr-dir> -o report.md`. Reads both JSON sets, joins on +board name, emits the markdown of §5. No network, no subprocess — fixture +JSONs in, deterministic markdown out. + +## 4. Workloads + +Named functions inside `sysview_ci.py`, selected by name from `tinyusb.json`: + +- **`cdc_burst`** (default): open the board's CDC node + (`/dev/serial/by-id/usb-TinyUSB_*`), fixed pattern — 300 ms of 64-byte + write/read bursts, 1.0 s idle, repeated for `duration_s`. Deterministic, so + run-to-run deltas are meaningful; the idle gaps double as the timestamp + sanity check (median gap ≈ 1.000 s). +- **`idle`**: enumerate and sit. For boards with no drivable node (host-role + boards drive their attached devices themselves). + +Adding a workload = one function + a name; no schema change. + +## 5. The comment + +Appended to the existing sticky comment. Canonical example (real numbers from +the 2026-07 pool dogfoods; one `###` section per board): + +--- + +## ⚡ SystemView performance — HIL + +*cdc_msc `SYSVIEW=4`, workload `cdc_burst` 15 s, OpenOCD rtt @1 ms · base `2d56dc5` → PR `a1b2c3d`* + +### stm32f407disco (M4 · DWT) — live 14.2 s + +| metric | base | PR | Δ | +|---|---:|---:|---:| +| ISR 83 p50 / p99 | 6.7 / 9.2 µs | 6.8 / 9.2 µs | +1.5% / — | +| `tud_task` p50 | 7.4 µs | 7.4 µs | — | +| `tud_cdc_read` p50 | 6.9 µs | 6.2 µs | **−10.1%** ✅ | +| `dcd_edpt_xfer` p50 | 5.9 µs | 6.1 µs | +3.4% | +| CPU load (usbd ctx) | 4.6 % | 4.4 % | −0.2 pt | +| `usbd` stack high-water | 684 B | 684 B | — | + +```mermaid +xychart-beta + title "hot functions p50 µs (base vs PR)" + x-axis [tud_task, cdc_read, cdc_flush, mscd_cb, dcd_xfer] + y-axis "µs" 0 --> 12 + bar [7.4, 6.9, 2.8, 10.2, 5.9] + bar [7.4, 6.2, 2.9, 10.0, 6.1] +``` + +### raspberry_pi_pico (M0+ · 1 MHz timer) — live 14.3 s + +| metric | base | PR | Δ | +|---|---:|---:|---:| +| `tud_task` p50 | 19.0 µs | 19.0 µs | — | +| `tud_cdc_read` p50 | 24.1 µs | 21.8 µs | **−9.5%** ✅ | +| `dcd_edpt_xfer` p50 | 10.0 µs | – ⚠︎ overflow 3 | — | +| CPU load (usbd ctx) | 7.1 % | 7.0 % | — | + +<sub>**Legend** — **p50/p99**: median / 99th-percentile duration over all calls +in the capture window (µs; p50 = typical cost, p99 = tail latency). **Δ**: +change vs base branch; **−** is faster/better. **pt**: percentage points. +**CPU load**: context's share of the live capture window. **stack +high-water**: peak bytes of stack used. Function rows/bars are ordered by CPU +occupancy (calls × p50) in the PR capture, hottest first. **– ⚠︎**: metric withheld — RTT ring +overflowed (`overflow N`) or too few samples (`n<50`); withheld beats wrong. +`max` is never shown: under overflow it splices two invocations into one bogus +duration. Capture: OpenOCD RTT @1 ms poll — p50/p99 match the J-Link recorder +within ~1%.</sub> + +--- + +Metric rows per board: ISR p50/p99 first (one row per USB ISR), then the +function rows **sorted by CPU occupancy** — `n × p50` over the live window, +descending, computed from the PR side (base side follows the same row order) — +then per-context CPU load (usbd/usbh context) and stack high-water per task +(FreeRTOS builds only). The function universe is the instrumented +`tu_sysview_id_t` sites, so today ≤6 rows per role appear; sorting makes the +first row "biggest CPU consumer", and the layout holds unchanged if +instrumentation grows. One mermaid chart per board: functions in the same +occupancy order, **capped at 8 bars**, two bar series (base, PR). The legend +appears once, after the last board section (and defines occupancy ordering). + +## 6. Validity gates + +Wrong numbers are worse than no numbers; every rule is mechanical: + +| condition | rendering | +|---|---| +| `metrics.overflow > 0` (single source; not duplicated in `capture`) | every duration metric from that capture: `– ⚠︎ overflow N` | +| metric `n < 50` | that metric: `– ⚠︎ n=<n>` | +| gate fails on either side | Δ omitted; passing side shown absolute | +| capture/decode error | board section: `capture failed: <reason>` (from `error` field) | +| board absent from baseline | absolute values, Δ column `new` | +| no flagged board ran | entire comment section omitted | +| `max` | never rendered, anywhere | + +## 7. Reporter prerequisites (bug fixes in `sysview_report.py`) + +Both are existing silent-wrong-answer bugs; deltas built on them would lie. + +1. **Spliced pairs.** A lost record makes the decoder pair one invocation's + CALL with a later invocation's RET (measured: 134 ms "max" on a function + whose p99 is 10 µs). Fix: track pairing depth per function id; when a CALL + arrives while one is open, or a RET arrives with none open, discard that + sample and count it in a new `dropped_pairs` field instead of emitting a + duration. +2. **Stale boot window.** The export contains boot-time records still in the + RTT ring, so a quiet capture's `cpu%` averages over dead air (measured: + 111 s span, 98 s of it idle). Fix: split the event stream on any + inter-event gap > 2 s before the first workload event; compute all + statistics over the live window only; report `live_window_s`. + +Both fixes land in `sysview_report.py` itself (with `--json` fields +`dropped_pairs` and `live_window_s`), so the skill's ad-hoc use benefits too. + +## 8. Workflow wiring + +- `build.yml` HIL jobs: after the `hil_test.py` step, a + `continue-on-error` capture step — + `python3 test/hil/sysview_ci.py capture --json $HIL_JSON --out sysview/` + (internally: only boards this job just tested AND flagged in the json) — + then `upload-artifact sysview-<rig>`. +- Master pushes upload the same artifact; it is the next PR's baseline. +- Report job (ubuntu, alongside the code-metrics comment job): download both + sides, run `report`, append to the sticky comment. Skips cleanly when the + PR produced no sysview artifacts. + +## 9. Testing + +- **TDD on `report`** (pure): fixture base/PR JSON pairs → expected markdown. + Cases: normal delta, gated metric (overflow, low-n), one-sided gate, + missing baseline, capture-failed board, empty intersection, legend + presence, mermaid series alignment. +- **Reporter fixes**: unit fixtures with synthetic event streams — spliced + CALL/RET sequences and a stale-boot + live-window stream — asserting + `dropped_pairs` / `live_window_s` and the corrected statistics. +- **Capture live-verified on ci.lan** against two flagged boards + (stm32f407disco, raspberry_pi_pico) before any workflow edit. +- **Comment rendering**: paste generated markdown into a scratch PR comment + once to confirm GitHub renders the mermaid blocks. + +## Out of scope (deliberately) + +Regression thresholds/auto-flagging (needs jitter data first — collect it from +real PR traffic), manual-parity extras (Time Interrupted, quartile charts, +run-time/s jitter — additive later), job-summary second tier, trend storage +beyond the single rolling baseline artifact, Espressif boards (no SYSVIEW +build path), Make builds. |