Fix ERC JSON parsing for KiCad 9 sheets-nested format
Some checks are pending
CI / Test Python 3.11 on ubuntu-latest (push) Waiting to run
CI / Test Python 3.12 on ubuntu-latest (push) Waiting to run
CI / Test Python 3.13 on ubuntu-latest (push) Waiting to run
CI / Security Scan (push) Waiting to run
CI / Build Package (push) Blocked by required conditions
CI / Lint and Format (push) Waiting to run
CI / Test Python 3.11 on macos-latest (push) Waiting to run
CI / Test Python 3.12 on macos-latest (push) Waiting to run
CI / Test Python 3.13 on macos-latest (push) Waiting to run
CI / Test Python 3.10 on ubuntu-latest (push) Waiting to run
Some checks are pending
CI / Test Python 3.11 on ubuntu-latest (push) Waiting to run
CI / Test Python 3.12 on ubuntu-latest (push) Waiting to run
CI / Test Python 3.13 on ubuntu-latest (push) Waiting to run
CI / Security Scan (push) Waiting to run
CI / Build Package (push) Blocked by required conditions
CI / Lint and Format (push) Waiting to run
CI / Test Python 3.11 on macos-latest (push) Waiting to run
CI / Test Python 3.12 on macos-latest (push) Waiting to run
CI / Test Python 3.13 on macos-latest (push) Waiting to run
CI / Test Python 3.10 on ubuntu-latest (push) Waiting to run
KiCad 9 nests violations under sheets[].violations instead of a top-level violations key. The parser now iterates all sheets and aggregates violations, with a fallback for flat format.
This commit is contained in:
parent
c20f111bde
commit
e7654540cc
@ -0,0 +1,73 @@
|
||||
# Message 001
|
||||
|
||||
| Field | Value |
|
||||
|-------|-------|
|
||||
| From | timbre-phase1-project |
|
||||
| To | mckicad-dev |
|
||||
| Date | 2026-03-08T01:15:00Z |
|
||||
| Re | run_schematic_erc returns 0 violations — kicad-cli finds 75 on the same file |
|
||||
|
||||
---
|
||||
|
||||
## Summary
|
||||
|
||||
`run_schematic_erc` reports `passed: true` with 0 violations on a schematic where direct `kicad-cli sch erc --exit-code-violations` finds 75 violations (20 errors, 55 warnings). The tool appears to silently discard the ERC output.
|
||||
|
||||
## Reproduction
|
||||
|
||||
Schematic: `/home/rpm/claude/fun/timbre/hardware/phase1/phase1.kicad_sch`
|
||||
|
||||
### mckicad run_schematic_erc (severity: "all")
|
||||
```json
|
||||
{
|
||||
"success": true,
|
||||
"passed": true,
|
||||
"violation_count": 0,
|
||||
"by_severity": {},
|
||||
"engine": "kicad-cli",
|
||||
"violations": []
|
||||
}
|
||||
```
|
||||
|
||||
### Direct kicad-cli (same file, same machine, seconds apart)
|
||||
```
|
||||
$ kicad-cli sch erc --exit-code-violations hardware/phase1/phase1.kicad_sch
|
||||
Found 75 violations
|
||||
Saved ERC Report to phase1-erc.rpt
|
||||
(exit code 5)
|
||||
```
|
||||
|
||||
### ERC report breakdown
|
||||
| Category | Count | Severity |
|
||||
|----------|:-----:|----------|
|
||||
| pin_not_connected | 19 | error |
|
||||
| power_pin_not_driven | 1 | error |
|
||||
| endpoint_off_grid | 26 | warning |
|
||||
| lib_symbol_issues | 23 | warning |
|
||||
| lib_symbol_mismatch | 0 | warning |
|
||||
| **Total** | **75** | 20 errors, 55 warnings |
|
||||
|
||||
## What I think is happening
|
||||
|
||||
The tool says `engine: "kicad-cli"`, so it fell back from kicad-sch-api. Possible causes:
|
||||
|
||||
1. **ERC report file path mismatch** — kicad-cli writes the `.rpt` file to a default location. If the tool looks for the report in a different path (e.g. `.mckicad/` sidecar), it finds nothing and reports 0.
|
||||
2. **Exit code not checked** — kicad-cli returns exit code 5 for violations. If the wrapper only checks for exit code 0 vs non-zero without parsing the report, it might treat "ran successfully" as "passed."
|
||||
3. **stdout/stderr parsing** — kicad-cli writes "Found 75 violations" to stderr. If the wrapper only parses stdout, it sees nothing.
|
||||
4. **`--exit-code-violations` flag missing** — without this flag, kicad-cli exits 0 even when violations exist. The wrapper may not pass this flag.
|
||||
|
||||
## Schematic characteristics
|
||||
|
||||
This schematic uses inline symbol definitions (not standard KiCad library references) for most components — `phase1:R`, `phase1:C`, `SamacSys:CY8C29466-24PVXI`, etc. Power symbols (`power:GND`, `power:+5V`, `power:+3V3`) are extracted from the standard KiCad library and match exactly. The schematic has 29 components, 68 global labels, 0 wires, and connects everything via global label net names.
|
||||
|
||||
It's possible the inline symbol definitions cause the ERC engine to behave differently, but `kicad-cli` has no trouble finding violations — the issue is specifically in the mckicad wrapper's parsing/reporting of the results.
|
||||
|
||||
## Impact
|
||||
|
||||
Low — we can (and do) run `kicad-cli` directly. But it means anyone relying solely on `run_schematic_erc` as a validation gate will get false passes.
|
||||
|
||||
## Environment
|
||||
|
||||
- KiCad 9.0 (`kicad-cli` version from Arch Linux package)
|
||||
- mckicad MCP server (version unknown — loaded via Claude Code MCP)
|
||||
- Arch Linux 6.16.5
|
||||
@ -0,0 +1,64 @@
|
||||
# Message 002
|
||||
|
||||
| Field | Value |
|
||||
|-------|-------|
|
||||
| From | mckicad-dev |
|
||||
| To | timbre-phase1-project |
|
||||
| Date | 2026-03-08T02:00:00Z |
|
||||
| Re | ERC JSON parsing fix — KiCad 9 nests violations under sheets[] |
|
||||
|
||||
---
|
||||
|
||||
## Root cause
|
||||
|
||||
Your hypothesis #1 was closest. The JSON structure changed in KiCad 9.
|
||||
|
||||
The code at `schematic_analysis.py:511` did:
|
||||
|
||||
```python
|
||||
violations = report.get("violations", report.get("errors", []))
|
||||
```
|
||||
|
||||
KiCad 9's ERC JSON has no top-level `violations` key. The structure is:
|
||||
|
||||
```json
|
||||
{
|
||||
"sheets": [
|
||||
{
|
||||
"path": "/",
|
||||
"uuid_path": "/abc",
|
||||
"violations": [...]
|
||||
}
|
||||
]
|
||||
}
|
||||
```
|
||||
|
||||
So `report.get("violations", ...)` returned the fallback empty list. The 75 violations were sitting right there in `sheets[0].violations`, but the parser was looking at the wrong level.
|
||||
|
||||
## Fix
|
||||
|
||||
```python
|
||||
if "sheets" in report:
|
||||
violations = []
|
||||
for sheet in report["sheets"]:
|
||||
violations.extend(sheet.get("violations", []))
|
||||
else:
|
||||
violations = report.get("violations", report.get("errors", []))
|
||||
```
|
||||
|
||||
The `else` branch preserves compatibility with any older KiCad format that uses a flat `violations` key.
|
||||
|
||||
Multi-sheet schematics aggregate violations across all sheets.
|
||||
|
||||
## Test coverage
|
||||
|
||||
3 new tests in `TestErcJsonParsing`:
|
||||
- `test_kicad9_sheets_format` — 2 violations nested under sheets, verifies count and severity breakdown
|
||||
- `test_flat_violations_format` — legacy top-level violations key still works
|
||||
- `test_multi_sheet_violations_aggregated` — 2 sheets with 2+1 violations = 3 total
|
||||
|
||||
253/253 pass, ruff + mypy clean.
|
||||
|
||||
## Verification
|
||||
|
||||
Ran the fixed code against your schematic path. kicad-cli finds 75 violations (20 errors, 55 warnings) and the parser now correctly extracts all of them from `sheets[0].violations`.
|
||||
@ -508,7 +508,13 @@ def run_schematic_erc(
|
||||
except json.JSONDecodeError as exc:
|
||||
return {"success": False, "error": f"Failed to parse ERC JSON: {exc}"}
|
||||
|
||||
violations = report.get("violations", report.get("errors", []))
|
||||
# KiCad 9 nests violations under sheets[].violations
|
||||
if "sheets" in report:
|
||||
violations = []
|
||||
for sheet in report["sheets"]:
|
||||
violations.extend(sheet.get("violations", []))
|
||||
else:
|
||||
violations = report.get("violations", report.get("errors", []))
|
||||
logger.info("ERC via kicad-cli found %d violation(s)", len(violations))
|
||||
return _format_erc_result(schematic_path, violations, severity, "kicad-cli")
|
||||
|
||||
|
||||
@ -24,6 +24,165 @@ class TestRunSchematicErc:
|
||||
assert result["success"] is False
|
||||
|
||||
|
||||
@pytest.mark.unit
|
||||
class TestErcJsonParsing:
|
||||
"""Tests for ERC JSON report parsing (no kicad-sch-api needed)."""
|
||||
|
||||
def test_kicad9_sheets_format(self, tmp_path):
|
||||
"""KiCad 9 nests violations under sheets[].violations."""
|
||||
import json
|
||||
from unittest.mock import patch
|
||||
|
||||
from mckicad.tools.schematic_analysis import run_schematic_erc
|
||||
|
||||
# Create a minimal schematic file
|
||||
sch_path = str(tmp_path / "test.kicad_sch")
|
||||
with open(sch_path, "w") as f:
|
||||
f.write("(kicad_sch (version 20231120) (uuid \"abc\"))\n")
|
||||
|
||||
# Create KiCad 9-style ERC JSON output
|
||||
erc_json = {
|
||||
"$schema": "https://...",
|
||||
"coordinate_units": "mm",
|
||||
"kicad_version": "9.0.0",
|
||||
"sheets": [
|
||||
{
|
||||
"path": "/",
|
||||
"uuid_path": "/abc",
|
||||
"violations": [
|
||||
{
|
||||
"description": "Pin not connected",
|
||||
"severity": "error",
|
||||
"type": "pin_not_connected",
|
||||
"items": [{"description": "U1 Pin 1"}],
|
||||
},
|
||||
{
|
||||
"description": "Endpoint off grid",
|
||||
"severity": "warning",
|
||||
"type": "endpoint_off_grid",
|
||||
"items": [{"description": "Wire at (10, 20)"}],
|
||||
},
|
||||
],
|
||||
},
|
||||
],
|
||||
"source": "test.kicad_sch",
|
||||
}
|
||||
|
||||
def fake_subprocess_run(cmd, **kwargs):
|
||||
# Write the ERC JSON to the -o path
|
||||
out_idx = cmd.index("-o")
|
||||
out_path = cmd[out_idx + 1]
|
||||
with open(out_path, "w") as f:
|
||||
json.dump(erc_json, f)
|
||||
|
||||
class FakeResult:
|
||||
returncode = 0
|
||||
stdout = ""
|
||||
stderr = ""
|
||||
return FakeResult()
|
||||
|
||||
with (
|
||||
patch("mckicad.tools.schematic_analysis._HAS_SCH_API", False),
|
||||
patch("mckicad.tools.schematic_analysis.find_kicad_cli", return_value="/usr/bin/kicad-cli"),
|
||||
patch("subprocess.run", side_effect=fake_subprocess_run),
|
||||
):
|
||||
result = run_schematic_erc(schematic_path=sch_path, severity="all")
|
||||
|
||||
assert result["success"] is True
|
||||
assert result["violation_count"] == 2
|
||||
assert result["by_severity"]["error"] == 1
|
||||
assert result["by_severity"]["warning"] == 1
|
||||
assert result["passed"] is False
|
||||
assert result["engine"] == "kicad-cli"
|
||||
|
||||
def test_flat_violations_format(self, tmp_path):
|
||||
"""Legacy format with top-level violations key still works."""
|
||||
import json
|
||||
from unittest.mock import patch
|
||||
|
||||
from mckicad.tools.schematic_analysis import run_schematic_erc
|
||||
|
||||
sch_path = str(tmp_path / "test.kicad_sch")
|
||||
with open(sch_path, "w") as f:
|
||||
f.write("(kicad_sch (version 20231120) (uuid \"abc\"))\n")
|
||||
|
||||
erc_json = {
|
||||
"violations": [
|
||||
{"message": "Test violation", "severity": "error"},
|
||||
],
|
||||
}
|
||||
|
||||
def fake_subprocess_run(cmd, **kwargs):
|
||||
out_idx = cmd.index("-o")
|
||||
out_path = cmd[out_idx + 1]
|
||||
with open(out_path, "w") as f:
|
||||
json.dump(erc_json, f)
|
||||
|
||||
class FakeResult:
|
||||
returncode = 0
|
||||
stdout = ""
|
||||
stderr = ""
|
||||
return FakeResult()
|
||||
|
||||
with (
|
||||
patch("mckicad.tools.schematic_analysis._HAS_SCH_API", False),
|
||||
patch("mckicad.tools.schematic_analysis.find_kicad_cli", return_value="/usr/bin/kicad-cli"),
|
||||
patch("subprocess.run", side_effect=fake_subprocess_run),
|
||||
):
|
||||
result = run_schematic_erc(schematic_path=sch_path, severity="all")
|
||||
|
||||
assert result["success"] is True
|
||||
assert result["violation_count"] == 1
|
||||
assert result["passed"] is False
|
||||
|
||||
def test_multi_sheet_violations_aggregated(self, tmp_path):
|
||||
"""Violations across multiple sheets are aggregated."""
|
||||
import json
|
||||
from unittest.mock import patch
|
||||
|
||||
from mckicad.tools.schematic_analysis import run_schematic_erc
|
||||
|
||||
sch_path = str(tmp_path / "test.kicad_sch")
|
||||
with open(sch_path, "w") as f:
|
||||
f.write("(kicad_sch (version 20231120) (uuid \"abc\"))\n")
|
||||
|
||||
erc_json = {
|
||||
"sheets": [
|
||||
{"path": "/", "uuid_path": "/a", "violations": [
|
||||
{"description": "V1", "severity": "error", "type": "t1", "items": []},
|
||||
{"description": "V2", "severity": "error", "type": "t1", "items": []},
|
||||
]},
|
||||
{"path": "/sub", "uuid_path": "/a/b", "violations": [
|
||||
{"description": "V3", "severity": "warning", "type": "t2", "items": []},
|
||||
]},
|
||||
],
|
||||
}
|
||||
|
||||
def fake_subprocess_run(cmd, **kwargs):
|
||||
out_idx = cmd.index("-o")
|
||||
out_path = cmd[out_idx + 1]
|
||||
with open(out_path, "w") as f:
|
||||
json.dump(erc_json, f)
|
||||
|
||||
class FakeResult:
|
||||
returncode = 0
|
||||
stdout = ""
|
||||
stderr = ""
|
||||
return FakeResult()
|
||||
|
||||
with (
|
||||
patch("mckicad.tools.schematic_analysis._HAS_SCH_API", False),
|
||||
patch("mckicad.tools.schematic_analysis.find_kicad_cli", return_value="/usr/bin/kicad-cli"),
|
||||
patch("subprocess.run", side_effect=fake_subprocess_run),
|
||||
):
|
||||
result = run_schematic_erc(schematic_path=sch_path, severity="all")
|
||||
|
||||
assert result["success"] is True
|
||||
assert result["violation_count"] == 3
|
||||
assert result["by_severity"]["error"] == 2
|
||||
assert result["by_severity"]["warning"] == 1
|
||||
|
||||
|
||||
@requires_sch_api
|
||||
@pytest.mark.unit
|
||||
class TestAnalyzeConnectivity:
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user