Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 43 additions & 2 deletions silico/knowledge/esp32-audio.md
Original file line number Diff line number Diff line change
Expand Up @@ -148,9 +148,42 @@ For product audio that must sound better than bit-bang DAC:

Do not copy keypad/Web-UI surfaces into silico; extract only host techniques that every GCU might need.

## Two C paths to the built-in DAC — pick with the board, not the docs

On `language=c` GCUs there are **two** ways to feed the classic-ESP32 internal DAC, and the better one is board-dependent:

| Path | Use when | Measured trouble |
|------|----------|------------------|
| `dac_continuous` (DMA) | Short-to-medium PCM; boards where it stays fed | **M5GO: descriptor timeouts under load** on multi-minute streams |
| **I2S → built-in DAC** (`I2S_MODE_DAC_BUILT_IN`) | Multi-minute streams, or when `dac_continuous` times out | Channel-format trap below — silently plays **2× fast** |

Do not treat either as *the* C answer. Measure on the board, then record which one you shipped and why. Both sections below are live guidance.

### I2S → built-in DAC (measured on M5GO / classic ESP32)

The reliable long-stream path on M5-class cores. Shape that worked:

```text
mode = I2S_MODE_MASTER | I2S_MODE_TX | I2S_MODE_DAC_BUILT_IN
bits_per_sample = 16 # DAC takes the top 8 bits
channel_format = I2S_CHANNEL_FMT_RIGHT_LEFT # NOT ONLY_RIGHT — see trap
communication_fmt = I2S_COMM_FORMAT_STAND_MSB
dma_buf_count = 8
dma_buf_len = 256
use_apll = true
tx_desc_auto_clear = true
then: i2s_set_dac_mode(I2S_DAC_CHANNEL_BOTH_EN)
```

**The 2× fast trap (costs hours if you do not know it).** With `I2S_CHANNEL_FMT_ONLY_RIGHT`, content clocks about **twice as fast** as its true rate on this path. The symptom looks exactly like a sample-rate bug, so the natural (wrong) fix is to halve the configured rate — which then makes every other rate assumption in the product wrong. **Correct fix:** use `I2S_CHANNEL_FMT_RIGHT_LEFT` and duplicate the mono sample into both L and R slots. Suspect channel format *before* you touch the asset's sample rate.

**Feeding u8 mono.** Expand each unsigned byte to a signed 16-bit frame written to both channels. Digital gain belongs here, and it has a ceiling: on the M5 amp **4× clipped audibly; 2× was desk-loud and clean**. Gain is product behavior — put the factor in the shipped defaults table, not a literal buried in the expander, and keep the comment honest about which factor actually shipped.

**Stopping cleanly.** Park to **u8 mid (128 → `0x8000` in a 16-bit frame)** and stop there if more audio follows. Writing mid and then hard-writing **zero** steps the DAC to rail and **clicks** — the same cliff described under *Silence is hard*, just one abstraction up. Ramp mid → 0 if you truly want the pin at 0 V.

## ESP-IDF `dac_continuous` (C / native path) — queue depth is product behavior

Native ESP-IDF continuous DAC (DMA) is the right path for multi-minute PCM on `language=c` GCUs. **Queue depth is audible product behavior**, not a free tuning knob (#79 field report).
Native ESP-IDF continuous DAC (DMA) is a good path for PCM on `language=c` GCUs **where it stays fed** — see the table above before committing to it on an M5-class board. **Queue depth is audible product behavior**, not a free tuning knob (#79 field report).

### Queue depth vs pause / teardown

Expand Down Expand Up @@ -196,10 +229,14 @@ returns.

### Agent checklist (C audio)

- [ ] Path chosen by **measurement** on this board (`dac_continuous` vs I2S→DAC), and the reason recorded.
- [ ] If I2S→DAC: `RIGHT_LEFT` + mono duplicated. Pitch wrong? Suspect **channel format before sample rate**.
- [ ] Digital gain measured against the board's amp, and stored in the shipped defaults table.
- [ ] Queue depth chosen for **pause UX**, not max underrun margin alone.
- [ ] Stop path **drains** before disable.
- [ ] Stop path **drains** before disable; park at **mid**, never hard-zero after mid.
- [ ] Rate from header + probe validation.
- [ ] Operator forewarning before long boot riffs (AGENTS: announce surprising metal effects).
- [ ] UI paint cost checked against the feeder: per-pixel `fill_rect` text rendering can issue tens of thousands of SPI transactions per second and starve the audio task. Compose regions and blit once.

## Deploy / CDC interaction

Expand Down Expand Up @@ -240,3 +277,7 @@ Keep results in the GCU (script + notes). Promote durable bullets **here**.
- NeoPixel side-strip updates interleaved with DAC sample loops coupled noise into the amp; static sides during long PCM helped.
- Streaming u8 mono from flash in ~1 KiB chunks with button polls between/inside chunks allows pause without loading the whole file into RAM.
- `period = 1_000_000 // 11025` floors to 90 µs (~0.78% fast); use rounded period or remainder-accumulator (`base` + distribute `1e6 % hz`) for correct pitch/duration.
- **C / M5GO:** `dac_continuous` hit descriptor timeouts under load on multi-minute streams; I2S → built-in DAC (`I2S_MODE_DAC_BUILT_IN`, 8 × 256 DMA, APLL) streamed the full track reliably (measured 2026-07).
- **C / classic ESP32:** `I2S_CHANNEL_FMT_ONLY_RIGHT` on the built-in-DAC path clocked content **~2× fast**; `I2S_CHANNEL_FMT_RIGHT_LEFT` with the mono sample duplicated into both slots fixed pitch. The symptom mimics a sample-rate error and burned several commits chasing 44.1 k vs 22.05 k before the channel format was found.
- **C / M5 amp:** 4× digital gain on u8 PCM distorted; 2× was desk-loud and clean.
- Parking an I2S DAC at u8 mid and then writing a zero frame re-introduced the click that parking at mid exists to avoid.
27 changes: 25 additions & 2 deletions silico/plates/gcu-c/AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ exactly that; keep that seed when you extend the domain.
`silico doctor` reads it; check before installing another IDF.
- More: `silico/knowledge/macos-codex-esp-idf.md`.

## Identity (required on the link)
## Link command surface (identity + escape hatch)

**Boot-print alone is not enough** for `silico inspect` after a greeting or banner scrolls past (#78 / #79). The image **must answer** the host word `identity` (CR/LF framed) with:

Expand All @@ -98,4 +98,27 @@ fw_name=GCU fw_version=0.0.1

Plate `main.c` shows the pattern: print once at boot **and** respond when the host knocks. A boot-print-only app is invisible to inspect as soon as the banner is gone.

Escape hatch (`repl` / `reboot`) is a product requirement for reclaim without hard reset when possible.
`identity`, `repl`, and `reboot` all ship in the plate. The escape hatch is **not optional decoration**: `repl` parks outputs and releases the console so a host can redeploy without hardware gymnastics, and `reboot` parks then hard-resets. A build without the door cannot be reclaimed on a bench.

### Parsing lives in the domain, not in main.c

`gcu_parse_command` / `gcu_handle_command` are in `src/domain.c`; `firmware/main/main.c` only moves bytes. Keep it that way:

1. `silico inspect` knocks **`identity`** and nothing else. Whatever else your product declares is verified by **your** host tests or not at all — so put the surface where a host test can reach it.
2. A dispatcher inside `firmware/` is device-only code, which means "protocol parsing" can never be part of a host-green claim.

`host/test_protocol.c` covers identity, `repl` parking, deferred `reboot`, blank lines, and unknown input failing closed. **Add a row for every command your product spec declares** — including the ones that must be refused. If your spec says the listed commands are the complete surface, do not quietly ship a fourth one (diagnostic capture hooks included).

Outputs your board drives get quieted in the HAL's `park_outputs` — extend it as the product grows a speaker, strips, or actuators. `repl` handing back a console while the product is still singing is a defect.

## Product defaults and host coverage

`silico product-path` proves a host scenario **loads** `[host].product_defaults` — it does not prove your spec's normative behavior is covered. Those are different claims. When the product spec has normative tables (button map, state machine, cycle order, screen layout), give each one a host test; the plate's `host/` files are a **floor to build on, not a ceiling**. Three shipped test files is what the plate happens to need, not what your product needs.

Anything unmeasurable in the spec ("smooth", "comfortable", "a debounce") becomes a number the moment you implement it. Put it in `include/gcu/defaults.h` and tell the operator you chose it — do not let the choice exist only in your head.

## Display HAL granularity (screens)

If the product face is a screen, the HAL's drawing primitives are a performance contract, not a formality. Compose a region into a buffer and **blit it once**; do not render text or glyphs by calling a per-pixel `fill_rect` in a loop. A 5x7 glyph drawn pixel-by-pixel is ~35 SPI transactions (and ~35 allocations if the backend allocates per call) **per character** — a six-row sensor readout at 10 Hz becomes tens of thousands of transactions per second, which is exactly the load that fights an audio DMA feeder and shows up as stutter the operator can hear.

Keep partial paints regional (eye only, banner strip only, value fields only) and reserve full-screen clears for mode changes.
16 changes: 16 additions & 0 deletions silico/plates/gcu-c/firmware/main/hal_board.c
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
#include "hal_board.h"

#include "driver/gpio.h"
#include "esp_system.h"
#include "esp_timer.h"
#include "freertos/FreeRTOS.h"
#include "freertos/task.h"
Expand All @@ -27,10 +28,25 @@ static int64_t now_ms(gcu_hal_t *self) {
return esp_timer_get_time() / 1000;
}

/* Escape hatch: quiet everything this board drives. Extend as the product
* grows outputs (speaker to parked level, strips off, actuators safe) —
* `repl` must not hand the console back with the product still singing. */
static void park_outputs(gcu_hal_t *self) {
(void)self;
gpio_set_level(GCU_LED_GPIO, 0);
}

static void board_reboot(gcu_hal_t *self) {
(void)self;
esp_restart();
}

static gcu_hal_t board_hal = {
.set_led = set_led,
.delay_ms = delay_ms,
.now_ms = now_ms,
.park_outputs = park_outputs,
.reboot = board_reboot,
};

gcu_hal_t *gcu_make_board_hal(void) {
Expand Down
28 changes: 18 additions & 10 deletions silico/plates/gcu-c/firmware/main/main.c
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,15 @@
#include <unistd.h>

/*
* Link plumbing only. Parsing and dispatch live in portable domain code
* (gcu_handle_command) so the command surface is host-testable — this file
* moves bytes, it does not decide what a command means.
*
* Identity on the link (#78 / #79): boot-print alone is not enough for
* silico inspect after the greeting scrolls past. The app must also answer
* the host word "identity" (CR/LF framed) with fw_name=… fw_version=….
* `repl` and `reboot` are required alongside it — a build without the
* escape hatch cannot be reclaimed without hardware gymnastics.
*
* stdin MUST be non-blocking before the forever loop. Blocking getchar()
* would park app_main and kill the product face (tick/LED) until a host
Expand All @@ -35,7 +41,7 @@ static void stdin_set_nonblocking(void) {
}
}

static void drain_identity_command(void) {
static void drain_link_commands(gcu_state_t *st) {
static char line[48];
static int n;
int c;
Expand All @@ -48,15 +54,10 @@ static void drain_identity_command(void) {
while ((c = getchar()) != EOF) {
if (c == '\r' || c == '\n') {
if (n > 0) {
char reply[80];
line[n] = '\0';
char *p = line;
while (*p && isspace((unsigned char)*p)) {
p++;
}
if (strcmp(p, "identity") == 0) {
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 👍 / 👎.

printf("%s\n", reply);
fflush(stdout);
}
n = 0;
Expand Down Expand Up @@ -98,7 +99,14 @@ void app_main(void) {

gcu_init(&st, hal);
for (;;) {
drain_identity_command();
drain_link_commands(&st);
if (st.reboot_pending) {
/* Reply already flushed above; outputs already parked by the domain. */
st.reboot_pending = 0;
if (hal && hal->reboot) {
hal->reboot(hal);
}
Comment on lines +105 to +108

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 👍 / 👎.

}
gcu_tick(&st);
if (hal && hal->delay_ms) {
hal->delay_ms(hal, gcu_tick_sleep_ms(&st));
Expand Down
6 changes: 5 additions & 1 deletion silico/plates/gcu-c/host/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,10 @@ add_executable(test_time test_time.c)
target_link_libraries(test_time gcu_core)
add_test(NAME time64 COMMAND test_time)

add_executable(test_protocol test_protocol.c)
target_link_libraries(test_protocol gcu_core)
add_test(NAME protocol COMMAND test_protocol)

# Convenience: cmake --build build/host --target host_test runs ctest.
# Do not name this target "test" — CMake reserves that name when enable_testing() is on.
# Multi-config generators (Visual Studio) need -C; single-config (Ninja) use no -C.
Expand All @@ -29,6 +33,6 @@ else()
endif()
add_custom_target(host_test
COMMAND ${CMAKE_CTEST_COMMAND} --output-on-failure ${_gcu_ctest_config_args}
DEPENDS test_defaults test_time
DEPENDS test_defaults test_time test_protocol
WORKING_DIRECTORY ${CMAKE_BINARY_DIR}
)
165 changes: 165 additions & 0 deletions silico/plates/gcu-c/host/test_protocol.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,165 @@
/* Link command surface — host test (no hardware).
*
* Why this file exists: the escape hatch (`repl` / `reboot`) is a product
* requirement, and an escape hatch that is only ever exercised on metal is
* the one that turns out to be missing at the worst moment. Parsing and
* dispatch live in src/domain.c precisely so this test can run on the host.
*
* Extend this alongside the product's declared command surface: every command
* the product spec lists should have a row here, including the ones that must
* FAIL (unknown input fails closed with a short error, not a help essay).
*/
#include "gcu/defaults.h"
#include "gcu/domain.h"
#include "gcu/hal.h"
#include "gcu/version.h"

#include <stdio.h>
#include <string.h>

static int led_state;
static int parked_calls;
static int reboot_calls;

static void set_led(gcu_hal_t *self, int on) {
(void)self;
led_state = on;
}

static void delay_ms(gcu_hal_t *self, int ms) {
(void)self;
(void)ms;
}

static void park_outputs(gcu_hal_t *self) {
(void)self;
parked_calls++;
}

static void board_reboot(gcu_hal_t *self) {
(void)self;
reboot_calls++;
}

static int fail(const char *msg) {
fprintf(stderr, "FAIL: %s\n", msg);
return 1;
}

int main(void) {
gcu_hal_t hal = {
.set_led = set_led,
.delay_ms = delay_ms,
.park_outputs = park_outputs,
.reboot = board_reboot,
};
gcu_state_t st;
char reply[80];

/* --- parsing: whole tokens, surrounding whitespace tolerated --- */
if (gcu_parse_command("identity") != GCU_CMD_IDENTITY) {
return fail("identity not parsed");
}
if (gcu_parse_command(" repl \r\n") != GCU_CMD_REPL) {
return fail("repl not parsed with surrounding whitespace");
}
if (gcu_parse_command("reboot") != GCU_CMD_REBOOT) {
return fail("reboot not parsed");
}
if (gcu_parse_command("") != GCU_CMD_NONE ||
gcu_parse_command(" ") != GCU_CMD_NONE) {
return fail("blank line should be NONE");
}
if (gcu_parse_command(NULL) != GCU_CMD_NONE) {
return fail("NULL line should be NONE");
}
/* Prefix/substring must not match a command. */
if (gcu_parse_command("identityX") != GCU_CMD_UNKNOWN ||
gcu_parse_command("rep") != GCU_CMD_UNKNOWN) {
return fail("partial token matched a command");
}

/* --- identity --- */
gcu_init(&st, &hal);
if (!gcu_handle_command(&st, "identity", reply, (int)sizeof reply)) {
return fail("identity produced no reply");
}
if (strstr(reply, "fw_name=") == NULL || strstr(reply, "fw_version=") == NULL) {
return fail("identity reply missing fw_name/fw_version");
}
if (st.parked) {
return fail("identity must not park outputs");
}

/* --- blank line: no reply, no chatter on the link --- */
if (gcu_handle_command(&st, " ", reply, (int)sizeof reply)) {
return fail("blank line should produce no reply");
}

/* --- unknown fails closed and short --- */
if (!gcu_handle_command(&st, "sing", reply, (int)sizeof reply)) {
return fail("unknown command produced no reply");
}
if (strncmp(reply, "err", 3) != 0) {
return fail("unknown command should reply with a short error");
}
if (st.parked) {
return fail("unknown command must not park outputs");
}

/* --- repl parks outputs and releases the console --- */
gcu_init(&st, &hal);
led_state = 1;
parked_calls = 0;
if (!gcu_handle_command(&st, "repl", reply, (int)sizeof reply)) {
return fail("repl produced no reply");
}
if (!st.parked) {
return fail("repl did not set parked");
}
if (parked_calls != 1) {
return fail("repl did not call hal park_outputs");
}
if (led_state != 0) {
return fail("repl left the LED driven");
}
if (st.reboot_pending) {
return fail("repl must not request a reboot");
}
/* Parked means parked: further ticks do not resume driving the face. */
led_state = 1;
gcu_tick(&st);
gcu_tick(&st);
if (led_state != 1) {
return fail("tick drove outputs after repl parked them");
}

/* --- reboot parks, acks, and defers the reset to main --- */
gcu_init(&st, &hal);
parked_calls = 0;
reboot_calls = 0;
if (!gcu_handle_command(&st, "reboot", reply, (int)sizeof reply)) {
return fail("reboot produced no reply");
}
if (parked_calls != 1) {
return fail("reboot did not park outputs");
}
if (!st.reboot_pending) {
return fail("reboot did not set reboot_pending");
}
if (reboot_calls != 0) {
return fail("domain must not reset before the reply is flushed");
}

/* --- undersized reply buffer is refused, not overflowed --- */
gcu_init(&st, &hal);
{
char tiny[4];
if (gcu_handle_command(&st, "identity", tiny, (int)sizeof tiny)) {
return fail("undersized buffer should be refused");
}
}

printf("OK protocol identity+repl+reboot+unknown\n");
return 0;
}
Loading
Loading