Skip to content

fix(cad): parse whole s-expression blocks, and ship a board to run them on - #94

Merged
srpatcha merged 5 commits into
embeddedos-org:masterfrom
Kartikey1306:fix/cad-pipeline-parser
Sep 1, 2026
Merged

fix(cad): parse whole s-expression blocks, and ship a board to run them on#94
srpatcha merged 5 commits into
embeddedos-org:masterfrom
Kartikey1306:fix/cad-pipeline-parser

Conversation

@Kartikey1306

Copy link
Copy Markdown
Contributor

tools/cad_pipeline.py extracts its blocks with non-greedy regexes:

re.search(r'\(eos_cpu(.*?)\)', content, re.DOTALL)
re.finditer(r'\(eos_peripheral(.*?)\)', content, re.DOTALL)

(.*?)\) stops at the first ). In

(eos_cpu
  (arch "aarch64")
  (core "cortex-a57")

that is the paren closing (arch ...) — so the match ends after the block's first attribute, and every later one falls outside it and silently takes its dataclass default.

What that looks like

file:   core=cortex-a72  freq=1800  fpu=crypto-neon  entry=my_entry
parsed: {"arch": "aarch64", "core": "cortex-a57", "freq_mhz": 1000}   ← the defaults

Every eos_peripheral comes back as unknown / unknown / 0x0 / irq -1 regardless of what the file says. And the region and peripheral counts are correct, so nothing looks wrong.

Those values flow straight into the generated artifacts:

  • the DTS emits compatible = "unknown" and reg = <0x0 0x00000000> for every device
  • the CMake toolchain emits -mcpu= for the wrong core

A linker script and device tree describing hardware that is not the board is worse than a failure, because it builds.

_sexpr_body() replaces both regexes with a depth counter that skips quoted strings, so a block ends where it actually ends.

Why this was never caught

The pipeline has no test and no input in this repository. The only board it is ever pointed at is eFab/samples/eos_reference_board.kicad_pcb, and embeddedos-org/eFab does not exist:

$ gh api repos/embeddedos-org/eFab
{"message":"Not Found","status":"404"}

Both eos CI jobs that invoke this tool die at that checkout, so the tool had never run anywhere — not in CI, not locally.

samples/eos_reference_board.kicad_pcb gives it one: the five memory regions the eos simulation workflow asserts on (FLASH_NOR, RAM_LPDDR4, UART0_PL011, HEAP_REGION, OTA_SCRATCH), a full CPU block, and two peripherals. That also unblocks eos's CAD job, which can point at this instead of a repository that isn't there.

Tests

tests/unit/test_cad_pipeline.py — 7 tests covering the parse and the generated artifacts. Against the unfixed parser, three fail:

FAILED test_cpu_attributes_track_the_file_not_the_defaults
FAILED test_peripheral_fields_are_parsed
FAILED test_generated_artifacts_carry_the_board_values
  assert 'arm,pl011' in '... compatible = "unknown"; reg = <0x0 0x00000000> ...'

The decisive one is the second CPU test: a board whose every value differs from the defaults must not read back as the defaults. A test using only the sample's own values would pass against the broken parser, since the sample happens to match CpuDef's defaults.

Verification

pytest tests/ 302 passed
pipeline on the sample all 5 artifacts generated, non-empty

Stacked on #66 — on master the macOS leg still pins the retired macos-13 image, so a branch off master cannot get a green run at all.

🤖 Generated with Claude Code

@codecov-commenter

Copy link
Copy Markdown

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

Kartikey1306 and others added 4 commits September 1, 2026 15:07
Rebuilt on current master. Master has since fixed the dispatch.py SyntaxError
and restored NinjaBackend._object_path, so embeddedos-org#66's versions of those are dropped
in favour of what landed — including its choice of RuntimeError for an unknown
backend, which is now consistent across both dispatch test suites. What follows
is what master still does not have.

**build.ninja is invalid on Windows.** Ninja splits build statements on
unescaped spaces and colons, so a Windows absolute path puts a drive-letter
colon where Ninja expects the separator between outputs and the rule name:

    ninja: error: build.ninja:20: expected build command name
    build C:\...\main.o: cc main.c
          ^ near here

Every generated file was rejected before a command ran; a POSIX path containing
a space fails identically. _ninja_path() escapes `$`, `:` and ` `, applied to
build-statement paths only — variable values (cflags, ldflags) are read to end
of line and are left alone, since escaping them hands the compiler mangled
flags. Four regression tests, one asserting each build statement contains
exactly one unescaped colon.

**test_shared_library_uses_shared_link_rule fails on macOS.** It asserts the
literal "-shared", but _shared_flag() correctly returns "-dynamiclib" on
darwin. The implementation is right and the test was not; made it
platform-aware. This is the one test failing on master today.

**The Windows leg of the test matrix has never run.** `Run test suite` uses
backslash line continuations, which PowerShell rejects:

    ParserError: Missing expression after unary operator '--'.

Marked `shell: bash`, which GitHub provides on Windows runners.

**macos-13 is a retired runner image**, so those jobs are never assigned a
runner and sit queued until they time out. Every other workflow here already
uses macos-latest.

**`mypy .` checks nothing.** It aborts with `Duplicate module named "tests"`
because layers/eosuite/ vendors its own tests/ package. The step is
continue-on-error, so this went unnoticed. Excluding layers/ makes it check 84
files; it stays continue-on-error, so the 12 pre-existing findings are visible
without gating the build.

**ci.yml has no concurrency group**, alone among this repo's workflows, so
pushes pile up queued runs competing for the same scarce runners.

Also gitignored _build/, which the suite leaves in the repo root.

Verified: pytest 292 passed (287 + the fixed shared-flag test + 4 new).

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
With `shell: bash` the Windows leg of the matrix runs pytest for the first
time, and it fails four tests. Neither cause is new; both were simply never
executed.

`test_test_target_links_like_an_executable` picks the link edge out of
build.ninja with `l.split(":")[0]`, meaning "the text before the rule
separator". On Windows the first colon is the drive letter, so that expression
returns "build C" for every line, the `.o` filter never matches, and the test
selects the compile edge and asserts `": link "` against it.

Splitting on the first *unescaped* colon is what was meant, and is now
unambiguous: `_ninja_path()` writes the drive colon as `$:` and leaves exactly
one bare colon per statement, the separator.

    build C$:\...\obj\t_smoke\t.c.o: cc t.c      -> outputs end in .o   (compile)
    build C$:\...\t_smoke.exe: link ...          -> outputs do not      (link)

The three `test_integration_initramfs_security.py` cases fail with WinError 2:
`_create_initramfs()` drives find(1) and cpio(1) directly and neither exists on
a stock Windows runner, so they die before reaching the command-injection
behaviour they exist to check. Building a Linux initramfs is not a Windows
operation, so they skip when the tools are absent — the same shape as the
existing skip in test_ninja_backend.py when no host C compiler is present.

Verified: pytest 292 passed locally; the Windows selection logic checked
against an escaped drive-letter path directly.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…em on

tools/cad_pipeline.py extracts its blocks with non-greedy regexes:

    re.search(r'\(eos_cpu(.*?)\)', content, re.DOTALL)
    re.finditer(r'\(eos_peripheral(.*?)\)', content, re.DOTALL)

`(.*?)\)` stops at the FIRST `)`. In

    (eos_cpu
      (arch "aarch64")
      (core "cortex-a57")

that is the paren closing (arch ...), so the match ends after the block's first
attribute and every later one falls out of it and takes its dataclass default.

The result is silent and plausible. A board declaring cortex-a72 at 1800 MHz
parses as cortex-a57 at 1000 -- CpuDef's defaults -- and every eos_peripheral
comes back as unknown/unknown/0x0/irq -1 regardless of what the file says. The
region and peripheral *counts* are right, so nothing looks wrong:

    file:   core=cortex-a72 freq=1800 fpu=crypto-neon entry=my_entry
    parsed: {"arch": "aarch64", "core": "cortex-a57", "freq_mhz": 1000}

Those values flow straight into the generated artifacts. The DTS emitted
compatible = "unknown" and reg = <0x0 0x00000000> for every device; the CMake
toolchain emitted -mcpu for the wrong core. A linker script and device tree
describing hardware that is not the board is worse than a failure, because it
builds.

_sexpr_body() replaces both regexes with a depth counter that skips quoted
strings, so a block ends where it actually ends.

Why this was never caught: the pipeline had no test and no input in this
repository. The only board it is ever pointed at is
eFab/samples/eos_reference_board.kicad_pcb, and embeddedos-org/eFab does not
exist -- `gh api` returns 404 -- so both eos CI jobs that invoke this tool die
at the checkout and the tool itself had never run anywhere.

samples/eos_reference_board.kicad_pcb gives it one: the five memory regions the
eos simulation workflow asserts on (FLASH_NOR, RAM_LPDDR4, UART0_PL011,
HEAP_REGION, OTA_SCRATCH), a full CPU block, and two peripherals.

tests/unit/test_cad_pipeline.py covers the parse and the generated artifacts.
Against the unfixed parser three of its seven fail, including the decisive one
-- a board whose values differ from every default must not read back as the
defaults.

Verified: pytest 302 passed. Stacked on embeddedos-org#66; on master the macOS leg pins the
retired macos-13 image, so a branch off master cannot get a green run.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Caught by the new test on the Windows leg:

    UnicodeDecodeError: 'utf-8' codec can't decode byte 0x97 in position 152

cad_pipeline.py reads the board with encoding="utf-8" but writes all five
artifacts with a bare open(..., "w"), which uses the platform default -- cp1252
on Windows. The generated banners carry em dashes and box-drawing characters,
so on Windows the linker script, device tree, recipe, toolchain and manifest
were written in a different encoding than the one the tool itself reads.

Asymmetric read/write encoding in a code generator is a bug in the generator,
not in the caller that reads its output as UTF-8.

The artifact test now also decodes each file as UTF-8 rather than only checking
it is non-empty, so this cannot regress silently.

Verified: pytest 302 passed.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@Kartikey1306
Kartikey1306 force-pushed the fix/cad-pipeline-parser branch from 13ccbb5 to 4543fa6 Compare September 1, 2026 09:38
srpatcha
srpatcha previously approved these changes Sep 1, 2026
@srpatcha
srpatcha merged commit a52e1a1 into embeddedos-org:master Sep 1, 2026
15 of 24 checks passed
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.

3 participants