fix(add_schematic_wire): support hierarchical sub-sheets (#170)
WireManager.add_wire and add_polyline_wire bailed out with "No sheet_instances section found in schematic" when called on a hierarchical sub-sheet — that block only exists in the root .kicad_sch. The handler in kicad_interface.py converted the False return into a flat "Failed to add wire" message, leaving callers with no diagnostic. Apply the same fallback that WireManager.add_label has used since the hierarchical-design support: when (sheet_instances ...) is absent, append the new (wire ...) item at the end of the outer (kicad_sch ...) form. Adds tests/test_add_wire_sub_sheet.py with 6 regression tests covering both add_wire and add_polyline_wire on a sub-sheet, including paren balance, sexpdata round-trip, and segment count for an N-point polyline.
This commit is contained in:
@@ -174,7 +174,9 @@ class WireManager:
|
|||||||
start_point, end_point, stroke_width, stroke_type
|
start_point, end_point, stroke_width, stroke_type
|
||||||
)
|
)
|
||||||
|
|
||||||
# Find insertion point (before sheet_instances)
|
# Find insertion point (before sheet_instances on the root sheet,
|
||||||
|
# or appended to the end on a hierarchical sub-sheet which has no
|
||||||
|
# sheet_instances block).
|
||||||
sheet_instances_index = None
|
sheet_instances_index = None
|
||||||
for i, item in enumerate(sch_data):
|
for i, item in enumerate(sch_data):
|
||||||
if isinstance(item, list) and len(item) > 0 and item[0] == _SYM_SHEET_INSTANCES:
|
if isinstance(item, list) and len(item) > 0 and item[0] == _SYM_SHEET_INSTANCES:
|
||||||
@@ -182,10 +184,10 @@ class WireManager:
|
|||||||
break
|
break
|
||||||
|
|
||||||
if sheet_instances_index is None:
|
if sheet_instances_index is None:
|
||||||
logger.error("No sheet_instances section found in schematic")
|
# Sub-sheets in hierarchical designs don't have (sheet_instances).
|
||||||
return False
|
sheet_instances_index = len(sch_data)
|
||||||
|
|
||||||
# Insert wire before sheet_instances
|
# Insert wire before sheet_instances (or at end for sub-sheets)
|
||||||
sch_data.insert(sheet_instances_index, wire_sexp)
|
sch_data.insert(sheet_instances_index, wire_sexp)
|
||||||
logger.info(f"Injected wire from {start_point} to {end_point}")
|
logger.info(f"Injected wire from {start_point} to {end_point}")
|
||||||
|
|
||||||
@@ -249,7 +251,9 @@ class WireManager:
|
|||||||
for i in range(len(points) - 1)
|
for i in range(len(points) - 1)
|
||||||
]
|
]
|
||||||
|
|
||||||
# Find insertion point
|
# Find insertion point (before sheet_instances on the root sheet,
|
||||||
|
# or appended to the end on a hierarchical sub-sheet which has no
|
||||||
|
# sheet_instances block).
|
||||||
sheet_instances_index = None
|
sheet_instances_index = None
|
||||||
for i, item in enumerate(sch_data):
|
for i, item in enumerate(sch_data):
|
||||||
if isinstance(item, list) and len(item) > 0 and item[0] == _SYM_SHEET_INSTANCES:
|
if isinstance(item, list) and len(item) > 0 and item[0] == _SYM_SHEET_INSTANCES:
|
||||||
@@ -257,8 +261,8 @@ class WireManager:
|
|||||||
break
|
break
|
||||||
|
|
||||||
if sheet_instances_index is None:
|
if sheet_instances_index is None:
|
||||||
logger.error("No sheet_instances section found in schematic")
|
# Sub-sheets in hierarchical designs don't have (sheet_instances).
|
||||||
return False
|
sheet_instances_index = len(sch_data)
|
||||||
|
|
||||||
# Insert all segments (in reverse so order is preserved after inserts)
|
# Insert all segments (in reverse so order is preserved after inserts)
|
||||||
for wire_sexp in reversed(wire_sexps):
|
for wire_sexp in reversed(wire_sexps):
|
||||||
|
|||||||
135
tests/test_add_wire_sub_sheet.py
Normal file
135
tests/test_add_wire_sub_sheet.py
Normal file
@@ -0,0 +1,135 @@
|
|||||||
|
"""
|
||||||
|
Regression tests for WireManager.add_wire / add_polyline_wire on hierarchical
|
||||||
|
sub-sheets.
|
||||||
|
|
||||||
|
Sub-sheets do not carry a (sheet_instances ...) block — that block is only
|
||||||
|
emitted on the root .kicad_sch in a hierarchical design. Before the fix, both
|
||||||
|
methods bailed out with "No sheet_instances section found in schematic" and
|
||||||
|
returned False, so users saw "Failed to add wire" on every call into a child
|
||||||
|
sheet.
|
||||||
|
"""
|
||||||
|
|
||||||
|
import sys
|
||||||
|
from pathlib import Path
|
||||||
|
from typing import Any
|
||||||
|
|
||||||
|
import pytest
|
||||||
|
|
||||||
|
sys.path.insert(0, str(Path(__file__).parent.parent / "python"))
|
||||||
|
|
||||||
|
|
||||||
|
# Minimal sub-sheet content: same outer (kicad_sch ...) form as a root sheet
|
||||||
|
# but WITHOUT (sheet_instances ...).
|
||||||
|
SUB_SHEET_NO_SHEET_INSTANCES = """(kicad_sch
|
||||||
|
\t(version 20260306)
|
||||||
|
\t(generator "eeschema")
|
||||||
|
\t(generator_version "10.0")
|
||||||
|
\t(uuid "bbbb2222-2222-2222-2222-bbbbbbbbbbbb")
|
||||||
|
\t(paper "A4")
|
||||||
|
\t(lib_symbols)
|
||||||
|
)
|
||||||
|
"""
|
||||||
|
|
||||||
|
|
||||||
|
def _write_sub_sheet(tmp_path: Path) -> Path:
|
||||||
|
sch = tmp_path / "child.kicad_sch"
|
||||||
|
sch.write_text(SUB_SHEET_NO_SHEET_INSTANCES, encoding="utf-8")
|
||||||
|
return sch
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.unit
|
||||||
|
class TestAddWireSubSheet:
|
||||||
|
"""WireManager.add_wire must succeed on hierarchical sub-sheets."""
|
||||||
|
|
||||||
|
def setup_method(self) -> None:
|
||||||
|
from commands.wire_manager import WireManager
|
||||||
|
|
||||||
|
self.WireManager = WireManager
|
||||||
|
|
||||||
|
def test_add_wire_succeeds_on_sub_sheet(self, tmp_path: Any) -> None:
|
||||||
|
sch = _write_sub_sheet(tmp_path)
|
||||||
|
|
||||||
|
ok = self.WireManager.add_wire(sch, [40.0, 40.0], [40.0, 50.0])
|
||||||
|
|
||||||
|
assert ok is True
|
||||||
|
content = sch.read_text(encoding="utf-8")
|
||||||
|
# The sexpdata writer emits the wire form somewhere in the file.
|
||||||
|
assert "wire" in content
|
||||||
|
assert "40.0" in content or "40 " in content
|
||||||
|
|
||||||
|
def test_add_wire_keeps_outer_form_balanced_on_sub_sheet(self, tmp_path: Any) -> None:
|
||||||
|
sch = _write_sub_sheet(tmp_path)
|
||||||
|
|
||||||
|
self.WireManager.add_wire(sch, [10.0, 10.0], [10.0, 20.0])
|
||||||
|
|
||||||
|
content = sch.read_text(encoding="utf-8")
|
||||||
|
assert content.count("(") == content.count(
|
||||||
|
")"
|
||||||
|
), "Inserting into a sub-sheet must keep parens balanced"
|
||||||
|
|
||||||
|
import sexpdata
|
||||||
|
|
||||||
|
parsed = sexpdata.loads(content)
|
||||||
|
assert parsed[0] == sexpdata.Symbol("kicad_sch")
|
||||||
|
|
||||||
|
def test_add_wire_writes_endpoints_on_sub_sheet(self, tmp_path: Any) -> None:
|
||||||
|
"""The new wire must round-trip through sexpdata with the requested endpoints."""
|
||||||
|
import sexpdata
|
||||||
|
|
||||||
|
sch = _write_sub_sheet(tmp_path)
|
||||||
|
|
||||||
|
self.WireManager.add_wire(sch, [12.5, 22.5], [42.5, 22.5])
|
||||||
|
|
||||||
|
parsed = sexpdata.loads(sch.read_text(encoding="utf-8"))
|
||||||
|
wires = [
|
||||||
|
item
|
||||||
|
for item in parsed[1:]
|
||||||
|
if isinstance(item, list) and len(item) > 0 and item[0] == sexpdata.Symbol("wire")
|
||||||
|
]
|
||||||
|
assert wires, "expected at least one (wire ...) item at top level after insert"
|
||||||
|
# Find xy points among the wire's children
|
||||||
|
wire_text = sexpdata.dumps(wires[0])
|
||||||
|
assert "12.5" in wire_text and "22.5" in wire_text and "42.5" in wire_text
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.unit
|
||||||
|
class TestAddPolylineWireSubSheet:
|
||||||
|
"""WireManager.add_polyline_wire must succeed on hierarchical sub-sheets."""
|
||||||
|
|
||||||
|
def setup_method(self) -> None:
|
||||||
|
from commands.wire_manager import WireManager
|
||||||
|
|
||||||
|
self.WireManager = WireManager
|
||||||
|
|
||||||
|
def test_add_polyline_wire_succeeds_on_sub_sheet(self, tmp_path: Any) -> None:
|
||||||
|
sch = _write_sub_sheet(tmp_path)
|
||||||
|
|
||||||
|
ok = self.WireManager.add_polyline_wire(sch, [[10.0, 10.0], [20.0, 10.0], [20.0, 30.0]])
|
||||||
|
|
||||||
|
assert ok is True
|
||||||
|
|
||||||
|
def test_add_polyline_wire_writes_n_minus_1_segments(self, tmp_path: Any) -> None:
|
||||||
|
"""A 4-point polyline must insert 3 (wire ...) segments into the file."""
|
||||||
|
import sexpdata
|
||||||
|
|
||||||
|
sch = _write_sub_sheet(tmp_path)
|
||||||
|
|
||||||
|
self.WireManager.add_polyline_wire(
|
||||||
|
sch, [[0.0, 0.0], [10.0, 0.0], [10.0, 10.0], [20.0, 10.0]]
|
||||||
|
)
|
||||||
|
|
||||||
|
parsed = sexpdata.loads(sch.read_text(encoding="utf-8"))
|
||||||
|
wires = [
|
||||||
|
item
|
||||||
|
for item in parsed[1:]
|
||||||
|
if isinstance(item, list) and len(item) > 0 and item[0] == sexpdata.Symbol("wire")
|
||||||
|
]
|
||||||
|
assert len(wires) == 3, f"expected 3 wire segments, got {len(wires)}"
|
||||||
|
|
||||||
|
def test_add_polyline_wire_keeps_outer_form_balanced_on_sub_sheet(self, tmp_path: Any) -> None:
|
||||||
|
sch = _write_sub_sheet(tmp_path)
|
||||||
|
|
||||||
|
self.WireManager.add_polyline_wire(sch, [[0.0, 0.0], [10.0, 0.0], [10.0, 10.0]])
|
||||||
|
|
||||||
|
content = sch.read_text(encoding="utf-8")
|
||||||
|
assert content.count("(") == content.count(")")
|
||||||
Reference in New Issue
Block a user