Fix two regressions from PR #88 (rotate/mirror)
1. add_schematic_net_label failed on schematics with no existing labels. The clone-based path required a pre-existing label to copy from; the documented "fallback to sexpdata" was a misleading log line — the RuntimeError was caught and the call silently returned False. Restore hand-built sexpdata construction (without the buggy fields_autoplaced token, with orientation-aware justify). 2. get_pin_angle returned the wrong angle for every mirrored symbol (off by exactly 180°, all rotations, both mirror axes). The mirror_x and mirror_y formulas were swapped relative to the pin_world_xy convention — pin_world_xy mirrors a position by flipping its local axis component, so the matching angle transform is (180 - θ) for mirror_x and -θ for mirror_y. Add regression tests: - test_add_label_empty_schematic.py — first label on empty schematic, orientation-aware justify. - test_get_pin_angle.py — full 24-case matrix (4 rotations × 3 mirror states × 2 pins).
This commit is contained in:
Eugene Mikhantyev
105
tests/test_add_label_empty_schematic.py
Normal file
105
tests/test_add_label_empty_schematic.py
Normal file
@@ -0,0 +1,105 @@
|
||||
"""
|
||||
Regression tests for WireManager.add_label on schematics with zero existing labels.
|
||||
|
||||
Bug: PR #88 rewrote add_label to clone an existing label via kicad-skip; on a
|
||||
fresh schematic with no labels this returns False. The fix restores a hand-built
|
||||
sexpdata path (without the spurious fields_autoplaced token) as the primary path.
|
||||
"""
|
||||
|
||||
import importlib.util
|
||||
import os
|
||||
import sys
|
||||
from unittest.mock import MagicMock
|
||||
|
||||
import sexpdata
|
||||
from sexpdata import Symbol
|
||||
|
||||
# Stub heavy / optional deps before loading wire_manager
|
||||
for modname in ("pcbnew", "skip"):
|
||||
sys.modules.setdefault(modname, MagicMock())
|
||||
|
||||
_wm_spec = importlib.util.spec_from_file_location(
|
||||
"wire_manager",
|
||||
os.path.join(os.path.dirname(__file__), "..", "python", "commands", "wire_manager.py"),
|
||||
)
|
||||
_wm_mod = importlib.util.module_from_spec(_wm_spec)
|
||||
_wm_spec.loader.exec_module(_wm_mod)
|
||||
WireManager = _wm_mod.WireManager
|
||||
|
||||
|
||||
_EMPTY_SCH = """\
|
||||
(kicad_sch (version 20250114) (generator "test")
|
||||
(lib_symbols)
|
||||
(sheet_instances
|
||||
(path "/" (page "1"))
|
||||
)
|
||||
)
|
||||
"""
|
||||
|
||||
|
||||
def _find_label(sch_data, text):
|
||||
for item in sch_data:
|
||||
if (
|
||||
isinstance(item, list)
|
||||
and len(item) >= 2
|
||||
and isinstance(item[0], Symbol)
|
||||
and str(item[0]) == "label"
|
||||
and item[1] == text
|
||||
):
|
||||
return item
|
||||
return None
|
||||
|
||||
|
||||
def _has_token(label_sexp, name):
|
||||
for part in label_sexp[2:]:
|
||||
if isinstance(part, list) and part and isinstance(part[0], Symbol) and str(part[0]) == name:
|
||||
return True
|
||||
return False
|
||||
|
||||
|
||||
def _find_subtoken(label_sexp, name):
|
||||
for part in label_sexp[2:]:
|
||||
if isinstance(part, list) and part and isinstance(part[0], Symbol) and str(part[0]) == name:
|
||||
return part
|
||||
return None
|
||||
|
||||
|
||||
def test_add_label_to_empty_schematic_succeeds(tmp_path):
|
||||
sch_path = tmp_path / "empty.kicad_sch"
|
||||
sch_path.write_text(_EMPTY_SCH)
|
||||
|
||||
ok = WireManager.add_label(sch_path, "TEST", [50.0, 50.0], "label", 0)
|
||||
assert ok is True
|
||||
|
||||
sch_data = sexpdata.loads(sch_path.read_text())
|
||||
label = _find_label(sch_data, "TEST")
|
||||
assert label is not None, 'Expected a (label "TEST" ...) token in the file'
|
||||
|
||||
# The fields_autoplaced token must NOT be present (regression guard for PR #88's bug).
|
||||
assert not _has_token(
|
||||
label, "fields_autoplaced"
|
||||
), "fields_autoplaced should not be emitted by add_label"
|
||||
|
||||
|
||||
def test_add_label_orientation_180_uses_right_bottom_justify(tmp_path):
|
||||
sch_path = tmp_path / "empty180.kicad_sch"
|
||||
sch_path.write_text(_EMPTY_SCH)
|
||||
|
||||
ok = WireManager.add_label(sch_path, "NETR", [60.0, 60.0], "label", 180)
|
||||
assert ok is True
|
||||
|
||||
sch_data = sexpdata.loads(sch_path.read_text())
|
||||
label = _find_label(sch_data, "NETR")
|
||||
assert label is not None
|
||||
|
||||
effects = _find_subtoken(label, "effects")
|
||||
assert effects is not None, "label must carry an (effects ...) token"
|
||||
|
||||
justify = _find_subtoken(effects, "justify")
|
||||
assert justify is not None, "effects must carry a (justify ...) token"
|
||||
|
||||
justify_vals = [str(t) for t in justify[1:] if isinstance(t, Symbol)]
|
||||
assert justify_vals == [
|
||||
"right",
|
||||
"bottom",
|
||||
], f"orientation=180 should produce 'right bottom' justify, got {justify_vals}"
|
||||
163
tests/test_get_pin_angle.py
Normal file
163
tests/test_get_pin_angle.py
Normal file
@@ -0,0 +1,163 @@
|
||||
"""
|
||||
Matrix tests for PinLocator.get_pin_angle on a Device:R symbol.
|
||||
|
||||
For each combination of (symbol_rotation, mirror, pin), we construct a real
|
||||
.kicad_sch fixture, then compare:
|
||||
- actual: PinLocator().get_pin_angle(...)
|
||||
- expected: derived geometrically from WireDragger.pin_world_xy by extending
|
||||
the pin one length unit along its library angle and measuring the
|
||||
world-frame displacement direction.
|
||||
|
||||
This characterizes the post-PR-#88 angle reflection logic. Cases that disagree
|
||||
with the geometric expectation are marked xfail with a "PR #88 regression
|
||||
candidate" annotation.
|
||||
"""
|
||||
|
||||
import importlib.util
|
||||
import math
|
||||
import os
|
||||
import sys
|
||||
import textwrap
|
||||
from pathlib import Path
|
||||
from unittest.mock import MagicMock
|
||||
|
||||
import pytest
|
||||
import sexpdata
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Module loading — bypass pcbnew, mirror the test_rotate_schematic_mirror style
|
||||
# ---------------------------------------------------------------------------
|
||||
_PYTHON_DIR = os.path.join(os.path.dirname(__file__), "..", "python")
|
||||
if _PYTHON_DIR not in sys.path:
|
||||
sys.path.insert(0, _PYTHON_DIR)
|
||||
|
||||
# Stub pcbnew before importing pin_locator (skip is real and works fine)
|
||||
sys.modules.setdefault("pcbnew", MagicMock())
|
||||
|
||||
_pl_spec = importlib.util.spec_from_file_location(
|
||||
"pin_locator_under_test",
|
||||
os.path.join(_PYTHON_DIR, "commands", "pin_locator.py"),
|
||||
)
|
||||
_pl_mod = importlib.util.module_from_spec(_pl_spec)
|
||||
_pl_spec.loader.exec_module(_pl_mod)
|
||||
PinLocator = _pl_mod.PinLocator
|
||||
|
||||
_wd_spec = importlib.util.spec_from_file_location(
|
||||
"wire_dragger_under_test",
|
||||
os.path.join(_PYTHON_DIR, "commands", "wire_dragger.py"),
|
||||
)
|
||||
_wd_mod = importlib.util.module_from_spec(_wd_spec)
|
||||
_wd_spec.loader.exec_module(_wd_mod)
|
||||
WireDragger = _wd_mod.WireDragger
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Device:R pin definitions (per python/templates/empty.kicad_sch)
|
||||
# pin 1: (0, 3.81), library angle 270, length 1.27
|
||||
# pin 2: (0, -3.81), library angle 90, length 1.27
|
||||
# ---------------------------------------------------------------------------
|
||||
PIN_DEFS = {
|
||||
"1": {"x": 0.0, "y": 3.81, "angle": 270.0, "length": 1.27},
|
||||
"2": {"x": 0.0, "y": -3.81, "angle": 90.0, "length": 1.27},
|
||||
}
|
||||
|
||||
SYMBOL_X = 100.0
|
||||
SYMBOL_Y = 100.0
|
||||
|
||||
|
||||
def _make_sch_text(rotation: float, mirror: str | None) -> str:
|
||||
mirror_line = ""
|
||||
if mirror == "x":
|
||||
mirror_line = "(mirror x)"
|
||||
elif mirror == "y":
|
||||
mirror_line = "(mirror y)"
|
||||
|
||||
return textwrap.dedent(f"""\
|
||||
(kicad_sch (version 20250114) (generator "test")
|
||||
(lib_symbols
|
||||
(symbol "Device:R" (pin_numbers hide) (pin_names (offset 0))
|
||||
(symbol "R_1_1"
|
||||
(pin passive line (at 0 3.81 270) (length 1.27)
|
||||
(name "~" (effects (font (size 1.27 1.27))))
|
||||
(number "1" (effects (font (size 1.27 1.27))))
|
||||
)
|
||||
(pin passive line (at 0 -3.81 90) (length 1.27)
|
||||
(name "~" (effects (font (size 1.27 1.27))))
|
||||
(number "2" (effects (font (size 1.27 1.27))))
|
||||
)
|
||||
)
|
||||
)
|
||||
)
|
||||
(symbol (lib_id "Device:R") (at {SYMBOL_X} {SYMBOL_Y} {rotation})
|
||||
{mirror_line}
|
||||
(property "Reference" "R1" (at {SYMBOL_X} {SYMBOL_Y} 0))
|
||||
(property "Value" "10k" (at {SYMBOL_X} {SYMBOL_Y} 0))
|
||||
)
|
||||
)
|
||||
""")
|
||||
|
||||
|
||||
def _write_sch(tmp_path: Path, rotation: float, mirror: str | None) -> Path:
|
||||
p = tmp_path / f"r_rot{int(rotation)}_mirror{mirror or 'none'}.kicad_sch"
|
||||
p.write_text(_make_sch_text(rotation, mirror))
|
||||
return p
|
||||
|
||||
|
||||
def _expected_stub_angle(pin_num: str, rotation: float, mirror: str | None) -> float:
|
||||
"""Geometrically expected outward angle: extend in library coords by +length
|
||||
along library angle, transform to world, take atan2 of displacement."""
|
||||
pin = PIN_DEFS[pin_num]
|
||||
px, py = pin["x"], pin["y"]
|
||||
lib_angle_rad = math.radians(pin["angle"])
|
||||
ox = px + pin["length"] * math.cos(lib_angle_rad)
|
||||
oy = py + pin["length"] * math.sin(lib_angle_rad)
|
||||
|
||||
mirror_x = mirror == "x"
|
||||
mirror_y = mirror == "y"
|
||||
|
||||
wx_pin, wy_pin = WireDragger.pin_world_xy(
|
||||
px, py, SYMBOL_X, SYMBOL_Y, rotation, mirror_x, mirror_y
|
||||
)
|
||||
wx_out, wy_out = WireDragger.pin_world_xy(
|
||||
ox, oy, SYMBOL_X, SYMBOL_Y, rotation, mirror_x, mirror_y
|
||||
)
|
||||
|
||||
deg = math.degrees(math.atan2(wy_out - wy_pin, wx_out - wx_pin)) % 360.0
|
||||
# Snap to 0/90/180/270 (axis-aligned pins; FP noise tolerance)
|
||||
snapped = round(deg / 90.0) * 90.0 % 360.0
|
||||
if abs(((deg - snapped) + 540) % 360 - 180) < 1e-6:
|
||||
# within tolerance
|
||||
return snapped
|
||||
return snapped
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Parametrized matrix
|
||||
# ---------------------------------------------------------------------------
|
||||
ROTATIONS = [0, 90, 180, 270]
|
||||
MIRRORS = [None, "x", "y"]
|
||||
PINS = ["1", "2"]
|
||||
|
||||
|
||||
@pytest.mark.parametrize("rotation", ROTATIONS)
|
||||
@pytest.mark.parametrize("mirror", MIRRORS)
|
||||
@pytest.mark.parametrize("pin_num", PINS)
|
||||
def test_get_pin_angle_matches_geometric_expectation(tmp_path, rotation, mirror, pin_num):
|
||||
sch_path = _write_sch(tmp_path, rotation, mirror)
|
||||
expected = _expected_stub_angle(pin_num, rotation, mirror)
|
||||
|
||||
locator = PinLocator()
|
||||
actual = locator.get_pin_angle(sch_path, "R1", pin_num)
|
||||
|
||||
assert (
|
||||
actual is not None
|
||||
), f"get_pin_angle returned None for rot={rotation} mirror={mirror} pin={pin_num}"
|
||||
|
||||
# Normalize both to [0, 360)
|
||||
actual_n = actual % 360.0
|
||||
expected_n = expected % 360.0
|
||||
|
||||
assert abs(((actual_n - expected_n) + 540) % 360 - 180) < 1e-3, (
|
||||
f"actual={actual_n}, expected={expected_n} "
|
||||
f"(rotation={rotation}, mirror={mirror}, pin={pin_num})"
|
||||
)
|
||||
Reference in New Issue
Block a user