Repository navigation
ESP32-C first-ship learnings: idf-env.json discovery, doctor pre-flight, 64-bit time plate, knowledge (#87) - #88
Conversation
…ledge (#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>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1f341bb95e
ℹ️ 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".
| if tc.idf_py: | ||
| lines.append(f"OK: ESP-IDF tools available ({tc.idf_py})") |
There was a problem hiding this comment.
When ESP-IDF exists only in ~/.espressif/idf-env.json and its export.sh has not been sourced—the exact host state added here—discover_c_toolchain() returns an off-PATH tc.idf_py, so doctor reports OK. However, silico deploy --yes still checks deploy_idf.idf_py_available(), which only recognizes PATH or IDF_PATH, and aborts before using the discovered executable. This makes the C pre-flight falsely green while the documented doctor→deploy path cannot proceed; either wire catalog discovery into deploy or report this state as installed-but-needs-activation rather than available.
AGENTS.md reference: AGENTS.md:L983-L988
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 47efbd7, test-first (3 new tests in tests/test_doctor_host.py observed failing first). Doctor now keys the OK strictly off idf_py_available() — exactly what silico deploy checks — via a new esp_idf_summary_lines() helper. A catalog-only install (idf-env.json / EIM, export.sh not sourced) reports WARN: ESP-IDF installed but not activated (… via catalog); deploy needs idf.py on PATH or IDF_PATH — activate: . "…/export.sh". Deploy's own WARN/FAIL messages now point at activation too, so the doctor→deploy path can't be falsely green.
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>
|
Independent code review done on the branch diff. One medium finding: |
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>
1 participant
Fixes #87 (main body + macOS onboarding comment).
What this folds in
Toolchain discovery (comment: "ESP-IDF was already installed but silico couldn't see it")
silico/c_toolchain.pynow reads~/.espressif/idf-env.json(the dict-keyedidfInstalledschema written byinstall.sh/idf_tools.py) as a catalog fallback after EIM'seim_idf.json, before the bare$IDF_PATHfallback.$IDF_PATH/export.shwhen present; python env from~/.espressif/python_env/*/bin/python.silico env --printsurface the catalog and a bash. "$IDF_PATH/export.sh"hint; missing-catalog message now names both catalogs.load_eim_installsbug fixed along the way: a nonexistent explicit path was echoed as if it were a catalog.Doctor pre-flight (comment: broken Homebrew python, cmake off PATH)
host_python_health_lines(): importspyexpat; on failure WARNs with theuv venvfix (Homebrew linkage rot broke ESP-IDF installs on the Mac first ship).cmake_hint_lines(): when cmake is off PATH, probes/opt/homebrew/binand/usr/local/binand tells the agent exactly where it is (this very session hit it again).discover_c_toolchain()pass as the detail section, so they can't contradict.Plate gcu-c (issue items 1, 3)
int64_t (*now_ms)HAL hook with the ILP32 warning inline (longis 32-bit on Xtensa: overflow <10 h, wrap ~24.8 days; hostlonghides it).esp_timer_get_time()/1000; domain LED blink is wall-clock driven with tick fallback.clearerr(stdin)after the non-blocking identity drain (sticky EOF made the link go deaf while TX still worked).host/test_time.cseeds the fake clock past 2^31 and is wired into the host gate.Knowledge (issue items 2, 4, 5, 6)
esp32-audio.md: stop-must-disable-or-settle, done-flag on every audio-task exit path, played-vs-submitted position across DMA backlog.esp32-usb-serial.md: clearerr footgun row.esp32-lcd-ips.md: measured ILI9342C esp_lcd init (MADCTL 0x08, INVON, 26.7 MHz, no read dummy).m5-core.md: SK6812 x10 on GPIO15 via RMT; amp-noise coupling note.deploy-assets.md: ESP-IDF SPIFFS partition + gitignored regen-script pattern.macos-codex-esp-idf.md: venv shadowing idf.py, pyexpat/uv, idf-env.json pointer, sudo front-loading.AGENTS.md: workflow-scope push rejection recipe; plateAGENTS.md: time contract + env gotchas.Test-first
New tests written and observed failing before implementation:
tests/test_doctor_host.py(6), idf-env cases intests/test_c_toolchain.py(8, +1 hermeticity fix to an existing test), platehost/test_time.c. Full suite: 208 passed, 1 skipped; plate host gate 2/2.