nearxos/kicad-mcp-server
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Merge pull request #152 from mixelpixx/fix/post-pr145-test-fallout

Browse Source 
Align pin transform with eeschema (rotation, mirror, angle)
This commit is contained in:
Eugene Mikhantyev
2026-05-03 22:45:41 +01:00
committed by GitHub
parent 84ba778d60 496be317e9
commit 216d3f189d
9 changed files with 552 additions and 220 deletions
Download Patch File Download Diff File Expand all files Collapse all files

27
python/commands/pin_locator.py
View File

@@ -279,15 +279,26 @@ class PinLocator:
pin_def_angle = pins[pin_number].get("angle", 0)
# Mirror flips the angle before applying symbol rotation.
# mirror_x flips the X component of local vectors → reflects across Y axis → 180 - angle.
# mirror_y flips the Y component of local vectors → reflects across X axis → negate angle.
if mirror_x:
pin_def_angle = (180 - pin_def_angle) % 360
if mirror_y:
pin_def_angle = (-pin_def_angle) % 360
# Mirror this exactly the way WireDragger.pin_world_xy does, in the
# same order: Y-flip (lib Y-up → screen Y-down) → mirror → rotate.
#
# Y-flip on an angle: negate it (reflects across X axis).
pin_def_angle = (-pin_def_angle) % 360
absolute_angle = (pin_def_angle + symbol_rotation) % 360
# eeschema (symbol.h:43-44):
# (mirror x) = SYM_MIRROR_X = TRANSFORM(1,0,0,-1) → negates Y →
# reflect angle across X axis → -angle.
# (mirror y) = SYM_MIRROR_Y = TRANSFORM(-1,0,0,1) → negates X →
# reflect angle across Y axis → 180 - angle.
if mirror_x:
pin_def_angle = (-pin_def_angle) % 360
if mirror_y:
pin_def_angle = (180 - pin_def_angle) % 360
# eeschema's rotation TRANSFORM is screen-CCW in Y-down, which is
# math-CW in standard atan2 convention — so subtract the rotation
# to match `pin_world_xy`'s `_rotate(..., -rotation)` call.
absolute_angle = (pin_def_angle - symbol_rotation) % 360
return absolute_angle
except Exception:

26
python/commands/wire_dragger.py
View File

@@ -75,14 +75,17 @@ class WireDragger:
if not (isinstance(item, list) and item and item[0] == sym_k):
continue
# Check Reference property
# Check Reference property.
# kicad-skip may write a trailing "_" on references (e.g. "R1_") when
# cloning symbols; strip it so callers passing the canonical "R1"
# still find the symbol. Mirrors the rstrip in PinLocator.get_pin_location.
ref_val = None
for sub in item[1:]:
if isinstance(sub, list) and len(sub) >= 3 and sub[0] == prop_k:
if str(sub[1]).strip('"') == "Reference":
ref_val = str(sub[2]).strip('"')
break
if ref_val != reference:
if ref_val is None or ref_val.rstrip("_") != reference:
continue
old_x = old_y = rotation = 0.0
@@ -153,15 +156,22 @@ class WireDragger:
Compute the world coordinate of a pin given the symbol transform.
Library pins are stored Y-up; the schematic is Y-down. Order matches
eeschema: mirror in lib space → Y-flip to screen → rotate → translate.
Without the Y-flip, polarized parts get pin 1/pin 2 silently swapped.
eeschema: Y-flip to screen → mirror → rotate (screen-CCW) → translate.
eeschema's TRANSFORM matrix for rotation 90 is (0, 1, -1, 0) —
i.e. screen-CCW in Y-down: (x, y) → (y, -x). Our `_rotate` helper is
standard math (Y-up CCW), so we negate the rotation angle to convert.
Mirror axis semantics match eeschema's symbol.h:
(mirror x) = SYM_MIRROR_X = TRANSFORM(1, 0, 0, -1) → negates Y.
(mirror y) = SYM_MIRROR_Y = TRANSFORM(-1, 0, 0, 1) → negates X.
"""
lx, ly = px, py
lx, ly = px, -py # Y-flip: lib Y-up → screen Y-down
if mirror_x:
lx = -lx
ly = -ly # SYM_MIRROR_X negates screen-Y
if mirror_y:
ly = -ly
rx, ry = _rotate(lx, -ly, rotation)
lx = -lx # SYM_MIRROR_Y negates screen-X
rx, ry = _rotate(lx, ly, -rotation) # negate angle: math-CCW → screen-CCW
return sym_x + rx, sym_y + ry
@staticmethod

123
tests/test_add_schematic_component.py
View File

@@ -19,9 +19,7 @@ EMPTY_SCH = TEMPLATES_DIR / "empty.kicad_sch"
def _write_temp_sch(content: str) -> Path:
tmp = tempfile.NamedTemporaryFile(
suffix=".kicad_sch", delete=False, mode="w", encoding="utf-8"
)
tmp = tempfile.NamedTemporaryFile(suffix=".kicad_sch", delete=False, mode="w", encoding="utf-8")
tmp.write(content)
tmp.close()
return Path(tmp.name)
@@ -36,7 +34,10 @@ def _unit_values_in_file(path: Path) -> list[int]:
"""Return all (unit N) values written for symbol instances in the schematic."""
content = path.read_text()
# Match top-level symbol instances: (symbol (lib_id ...) (at ...) (unit N) ...)
return [int(n) for n in re.findall(r"\(symbol \(lib_id [^)]+\) \(at [^)]+\) \(unit (\d+)\)", content)]
return [
int(n)
for n in re.findall(r"\(symbol \(lib_id [^)]+\) \(at [^)]+\) \(unit (\d+)\)", content)
]
# ---------------------------------------------------------------------------
@@ -155,18 +156,20 @@ class TestHandlerAddSchematicComponent:
def test_unit_defaults_to_1_in_handler(self, tmp_path: Any) -> None:
sch = tmp_path / "test.kicad_sch"
shutil.copy(EMPTY_SCH, sch)
result = self._call_handler({
"schematicPath": str(sch),
"component": {
"library": "Device",
"type": "R",
"reference": "R99",
"value": "1k",
"x": 10,
"y": 10,
# no "unit" key — should default to 1
},
})
result = self._call_handler(
{
"schematicPath": str(sch),
"component": {
"library": "Device",
"type": "R",
"reference": "R99",
"value": "1k",
"x": 10,
"y": 10,
# no "unit" key — should default to 1
},
}
)
assert result["success"] is True
units = _unit_values_in_file(sch)
assert 1 in units
@@ -174,18 +177,82 @@ class TestHandlerAddSchematicComponent:
def test_unit_2_passed_through_handler(self, tmp_path: Any) -> None:
sch = tmp_path / "test.kicad_sch"
shutil.copy(EMPTY_SCH, sch)
result = self._call_handler({
"schematicPath": str(sch),
"component": {
"library": "Device",
"type": "R",
"reference": "U10",
"value": "TLP291-4",
"x": 25,
"y": 35,
"unit": 2,
},
})
result = self._call_handler(
{
"schematicPath": str(sch),
"component": {
"library": "Device",
"type": "R",
"reference": "U10",
"value": "TLP291-4",
"x": 25,
"y": 35,
"unit": 2,
},
}
)
assert result["success"] is True
units = _unit_values_in_file(sch)
assert 2 in units
# ---------------------------------------------------------------------------
# Mirror parameter — known gap
# ---------------------------------------------------------------------------
@pytest.mark.unit
class TestAddComponentMirrorParam:
"""ComponentManager.add_component does NOT honor a 'mirror' kwarg today.
The MCP add_schematic_component tool schema also doesn't expose mirror.
A mirror is currently only applicable post-add via rotate_schematic_component.
These tests pin down the silent-drop behavior so a fixture that passes
'mirror': 'x' and then asserts something against the resulting schematic
cannot accidentally pass for the wrong reason (the symbol ends up
unmirrored). If/when add_component grows real mirror support, update both
tests together — the second test then becomes the positive assertion."""
def setup_method(self) -> None:
from commands.component_schematic import ComponentManager
from commands.schematic import SchematicManager
self.ComponentManager = ComponentManager
self.SchematicManager = SchematicManager
def _add(self, sch_path: Path, mirror_value: Any) -> None:
sch = self.SchematicManager.load_schematic(str(sch_path))
params = {
"type": "R",
"reference": "R1",
"value": "10k",
"x": 100.0,
"y": 100.0,
"rotation": 0,
}
if mirror_value is not None:
params["mirror"] = mirror_value
self.ComponentManager.add_component(sch, params, sch_path)
self.SchematicManager.save_schematic(sch, str(sch_path))
def test_mirror_x_arg_is_silently_dropped(self, tmp_path: Any) -> None:
sch = tmp_path / "mirror_x.kicad_sch"
shutil.copy(EMPTY_SCH, sch)
self._add(sch, "x")
text = sch.read_text()
assert "(mirror x)" not in text, (
"ComponentManager.add_component now appears to honor mirror='x'. "
"Update _build_mirror_case in test_pin_world_xy_eeschema_truth.py "
"to drop the post-add mirror application and remove this test."
)
def test_mirror_y_arg_is_silently_dropped(self, tmp_path: Any) -> None:
sch = tmp_path / "mirror_y.kicad_sch"
shutil.copy(EMPTY_SCH, sch)
self._add(sch, "y")
text = sch.read_text()
assert "(mirror y)" not in text, (
"ComponentManager.add_component now appears to honor mirror='y'. "
"See sibling test_mirror_x_arg_is_silently_dropped."
)

71
tests/test_hierarchical_pad_net_map.py
View File

@@ -36,8 +36,7 @@ sys.path.insert(0, str(Path(__file__).parent.parent / "python"))
# pin "2" at ( 1.27, 0) → wire connects on the right
# ---------------------------------------------------------------------------
_SCH_WITH_TESTLIB_R = textwrap.dedent("""\
(kicad_sch (version 20231120)
_LIB_SYMBOLS_BLOCK = textwrap.dedent("""\
(lib_symbols
(symbol "TestLib:R"
(symbol "TestLib:R_1_1"
@@ -52,12 +51,40 @@ _SCH_WITH_TESTLIB_R = textwrap.dedent("""\
)
)
)
)
""")
_SCH_WITH_TESTLIB_R = "(kicad_sch (version 20231120)\n" + _LIB_SYMBOLS_BLOCK + ")\n"
_SCH_EMPTY = "(kicad_sch (version 20231120))"
def _build_sch_with_instances(
instances: "list[tuple[str, str, float, float, float]] | None" = None,
) -> str:
"""Build a .kicad_sch text with TestLib:R lib_symbols + the given symbol instances.
Each instance tuple: (reference, lib_id, x, y, rotation).
Required because PinLocator._get_symbol_transform reads the symbol position,
rotation, and lib_id directly from disk via sexpdata (it does not consult
the kicad-skip cache that the tests mock for labels/wires).
"""
parts = ["(kicad_sch (version 20231120)", _LIB_SYMBOLS_BLOCK]
for i, (ref, lib_id, x, y, rot) in enumerate(instances or [], start=1):
uuid = f"00000000-0000-0000-0000-{i:012d}"
parts.append(
f' (symbol (lib_id "{lib_id}") (at {x} {y} {rot}) (unit 1)\n'
f" (uuid {uuid})\n"
f' (property "Reference" "{ref}" (at {x} {y - 2.54} 0))\n'
f' (property "Value" "~" (at {x} {y + 2.54} 0))\n'
f' (pin "1" (uuid {uuid[:-1]}a))\n'
f' (pin "2" (uuid {uuid[:-1]}b))\n'
f" )\n"
)
parts.append(")\n")
return "".join(parts)
# ---------------------------------------------------------------------------
# Mock helpers
# ---------------------------------------------------------------------------
@@ -176,7 +203,7 @@ class TestLabelAtPin:
def test_global_label_pin1(self, iface, tmp_path):
"""Global label at pin-1 position → (R1, 1) in map."""
sch = tmp_path / "top.kicad_sch"
sch.write_text(_SCH_WITH_TESTLIB_R)
sch.write_text(_build_sch_with_instances([("R1", "TestLib:R", 10.0, 10.0, 0)]))
# R1 at (10, 10); pin 1 abs = (10 − 1.27, 10) = (8.73, 10)
mock_sch = _sch_mock(
symbols=[_sym_mock("R1", "TestLib:R", 10.0, 10.0)],
@@ -189,7 +216,7 @@ class TestLabelAtPin:
def test_global_label_pin2(self, iface, tmp_path):
"""Global label at pin-2 position → (R1, 2) in map."""
sch = tmp_path / "top.kicad_sch"
sch.write_text(_SCH_WITH_TESTLIB_R)
sch.write_text(_build_sch_with_instances([("R1", "TestLib:R", 10.0, 10.0, 0)]))
# pin 2 abs = (10 + 1.27, 10) = (11.27, 10)
mock_sch = _sch_mock(
symbols=[_sym_mock("R1", "TestLib:R", 10.0, 10.0)],
@@ -202,7 +229,7 @@ class TestLabelAtPin:
def test_both_pins_mapped(self, iface, tmp_path):
"""Labels at both pin positions → both (ref, pin) keys present."""
sch = tmp_path / "top.kicad_sch"
sch.write_text(_SCH_WITH_TESTLIB_R)
sch.write_text(_build_sch_with_instances([("R1", "TestLib:R", 10.0, 10.0, 0)]))
mock_sch = _sch_mock(
symbols=[_sym_mock("R1", "TestLib:R", 10.0, 10.0)],
global_labels=[
@@ -217,7 +244,7 @@ class TestLabelAtPin:
def test_local_label_also_works(self, iface, tmp_path):
"""Local (net) labels are treated identically to global labels."""
sch = tmp_path / "top.kicad_sch"
sch.write_text(_SCH_WITH_TESTLIB_R)
sch.write_text(_build_sch_with_instances([("R1", "TestLib:R", 10.0, 10.0, 0)]))
mock_sch = _sch_mock(
symbols=[_sym_mock("R1", "TestLib:R", 10.0, 10.0)],
labels=[_lbl_mock("LOCAL_NET", 8.73, 10.0)],
@@ -228,7 +255,7 @@ class TestLabelAtPin:
def test_hierarchical_label_also_works(self, iface, tmp_path):
"""Hierarchical labels are treated identically to global labels."""
sch = tmp_path / "top.kicad_sch"
sch.write_text(_SCH_WITH_TESTLIB_R)
sch.write_text(_build_sch_with_instances([("R1", "TestLib:R", 10.0, 10.0, 0)]))
mock_sch = _sch_mock(
symbols=[_sym_mock("R1", "TestLib:R", 10.0, 10.0)],
hier_labels=[_lbl_mock("HIER_NET", 8.73, 10.0)],
@@ -249,7 +276,7 @@ class TestLabelViaWire:
def test_label_one_hop_away(self, iface, tmp_path):
"""Label at wire start, wire end at pin → net assigned."""
sch = tmp_path / "top.kicad_sch"
sch.write_text(_SCH_WITH_TESTLIB_R)
sch.write_text(_build_sch_with_instances([("R1", "TestLib:R", 10.0, 10.0, 0)]))
# pin 1 at (8.73, 10); label at (5.0, 10); wire (5.0,10)→(8.73,10)
mock_sch = _sch_mock(
symbols=[_sym_mock("R1", "TestLib:R", 10.0, 10.0)],
@@ -262,7 +289,7 @@ class TestLabelViaWire:
def test_label_two_hops_away(self, iface, tmp_path):
"""Net propagates through two chained wire segments."""
sch = tmp_path / "top.kicad_sch"
sch.write_text(_SCH_WITH_TESTLIB_R)
sch.write_text(_build_sch_with_instances([("R1", "TestLib:R", 10.0, 10.0, 0)]))
# label at (3.0, 10); wire1: 3→6; wire2: 6→8.73 → pin 1
mock_sch = _sch_mock(
symbols=[_sym_mock("R1", "TestLib:R", 10.0, 10.0)],
@@ -330,7 +357,7 @@ class TestMultipleSubsheets:
sub_dir = tmp_path / "sheets"
sub_dir.mkdir()
sub = sub_dir / "component_sheet.kicad_sch"
sub.write_text(_SCH_WITH_TESTLIB_R)
sub.write_text(_build_sch_with_instances([("R2", "TestLib:R", 10.0, 10.0, 0)]))
top_mock = _sch_mock() # top sheet has no components
sub_mock = _sch_mock(
@@ -340,6 +367,7 @@ class TestMultipleSubsheets:
def _factory(path: str) -> MagicMock:
from pathlib import Path as _P
return sub_mock if _P(path).name == "component_sheet.kicad_sch" else top_mock
with (
@@ -354,11 +382,11 @@ class TestMultipleSubsheets:
def test_top_and_sub_components_merged(self, iface, tmp_path):
"""Components from both top-level and sub-sheet appear in the same map."""
top = tmp_path / "top.kicad_sch"
top.write_text(_SCH_WITH_TESTLIB_R)
top.write_text(_build_sch_with_instances([("R1", "TestLib:R", 10.0, 10.0, 0)]))
sub_dir = tmp_path / "sheets"
sub_dir.mkdir()
sub = sub_dir / "component_sheet.kicad_sch"
sub.write_text(_SCH_WITH_TESTLIB_R)
sub.write_text(_build_sch_with_instances([("R2", "TestLib:R", 10.0, 10.0, 0)]))
top_mock = _sch_mock(
symbols=[_sym_mock("R1", "TestLib:R", 10.0, 10.0)],
@@ -371,6 +399,7 @@ class TestMultipleSubsheets:
def _factory(path: str) -> MagicMock:
from pathlib import Path as _P
return sub_mock if _P(path).name == "component_sheet.kicad_sch" else top_mock
with (
@@ -391,17 +420,19 @@ class TestMultipleSubsheets:
@pytest.mark.unit
class TestRotatedSymbol:
def test_90_degree_rotation(self, iface, tmp_path):
"""A 90° CCW rotation maps pin (-1.27, 0) → (0, -1.27) in local coords."""
"""eeschema rot=90 is CCW in screen Y-down: TRANSFORM(0,1,-1,0).
Pin 1 lib (-1.27, 0) → internal (-1.27, 0) → (0,1,-1,0) applied = (0, 1.27)
→ world (10.0, 11.27).
Pin 2 lib ( 1.27, 0) → internal ( 1.27, 0) → (0,1,-1,0) applied = (0, -1.27)
→ world (10.0, 8.73).
Verified vs kicad-cli netlist on a Device:D rotated 90."""
sch = tmp_path / "top.kicad_sch"
sch.write_text(_SCH_WITH_TESTLIB_R)
# R1 at (10, 10), rotation=90°
# pin 1 (-1.27, 0) rotated 90° CCW → (0, -1.27) → abs (10.0, 8.73)
# pin 2 ( 1.27, 0) rotated 90° CCW → (0, 1.27) → abs (10.0, 11.27)
sch.write_text(_build_sch_with_instances([("R1", "TestLib:R", 10.0, 10.0, 90)]))
mock_sch = _sch_mock(
symbols=[_sym_mock("R1", "TestLib:R", 10.0, 10.0, rotation=90)],
global_labels=[
_lbl_mock("UP_NET", 10.0, 8.73),
_lbl_mock("DN_NET", 10.0, 11.27),
_lbl_mock("UP_NET", 10.0, 11.27),
_lbl_mock("DN_NET", 10.0, 8.73),
],
)
pad_net_map, _ = _call(iface, sch, mock_sch)

7
tests/test_move_with_wire_preservation.py
View File

@@ -222,13 +222,14 @@ class TestComputePinPositions:
assert abs(new2[1] - 23.81) < 1e-4
def test_resistor_rotated_90(self) -> None:
"""Device:R at (100, 100) rot=90. Pin 1 lib (0, +3.81) → y-flipped to
(0, -3.81) → rotated 90° → (3.81, 0) → world (103.81, 100)."""
"""Device:R at (100, 100) rot=90. Pin 1 lib (0, +3.81), Y-flip → (0, -3.81),
eeschema rot=90 TRANSFORM(0,1,-1,0): (0*0+1*-3.81, -1*0+0*-3.81) = (-3.81, 0).
World (96.19, 100). Verified vs kicad-cli netlist."""
sch = _make_sch_data([_make_symbol("R1", 100, 100, rotation=90)])
positions = WireDragger.compute_pin_positions(sch, "R1", 100, 100)
old1, _ = positions["1"]
old2, _ = positions["2"]
assert abs(old1[0] - 103.81) < 1e-3
assert abs(old1[0] - 96.19) < 1e-3
assert abs(old1[1] - 100) < 1e-3
def test_returns_empty_for_missing_component(self) -> None:

183
tests/test_pin_locator_and_component.py
View File

@@ -1,17 +1,21 @@
"""
Regression tests for three bugs fixed in PR #103:
Regression tests for bugs originally fixed in PR #103 and updated for PR #145.
1. component_schematic.py: clone() + redundant append() causes trailing "_" on reference
2. pin_locator.py: pin_rel_y must be negated (lib y-up → schematic y-down)
3. pin_locator.py: reference comparison must tolerate trailing "_" from kicad-skip
2. pin_locator.py: reference comparison must tolerate trailing "_" from kicad-skip
(this also covers WireDragger.find_symbol, used by _get_symbol_transform)
The pre-PR-145 y-axis-negation tests were removed: their assertions encoded the
correct post-PR-145 convention, but their MagicMock setup bypassed
_get_symbol_transform (which reads the .kicad_sch file directly via sexpdata).
The y-flip behaviour is now covered end-to-end against eeschema in
tests/test_pin_locator_y_flip.py — duplicating it with mocks added no value.
"""
import shutil
import sys
import tempfile
import types
from pathlib import Path
from unittest.mock import MagicMock, patch
import pytest
@@ -26,15 +30,6 @@ sys.path.insert(0, str(PYTHON_DIR))
_TEMPLATE_SCH = TEMPLATES_DIR / "template_with_symbols.kicad_sch"
def _stub_symbol(ref: str, at: list, lib_id: str = "Device:R") -> MagicMock:
"""Build a minimal kicad-skip symbol stub."""
sym = MagicMock()
sym.property.Reference.value = ref
sym.at.value = at
sym.lib_id.value = lib_id
return sym
# ===========================================================================
# 1. component_schematic — no trailing underscore after clone()
# ===========================================================================
@@ -71,118 +66,78 @@ class TestAddComponentNoTrailingUnderscore:
# ===========================================================================
# 2. pin_locator — y-axis sign (lib y-up → schematic y-down)
# 2. pin_locator — .rstrip("_") tolerance in reference lookup
# ===========================================================================
@pytest.mark.unit
class TestPinLocatorYAxisNegation:
"""
Device:R pin 1 is at library y=+3.81 (y-up).
For a symbol centred at (100, 100) with rotation=0, the schematic absolute y
must be 100 - 3.81 = 96.19, NOT 100 + 3.81 = 103.81.
"""
@pytest.fixture(autouse=True)
def setup(self):
# Stub sexpdata and skip so the module can be imported without them installed
for mod_name in ("sexpdata", "skip"):
sys.modules.setdefault(mod_name, types.ModuleType(mod_name))
from commands.pin_locator import PinLocator
self.locator = PinLocator()
def test_pin1_y_above_center_for_rotation_0(self):
"""Pin at lib y=+3.81 should appear *above* the symbol centre (lower y value)."""
sym = _stub_symbol("R1", at=[100.0, 100.0, 0.0])
self.locator._schematic_cache["test.kicad_sch"] = MagicMock(symbol=[sym])
# Patch get_symbol_pins to return a Device:R-like pin definition
with patch.object(
self.locator,
"get_symbol_pins",
return_value={"1": {"x": 0.0, "y": 3.81, "angle": 270, "name": "~"}},
):
result = self.locator.get_pin_location(Path("test.kicad_sch"), "R1", "1")
assert result is not None
x, y = result
assert abs(x - 100.0) < 1e-6, f"x should be 100.0, got {x}"
assert abs(y - 96.19) < 1e-4, (
f"y should be ~96.19 (above centre), got {y}. "
"y was not negated — library y-up convention mismatch."
)
def test_pin2_y_below_center_for_rotation_0(self):
"""Pin at lib y=-3.81 should appear *below* the symbol centre (higher y value)."""
sym = _stub_symbol("R1", at=[100.0, 100.0, 0.0])
self.locator._schematic_cache["test.kicad_sch"] = MagicMock(symbol=[sym])
with patch.object(
self.locator,
"get_symbol_pins",
return_value={"2": {"x": 0.0, "y": -3.81, "angle": 90, "name": "~"}},
):
result = self.locator.get_pin_location(Path("test.kicad_sch"), "R1", "2")
assert result is not None
_, y = result
assert abs(y - 103.81) < 1e-4, f"y should be ~103.81 (below centre), got {y}."
def test_pin1_rotated_90(self):
"""
Symbol rotated 90°. Pin at lib (x=0, y=+3.81).
After y-negation: (0, -3.81). After 90° CCW rotation: (x=3.81, y=0).
Absolute: (100+3.81, 100+0) = (103.81, 100).
"""
sym = _stub_symbol("C1", at=[100.0, 100.0, 90.0])
self.locator._schematic_cache["test.kicad_sch"] = MagicMock(symbol=[sym])
with patch.object(
self.locator,
"get_symbol_pins",
return_value={"1": {"x": 0.0, "y": 3.81, "angle": 270, "name": "~"}},
):
result = self.locator.get_pin_location(Path("test.kicad_sch"), "C1", "1")
assert result is not None
x, y = result
assert abs(x - 103.81) < 1e-4, f"x should be ~103.81, got {x}"
assert abs(y - 100.0) < 1e-4, f"y should be ~100.0, got {y}"
# ===========================================================================
# 3. pin_locator — .rstrip("_") tolerance in reference lookup
# ===========================================================================
@pytest.mark.unit
@pytest.mark.integration
class TestPinLocatorReferenceRstrip:
"""kicad-skip may write 'R1_' — lookups must still find 'R1'."""
"""
kicad-skip may write 'R1_' on disk after a clone; lookups for 'R1' must
still resolve. This must hold for *both* lookup paths inside
get_pin_location: the kicad-skip Schematic scan AND the sexpdata-based
_get_symbol_transform (via WireDragger.find_symbol).
"""
@pytest.fixture(autouse=True)
def setup(self):
for mod_name in ("sexpdata", "skip"):
sys.modules.setdefault(mod_name, types.ModuleType(mod_name))
from commands.pin_locator import PinLocator
def _write_sch_with_underscored_ref(self, sch_path: Path) -> None:
"""Add R1, then mangle the on-disk reference to 'R1_' to simulate the kicad-skip artifact."""
from commands.component_schematic import ComponentManager
from commands.schematic import SchematicManager
self.locator = PinLocator()
shutil.copy(_TEMPLATE_SCH, sch_path)
sch = SchematicManager.load_schematic(str(sch_path))
ComponentManager.add_component(
sch,
{"type": "R", "reference": "R1", "value": "10k", "x": 100.0, "y": 100.0, "rotation": 0},
sch_path,
)
SchematicManager.save_schematic(sch, str(sch_path))
# Rewrite the saved file, replacing the Reference "R1" with "R1_"
text = sch_path.read_text(encoding="utf-8")
text = text.replace('(property "Reference" "R1"', '(property "Reference" "R1_"', 1)
sch_path.write_text(text, encoding="utf-8")
def test_get_pin_location_finds_symbol_with_trailing_underscore(self):
# Symbol stored in schematic with reference 'R1_' (kicad-skip artifact)
sym = _stub_symbol("R1_", at=[50.0, 50.0, 0.0])
self.locator._schematic_cache["sch.kicad_sch"] = MagicMock(symbol=[sym])
with patch.object(
self.locator,
"get_symbol_pins",
return_value={"1": {"x": 0.0, "y": 3.81, "angle": 270, "name": "~"}},
):
# Caller uses clean reference 'R1'; should still resolve
result = self.locator.get_pin_location(Path("sch.kicad_sch"), "R1", "1")
from commands.pin_locator import PinLocator
with tempfile.TemporaryDirectory() as tmp:
sch_path = Path(tmp) / "sch.kicad_sch"
self._write_sch_with_underscored_ref(sch_path)
locator = PinLocator()
# Caller uses clean reference 'R1'; should still resolve through both
# the kicad-skip path and the sexpdata _get_symbol_transform path.
result = locator.get_pin_location(sch_path, "R1", "1")
assert (
result is not None
), "get_pin_location returned None for reference 'R1' when schematic stores 'R1_'"
def test_get_pin_location_returns_none_for_genuinely_missing_symbol(self):
sym = _stub_symbol("R2", at=[50.0, 50.0, 0.0])
self.locator._schematic_cache["sch.kicad_sch"] = MagicMock(symbol=[sym])
result = self.locator.get_pin_location(Path("sch.kicad_sch"), "R1", "1")
from commands.component_schematic import ComponentManager
from commands.pin_locator import PinLocator
from commands.schematic import SchematicManager
with tempfile.TemporaryDirectory() as tmp:
sch_path = Path(tmp) / "sch.kicad_sch"
shutil.copy(_TEMPLATE_SCH, sch_path)
sch = SchematicManager.load_schematic(str(sch_path))
ComponentManager.add_component(
sch,
{
"type": "R",
"reference": "R2",
"value": "1k",
"x": 50.0,
"y": 50.0,
"rotation": 0,
},
sch_path,
)
SchematicManager.save_schematic(sch, str(sch_path))
locator = PinLocator()
result = locator.get_pin_location(sch_path, "R1", "1")
assert result is None

9
tests/test_pin_locator_y_flip.py
View File

@@ -105,8 +105,9 @@ def test_rotated_capacitor_pin_x_matches_render_convention():
p1 = locator.get_pin_location(sch_path, "C1", "1")
assert p1 is not None
# Device:C pin 1 is at symbol (0, +3.81). After y-negate → (0, -3.81).
# Rotated 90° CCW in screen coords: (0, -3.81) → (3.81, 0).
# Absolute: (150+3.81, 100+0) = (153.81, 100).
assert p1[0] == pytest.approx(153.81), f"rotated pin 1 X wrong: {p1[0]}"
# Device:C pin 1 lib (0, +3.81). parseXY(invertY=true) → internal (0, -3.81).
# Rotation 90 in eeschema is CCW in screen Y-down: TRANSFORM(0,1,-1,0).
# Apply: (0*0 + 1*(-3.81), -1*0 + 0*(-3.81)) = (-3.81, 0).
# World: (150-3.81, 100) = (146.19, 100). Verified vs kicad-cli netlist.
assert p1[0] == pytest.approx(146.19), f"rotated pin 1 X wrong: {p1[0]}"
assert p1[1] == pytest.approx(100.0), f"rotated pin 1 Y wrong: {p1[1]}"

230
tests/test_pin_world_xy_eeschema_truth.py Normal file
View File

@@ -0,0 +1,230 @@
"""
Ground-truth regression for WireDragger.pin_world_xy + label snap.
The oracle is eeschema itself, accessed via `kicad-cli sch export netlist`.
For each (rotation, mirror) corner case, we:
1. Build a schematic with a polarized component (Device:D, K=pin1, A=pin2).
2. Apply the transform via the MCP rotate handler.
3. Snap a label "<ref>_K" to pin 1 and "<ref>_A" to pin 2 via PinLocator coords.
4. Run kicad-cli to extract the netlist.
5. Assert each label's net binds to the *named* pin in the netlist —
i.e. label "_K" must end up on pin 1 (K), not pin 2 (A).
If our pin coords agree with eeschema's render, the labels land on the
intended pins. If they disagree, the netlist swaps them, exposing the bug.
Skips if kicad-cli or the system Device library aren't available.
"""
import os
import shutil
import subprocess
import sys
import tempfile
import xml.etree.ElementTree as ET
from pathlib import Path
import pytest
PYTHON_DIR = Path(__file__).parent.parent / "python"
sys.path.insert(0, str(PYTHON_DIR))
from commands.component_schematic import ComponentManager # noqa: E402
from commands.pin_locator import PinLocator # noqa: E402
from commands.schematic import SchematicManager # noqa: E402
from commands.wire_dragger import WireDragger # noqa: E402
_KICAD_CLI = shutil.which("kicad-cli")
_DEVICE_LIB = Path("/usr/share/kicad/symbols/Device.kicad_sym")
pytestmark = [
pytest.mark.integration,
pytest.mark.skipif(_KICAD_CLI is None, reason="kicad-cli not on PATH"),
pytest.mark.skipif(not _DEVICE_LIB.exists(), reason="Device lib not installed"),
]
def _add_labels_to_file(sch_path: Path, labels: list[tuple[str, float, float]]) -> None:
"""Inject (label ...) tokens before the closing ')' of a .kicad_sch file."""
text = sch_path.read_text()
block = "\n"
for name, x, y in labels:
block += (
f' (label "{name}"\n'
f" (at {x} {y} 0)\n"
f" (effects (font (size 1.27 1.27)) (justify left bottom))\n"
f' (uuid "00000000-0000-0000-0000-{abs(hash(name)) % 10**12:012d}")\n'
f" )\n"
)
last = text.rstrip().rfind(")")
sch_path.write_text(text[:last] + block + text[last:])
def _extract_pin_to_net(netlist_xml: Path) -> dict:
"""Return {(ref, pin_num): net_name} from a kicad XML netlist."""
tree = ET.parse(netlist_xml)
root = tree.getroot()
out = {}
for net in root.findall(".//net"):
net_name = net.attrib.get("name", "").lstrip("/")
for node in net.findall("node"):
ref = node.attrib.get("ref")
pin = node.attrib.get("pin")
if ref and pin:
out[(ref, pin)] = net_name
return out
def _build_diode_case(tmp: Path, rotation: int) -> tuple[Path, dict]:
"""Place a Device:D, rotate, snap labels, save. Returns (sch_path, expected_map)."""
sch_path = tmp / f"diode_rot{rotation}.kicad_sch"
template = PYTHON_DIR / "templates" / "template_with_symbols.kicad_sch"
shutil.copy(template, sch_path)
sch = SchematicManager.load_schematic(str(sch_path))
ComponentManager.add_component(
sch,
{
"type": "D",
"reference": "D1",
"value": "1N4148",
"x": 100.0,
"y": 100.0,
"rotation": rotation,
},
sch_path,
)
SchematicManager.save_schematic(sch, str(sch_path))
locator = PinLocator()
p_k = locator.get_pin_location(sch_path, "D1", "1")
p_a = locator.get_pin_location(sch_path, "D1", "2")
assert p_k is not None and p_a is not None
_add_labels_to_file(sch_path, [("D1_K", p_k[0], p_k[1]), ("D1_A", p_a[0], p_a[1])])
return sch_path, {("D1", "1"): "D1_K", ("D1", "2"): "D1_A"}
def _apply_mirror_to_file(sch_path: Path, reference: str, axis: str) -> None:
"""Apply (mirror x|y) to a placed symbol via direct sexpr mutation.
ComponentManager.add_component silently drops a 'mirror' kwarg, so this
fixture goes around it via the same low-level helper rotate_schematic_component
uses (WireDragger.update_symbol_rotation_mirror)."""
import sexpdata
sch_data = sexpdata.loads(sch_path.read_text())
if not WireDragger.update_symbol_rotation_mirror(sch_data, reference, 0, axis):
raise RuntimeError(f"Failed to apply mirror={axis} to {reference}")
sch_path.write_text(sexpdata.dumps(sch_data))
def _build_mirror_case(tmp: Path, axis: str) -> tuple[Path, dict]:
sch_path = tmp / f"resistor_mirror_{axis}.kicad_sch"
template = PYTHON_DIR / "templates" / "template_with_symbols.kicad_sch"
shutil.copy(template, sch_path)
sch = SchematicManager.load_schematic(str(sch_path))
ComponentManager.add_component(
sch,
{"type": "R", "reference": "R1", "value": "10k", "x": 100.0, "y": 100.0, "rotation": 0},
sch_path,
)
SchematicManager.save_schematic(sch, str(sch_path))
_apply_mirror_to_file(sch_path, "R1", axis)
if f"(mirror {axis})" not in sch_path.read_text():
raise RuntimeError(
f"Fixture failed to write (mirror {axis}) — the kicad-cli oracle would "
f"silently match our pin coords for an unmirrored symbol."
)
locator = PinLocator()
p1 = locator.get_pin_location(sch_path, "R1", "1")
p2 = locator.get_pin_location(sch_path, "R1", "2")
if p1 is None or p2 is None:
raise RuntimeError(f"PinLocator returned None for R1 mirror={axis}")
_add_labels_to_file(sch_path, [("R1_PIN1", p1[0], p1[1]), ("R1_PIN2", p2[0], p2[1])])
return sch_path, {("R1", "1"): "R1_PIN1", ("R1", "2"): "R1_PIN2"}
def _run_netlist(sch_path: Path) -> dict:
out = sch_path.with_suffix(".net")
env = {**os.environ, "KICAD_SYMBOL_DIR": "/usr/share/kicad/symbols"}
subprocess.run(
[
_KICAD_CLI,
"sch",
"export",
"netlist",
"--format",
"kicadxml",
"-o",
str(out),
str(sch_path),
],
check=True,
capture_output=True,
env=env,
)
return _extract_pin_to_net(out)
@pytest.mark.parametrize("rotation", [0, 90, 180, 270])
def test_diode_label_polarity_through_eeschema(rotation):
"""Snap-labelled K must show up on pin 1 in the kicad-cli netlist."""
with tempfile.TemporaryDirectory() as td:
sch_path, expected = _build_diode_case(Path(td), rotation)
actual = _run_netlist(sch_path)
for (ref, pin), net in expected.items():
assert actual.get((ref, pin)) == net, (
f"rotation={rotation}: D1.{pin} label landed on wrong pin. "
f"Expected net={net}, got {actual.get((ref, pin))}. "
f"Full mapping: {actual}"
)
@pytest.mark.parametrize("axis", ["x", "y"])
def test_mirrored_resistor_label_through_eeschema(axis):
"""Snap-labelled pin 1 must show up on pin 1 after (mirror x) / (mirror y)."""
with tempfile.TemporaryDirectory() as td:
sch_path, expected = _build_mirror_case(Path(td), axis)
actual = _run_netlist(sch_path)
for (ref, pin), net in expected.items():
assert actual.get((ref, pin)) == net, (
f"mirror={axis}: R1.{pin} label landed on wrong pin. "
f"Expected net={net}, got {actual.get((ref, pin))}. "
f"Full mapping: {actual}"
)
def test_pin_world_xy_rot90_matches_eeschema_transform():
"""Pure-math regression: pin_world_xy for Device:R rot=90 must match
eeschema's TRANSFORM(0,1,-1,0) applied to internal Y-flipped pin."""
# Device:R pin 1: lib (0, +3.81). parseXY(invertY=true) → internal (0, -3.81).
# TRANSFORM(0,1,-1,0) applied: (0*0 + 1*-3.81, -1*0 + 0*-3.81) = (-3.81, 0).
# Symbol at (100, 100) → world (96.19, 100).
wx, wy = WireDragger.pin_world_xy(0.0, 3.81, 100.0, 100.0, 90, False, False)
assert wx == pytest.approx(96.19), f"rot=90 X wrong: {wx} (expected 96.19)"
assert wy == pytest.approx(100.0), f"rot=90 Y wrong: {wy} (expected 100.0)"
def test_pin_world_xy_mirror_x_matches_eeschema():
"""(mirror x) = SYM_MIRROR_X = TRANSFORM(1,0,0,-1) → negates internal Y.
Device:R pin 1 lib (0, +3.81) → internal (0, -3.81) → mirror_x → (0, 3.81)
→ symbol (100, 100) → world (100, 103.81). Pin should NOT be at (100, 96.19)."""
wx, wy = WireDragger.pin_world_xy(0.0, 3.81, 100.0, 100.0, 0, True, False)
assert wx == pytest.approx(100.0)
assert wy == pytest.approx(103.81), f"mirror_x Y wrong: {wy} (expected 103.81)"
def test_pin_world_xy_mirror_y_matches_eeschema():
"""(mirror y) = SYM_MIRROR_Y = TRANSFORM(-1,0,0,1) → negates internal X.
Device:R pin 1 lib (0, +3.81) → internal (0, -3.81) → mirror_y → (0, -3.81)
→ world (100, 96.19). Y of pin 1 is unchanged by mirror across Y axis."""
wx, wy = WireDragger.pin_world_xy(0.0, 3.81, 100.0, 100.0, 0, False, True)
assert wx == pytest.approx(100.0)
assert wy == pytest.approx(96.19), f"mirror_y Y wrong: {wy} (expected 96.19)"

96
tests/test_rotate_schematic_mirror.py
View File

@@ -6,13 +6,13 @@ Tests are split into two layers:
2. Handler integration smoke test — patches SchematicManager away.
"""
import importlib.util
import math
import os
import sys
import math
import textwrap
import tempfile
import importlib.util
from unittest.mock import patch, MagicMock
import textwrap
from unittest.mock import MagicMock, patch
import sexpdata
from sexpdata import Symbol
@@ -40,6 +40,7 @@ WireDragger = _wd_mod.WireDragger
# Helpers
# ---------------------------------------------------------------------------
def _parse(text: str) -> list:
return sexpdata.loads(text)
@@ -80,6 +81,7 @@ def _make_sch(sym_extra: str = "", wires: str = "") -> list:
# Tests: update_symbol_rotation_mirror
# ---------------------------------------------------------------------------
def test_update_rotation_sets_angle():
sch = _make_sch()
result = WireDragger.update_symbol_rotation_mirror(sch, "Q1", 90.0, None)
@@ -130,6 +132,7 @@ def test_update_unknown_reference_returns_false():
# Tests: compute_pin_positions_for_rotation
# ---------------------------------------------------------------------------
def test_pin_positions_change_on_rotation():
"""Pins at non-zero local offsets should move when the symbol rotates."""
sch = _make_sch()
@@ -160,25 +163,28 @@ def test_pin_positions_unchanged_at_same_transform():
assert old_xy == new_xy
def test_pin_positions_mirror_x_flips_x():
"""mirror_x should negate the local X coordinate before rotation."""
def test_pin_positions_mirror_x_flips_y():
"""mirror_x = SYM_MIRROR_X = TRANSFORM(1,0,0,-1) negates the screen-Y
coordinate (eeschema symbol.h:43-44), not X. With the lib→screen Y-flip
applied first, this means the pin's screen Y is reflected back to lib Y."""
sch = _make_sch() # at (75, 105, 0), no mirror
fake_pins = {"1": {"x": 2.0, "y": 0.0}}
fake_pins = {"1": {"x": 0.0, "y": 2.0}}
with patch.object(WireDragger, "get_pin_defs", return_value=fake_pins):
pos = WireDragger.compute_pin_positions_for_rotation(sch, "Q1", 0.0, True, False)
_, (old_xy, new_xy) = next(iter(pos.items()))
# old: pin at local (2, 0), world = (75+2, 105) = (77, 105)
assert abs(old_xy[0] - 77.0) < 1e-4
# new: mirror_x → local (-2, 0), world = (75-2, 105) = (73, 105)
assert abs(new_xy[0] - 73.0) < 1e-4
# old: pin at lib (0, 2). Y-flip → (0, -2). No mirror. World = (75, 105-2) = (75, 103).
assert abs(old_xy[1] - 103.0) < 1e-4
# new: mirror_x → negate screen-Y → (0, 2). World = (75, 105+2) = (75, 107).
assert abs(new_xy[1] - 107.0) < 1e-4
# ---------------------------------------------------------------------------
# Integration smoke test: handler uses sexpdata, not kicad-skip
# ---------------------------------------------------------------------------
def test_rotate_handler_no_crash(tmp_path):
"""_handle_rotate_schematic_component should succeed without kicad-skip."""
# Ensure python/ is on sys.path so commands.* imports resolve
@@ -186,26 +192,44 @@ def test_rotate_handler_no_crash(tmp_path):
if _python_dir not in sys.path:
sys.path.insert(0, _python_dir)
# Stub heavy imports before loading kicad_interface
for modname in ("pcbnew", "skip", "resources", "schemas",
"resources.resource_definitions", "schemas.tool_schemas",
"annotations"):
sys.modules.setdefault(modname, MagicMock())
sys.modules["resources.resource_definitions"].RESOURCE_DEFINITIONS = {}
sys.modules["resources.resource_definitions"].handle_resource_read = MagicMock()
sys.modules["schemas.tool_schemas"].TOOL_SCHEMAS = []
_pcbnew = sys.modules["pcbnew"]
_pcbnew.__file__ = "/fake/pcbnew.so"
_pcbnew.GetBuildVersion.return_value = "9.0.0"
ki_spec = importlib.util.spec_from_file_location(
"kicad_interface_smoke",
os.path.join(os.path.dirname(__file__), "..", "python", "kicad_interface.py"),
# Stub heavy imports before loading kicad_interface. Save and restore
# sys.modules state so we don't pollute already-imported real modules
# (notably schemas.tool_schemas, whose TOOL_SCHEMAS dict is shared across
# the test session).
_stub_modnames = (
"pcbnew",
"skip",
"resources",
"schemas",
"resources.resource_definitions",
"schemas.tool_schemas",
"annotations",
)
ki_mod = importlib.util.module_from_spec(ki_spec)
ki_spec.loader.exec_module(ki_mod)
KiCADInterface = ki_mod.KiCADInterface
_saved_modules = {n: sys.modules.get(n) for n in _stub_modnames}
try:
for modname in _stub_modnames:
sys.modules[modname] = MagicMock()
sys.modules["resources.resource_definitions"].RESOURCE_DEFINITIONS = {}
sys.modules["resources.resource_definitions"].handle_resource_read = MagicMock()
sys.modules["schemas.tool_schemas"].TOOL_SCHEMAS = []
_pcbnew = sys.modules["pcbnew"]
_pcbnew.__file__ = "/fake/pcbnew.so"
_pcbnew.GetBuildVersion.return_value = "9.0.0"
ki_spec = importlib.util.spec_from_file_location(
"kicad_interface_smoke",
os.path.join(os.path.dirname(__file__), "..", "python", "kicad_interface.py"),
)
ki_mod = importlib.util.module_from_spec(ki_spec)
ki_spec.loader.exec_module(ki_mod)
KiCADInterface = ki_mod.KiCADInterface
finally:
for modname, mod in _saved_modules.items():
if mod is None:
sys.modules.pop(modname, None)
else:
sys.modules[modname] = mod
# Write a minimal schematic file
sch_path = str(tmp_path / "test.kicad_sch")
@@ -233,11 +257,13 @@ def test_rotate_handler_no_crash(tmp_path):
f.write(sch_content)
iface = KiCADInterface.__new__(KiCADInterface)
result = iface._handle_rotate_schematic_component({
"schematicPath": sch_path,
"reference": "R1",
"angle": 90,
})
result = iface._handle_rotate_schematic_component(
{
"schematicPath": sch_path,
"reference": "R1",
"angle": 90,
}
)
assert result["success"] is True
assert result["angle"] == 90