Skip to content

plate(gcu-c): ship the escape hatch, host-test the link surface, record measured I2S DAC truths - #115

Open
tig wants to merge 1 commit into
mainfrom
claude/esphome-silico-integration-ji9tag
Open

tig wants to merge 1 commit into
mainfrom
claude/esphome-silico-integration-ji9tag

Conversation

@tig

@tig tig commented Jul 31, 2026

Copy link
Copy Markdown
Owner

Reverse-engineered from reviewing the tig/xuss-c first ship against its spec.md. Five unrelated findings had one mechanism behind them: agents extend what the plate ships and ignore what the plate only says.

Closes nothing outright; advances #111, #112, #113, #114.

The escape hatch now exists in code, not just in prose

The plate's AGENTS.md said "Escape hatch (repl / reboot) is a product requirement." The plate's main.c implemented identity only. xuss-c inherited the code — same function name, same shape — and shipped without a door, which its own spec says fails L1.

Compounding it, silico inspect knocks identity and nothing else, so the one command silico verifies is the one that got built.

  • gcu_parse_command / gcu_handle_command in src/domain.c
  • repl parks outputs and sets parked; gcu_tick then stops driving the face until next boot
  • reboot parks, acks, and sets reboot_pending — main.c flushes the reply then calls hal->reboot, so the host sees the ack
  • unknown input fails closed with a short err, not a help essay
  • blank lines produce no reply (no chatter on the link)
  • new optional HAL hooks park_outputs and reboot; hal_board.c implements both

Parsing moved across the HAL seam

Dispatch used to live in firmware/main/main.c — device-side, outside the host build. That made spec-layer L0's "protocol parsing" requirement structurally impossible to satisfy, on any product using this plate. main.c now only moves bytes.

A third test file

host/test_protocol.c covers identity, repl parking (including that ticks stay parked afterward), deferred reboot, whole-token matching (identityX and rep must not match), blank lines, and an undersized reply buffer being refused rather than overflowed.

It exists partly because three files read differently than two: xuss-c shipped exactly the two test files the plate shipped, and its spec's normative tables — theme cycle order, song state machine, button map — got none. The plate should read as a floor.

[[deploy.data]] is discoverable now

The feature has existed since #79, with tests, and its docstring says it exists so there is "no free-form esptool essay." It appeared nowhere in this plate. xuss-c consequently hand-rolled a two-command gen_spiffs_image.py + esptool write_flash wall into its operator docs — and because the asset ships out-of-band, its firmware grew a fallback that plays the boot riff when the track is missing, where the spec requires a clear refusal.

Commented block in silico.toml, plus an install/README.md note about not pasting raw write_flash into operator docs.

knowledge/esp32-audio.md — the compounding miss

The C section presented dac_continuous as the native path. On M5GO that is measurably wrong: descriptor timeouts under load on multi-minute streams. Six audio commits of thrash in xuss-c produced durable board truths that were never written back, so the next agent would have repeated them.

Added, with the two paths now presented as a board-dependent choice rather than one answer:

  • I2S → built-in DAC config that worked (RIGHT_LEFT, BOTH_EN, 8×256 DMA, APLL)
  • the 2× fast trap: I2S_CHANNEL_FMT_ONLY_RIGHT clocks content ~2× fast and the symptom mimics a sample-rate bug — the natural wrong fix is halving the rate. Suspect channel format before touching the asset rate.
  • amp gain ceiling (4× distorts, 2× is desk-loud) and that gain belongs in the defaults table
  • parking at mid then hard-writing zero re-introduces the click that parking at mid avoids
  • four friction-log bullets

AGENTS.md

product-path proves defaults are loaded, not that normative behavior is covered — different claims. Plus display HAL granularity: compose a region and blit once; a 5×7 glyph drawn per-pixel is ~35 SPI transactions per character, and a six-row readout at 10 Hz becomes tens of thousands per second, which is what fights an audio DMA feeder and reaches the operator as stutter.

Verification

$ cmake --build build/host --target host_test
1/3 Test #1: defaults .......   Passed
2/3 Test #2: time64 .........   Passed
3/3 Test #3: protocol .......   Passed
100% tests passed, 0 tests failed out of 3

$ python -m pytest tests/ -q
213 passed

Also compiled clean under -Wall -Wextra -Wpedantic -std=c11.

Scope

Plate and knowledge only — no CLI behavior change. The real fix for the inspect gap is [protocol].required in silico.toml so inspect exercises a declared surface instead of just identity; that is left open in #111 because it needs a maintainer call on warn-vs-fail.

🤖 Generated with Claude Code

https://claude.ai/code/session_01PDzUSCHwM6cs47eHGXX5y9


Generated by Claude Code

…rd I2S DAC truths

Reverse-engineered from the tig/xuss-c first ship. The pattern across every
finding was the same: the plate is the spec the agent actually follows.

- Link surface: the plate required `repl` / `reboot` in prose but shipped only
  `identity` in code, so the product inherited the code and shipped without an
  escape hatch. Parsing/dispatch moves to src/domain.c (gcu_parse_command /
  gcu_handle_command) with park_outputs + reboot HAL hooks; main.c now only
  moves bytes. `repl` parks outputs and stops the tick driving them; `reboot`
  defers the reset so the ack flushes first; unknown input fails closed.

- Host coverage: dispatch in firmware/main.c was device-only code, so "protocol
  parsing" could never be part of a host-green claim. host/test_protocol.c
  covers identity, repl parking, deferred reboot, blank lines, whole-token
  matching, and undersized buffers. Third test file exists partly so the plate
  reads as a floor rather than a ceiling — the product shipped exactly the two
  test files the plate happened to ship.

- Deploy assets: [[deploy.data]] has existed since #79 but appeared nowhere in
  this plate, so the product hand-rolled the esptool write_flash wall the
  feature was built to delete. Commented example in silico.toml plus a note in
  install/README.md.

- knowledge/esp32-audio.md: the C section presented dac_continuous as the path,
  which is measurably wrong on M5GO (descriptor timeouts under load). Adds the
  I2S -> built-in DAC recipe and the ONLY_RIGHT 2x-fast trap that mimics a
  sample-rate bug, plus amp gain ceiling and the park-at-mid-then-zero click.

- AGENTS.md: product-path proves defaults are loaded, not that normative
  behavior is covered; display HAL granularity (compose-and-blit, not per-pixel
  fill_rect text).

Host gate green: 3/3 ctest on the plate, full silico pytest suite passing.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PDzUSCHwM6cs47eHGXX5y9
@cursor

cursor Bot commented Jul 31, 2026

Copy link
Copy Markdown

Bugbot is not enabled for your account, so this pull request was not reviewed.

Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 6f1550f405

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

char id[64];
gcu_identity_line(id, (int)sizeof id);
printf("%s\n", id);
if (gcu_handle_command(st, line, reply, (int)sizeof reply)) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Discard the remainder of an overlong serial line

When a serial line exceeds 47 bytes, the existing overflow branch resets n to zero but continues collecting the suffix, and this new dispatch call treats that suffix as a complete command. Thus an oversized or malformed line ending in repl or reboot can unexpectedly park the product or reset it instead of failing closed; retain an overflow flag and ignore all bytes until the next delimiter.

AGENTS.md reference: silico/plates/gcu-c/AGENTS.md:L105-L110

Useful? React with 👍 / 👎.

Comment on lines +105 to +108
st.reboot_pending = 0;
if (hal && hal->reboot) {
hal->reboot(hal);
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Refuse reboot when the HAL cannot reset

When a derived product leaves the explicitly optional hal->reboot hook as NULL, the command has already replied ok reboot and parked its outputs, but this block clears reboot_pending and performs no reset. The device then remains parked indefinitely while the host believes it rebooted, defeating the escape hatch; reject reboot before acknowledging it when the hook is unavailable.

AGENTS.md reference: silico/plates/gcu-c/AGENTS.md:L99-L101

Useful? React with 👍 / 👎.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants