From 1f341bb95e11a229c69ad73bf49b889d2dc3f6e4 Mon Sep 17 00:00:00 2001 From: Tig Date: Fri, 24 Jul 2026 18:46:24 -0600 Subject: [PATCH 1/3] Fold ESP32-C first-ship learnings into toolchain, doctor, plate, knowledge (#87) - c_toolchain: discover ~/.espressif/idf-env.json (idf_tools.py dict schema) as a catalog fallback after EIM; surface it in doctor lines and env --print (bash export.sh activation hint); treat a missing eim_idf.json path as no catalog instead of echoing it. - doctor: pyexpat linkage health check (Homebrew python rot -> uv hint); cmake off-PATH probe (/opt/homebrew/bin, /usr/local/bin); language=c ESP-IDF summary line now uses the same discovery as the section below. - plate gcu-c: int64_t now_ms HAL hook (long is 32-bit on Xtensa), esp_timer-backed board impl, wall-clock LED blink with tick fallback, clearerr(stdin) after non-blocking identity drain, host/test_time.c seeding the clock past 2^31 wired into the host gate. - knowledge: dac_continuous stop/done-flag/position-vs-backlog, clearerr footgun row, measured ILI9342C init (MADCTL 0x08, INVON, 26.7 MHz), SK6812-via-RMT note, SPIFFS asset partition + regen-script pattern, macOS venv-shadowing/pyexpat/idf-env.json/sudo notes; AGENTS workflow- scope push gotcha; plate AGENTS time contract. Tests first: tests/test_doctor_host.py (new), idf-env cases in tests/test_c_toolchain.py, plate host test_time.c. Full suite green. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- AGENTS.md | 11 ++ silico/c_toolchain.py | 171 +++++++++++++++++- silico/doctor.py | 61 ++++++- silico/knowledge/deploy-assets.md | 18 ++ silico/knowledge/esp32-audio.md | 18 ++ silico/knowledge/esp32-lcd-ips.md | 15 ++ silico/knowledge/esp32-usb-serial.md | 1 + silico/knowledge/m5-core.md | 2 +- silico/knowledge/macos-codex-esp-idf.md | 21 +++ silico/plates/gcu-c/AGENTS.md | 17 ++ silico/plates/gcu-c/firmware/main/hal_board.c | 9 + silico/plates/gcu-c/firmware/main/main.c | 5 +- silico/plates/gcu-c/host/CMakeLists.txt | 6 +- silico/plates/gcu-c/host/test_time.c | 93 ++++++++++ silico/plates/gcu-c/include/gcu/domain.h | 3 + silico/plates/gcu-c/include/gcu/hal.h | 7 + silico/plates/gcu-c/src/domain.c | 19 +- tests/test_c_toolchain.py | 122 ++++++++++++- tests/test_doctor_host.py | 55 ++++++ 19 files changed, 630 insertions(+), 24 deletions(-) create mode 100644 silico/plates/gcu-c/host/test_time.c create mode 100644 tests/test_doctor_host.py diff --git a/AGENTS.md b/AGENTS.md index 079baa5..a9e5c06 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -393,6 +393,17 @@ When the operator’s goal is a **fresh first-ship harness run**, all three prac **Anti-pattern:** invent a PR workflow on **tig/xuss**, **tig/xuss-c**, or **tig/xuss-lame** because of leftover harness PR history. **Anti-pattern:** five open PRs titled variations of “first ship scaffold,” “L0 product face,” … with no operator request to split. **Anti-pattern:** choose PRs and never watch CI — that skips the reason PRs exist for that team. + +##### Push rejected: workflow scope + +If the branch being pushed **adds or edits `.github/workflows/*`**, a push +with a token lacking the `workflow` scope is **refused by GitHub** (clear +`refusing to allow ... workflow` error). Ambient CI tokens (`GH_TOKEN` / +`GITHUB_TOKEN`) often lack it. Fix: push with the operator's keyring token — +`KT=$(env -u GH_TOKEN -u GITHUB_TOKEN gh auth token)` then +`git push "https://x-access-token:${KT}@github.com//.git" ` — +and scrub the tokenized remote/upstream afterwards. Do not drop the workflow +file to make the push pass. **Anti-pattern:** first-ship mutate on a practice GCU whose `origin/main` tip is not the product-only clean start, without telling the operator. **Anti-pattern:** tip is clean start (docs only) but the agent cherry-picks or replays a previous attempt’s display/audio/UI firmware from older commits / other branches / “last session worked” — that is **past-HEAD salvage** (see **Product truth is HEAD**). Verboten. diff --git a/silico/c_toolchain.py b/silico/c_toolchain.py index f8959f8..cbab04f 100644 --- a/silico/c_toolchain.py +++ b/silico/c_toolchain.py @@ -1,8 +1,18 @@ -"""Windows / Espressif EIM C-toolchain discovery (language=c host path). +"""Espressif C-toolchain discovery (language=c host path). Agents should not hand-parse ``eim_idf.json`` or re-probe ``C:\\Espressif\\tools`` every shell. ``silico doctor`` and ``silico env --print`` surface activation scripts and resolved tool paths (tig/silico#79). + +Two catalogs exist in the field: + +* **EIM** (``eim_idf.json``): list-shaped ``idfInstalled`` with activation + scripts (Windows-first). +* **idf_tools.py** (``~/.espressif/idf-env.json``): dict-shaped + ``idfInstalled`` keyed by install id — the normal macOS/Linux + ``install.sh`` result. A pre-installed IDF registered here is invisible on + PATH until ``export.sh`` is sourced; discovery must find it before + concluding "no ESP-IDF" (tig/silico#87). """ from __future__ import annotations @@ -42,6 +52,7 @@ class CToolchainReport: ninja: str | None = None idf_py: str | None = None eim_json: Path | None = None + idf_env_json: Path | None = None def eim_json_search_paths( @@ -119,7 +130,7 @@ def load_eim_installs( ) -> tuple[Path | None, list[IdfInstall], str | None]: """Load installs from *path* or auto-discovered eim_idf.json.""" p = path or find_eim_json(env=env, extra=extra) - if p is None: + if p is None or not p.is_file(): return None, [], None try: data = json.loads(p.read_text(encoding="utf-8")) @@ -144,6 +155,130 @@ def select_install( return installs[0] +# --- idf_tools.py catalog (~/.espressif/idf-env.json) — macOS/Linux (#87) --- + + +def idf_env_search_paths( + *, + env: dict[str, str] | None = None, + extra: list[Path] | None = None, +) -> list[Path]: + """Ordered paths to probe for idf_tools.py's idf-env.json.""" + env = env if env is not None else os.environ + out: list[Path] = [] + if extra: + out.extend(extra) + tools = env.get("IDF_TOOLS_PATH") + if tools: + out.append(Path(tools) / "idf-env.json") + out.append(Path.home() / ".espressif" / "idf-env.json") + seen: set[str] = set() + uniq: list[Path] = [] + for p in out: + key = str(p).lower() + if key not in seen: + seen.add(key) + uniq.append(p) + return uniq + + +def find_idf_env_json( + *, + env: dict[str, str] | None = None, + extra: list[Path] | None = None, +) -> Path | None: + for p in idf_env_search_paths(env=env, extra=extra): + if p.is_file(): + return p + return None + + +def _python_env_for(espressif_root: Path | None, version: str) -> str: + """Best python under /python_env for an IDF *version* (e.g. '5.3').""" + if espressif_root is None: + return "" + env_root = espressif_root / "python_env" + if not env_root.is_dir(): + return "" + exact: list[Path] = [] + other: list[Path] = [] + for d in sorted(env_root.iterdir()): + if not d.is_dir(): + continue + py = d / "bin" / "python" + if not py.is_file(): + py = d / "Scripts" / "python.exe" + if not py.is_file(): + continue + if version and d.name.startswith(f"idf{version}_"): + exact.append(py) + else: + other.append(py) + # Prefer an env matching the install's version; latest name wins either way. + if exact: + return str(exact[-1]) + if other: + return str(other[-1]) + return "" + + +def parse_idf_env_json( + data: dict, + *, + espressif_root: Path | None = None, +) -> tuple[list[IdfInstall], str | None]: + """Parse idf_tools.py idf-env.json body → (installs, selected_id). + + Schema differs from EIM: ``idfInstalled`` is a **dict** keyed by install + id, each value carrying ``version`` and ``path``. The bash activation is + the install's ``export.sh`` (there is no activationScript field). + """ + raw = data.get("idfInstalled") + installs: list[IdfInstall] = [] + if isinstance(raw, dict): + for key, item in raw.items(): + if not isinstance(item, dict): + continue + path = str(item.get("path") or "").strip() + if not path: + continue + version = str(item.get("version") or "").strip() + export_sh = Path(path) / "export.sh" + installs.append( + IdfInstall( + name=f"ESP-IDF {version}".strip() if version else path, + path=path, + idf_tools_path=str(espressif_root) if espressif_root else "", + activation_script=str(export_sh) if export_sh.is_file() else "", + python=_python_env_for(espressif_root, version), + id=str(key), + ) + ) + selected = data.get("idfSelectedId") + selected_id = str(selected).strip() if selected else None + return installs, selected_id + + +def load_idf_env_installs( + path: Path | None = None, + *, + env: dict[str, str] | None = None, + extra: list[Path] | None = None, +) -> tuple[Path | None, list[IdfInstall], str | None]: + """Load installs from *path* or auto-discovered idf-env.json.""" + p = path or find_idf_env_json(env=env, extra=extra) + if p is None: + return None, [], None + try: + data = json.loads(p.read_text(encoding="utf-8")) + except (OSError, json.JSONDecodeError): + return p, [], None + if not isinstance(data, dict): + return p, [], None + installs, selected_id = parse_idf_env_json(data, espressif_root=p.parent) + return p, installs, selected_id + + def _which_under(tools_root: Path | None, name: str) -> str | None: """Find *name* on PATH or under Espressif tools trees. @@ -204,9 +339,18 @@ def discover_c_toolchain( eim_json, env=env, extra=extra_json_paths ) selected = select_install(installs, selected_id) + + # Fall back to idf_tools.py's ~/.espressif/idf-env.json (macOS/Linux, #87): + # a pre-installed IDF lives there without touching PATH or IDF_PATH. + idf_env_path: Path | None = None + if selected is None: + idf_env_path, env_installs, env_selected_id = load_idf_env_installs(env=env) + if env_installs: + installs = env_installs + selected = select_install(env_installs, env_selected_id) cmake, ninja, idf_py = resolve_tools(selected) - # Fall back to env IDF_PATH if EIM missing but IDF is active + # Fall back to env IDF_PATH if catalogs missing but IDF is active if selected is None and env.get("IDF_PATH"): selected = IdfInstall( name="IDF_PATH", @@ -226,6 +370,7 @@ def discover_c_toolchain( ninja=ninja, idf_py=idf_py, eim_json=path, + idf_env_json=idf_env_path, ) return report @@ -240,13 +385,16 @@ def doctor_c_toolchain_lines( lines: list[str] = [] if r.eim_json: lines.append(f"EIM catalog: {r.eim_json}") + elif r.idf_env_json: + lines.append(f"IDF catalog: {r.idf_env_json} (idf_tools.py install.sh)") else: lines.append( - "INFO: no eim_idf.json found (checked IDF_TOOLS_PATH and C:\\Espressif\\tools). " - "Install Espressif EIM or export IDF_PATH." + "INFO: no ESP-IDF catalog found (checked eim_idf.json under " + "IDF_TOOLS_PATH / C:\\Espressif\\tools, and ~/.espressif/idf-env.json). " + "Install ESP-IDF (install.sh / EIM) or export IDF_PATH." ) if r.installs: - lines.append(f"IDF installs known to EIM: {len(r.installs)}") + lines.append(f"IDF installs known: {len(r.installs)}") for inst in r.installs: mark = " (selected)" if r.selected and inst.id == r.selected.id else "" lines.append(f" - {inst.name}: {inst.path}{mark}") @@ -254,9 +402,14 @@ def doctor_c_toolchain_lines( lines.append(f"Active IDF: {r.selected.name} @ {r.selected.path}") if r.selected.activation_script: lines.append(f" activation: {r.selected.activation_script}") - lines.append( - " PowerShell: . '" + r.selected.activation_script.replace("'", "''") + "'" - ) + if r.selected.activation_script.endswith(".sh"): + lines.append( + ' bash/zsh: . "' + r.selected.activation_script + '"' + ) + else: + lines.append( + " PowerShell: . '" + r.selected.activation_script.replace("'", "''") + "'" + ) if r.selected.idf_tools_path: lines.append(f" IDF_TOOLS_PATH: {r.selected.idf_tools_path}") if r.selected.python: diff --git a/silico/doctor.py b/silico/doctor.py index 7317654..0299768 100644 --- a/silico/doctor.py +++ b/silico/doctor.py @@ -2,6 +2,7 @@ from __future__ import annotations +import importlib import shutil import sys from dataclasses import dataclass, field @@ -22,6 +23,43 @@ class DoctorReport: lines: list[str] = field(default_factory=list) +def host_python_health_lines(*, import_module=None) -> tuple[bool, list[str]]: + """Check stdlib C-extension linkage that ESP-IDF tooling depends on (#87). + + Homebrew pythons can rot (pyexpat linked against a removed libexpat), + which breaks idf_tools.py / install.sh in confusing ways. Surface it + here with an actionable fix instead of letting ESP-IDF installs fail. + """ + imp = import_module or importlib.import_module + try: + imp("pyexpat") + except Exception as e: # ImportError or loader-level OSError + return False, [ + f"WARN: stdlib pyexpat is broken ({e}) — typical Homebrew python " + "linkage rot; ESP-IDF installers need it. Fix: use a uv-managed " + "interpreter (`uv venv --python 3.11`) or reinstall python.", + ] + return True, ["OK: pyexpat imports (stdlib XML linkage healthy)"] + + +_CMAKE_PROBE_DIRS = (Path("/opt/homebrew/bin"), Path("/usr/local/bin")) + + +def cmake_hint_lines(*, which=shutil.which, probe_dirs=None) -> list[str]: + """cmake presence for the C host gate, with an off-PATH rescue hint (#87).""" + if which("cmake"): + return ["OK: cmake on PATH (C host gate)"] + dirs = _CMAKE_PROBE_DIRS if probe_dirs is None else probe_dirs + for d in dirs: + cand = Path(d) / "cmake" + if cand.is_file(): + return [ + f"WARN: cmake not on PATH but found at {cand} — add {d} to PATH " + '(e.g. `eval "$(/opt/homebrew/bin/brew shellenv)"`).' + ] + return ["WARN: cmake not on PATH (needed for C host gate) — install via brew/apt or EIM"] + + def run_doctor(*, root: Path | None = None) -> DoctorReport: lines: list[str] = [] ok = True @@ -82,6 +120,8 @@ def run_doctor(*, root: Path | None = None) -> DoctorReport: ok = False else: lines.append("OK: Python >= 3.11") + _, py_health = host_python_health_lines() + lines.extend(py_health) if shutil.which("git"): lines.append("OK: git on PATH") @@ -95,19 +135,22 @@ def run_doctor(*, root: Path | None = None) -> DoctorReport: # Tooling hints for C intent even if config has FAILs (language still "c"). if cfg.language == "c": - if idf_py_available(): + # One discovery pass drives both the summary line and the section + # below, so they can't contradict each other (#87). + from silico.c_toolchain import discover_c_toolchain, doctor_c_toolchain_lines + + tc = discover_c_toolchain() + if tc.idf_py: + lines.append(f"OK: ESP-IDF tools available ({tc.idf_py})") + elif idf_py_available(): lines.append("OK: ESP-IDF tools available (idf.py or IDF_PATH)") else: lines.append( - "WARN: ESP-IDF tools not found — language=c deploy needs idf.py / IDF_PATH" + "WARN: ESP-IDF tools not found — language=c deploy needs idf.py " + "(PATH, IDF_PATH, EIM, or ~/.espressif/idf-env.json)" ) - if shutil.which("cmake"): - lines.append("OK: cmake on PATH (C host gate)") - else: - lines.append("WARN: cmake not on PATH (needed for C host gate)") - # EIM / Espressif tool roots (#79) — resolved paths, not hand-parsed JSON. - from silico.c_toolchain import doctor_c_toolchain_lines - + lines.extend(cmake_hint_lines()) + # EIM / idf_tools.py catalogs (#79, #87) — resolved paths, not hand-parsed JSON. lines.append("--- C toolchain (EIM / IDF) ---") lines.extend(doctor_c_toolchain_lines()) else: diff --git a/silico/knowledge/deploy-assets.md b/silico/knowledge/deploy-assets.md index 0db6a45..fb27a43 100644 --- a/silico/knowledge/deploy-assets.md +++ b/silico/knowledge/deploy-assets.md @@ -19,3 +19,21 @@ Host notes for GCUs that ship **binary files** next to firmware (PCM riffs, font 1. List required non-`.py` assets and expected byte sizes in the GCU (README or deploy notes). 2. After deploy, verify each large asset size before operator audio acceptance. 3. If you invent a more reliable upload path, extend **this file** or improve `silico deploy` — do not leave the recipe only in chat. + +## ESP-IDF path: SPIFFS partition (language=c) + +Under ESP-IDF there is no mpremote filesystem — ship multi-MB assets in a +dedicated flash partition instead of embedding them in the app binary: + +1. Add a `storage` **SPIFFS** partition to `partitions.csv` sized for the + asset (a 16 MB flash module fits app + multi-MB audio comfortably; + confirm the module size before assuming >4 MB). +2. `spiffs_create_partition_image(storage FLASH_IN_PROJECT)` in the + component CMake builds and flashes the image with `idf.py flash`. +3. **Do not commit the generated binary asset.** Keep the *source* asset or + a regeneration script (e.g. `tools/make_song.sh` ffmpeg → raw u8 PCM) in + the repo, gitignore the output, and document “regenerate before first + build on a fresh checkout” in the GCU README. +4. Firmware fails closed on missing/zero-length asset (rule 4 above) — a + fresh checkout that skipped regeneration must show a clear error, not + silence. diff --git a/silico/knowledge/esp32-audio.md b/silico/knowledge/esp32-audio.md index 00403f6..53feade 100644 --- a/silico/knowledge/esp32-audio.md +++ b/silico/knowledge/esp32-audio.md @@ -170,6 +170,24 @@ Native ESP-IDF continuous DAC (DMA) is the right path for multi-minute PCM on `l Hard-disable mid-queue = click or multi-hundred-ms “catch-up” dump. +**Stop must actually stop.** A “stopped” player that leaves the channel +enabled keeps the last DMA sample on the pin — a DC level biasing the amp +(warm amp, hiss, faster battery drain). After drain, either disable the +channel or write a silence/mid-level buffer and verify the pin parks. + +**Pause position ≠ submitted position.** If pause records how much you +*submitted*, resume skips whatever was still in the DMA backlog when you +stopped feeding (an audible gap at deep queues). Track the *played* position +(bytes drained / conversion-done callback), or keep the queue shallow enough +that the difference is inaudible. + +**Signal “done” on every task exit path.** The audio task must set its +done/idle flag (and release the channel) on *all* exits — end of file, +stop request, **and** open/enable/config failure. A task that errors out +before the main loop and never signals leaves the UI stuck on “playing” +forever. Structure as single exit (`goto out` / cleanup block), not early +returns. + ### Sample rate from the asset - Clock the DAC continuous config from the **asset header rate** (or sidecar), not a hard-coded constant that drifts from the file. diff --git a/silico/knowledge/esp32-lcd-ips.md b/silico/knowledge/esp32-lcd-ips.md index b7ac0e7..9d1e76f 100644 --- a/silico/knowledge/esp32-lcd-ips.md +++ b/silico/knowledge/esp32-lcd-ips.md @@ -34,6 +34,21 @@ Do **not** copy this blindly to every ILI9341 module; **measure**. Document the - Fix **panel path** first; only then tune theme RGB tables. - Pure primaries on LEDs (`(255,0,0)` not `(255,48,48)`) make mismatches obvious. +### Measured: ILI9342C on M5GO / Core (ESP-IDF `esp_lcd`) + +One bench-verified init that produced correct colors and stable SPI (#87): + +- `esp_lcd_new_panel_ili9341`-style driver works for ILI9342C with: + **MADCTL = 0x08** (BGR bit only — no row/col exchange for landscape M5), + **INVON** (`esp_lcd_panel_invert_color(panel, true)`), + no gap, no mirror. +- Pixel clock **26.7 MHz** (80 MHz / 3) was stable; 40 MHz corrupted on + some units/wiring. +- The panel returns no dummy byte on reads — if the driver exposes it, set + the “no dummy” / skip-read-dummy flag rather than fighting shifted reads. +- Landscape 320×240 comes from the panel’s native orientation with this + MADCTL; verify with an asymmetric test pattern before layout work. + ## Partial updates (SPI thrash) Full-panel `fill` every animation tick: diff --git a/silico/knowledge/esp32-usb-serial.md b/silico/knowledge/esp32-usb-serial.md index 94204ae..7e4f140 100644 --- a/silico/knowledge/esp32-usb-serial.md +++ b/silico/knowledge/esp32-usb-serial.md @@ -31,6 +31,7 @@ If inspect/deploy says app owns console / door stayed shut: | Deploy `--verify` then soft-reset into deaf app | Verify used REPL; app may never hear host (#49 race) | | DTR/RTS pulse before C identity knock on CH9102 | Resets into ROM / download; `silico inspect` captures 0 bytes while bare pyserial (dtr=rts=False + `identity`) works (#78) | | Clear RX after boot wait on pulse path | Discards boot-printed `fw_name=` on boot-only C plates (#81 CR) | +| Non-blocking stdio drain without `clearerr(stdin)` | EOF/error flags latch on `FILE*`; next drain sees phantom EOF and the link goes deaf while TX still works (#87) — clear both `clearerr(stdin)` and sticky `EAGAIN` errno after each empty drain | ## C / ESP-IDF identity (language=c) diff --git a/silico/knowledge/m5-core.md b/silico/knowledge/m5-core.md index d3a220f..5920d96 100644 --- a/silico/knowledge/m5-core.md +++ b/silico/knowledge/m5-core.md @@ -8,7 +8,7 @@ Related: [esp32-audio.md](esp32-audio.md), [esp32-lcd-ips.md](esp32-lcd-ips.md), | Surface | Candidate | Notes | |---------|-----------|--------| -| Side status strip | GPIO **15** | Often NeoPixel bus; confirm LED count/order | +| Side status strip | GPIO **15** | **SK6812** ×10 (5/side) on M5GO base; drive via **RMT** under ESP-IDF (`led_strip` component, GRB order) — bit-bang loops fight the flash cache. Updates during tight DAC loops couple amp noise; batch or pause strip refresh while streaming audio | | Speaker | GPIO **25** | DAC1 / amp; see esp32-audio | | IPS | **ILI9342C** on **SPI3** | SCLK **18**, MOSI **23**, CS **14**, DC **27**, RST **33**, BL **32** | | Display flags | invert **ON**, color **BGR** | Wrong flags → washed/inverted colors (see esp32-lcd-ips) | diff --git a/silico/knowledge/macos-codex-esp-idf.md b/silico/knowledge/macos-codex-esp-idf.md index 7ebc4f6..f6f6302 100644 --- a/silico/knowledge/macos-codex-esp-idf.md +++ b/silico/knowledge/macos-codex-esp-idf.md @@ -22,8 +22,29 @@ idf.py --version - `install.sh esp32` with **system Python 3.9** often creates a bad checker/env. - Prepend a **3.11+** (3.12 preferred) interpreter, then install; set `IDF_PYTHON_ENV_PATH` to the resulting `idf*_py3.12_env`. +- **Homebrew python linkage rot:** a brew-installed python can fail with + `pyexpat` / `libexpat` symbol errors, which breaks `install.sh` and + `idf_tools.py` in confusing ways. `silico doctor` now checks pyexpat; + fix by using a **uv-managed interpreter** (`uv venv --python 3.11`) rather + than fighting brew relinks. +- **Project venv shadows the IDF env.** If the GCU/agent venv is on PATH + ahead of the IDF python env, `idf.py` resolves the wrong interpreter and + fails on missing IDF packages. Deactivate (or strip the venv `bin` from + PATH) before `. $IDF_PATH/export.sh`, or invoke the IDF python explicitly. +- **Find an existing install before installing a new one:** + `~/.espressif/idf-env.json` (written by `install.sh`/`idf_tools.py`) lists + installed IDF versions and paths. `silico doctor` / `silico env --print` + read it (dict-keyed `idfInstalled` schema) in addition to EIM's + `eim_idf.json`. Check it before spending 20 minutes re-installing an IDF + that is already on the machine. - `silico doctor` (language=c) and `silico env --print` surface activation and tool paths when EIM/IDF is present. +## Sandbox / sudo + +- Agent sandboxes often have **no interactive sudo**. Anything needing + admin (driver install, `brew install` of casks, udev-ish permissions) + must be front-loaded with the operator; don't discover it mid-flash. + ## Serial - CH9102: keep default identity probe **without** DTR/RTS pulse; do not run `monitor` and `inspect`/`deploy --verify` on the same port in parallel (port busy → clear HINT). diff --git a/silico/plates/gcu-c/AGENTS.md b/silico/plates/gcu-c/AGENTS.md index 8a3b5c3..ba1af78 100644 --- a/silico/plates/gcu-c/AGENTS.md +++ b/silico/plates/gcu-c/AGENTS.md @@ -69,6 +69,23 @@ ESP-IDF must be installed (`idf.py` or `IDF_PATH`). First flash and update flash Portable domain under `include/` + `src/` must not include freertos / esp_* / driver headers. Only stems listed in `[hal].allow_device_headers` (default `hal_board`) may touch device headers. +### Time is int64_t milliseconds + +The HAL clock hook is `int64_t now_ms` (see `include/gcu/hal.h`). On ESP32 +(ILP32) `long` is **32 bits**: millisecond math in `long`/`int` overflows in +under 10 hours and wraps at ~24.8 days. Host `long` is 64-bit, so host tests +only catch this if they seed the clock past 2^31 — `host/test_time.c` does +exactly that; keep that seed when you extend the domain. + +## ESP-IDF environment gotchas + +- If the agent/GCU **venv is on PATH ahead of the IDF python env**, `idf.py` + resolves the wrong interpreter and fails on missing packages. Deactivate or + strip the venv from PATH before `. $IDF_PATH/export.sh`. +- An existing install is usually recorded in `~/.espressif/idf-env.json` — + `silico doctor` reads it; check before installing another IDF. +- More: `silico/knowledge/macos-codex-esp-idf.md`. + ## Identity (required on the link) **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: diff --git a/silico/plates/gcu-c/firmware/main/hal_board.c b/silico/plates/gcu-c/firmware/main/hal_board.c index e343783..0f82443 100644 --- a/silico/plates/gcu-c/firmware/main/hal_board.c +++ b/silico/plates/gcu-c/firmware/main/hal_board.c @@ -2,6 +2,7 @@ #include "hal_board.h" #include "driver/gpio.h" +#include "esp_timer.h" #include "freertos/FreeRTOS.h" #include "freertos/task.h" @@ -19,9 +20,17 @@ static void delay_ms(gcu_hal_t *self, int ms) { vTaskDelay(pdMS_TO_TICKS(ms > 0 ? ms : 1)); } +static int64_t now_ms(gcu_hal_t *self) { + (void)self; + /* esp_timer_get_time() is int64_t µs since boot; keep the division in + * 64-bit. Do NOT narrow to long/int (32-bit here — wraps at ~24.8 days). */ + return esp_timer_get_time() / 1000; +} + static gcu_hal_t board_hal = { .set_led = set_led, .delay_ms = delay_ms, + .now_ms = now_ms, }; gcu_hal_t *gcu_make_board_hal(void) { diff --git a/silico/plates/gcu-c/firmware/main/main.c b/silico/plates/gcu-c/firmware/main/main.c index 57f34f6..70fb164 100644 --- a/silico/plates/gcu-c/firmware/main/main.c +++ b/silico/plates/gcu-c/firmware/main/main.c @@ -69,7 +69,10 @@ static void drain_identity_command(void) { n = 0; /* overflow: drop */ } } - /* Clear sticky errno from EAGAIN/EWOULDBLOCK after empty non-block read. */ + /* Clear sticky stream state after empty non-blocking reads: EOF/error + * flags latch on FILE* and errno keeps EAGAIN. Without both resets the + * NEXT drain can see a phantom EOF and never read again (#87). */ + clearerr(stdin); if (errno == EAGAIN || errno == EWOULDBLOCK) { errno = 0; } diff --git a/silico/plates/gcu-c/host/CMakeLists.txt b/silico/plates/gcu-c/host/CMakeLists.txt index f26c0e4..6b6bb1c 100644 --- a/silico/plates/gcu-c/host/CMakeLists.txt +++ b/silico/plates/gcu-c/host/CMakeLists.txt @@ -14,10 +14,14 @@ add_executable(test_defaults test_defaults.c) target_link_libraries(test_defaults gcu_core) add_test(NAME defaults COMMAND test_defaults) +add_executable(test_time test_time.c) +target_link_libraries(test_time gcu_core) +add_test(NAME time64 COMMAND test_time) + # 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. add_custom_target(host_test COMMAND ${CMAKE_CTEST_COMMAND} --output-on-failure - DEPENDS test_defaults + DEPENDS test_defaults test_time WORKING_DIRECTORY ${CMAKE_BINARY_DIR} ) diff --git a/silico/plates/gcu-c/host/test_time.c b/silico/plates/gcu-c/host/test_time.c new file mode 100644 index 0000000..e30a13b --- /dev/null +++ b/silico/plates/gcu-c/host/test_time.c @@ -0,0 +1,93 @@ +/* Time contract: hal now_ms is int64_t milliseconds (#87). + * + * On ESP32 (Xtensa ILP32) `long` is 32 bits: millisecond math in `long` + * overflows in <10 hours of uptime arithmetic and wraps at ~24.8 days. + * Host `long` is 64-bit, so a host test only catches the trap if it seeds + * the clock PAST 2^31 — which this test does. Keep the seed; do not + * "simplify" it to small numbers. + */ +#include "gcu/defaults.h" +#include "gcu/domain.h" +#include "gcu/hal.h" + +#include +#include + +static int led; +static int led_writes; +static int64_t fake_now; + +static void set_led(gcu_hal_t *self, int on) { + (void)self; + led = on; + led_writes++; +} + +static void delay_ms(gcu_hal_t *self, int ms) { + (void)self; + (void)ms; +} + +static int64_t now_ms(gcu_hal_t *self) { + (void)self; + return fake_now; +} + +int main(void) { + gcu_hal_t hal = {.set_led = set_led, .delay_ms = delay_ms, .now_ms = now_ms}; + gcu_state_t st; + const int period = GCU_DEFAULTS.tick_sleep_ms; + + /* Seed past 2^31 ms so 32-bit intermediate math would go negative. */ + fake_now = (INT64_C(1) << 31) + 12345; + gcu_init(&st, &hal); + + /* Same instant: no blink edge yet. */ + led_writes = 0; + gcu_tick(&st); + if (led_writes != 0) { + fprintf(stderr, "blinked with no elapsed time\n"); + return 1; + } + + /* One period later: exactly one toggle, on. */ + fake_now += period; + gcu_tick(&st); + if (led_writes != 1 || led != 1) { + fprintf(stderr, "expected one on-toggle after %d ms\n", period); + return 1; + } + + /* Sub-period ticks must not toggle (wall clock, not tick count). */ + fake_now += period / 2; + gcu_tick(&st); + if (led_writes != 1) { + fprintf(stderr, "toggled before period elapsed\n"); + return 1; + } + + /* Large jump (past another 2^31 ms) still behaves: one toggle. */ + fake_now += (INT64_C(1) << 31) + period; + gcu_tick(&st); + if (led_writes != 2 || led != 0) { + fprintf(stderr, "64-bit delta mishandled after big clock jump\n"); + return 1; + } + + /* Fallback: no now_ms hook -> every tick toggles (plate default). */ + { + gcu_hal_t hal2 = {.set_led = set_led, .delay_ms = delay_ms}; + gcu_state_t st2; + gcu_init(&st2, &hal2); + led_writes = 0; + gcu_tick(&st2); + gcu_tick(&st2); + if (led_writes != 2) { + fprintf(stderr, "tick fallback broken without now_ms\n"); + return 1; + } + } + + printf("OK time64\n"); + return 0; +} diff --git a/silico/plates/gcu-c/include/gcu/domain.h b/silico/plates/gcu-c/include/gcu/domain.h index 5ed5d45..0d10481 100644 --- a/silico/plates/gcu-c/include/gcu/domain.h +++ b/silico/plates/gcu-c/include/gcu/domain.h @@ -3,11 +3,14 @@ #include "gcu/hal.h" +#include + typedef struct { gcu_hal_t *hal; int tick_count; int led_on; int tick_sleep_ms; + int64_t last_blink_ms; /* wall-clock blink edge; unused without now_ms */ } gcu_state_t; void gcu_identity_line(char *out, int out_len); diff --git a/silico/plates/gcu-c/include/gcu/hal.h b/silico/plates/gcu-c/include/gcu/hal.h index 7831637..6ae4189 100644 --- a/silico/plates/gcu-c/include/gcu/hal.h +++ b/silico/plates/gcu-c/include/gcu/hal.h @@ -3,11 +3,18 @@ /* Portable HAL contract — no device headers here. */ +#include + typedef struct gcu_hal gcu_hal_t; struct gcu_hal { void (*set_led)(gcu_hal_t *self, int on); void (*delay_ms)(gcu_hal_t *self, int ms); + /* Monotonic wall clock in milliseconds since boot, or NULL if the board + * has none. MUST be int64_t: on ESP32 (ILP32) `long` is 32 bits, so + * millisecond math in `long` overflows in <10 h and wraps at ~24.8 days. + * Host `long` is 64-bit and hides the trap — see host/test_time.c. */ + int64_t (*now_ms)(gcu_hal_t *self); }; #endif diff --git a/silico/plates/gcu-c/src/domain.c b/silico/plates/gcu-c/src/domain.c index 2f6a158..076e0b0 100644 --- a/silico/plates/gcu-c/src/domain.c +++ b/silico/plates/gcu-c/src/domain.c @@ -17,14 +17,29 @@ void gcu_init(gcu_state_t *st, gcu_hal_t *hal) { st->tick_count = 0; st->led_on = 0; st->tick_sleep_ms = GCU_DEFAULTS.tick_sleep_ms; + st->last_blink_ms = (hal && hal->now_ms) ? hal->now_ms(hal) : 0; } -void gcu_tick(gcu_state_t *st) { - st->tick_count += 1; +static void toggle_led(gcu_state_t *st) { st->led_on = !st->led_on; if (st->hal && st->hal->set_led) { st->hal->set_led(st->hal, st->led_on); } } +void gcu_tick(gcu_state_t *st) { + st->tick_count += 1; + if (st->hal && st->hal->now_ms) { + /* Wall-clock blink: robust to variable tick latency. All millisecond + * math stays in int64_t — `long` is 32-bit on ESP32 (see hal.h). */ + int64_t now = st->hal->now_ms(st->hal); + if (now - st->last_blink_ms >= (int64_t)st->tick_sleep_ms) { + st->last_blink_ms = now; + toggle_led(st); + } + } else { + toggle_led(st); /* no clock: blink per tick */ + } +} + int gcu_tick_sleep_ms(const gcu_state_t *st) { return st->tick_sleep_ms; } diff --git a/tests/test_c_toolchain.py b/tests/test_c_toolchain.py index 3ed30d6..db6c58a 100644 --- a/tests/test_c_toolchain.py +++ b/tests/test_c_toolchain.py @@ -88,7 +88,9 @@ def test_env_print_powershell_block(tmp_path: Path): assert "PowerShell_profile.ps1" in joined -def test_env_print_missing_is_honest(tmp_path: Path): +def test_env_print_missing_is_honest(tmp_path: Path, monkeypatch): + # Pin home so a real ~/.espressif on the dev machine cannot leak in (#87). + monkeypatch.setattr(Path, "home", staticmethod(lambda: tmp_path)) missing = tmp_path / "nope.json" lines = env_print_block(env={}, eim_json=missing, shell="bash") assert any("No IDF" in ln for ln in lines) @@ -145,3 +147,121 @@ def _discover(**kwargs): assert rc == 0 assert "IDF_PATH" in out assert "v5.3.2" in out or "esp-idf" in out + + +# --- macOS/Linux idf_tools.py catalog: ~/.espressif/idf-env.json (#87) --- + + +def _sample_idf_env() -> dict: + """Real idf_tools.py schema: idfInstalled is a dict keyed by install id.""" + return { + "idfInstalled": { + "/Users/dev/esp/esp-idf-v5.3.2-v5.3": { + "version": "5.3", + "path": "/Users/dev/esp/esp-idf-v5.3.2", + "features": ["core"], + "targets": ["esp32"], + } + } + } + + +def _write_idf_env_tree(tmp_path: Path) -> tuple[Path, Path]: + """Materialize a fake ~/.espressif + IDF tree; returns (idf_env_json, idf_path).""" + espressif = tmp_path / ".espressif" + espressif.mkdir() + idf = tmp_path / "esp" / "esp-idf-v5.3.2" + (idf / "tools").mkdir(parents=True) + (idf / "export.sh").write_text("# export\n", encoding="utf-8") + (idf / "tools" / "idf.py").write_text("# idf.py\n", encoding="utf-8") + py_env = espressif / "python_env" / "idf5.3_py3.12_env" / "bin" + py_env.mkdir(parents=True) + (py_env / "python").write_text("", encoding="utf-8") + body = { + "idfInstalled": { + "id-1": {"version": "5.3", "path": str(idf), "targets": ["esp32"]} + } + } + catalog = espressif / "idf-env.json" + catalog.write_text(json.dumps(body), encoding="utf-8") + return catalog, idf + + +def test_parse_idf_env_json_dict_schema(tmp_path: Path): + from silico.c_toolchain import parse_idf_env_json + + catalog, idf = _write_idf_env_tree(tmp_path) + data = json.loads(catalog.read_text(encoding="utf-8")) + installs, selected_id = parse_idf_env_json(data, espressif_root=catalog.parent) + assert len(installs) == 1 + inst = installs[0] + assert inst.path == str(idf) + assert "5.3" in inst.name + # export.sh is the bash activation for idf_tools installs + assert inst.activation_script.endswith("export.sh") + # python env under ~/.espressif/python_env resolved when it matches the version + assert "idf5.3_py3.12_env" in inst.python + + +def test_parse_idf_env_json_ignores_junk(): + from silico.c_toolchain import parse_idf_env_json + + installs, selected_id = parse_idf_env_json( + {"idfInstalled": {"x": "not-a-dict", "y": {"version": "5.1"}}}, + espressif_root=None, + ) + assert installs == [] + assert selected_id is None + + +def test_find_idf_env_json_home_and_tools_path(tmp_path: Path, monkeypatch): + from silico.c_toolchain import find_idf_env_json + + catalog, _idf = _write_idf_env_tree(tmp_path) + # via IDF_TOOLS_PATH + found = find_idf_env_json(env={"IDF_TOOLS_PATH": str(catalog.parent)}) + assert found == catalog + # via home (~/.espressif) + monkeypatch.setattr(Path, "home", staticmethod(lambda: tmp_path)) + found = find_idf_env_json(env={}) + assert found == catalog + + +def test_discover_falls_back_to_idf_env_json(tmp_path: Path, monkeypatch): + """No EIM catalog + no $IDF_PATH: an existing ~/.espressif install must be found.""" + catalog, idf = _write_idf_env_tree(tmp_path) + monkeypatch.setattr(Path, "home", staticmethod(lambda: tmp_path)) + monkeypatch.setattr("silico.c_toolchain.shutil.which", lambda _n: None) + r = discover_c_toolchain(env={}, eim_json=tmp_path / "no-eim.json") + assert r.ok + assert r.selected is not None + assert r.selected.path == str(idf) + assert r.idf_py is not None and r.idf_py.endswith("idf.py") + + +def test_doctor_lines_surface_idf_env_install(tmp_path: Path, monkeypatch): + catalog, idf = _write_idf_env_tree(tmp_path) + monkeypatch.setattr(Path, "home", staticmethod(lambda: tmp_path)) + monkeypatch.setattr("silico.c_toolchain.shutil.which", lambda _n: None) + lines = doctor_c_toolchain_lines(env={}, eim_json=tmp_path / "no-eim.json") + joined = "\n".join(lines) + assert str(idf) in joined + # actionable bash activation, not a bare "not found" + assert "export.sh" in joined + assert "WARN: no ESP-IDF install resolved" not in joined + + +def test_doctor_missing_message_mentions_idf_env(tmp_path: Path, monkeypatch): + monkeypatch.setattr(Path, "home", staticmethod(lambda: tmp_path)) + lines = doctor_c_toolchain_lines(env={}, eim_json=tmp_path / "no-eim.json") + joined = "\n".join(lines) + assert "idf-env.json" in joined + + +def test_env_print_bash_sources_export_sh(tmp_path: Path, monkeypatch): + catalog, idf = _write_idf_env_tree(tmp_path) + monkeypatch.setattr(Path, "home", staticmethod(lambda: tmp_path)) + lines = env_print_block(env={}, eim_json=tmp_path / "no-eim.json", shell="bash") + joined = "\n".join(lines) + assert f'export IDF_PATH="{idf}"' in joined + assert "export.sh" in joined diff --git a/tests/test_doctor_host.py b/tests/test_doctor_host.py new file mode 100644 index 0000000..a18a452 --- /dev/null +++ b/tests/test_doctor_host.py @@ -0,0 +1,55 @@ +"""Host Python / tool health pre-flight checks (#87 Mac first-ship friction).""" + +from __future__ import annotations + +from pathlib import Path + +from silico.doctor import cmake_hint_lines, host_python_health_lines, run_doctor + + +def test_pyexpat_ok_on_healthy_python(): + ok, lines = host_python_health_lines() + # The test interpreter itself must be healthy — and report OK. + assert ok + assert any("pyexpat" in ln and ln.startswith("OK") for ln in lines) + + +def test_pyexpat_broken_reports_uv_hint(): + def broken(_name): + raise ImportError("symbol not found: _XML_SetAllocTrackerActivationThreshold") + + ok, lines = host_python_health_lines(import_module=broken) + assert not ok + joined = "\n".join(lines) + assert "WARN" in joined + assert "pyexpat" in joined + # Actionable: broken Homebrew python linkage → use a uv-managed interpreter. + assert "uv venv" in joined + + +def test_cmake_hint_when_off_path(tmp_path: Path): + bindir = tmp_path / "opt-homebrew-bin" + bindir.mkdir() + (bindir / "cmake").write_text("", encoding="utf-8") + lines = cmake_hint_lines(which=lambda _n: None, probe_dirs=[bindir]) + joined = "\n".join(lines) + assert "WARN" in joined + assert str(bindir / "cmake") in joined + assert "PATH" in joined + + +def test_cmake_no_hint_when_on_path(tmp_path: Path): + lines = cmake_hint_lines(which=lambda _n: "/usr/bin/cmake", probe_dirs=[tmp_path]) + assert lines == ["OK: cmake on PATH (C host gate)"] + + +def test_cmake_missing_everywhere(tmp_path: Path): + lines = cmake_hint_lines(which=lambda _n: None, probe_dirs=[tmp_path]) + joined = "\n".join(lines) + assert "WARN" in joined and "cmake" in joined + + +def test_run_doctor_includes_python_health(): + report = run_doctor() + joined = "\n".join(report.lines) + assert "pyexpat" in joined From 23f4ea13f625ee388b66085c2440eedcc0b7a01c Mon Sep 17 00:00:00 2001 From: Tig Date: Fri, 24 Jul 2026 18:52:28 -0600 Subject: [PATCH 2/3] Fix python_env selection: numeric version sort, not lexicographic (#87) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review finding on PR #88: sorted() put idf5.3_py3.9_env after idf5.3_py3.12_env, so a stale system-3.9 env from a bad first install shadowed the good 3.12 reinstall — the exact macOS failure mode this PR documents. Digit runs now compare as integers (also fixes the cross-IDF fallback, 5.10 > 5.9). Tests first: two new cases observed failing. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- silico/c_toolchain.py | 17 +++++- tests/test_c_toolchain.py | 26 +++++++++ uv.lock | 119 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 160 insertions(+), 2 deletions(-) create mode 100644 uv.lock diff --git a/silico/c_toolchain.py b/silico/c_toolchain.py index cbab04f..68a05aa 100644 --- a/silico/c_toolchain.py +++ b/silico/c_toolchain.py @@ -19,6 +19,7 @@ import json import os +import re import shutil import sys from dataclasses import dataclass, field @@ -193,6 +194,16 @@ def find_idf_env_json( return None +def _env_dir_key(name: str) -> tuple: + """Numeric sort key for idf_tools python_env dir names. + + Lexicographic order inverts versions (py3.9 > py3.12, idf5.9 > idf5.10); + compare every digit run as an integer instead. + """ + parts = re.split(r"(\d+)", name) + return tuple(int(p) if p.isdigit() else p for p in parts) + + def _python_env_for(espressif_root: Path | None, version: str) -> str: """Best python under /python_env for an IDF *version* (e.g. '5.3').""" if espressif_root is None: @@ -202,7 +213,7 @@ def _python_env_for(espressif_root: Path | None, version: str) -> str: return "" exact: list[Path] = [] other: list[Path] = [] - for d in sorted(env_root.iterdir()): + for d in sorted(env_root.iterdir(), key=lambda p: _env_dir_key(p.name)): if not d.is_dir(): continue py = d / "bin" / "python" @@ -214,7 +225,9 @@ def _python_env_for(espressif_root: Path | None, version: str) -> str: exact.append(py) else: other.append(py) - # Prefer an env matching the install's version; latest name wins either way. + # Prefer an env matching the install's version; newest (numeric) wins + # either way — a stale py3.9 env from a bad first install must not + # shadow the good py3.12 reinstall (PR #88 review). if exact: return str(exact[-1]) if other: diff --git a/tests/test_c_toolchain.py b/tests/test_c_toolchain.py index db6c58a..61a89f5 100644 --- a/tests/test_c_toolchain.py +++ b/tests/test_c_toolchain.py @@ -265,3 +265,29 @@ def test_env_print_bash_sources_export_sh(tmp_path: Path, monkeypatch): joined = "\n".join(lines) assert f'export IDF_PATH="{idf}"' in joined assert "export.sh" in joined + +def test_python_env_prefers_newest_python_not_lexicographic(tmp_path: Path): + # Review finding (PR #88): sorted() is lexicographic, so py3.9 sorts + # AFTER py3.12 and used to win. This is the exact macOS failure mode: + # a bad system-3.9 env installed first, then a good 3.12 reinstall. + from silico.c_toolchain import _python_env_for + + espressif = tmp_path / ".espressif" + for name in ("idf5.3_py3.9_env", "idf5.3_py3.12_env"): + b = espressif / "python_env" / name / "bin" + b.mkdir(parents=True) + (b / "python").write_text("", encoding="utf-8") + assert "py3.12" in _python_env_for(espressif, "5.3") + + +def test_python_env_fallback_prefers_newest_idf(tmp_path: Path): + from silico.c_toolchain import _python_env_for + + espressif = tmp_path / ".espressif" + for name in ("idf5.9_py3.12_env", "idf5.10_py3.12_env"): + b = espressif / "python_env" / name / "bin" + b.mkdir(parents=True) + (b / "python").write_text("", encoding="utf-8") + # No env matches version 6.0 -> fall back to the newest IDF env, which + # is 5.10 (numeric), not 5.9 (lexicographic winner). + assert "idf5.10_" in _python_env_for(espressif, "6.0") diff --git a/uv.lock b/uv.lock new file mode 100644 index 0000000..f976340 --- /dev/null +++ b/uv.lock @@ -0,0 +1,119 @@ +version = 1 +revision = 3 +requires-python = ">=3.11" + +[[package]] +name = "colorama" +version = "0.4.6" +source = { registry = "https://pypi.org/simple" } +sdist = { url = "https://files.pythonhosted.org/packages/d8/53/6f443c9a4a8358a93a6792e2acffb9d9d5cb0a5cfd8802644b7b1c9a02e4/colorama-0.4.6.tar.gz", hash = "sha256:08695f5cb7ed6e0531a20572697297273c47b8cae5a63ffc6d6ed5c201be6e44", size = 27697, upload-time = "2022-10-25T02:36:22.414Z" } +wheels = [ + { url = "https://files.pythonhosted.org/packages/d1/d6/3965ed04c63042e047cb6a3e6ed1a63a35087b6a609aa3a15ed8ac56c221/colorama-0.4.6-py2.py3-none-any.whl", hash = "sha256:4f1d9991f5acc0ca119f9d443620b77f9d6b33703e51011c16baf57afb285fc6", size = 25335, upload-time = "2022-10-25T02:36:20.889Z" }, +] + +[[package]] +name = "iniconfig" +version = "2.3.0" +source = { registry = "https://pypi.org/simple" } +sdist = { url = "https://files.pythonhosted.org/packages/72/34/14ca021ce8e5dfedc35312d08ba8bf51fdd999c576889fc2c24cb97f4f10/iniconfig-2.3.0.tar.gz", hash = "sha256:c76315c77db068650d49c5b56314774a7804df16fee4402c1f19d6d15d8c4730", size = 20503, upload-time = "2025-10-18T21:55:43.219Z" } +wheels = [ + { url = "https://files.pythonhosted.org/packages/cb/b1/3846dd7f199d53cb17f49cba7e651e9ce294d8497c8c150530ed11865bb8/iniconfig-2.3.0-py3-none-any.whl", hash = "sha256:f631c04d2c48c52b84d0d0549c99ff3859c98df65b3101406327ecc7d53fbf12", size = 7484, upload-time = "2025-10-18T21:55:41.639Z" }, +] + +[[package]] +name = "mpremote" +version = "1.28.0" +source = { registry = "https://pypi.org/simple" } +dependencies = [ + { name = "platformdirs" }, + { name = "pyserial" }, +] +sdist = { url = "https://files.pythonhosted.org/packages/20/1b/d008342d1b018fb60e1196bf72a164ea71a567f924db11934f0e3a0a10bc/mpremote-1.28.0.tar.gz", hash = "sha256:fdb5626be83dff4e53c0184f8950814cb519b524dba7f1f8b1668aa477257a31", size = 31412, upload-time = "2026-04-06T13:21:14.976Z" } +wheels = [ + { url = "https://files.pythonhosted.org/packages/0f/d7/634a63290002a9df2708e05983fc769447d64f074ac2e8d8990316c70fd0/mpremote-1.28.0-py3-none-any.whl", hash = "sha256:2df2a50f3c8098cae8c732dbf2541e7e58185e7896513b45d05196901e049334", size = 36098, upload-time = "2026-04-06T13:21:13.368Z" }, +] + +[[package]] +name = "packaging" +version = "26.2" +source = { registry = "https://pypi.org/simple" } +sdist = { url = "https://files.pythonhosted.org/packages/d7/f1/e7a6dd94a8d4a5626c03e4e99c87f241ba9e350cd9e6d75123f992427270/packaging-26.2.tar.gz", hash = "sha256:ff452ff5a3e828ce110190feff1178bb1f2ea2281fa2075aadb987c2fb221661", size = 228134, upload-time = "2026-04-24T20:15:23.917Z" } +wheels = [ + { url = "https://files.pythonhosted.org/packages/df/b2/87e62e8c3e2f4b32e5fe99e0b86d576da1312593b39f47d8ceef365e95ed/packaging-26.2-py3-none-any.whl", hash = "sha256:5fc45236b9446107ff2415ce77c807cee2862cb6fac22b8a73826d0693b0980e", size = 100195, upload-time = "2026-04-24T20:15:22.081Z" }, +] + +[[package]] +name = "platformdirs" +version = "4.11.0" +source = { registry = "https://pypi.org/simple" } +sdist = { url = "https://files.pythonhosted.org/packages/78/9b/560e4be8e26f6fd133a03630a8df0c663b9e8d61b4ade152b72005aec83b/platformdirs-4.11.0.tar.gz", hash = "sha256:0555d18370482847566ffabcaa53ad7c6c1c29f195989ae1ed634a05f76ea1e0", size = 31953, upload-time = "2026-07-21T13:09:36.565Z" } +wheels = [ + { url = "https://files.pythonhosted.org/packages/7d/68/d8d58938dfb1370b266a1a729e6d77a985be23689a0496498ee17b2cbf90/platformdirs-4.11.0-py3-none-any.whl", hash = "sha256:360ccded2b7fce0af0ff80cc8f5942a1c5d99b0e856033acb030bfc634709e74", size = 23247, upload-time = "2026-07-21T13:09:35.422Z" }, +] + +[[package]] +name = "pluggy" +version = "1.6.0" +source = { registry = "https://pypi.org/simple" } +sdist = { url = "https://files.pythonhosted.org/packages/f9/e2/3e91f31a7d2b083fe6ef3fa267035b518369d9511ffab804f839851d2779/pluggy-1.6.0.tar.gz", hash = "sha256:7dcc130b76258d33b90f61b658791dede3486c3e6bfb003ee5c9bfb396dd22f3", size = 69412, upload-time = "2025-05-15T12:30:07.975Z" } +wheels = [ + { url = "https://files.pythonhosted.org/packages/54/20/4d324d65cc6d9205fabedc306948156824eb9f0ee1633355a8f7ec5c66bf/pluggy-1.6.0-py3-none-any.whl", hash = "sha256:e920276dd6813095e9377c0bc5566d94c932c33b27a3e3945d8389c374dd4746", size = 20538, upload-time = "2025-05-15T12:30:06.134Z" }, +] + +[[package]] +name = "pygments" +version = "2.20.0" +source = { registry = "https://pypi.org/simple" } +sdist = { url = "https://files.pythonhosted.org/packages/c3/b2/bc9c9196916376152d655522fdcebac55e66de6603a76a02bca1b6414f6c/pygments-2.20.0.tar.gz", hash = "sha256:6757cd03768053ff99f3039c1a36d6c0aa0b263438fcab17520b30a303a82b5f", size = 4955991, upload-time = "2026-03-29T13:29:33.898Z" } +wheels = [ + { url = "https://files.pythonhosted.org/packages/f4/7e/a72dd26f3b0f4f2bf1dd8923c85f7ceb43172af56d63c7383eb62b332364/pygments-2.20.0-py3-none-any.whl", hash = "sha256:81a9e26dd42fd28a23a2d169d86d7ac03b46e2f8b59ed4698fb4785f946d0176", size = 1231151, upload-time = "2026-03-29T13:29:30.038Z" }, +] + +[[package]] +name = "pyserial" +version = "3.5" +source = { registry = "https://pypi.org/simple" } +sdist = { url = "https://files.pythonhosted.org/packages/1e/7d/ae3f0a63f41e4d2f6cb66a5b57197850f919f59e558159a4dd3a818f5082/pyserial-3.5.tar.gz", hash = "sha256:3c77e014170dfffbd816e6ffc205e9842efb10be9f58ec16d3e8675b4925cddb", size = 159125, upload-time = "2020-11-23T03:59:15.045Z" } +wheels = [ + { url = "https://files.pythonhosted.org/packages/07/bc/587a445451b253b285629263eb51c2d8e9bcea4fc97826266d186f96f558/pyserial-3.5-py2.py3-none-any.whl", hash = "sha256:c4451db6ba391ca6ca299fb3ec7bae67a5c55dde170964c7a14ceefec02f2cf0", size = 90585, upload-time = "2020-11-23T03:59:13.41Z" }, +] + +[[package]] +name = "pytest" +version = "9.1.1" +source = { registry = "https://pypi.org/simple" } +dependencies = [ + { name = "colorama", marker = "sys_platform == 'win32'" }, + { name = "iniconfig" }, + { name = "packaging" }, + { name = "pluggy" }, + { name = "pygments" }, +] +sdist = { url = "https://files.pythonhosted.org/packages/e4/47/b9efed96c114afcfa3c9d3fe98a76a1d14c74a9e266d397cf6eb64be5e01/pytest-9.1.1.tar.gz", hash = "sha256:1088fbde8f2b49d95a549a195707afa7a76a3ce9bcadc26b6d71f0ffda5fe313", size = 1636369, upload-time = "2026-06-19T10:58:32.857Z" } +wheels = [ + { url = "https://files.pythonhosted.org/packages/24/25/1de2678b631f5a49215c6c96fff41ba892b0a34df68d6d80292b1b48aa7f/pytest-9.1.1-py3-none-any.whl", hash = "sha256:37a86b45efb9a47a61a36449063e8e18d0cab3161329fc099eb21783169c4f0c", size = 386536, upload-time = "2026-06-19T10:58:31.347Z" }, +] + +[[package]] +name = "tig-silico" +version = "0.1.4" +source = { editable = "." } +dependencies = [ + { name = "pyserial" }, +] + +[package.optional-dependencies] +dev = [ + { name = "pytest" }, +] +device = [ + { name = "mpremote" }, +] + +[package.metadata] +requires-dist = [ + { name = "mpremote", marker = "extra == 'device'", specifier = ">=1.24" }, + { name = "pyserial", specifier = ">=3.5" }, + { name = "pytest", marker = "extra == 'dev'", specifier = ">=8" }, +] +provides-extras = ["dev", "device"] From 47efbd79394484d67da83dbf2dd6b1aca6988a94 Mon Sep 17 00:00:00 2001 From: Tig Date: Fri, 24 Jul 2026 19:38:55 -0600 Subject: [PATCH 3/3] Doctor: catalog-only ESP-IDF is needs-activation, not deploy-ready (#87) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PR #88 review (P1): when idf.py resolved only via a catalog (idf-env.json / EIM), doctor said OK while silico deploy — which accepts only PATH or IDF_PATH — aborted. New esp_idf_summary_lines() keys the OK strictly off idf_py_available() (what deploy checks) and reports installed-but-not-activated with the export.sh hint otherwise. Deploy WARN/FAIL messages now point at activation too. Tests first: 3 new cases observed failing. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- silico/deploy_idf.py | 9 +++++--- silico/doctor.py | 34 +++++++++++++++++++++-------- tests/test_doctor_host.py | 46 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 77 insertions(+), 12 deletions(-) diff --git a/silico/deploy_idf.py b/silico/deploy_idf.py index 73f351d..cdc38f2 100644 --- a/silico/deploy_idf.py +++ b/silico/deploy_idf.py @@ -128,7 +128,9 @@ def plan_idf_deploy( if not idf_py_available(): lines.append( - "WARN: idf.py not on PATH and IDF_PATH unset — write will fail until ESP-IDF is installed." + "WARN: idf.py not on PATH and IDF_PATH unset — write will fail until " + "ESP-IDF is activated (installed catalog? `. \"$IDF_PATH/export.sh\"`; " + "silico doctor shows the path)." ) if yes: lines.append( @@ -250,8 +252,9 @@ def deploy_idf( if not idf_py_available(): log( - "FAIL: ESP-IDF tools not found (idf.py / IDF_PATH). " - "Install per Espressif getting started; silico doctor reports this." + "FAIL: ESP-IDF tools not activated (need idf.py on PATH or IDF_PATH). " + "If an install exists (silico doctor lists catalogs), source its " + "export.sh first; otherwise install per Espressif getting started." ) return DeployResult(False, log.lines) diff --git a/silico/doctor.py b/silico/doctor.py index 0299768..c722b1c 100644 --- a/silico/doctor.py +++ b/silico/doctor.py @@ -60,6 +60,30 @@ def cmake_hint_lines(*, which=shutil.which, probe_dirs=None) -> list[str]: return ["WARN: cmake not on PATH (needed for C host gate) — install via brew/apt or EIM"] +def esp_idf_summary_lines(tc, deploy_ready: bool) -> list[str]: + """One honest ESP-IDF availability line for language=c (#87, PR #88 CR). + + ``deploy_ready`` must mirror what `silico deploy` actually accepts + (idf.py on PATH or IDF_PATH). A catalog-only install (idf-env.json / + EIM) is real but NOT deploy-ready until its export script is sourced — + report needs-activation, never a false OK. + """ + if deploy_ready: + return [f"OK: ESP-IDF tools available ({tc.idf_py or 'idf.py on PATH / IDF_PATH'})"] + if tc.idf_py: + hint = "" + if tc.selected and tc.selected.activation_script: + hint = f' — activate: . "{tc.selected.activation_script}"' + return [ + f"WARN: ESP-IDF installed but not activated ({tc.idf_py} via catalog); " + "deploy needs idf.py on PATH or IDF_PATH" + hint + ] + return [ + "WARN: ESP-IDF tools not found — language=c deploy needs idf.py " + "(PATH, IDF_PATH, EIM, or ~/.espressif/idf-env.json)" + ] + + def run_doctor(*, root: Path | None = None) -> DoctorReport: lines: list[str] = [] ok = True @@ -140,15 +164,7 @@ def run_doctor(*, root: Path | None = None) -> DoctorReport: from silico.c_toolchain import discover_c_toolchain, doctor_c_toolchain_lines tc = discover_c_toolchain() - if tc.idf_py: - lines.append(f"OK: ESP-IDF tools available ({tc.idf_py})") - elif idf_py_available(): - lines.append("OK: ESP-IDF tools available (idf.py or IDF_PATH)") - else: - lines.append( - "WARN: ESP-IDF tools not found — language=c deploy needs idf.py " - "(PATH, IDF_PATH, EIM, or ~/.espressif/idf-env.json)" - ) + lines.extend(esp_idf_summary_lines(tc, deploy_ready=idf_py_available())) lines.extend(cmake_hint_lines()) # EIM / idf_tools.py catalogs (#79, #87) — resolved paths, not hand-parsed JSON. lines.append("--- C toolchain (EIM / IDF) ---") diff --git a/tests/test_doctor_host.py b/tests/test_doctor_host.py index a18a452..2d09473 100644 --- a/tests/test_doctor_host.py +++ b/tests/test_doctor_host.py @@ -53,3 +53,49 @@ def test_run_doctor_includes_python_health(): report = run_doctor() joined = "\n".join(report.lines) assert "pyexpat" in joined + +def _report(idf_py=None, activation=""): + from silico.c_toolchain import CToolchainReport, IdfInstall + + selected = None + if activation: + selected = IdfInstall( + name="esp-idf-5.3", + path="/home/u/esp/esp-idf", + idf_tools_path="/home/u/.espressif", + activation_script=activation, + python="", + ) + return CToolchainReport(ok=True, idf_py=idf_py, selected=selected) + + +def test_esp_idf_summary_deploy_ready_is_ok(): + from silico.doctor import esp_idf_summary_lines + + lines = esp_idf_summary_lines(_report(idf_py="/usr/bin/idf.py"), deploy_ready=True) + assert lines[0].startswith("OK") + + +def test_esp_idf_summary_catalog_only_is_not_ok(): + # PR #88 review (P1): catalog-only IDF must not report deploy-ready — + # `silico deploy` only accepts idf.py on PATH or IDF_PATH. + from silico.doctor import esp_idf_summary_lines + + lines = esp_idf_summary_lines( + _report(idf_py="/home/u/esp/esp-idf/tools/idf.py", + activation="/home/u/esp/esp-idf/export.sh"), + deploy_ready=False, + ) + joined = "\n".join(lines) + assert not any(ln.startswith("OK") for ln in lines) + assert "WARN" in joined + assert "not activated" in joined or "needs activation" in joined + assert "export.sh" in joined # actionable activation hint + + +def test_esp_idf_summary_missing_is_warn(): + from silico.doctor import esp_idf_summary_lines + + lines = esp_idf_summary_lines(_report(), deploy_ready=False) + joined = "\n".join(lines) + assert "WARN" in joined and "not found" in joined