From 7525f3dcdccdb017820cb9959bdb66863ee7cc6f Mon Sep 17 00:00:00 2001 From: Ryan Malloy Date: Wed, 4 Mar 2026 20:06:06 -0700 Subject: [PATCH] Fix add_label persistence and add_power_symbol custom library fallback MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit add_label bypasses kicad-sch-api serializer entirely — generates s-expression strings and inserts them directly into the .kicad_sch file via atomic write. Fixes two upstream bugs: global labels silently dropped on save (serializer never iterates "global_label" key), and local labels raising TypeError (parameter signature mismatch in LabelCollection.add()). add_power_symbol now falls back to sexp pin parsing when the API returns None for custom library symbols (e.g. SMF5.0CA). Extracts shared resolve_pin_position() utility used by both add_power_symbol and batch operations. Batch labels also fixed — collected as sexp strings during the batch loop and inserted after sch.save() so the serializer can't overwrite them. --- src/mckicad/tools/batch.py | 67 ++++++-- src/mckicad/tools/power_symbols.py | 9 +- src/mckicad/tools/schematic.py | 35 ++-- src/mckicad/utils/sexp_parser.py | 247 ++++++++++++++++++++++++++++- tests/test_batch.py | 28 ++++ tests/test_power_symbols.py | 17 ++ tests/test_schematic.py | 81 ++++++++++ tests/test_sexp_parser.py | 184 +++++++++++++++++++++ 8 files changed, 638 insertions(+), 30 deletions(-) diff --git a/src/mckicad/tools/batch.py b/src/mckicad/tools/batch.py index 8cd8729..13a087f 100644 --- a/src/mckicad/tools/batch.py +++ b/src/mckicad/tools/batch.py @@ -209,15 +209,35 @@ def _validate_batch_data(data: dict[str, Any], sch: Any) -> list[str]: return errors -def _apply_batch_operations(sch: Any, data: dict[str, Any]) -> dict[str, Any]: - """Apply all batch operations to a loaded schematic. Returns summary.""" +def _apply_batch_operations( + sch: Any, data: dict[str, Any], schematic_path: str, +) -> dict[str, Any]: + """Apply all batch operations to a loaded schematic. Returns summary. + + Labels are returned as pending sexp strings (``_pending_label_sexps``) + that must be inserted into the file *after* ``sch.save()``, because + kicad-sch-api's serializer drops global labels and raises TypeError + on local labels. + + Args: + sch: A kicad-sch-api SchematicDocument instance. + data: Parsed batch JSON data. + schematic_path: Path to the .kicad_sch file (needed for sexp + pin fallback and label insertion). + """ from mckicad.patterns._geometry import add_power_symbol_to_pin + from mckicad.utils.sexp_parser import ( + generate_global_label_sexp, + generate_label_sexp, + resolve_pin_position, + ) placed_components: list[str] = [] placed_power: list[dict[str, Any]] = [] placed_wires: list[str] = [] placed_labels: list[str] = [] placed_no_connects = 0 + pending_label_sexps: list[str] = [] # 1. Components for comp in data.get("components", []): @@ -235,10 +255,12 @@ def _apply_batch_operations(sch: Any, data: dict[str, Any]) -> dict[str, Any]: placed.rotate(comp["rotation"]) placed_components.append(ref or comp["lib_id"]) - # 2. Power symbols + # 2. Power symbols (with sexp pin fallback) for ps in data.get("power_symbols", []): - pin_pos = sch.get_component_pin_position(ps["pin_ref"], ps["pin_number"]) - if pin_pos is None: + pin_pos_tuple = resolve_pin_position( + sch, schematic_path, ps["pin_ref"], ps["pin_number"], + ) + if pin_pos_tuple is None: logger.warning( "Skipping power symbol: pin %s.%s not found", ps["pin_ref"], @@ -247,7 +269,7 @@ def _apply_batch_operations(sch: Any, data: dict[str, Any]) -> dict[str, Any]: continue result = add_power_symbol_to_pin( sch=sch, - pin_position=(pin_pos.x, pin_pos.y), + pin_position=pin_pos_tuple, net=ps["net"], lib_id=ps.get("lib_id"), stub_length=ps.get("stub_length"), @@ -270,20 +292,27 @@ def _apply_batch_operations(sch: Any, data: dict[str, Any]) -> dict[str, Any]: ) placed_wires.append(str(wire_id)) - # 4. Labels + # 4. Labels — generate sexp strings for post-save insertion for label in data.get("labels", []): is_global = label.get("global", False) + rotation = label.get("rotation", 0) if is_global: - label_id = sch.add_global_label( + sexp = generate_global_label_sexp( text=label["text"], - position=(label["x"], label["y"]), + x=label["x"], + y=label["y"], + rotation=rotation, + shape=label.get("shape", "bidirectional"), ) else: - label_id = sch.add_label( + sexp = generate_label_sexp( text=label["text"], - position=(label["x"], label["y"]), + x=label["x"], + y=label["y"], + rotation=rotation, ) - placed_labels.append(str(label_id)) + pending_label_sexps.append(sexp) + placed_labels.append(label["text"]) # 5. No-connects for nc in data.get("no_connects", []): @@ -307,6 +336,7 @@ def _apply_batch_operations(sch: Any, data: dict[str, Any]) -> dict[str, Any]: + len(placed_labels) + placed_no_connects ), + "_pending_label_sexps": pending_label_sexps, } @@ -440,11 +470,20 @@ def apply_batch( } # Apply all operations - summary = _apply_batch_operations(sch, data) + summary = _apply_batch_operations(sch, data, schematic_path) - # Single save + # Single save (components, power symbols, wires, no-connects) sch.save(schematic_path) + # Insert labels via sexp AFTER save — kicad-sch-api's serializer + # drops global labels and raises TypeError on local labels. + pending_sexps = summary.pop("_pending_label_sexps", []) + if pending_sexps: + from mckicad.utils.sexp_parser import insert_sexp_before_close + + combined_sexp = "".join(pending_sexps) + insert_sexp_before_close(schematic_path, combined_sexp) + logger.info( "Batch applied: %d operations from %s to %s", summary["total_operations"], diff --git a/src/mckicad/tools/power_symbols.py b/src/mckicad/tools/power_symbols.py index aca03bb..766efc5 100644 --- a/src/mckicad/tools/power_symbols.py +++ b/src/mckicad/tools/power_symbols.py @@ -110,12 +110,13 @@ def add_power_symbol( try: from mckicad.patterns._geometry import add_power_symbol_to_pin + from mckicad.utils.sexp_parser import resolve_pin_position sch = _ksa_load(schematic_path) - # Look up the target pin position - pin_pos = sch.get_component_pin_position(pin_ref, pin_number) - if pin_pos is None: + # Look up the target pin position (with sexp fallback for custom symbols) + pin_pos_tuple = resolve_pin_position(sch, schematic_path, pin_ref, pin_number) + if pin_pos_tuple is None: return { "success": False, "error": ( @@ -127,7 +128,7 @@ def add_power_symbol( result = add_power_symbol_to_pin( sch=sch, - pin_position=(pin_pos.x, pin_pos.y), + pin_position=pin_pos_tuple, net=net, lib_id=lib_id, stub_length=stub_length, diff --git a/src/mckicad/tools/schematic.py b/src/mckicad/tools/schematic.py index eb4fbcf..36a41c6 100644 --- a/src/mckicad/tools/schematic.py +++ b/src/mckicad/tools/schematic.py @@ -17,6 +17,9 @@ from mckicad.config import INLINE_RESULT_THRESHOLD from mckicad.server import mcp from mckicad.utils.file_utils import write_detail_file from mckicad.utils.sexp_parser import ( + generate_global_label_sexp, + generate_label_sexp, + insert_sexp_before_close, parse_global_labels, parse_lib_symbol_pins, transform_pin_to_schematic, @@ -427,6 +430,8 @@ def add_label( x: float, y: float, global_label: bool = False, + rotation: float = 0, + shape: str = "bidirectional", ) -> dict[str, Any]: """Add a net label or global label to a schematic. @@ -434,6 +439,10 @@ def add_label( nets across hierarchical sheets -- use global labels for power rails, clock signals, and inter-sheet buses. + Uses direct s-expression file insertion to bypass kicad-sch-api + serializer bugs that silently drop global labels and raise TypeError + on local labels. + Args: schematic_path: Path to an existing .kicad_sch file. text: Label text (becomes the net name, e.g. ``GND``, ``SPI_CLK``). @@ -441,30 +450,36 @@ def add_label( y: Vertical position in schematic units. global_label: When True, creates a global label visible across all hierarchical sheets. Defaults to a local label. + rotation: Label rotation in degrees (0, 90, 180, 270). + shape: Global label shape (``bidirectional``, ``input``, ``output``, + ``tri_state``, ``passive``). Ignored for local labels. Returns: Dictionary with ``success``, the ``label_id``, and label type. """ - err = _require_sch_api() - if err: - return err - schematic_path = _expand(schematic_path) verr = _validate_schematic_path(schematic_path) if verr: return verr - try: - sch = _ksa_load(schematic_path) + if not text: + return {"success": False, "error": "Label text must be a non-empty string"} + try: if global_label: - label_id = sch.add_global_label(text=text, position=(x, y)) + sexp = generate_global_label_sexp(text, x, y, rotation=rotation, shape=shape) label_type = "global" else: - label_id = sch.add_label(text=text, position=(x, y)) + sexp = generate_label_sexp(text, x, y, rotation=rotation) label_type = "local" - sch.save(schematic_path) + insert_sexp_before_close(schematic_path, sexp) + + # Extract the UUID we generated from the sexp block + import re as _re + + uuid_match = _re.search(r'\(uuid "([^"]+)"\)', sexp) + label_id = uuid_match.group(1) if uuid_match else "unknown" logger.info( "Added %s label '%s' at (%.1f, %.1f) in %s", label_type, text, x, y, schematic_path @@ -476,7 +491,7 @@ def add_label( "label_type": label_type, "position": {"x": x, "y": y}, "schematic_path": schematic_path, - "engine": _get_schematic_engine(), + "engine": "sexp-direct", } except Exception as e: logger.error("Failed to add label '%s' in %s: %s", text, schematic_path, e) diff --git a/src/mckicad/utils/sexp_parser.py b/src/mckicad/utils/sexp_parser.py index 0d7fd1e..a93b737 100644 --- a/src/mckicad/utils/sexp_parser.py +++ b/src/mckicad/utils/sexp_parser.py @@ -10,13 +10,29 @@ kicad-sch-api v0.5.5 has two known parsing gaps: attribute — ``sch.labels`` only contains local labels, and there is no ``sch.global_labels`` attribute. -This module provides focused parsers for these two cases, reading directly -from the raw ``.kicad_sch`` file. +3. ``parser._schematic_data_to_sexp()`` never iterates over the + ``"global_label"`` key — global labels added via TextElementManager + are silently dropped on save. + +4. ``LabelCollection.add()`` has a parameter signature mismatch — it + doesn't accept ``justify_h``/``justify_v``/``uuid`` kwargs that + ``Schematic.add_label()`` passes, causing TypeError on local labels. + +This module provides focused parsers for these cases, plus direct +s-expression generation and file insertion to bypass the broken +serializer entirely. """ +import contextlib +import logging import math +import os import re +import tempfile from typing import Any +import uuid as _uuid_mod + +logger = logging.getLogger(__name__) # --------------------------------------------------------------------------- # Global labels @@ -217,3 +233,230 @@ def _extract_named_section(content: str, keyword: str, name: str) -> str | None: if depth == 0: return content[start : i + 1] return None + + +# --------------------------------------------------------------------------- +# S-expression generation — bypasses kicad-sch-api serializer bugs +# --------------------------------------------------------------------------- + + +def _rc(value: float) -> str: + """Round a coordinate to 2 decimal places for s-expression output.""" + return f"{value:.2f}".rstrip("0").rstrip(".") + + +def _escape_sexp_string(text: str) -> str: + """Escape a string for safe embedding in an s-expression double-quoted value.""" + return text.replace("\\", "\\\\").replace('"', '\\"') + + +def generate_label_sexp( + text: str, + x: float, + y: float, + rotation: float = 0, + uuid_str: str | None = None, +) -> str: + """Generate a KiCad ``(label ...)`` s-expression block. + + Args: + text: Label text (net name). + x: Horizontal position in schematic units. + y: Vertical position in schematic units. + rotation: Label rotation in degrees (0, 90, 180, 270). + uuid_str: Explicit UUID string. Auto-generated if None. + + Returns: + Complete ``(label ...)`` s-expression string ready for file insertion. + """ + if uuid_str is None: + uuid_str = str(_uuid_mod.uuid4()) + safe_text = _escape_sexp_string(text) + rot = _rc(rotation) + return ( + f' (label "{safe_text}"\n' + f" (at {_rc(x)} {_rc(y)} {rot})\n" + f' (effects (font (size 1.27 1.27)) (justify left bottom))\n' + f' (uuid "{uuid_str}")\n' + f" )\n" + ) + + +def generate_global_label_sexp( + text: str, + x: float, + y: float, + rotation: float = 0, + shape: str = "bidirectional", + uuid_str: str | None = None, +) -> str: + """Generate a KiCad ``(global_label ...)`` s-expression block. + + Includes the ``Intersheetrefs`` property required by KiCad 9+. + + Args: + text: Label text (net name). + x: Horizontal position in schematic units. + y: Vertical position in schematic units. + rotation: Label rotation in degrees (0, 90, 180, 270). + shape: Label shape — ``bidirectional``, ``input``, ``output``, + ``tri_state``, or ``passive``. + uuid_str: Explicit UUID string. Auto-generated if None. + + Returns: + Complete ``(global_label ...)`` s-expression string. + """ + if uuid_str is None: + uuid_str = str(_uuid_mod.uuid4()) + safe_text = _escape_sexp_string(text) + rot = _rc(rotation) + return ( + f' (global_label "{safe_text}"\n' + f" (shape {shape})\n" + f" (at {_rc(x)} {_rc(y)} {rot})\n" + f' (effects (font (size 1.27 1.27)) (justify left))\n' + f' (uuid "{uuid_str}")\n' + f' (property "Intersheetrefs" "${{INTERSHEET_REFS}}"\n' + f" (at {_rc(x)} {_rc(y)} {rot})\n" + f" (effects (font (size 1.27 1.27)) (hide yes))\n" + f" )\n" + f" )\n" + ) + + +def insert_sexp_before_close(filepath: str, sexp_block: str) -> None: + """Insert an s-expression block into a ``.kicad_sch`` file before its final ``)``. + + Performs an atomic write (temp file + ``os.replace()``) to avoid + corrupting the schematic if interrupted. + + Args: + filepath: Path to an existing ``.kicad_sch`` file. + sexp_block: The s-expression text to insert. + + Raises: + ValueError: If the file doesn't start with ``(kicad_sch``. + FileNotFoundError: If the file doesn't exist. + """ + with open(filepath, encoding="utf-8") as f: + content = f.read() + + stripped = content.lstrip() + if not stripped.startswith("(kicad_sch"): + raise ValueError(f"Not a KiCad schematic file: {filepath}") + + # Find the last closing paren + last_close = content.rfind(")") + if last_close == -1: + raise ValueError(f"Malformed schematic file (no closing paren): {filepath}") + + new_content = content[:last_close] + sexp_block + content[last_close:] + + # Atomic write: temp file in same directory, then rename + fd, tmp_path = tempfile.mkstemp( + dir=os.path.dirname(filepath) or ".", + suffix=".kicad_sch.tmp", + ) + try: + with os.fdopen(fd, "w", encoding="utf-8") as tmp_f: + tmp_f.write(new_content) + os.replace(tmp_path, filepath) + except BaseException: + # Clean up temp file on any failure + with _suppress_os_error(): + os.unlink(tmp_path) + raise + + +def _suppress_os_error(): + """Context manager that suppresses OSError (for cleanup paths).""" + return contextlib.suppress(OSError) + + +# --------------------------------------------------------------------------- +# Shared pin resolution — "try API, fall back to sexp" pattern +# --------------------------------------------------------------------------- + + +def resolve_pin_position( + sch: Any, + schematic_path: str, + reference: str, + pin_number: str, +) -> tuple[float, float] | None: + """Resolve a component pin's schematic-space position. + + Tries the kicad-sch-api ``get_component_pin_position()`` first. If that + returns None (common for custom library symbols), falls back to parsing + the pin from the raw ``(lib_symbols ...)`` section and transforming it + to schematic coordinates. + + Args: + sch: A kicad-sch-api SchematicDocument instance. + schematic_path: Path to the ``.kicad_sch`` file (for sexp fallback). + reference: Component reference designator (e.g. ``U1``, ``D1``). + pin_number: Pin number string (e.g. ``1``, ``2``). + + Returns: + ``(x, y)`` tuple in schematic coordinates, or None if the pin + cannot be found by either method. + """ + # 1. Try kicad-sch-api + try: + pos = sch.get_component_pin_position(reference, pin_number) + if pos is not None: + return (float(pos.x), float(pos.y)) + except Exception: + pass + + # 2. Fall back to sexp parsing + comp = None + with contextlib.suppress(Exception): + comp = sch.components.get(reference) + + if comp is None: + return None + + lib_id = getattr(comp, "lib_id", None) + if not lib_id: + return None + + sexp_pins = parse_lib_symbol_pins(schematic_path, str(lib_id)) + if not sexp_pins: + return None + + # Find the matching pin + target_pin = None + for pin in sexp_pins: + if pin["number"] == pin_number: + target_pin = pin + break + + if target_pin is None: + return None + + # Get component transform data + comp_pos = getattr(comp, "position", None) + comp_rot = float(getattr(comp, "rotation", 0) or 0) + comp_mirror = getattr(comp, "mirror", None) + mirror_x = comp_mirror in ("x", True) if comp_mirror else False + + cx = 0.0 + cy = 0.0 + if comp_pos is not None: + if hasattr(comp_pos, "x"): + cx = float(comp_pos.x) + cy = float(comp_pos.y) + elif isinstance(comp_pos, (list, tuple)) and len(comp_pos) >= 2: + cx = float(comp_pos[0]) + cy = float(comp_pos[1]) + + sx, sy = transform_pin_to_schematic( + target_pin["x"], target_pin["y"], cx, cy, comp_rot, mirror_x + ) + + logger.debug( + "Resolved pin %s.%s via sexp fallback: (%.2f, %.2f)", + reference, pin_number, sx, sy, + ) + return (sx, sy) diff --git a/tests/test_batch.py b/tests/test_batch.py index 4810b5f..63d9902 100644 --- a/tests/test_batch.py +++ b/tests/test_batch.py @@ -186,3 +186,31 @@ class TestBatchApply: assert result["success"] is True assert result["labels_placed"] == 1 + + def test_batch_labels_persist_in_file(self, populated_schematic_with_ic, tmp_output_dir): + """Batch labels (local and global) must appear in the saved file.""" + from mckicad.tools.batch import apply_batch + + data = { + "labels": [ + {"text": "BATCH_LOCAL", "x": 150, "y": 100}, + {"text": "BATCH_GLOBAL", "x": 160, "y": 110, "global": True}, + ], + } + batch_path = os.path.join(tmp_output_dir, "label_persist_batch.json") + with open(batch_path, "w") as f: + json.dump(data, f) + + result = apply_batch( + schematic_path=populated_schematic_with_ic, + batch_file=batch_path, + ) + + assert result["success"] is True + assert result["labels_placed"] == 2 + + with open(populated_schematic_with_ic) as f: + content = f.read() + + assert '(label "BATCH_LOCAL"' in content + assert '(global_label "BATCH_GLOBAL"' in content diff --git a/tests/test_power_symbols.py b/tests/test_power_symbols.py index f584c11..65fb2ea 100644 --- a/tests/test_power_symbols.py +++ b/tests/test_power_symbols.py @@ -176,3 +176,20 @@ class TestAddPowerSymbolTool: ) assert result["success"] is False + + +class TestPowerSymbolSexpFallback: + """Test that add_power_symbol uses sexp pin fallback for custom symbols.""" + + def test_resolve_pin_position_used_on_api_failure(self): + """Verify resolve_pin_position handles None from API gracefully.""" + from unittest.mock import MagicMock + + from mckicad.utils.sexp_parser import resolve_pin_position + + sch = MagicMock() + sch.get_component_pin_position.return_value = None + sch.components.get.return_value = None + + result = resolve_pin_position(sch, "/fake.kicad_sch", "D1", "1") + assert result is None diff --git a/tests/test_schematic.py b/tests/test_schematic.py index b14274c..a290865 100644 --- a/tests/test_schematic.py +++ b/tests/test_schematic.py @@ -168,3 +168,84 @@ def test_sidecar_per_schematic_isolation(tmp_output_dir): assert path_a != path_b assert os.path.join(".mckicad", "power", "connectivity.json") in path_a assert os.path.join(".mckicad", "esp32_p4_core", "connectivity.json") in path_b + + +class TestAddLabelPersistence: + """Labels added via add_label must actually appear in the saved file.""" + + @pytest.mark.unit + def test_local_label_persists(self, tmp_output_dir): + from mckicad.tools.schematic import add_label + + path = os.path.join(tmp_output_dir, "label_test.kicad_sch") + with open(path, "w") as f: + f.write("(kicad_sch\n (version 20231120)\n (uuid \"abc\")\n)\n") + + result = add_label(schematic_path=path, text="SPI_CLK", x=100.0, y=200.0) + assert result["success"] is True + assert result["label_type"] == "local" + assert result["engine"] == "sexp-direct" + + with open(path) as f: + content = f.read() + assert '(label "SPI_CLK"' in content + assert "(at 100 200 0)" in content + + @pytest.mark.unit + def test_global_label_persists(self, tmp_output_dir): + from mckicad.tools.schematic import add_label + + path = os.path.join(tmp_output_dir, "glabel_test.kicad_sch") + with open(path, "w") as f: + f.write("(kicad_sch\n (version 20231120)\n (uuid \"abc\")\n)\n") + + result = add_label( + schematic_path=path, text="VBUS_OUT", x=187.96, y=114.3, + global_label=True, + ) + assert result["success"] is True + assert result["label_type"] == "global" + + with open(path) as f: + content = f.read() + assert '(global_label "VBUS_OUT"' in content + assert "(shape bidirectional)" in content + assert "Intersheetrefs" in content + + @pytest.mark.unit + def test_multiple_labels_all_persist(self, tmp_output_dir): + from mckicad.tools.schematic import add_label + + path = os.path.join(tmp_output_dir, "multi_label.kicad_sch") + with open(path, "w") as f: + f.write("(kicad_sch\n (version 20231120)\n (uuid \"abc\")\n)\n") + + labels = ["NET_A", "NET_B", "NET_C", "GLOBAL_D"] + for i, name in enumerate(labels): + is_global = name.startswith("GLOBAL_") + result = add_label( + schematic_path=path, text=name, + x=100.0 + i * 10, y=200.0, + global_label=is_global, + ) + assert result["success"] is True + + with open(path) as f: + content = f.read() + + for name in labels: + if name.startswith("GLOBAL_"): + assert f'(global_label "{name}"' in content + else: + assert f'(label "{name}"' in content + + @pytest.mark.unit + def test_empty_text_rejected(self, tmp_output_dir): + from mckicad.tools.schematic import add_label + + path = os.path.join(tmp_output_dir, "empty_label.kicad_sch") + with open(path, "w") as f: + f.write("(kicad_sch\n (version 20231120)\n (uuid \"abc\")\n)\n") + + result = add_label(schematic_path=path, text="", x=0, y=0) + assert result["success"] is False diff --git a/tests/test_sexp_parser.py b/tests/test_sexp_parser.py index d9a7e9b..38c82e6 100644 --- a/tests/test_sexp_parser.py +++ b/tests/test_sexp_parser.py @@ -9,6 +9,9 @@ import tempfile import pytest from mckicad.utils.sexp_parser import ( + generate_global_label_sexp, + generate_label_sexp, + insert_sexp_before_close, parse_global_labels, parse_lib_symbol_pins, transform_pin_to_schematic, @@ -290,3 +293,184 @@ class TestTransformPinToSchematic: sx, sy = transform_pin_to_schematic(5, 0, 100, 100, 0, mirror_x=True) assert sx == pytest.approx(95.0) assert sy == pytest.approx(100.0) + + +class TestGenerateLabelSexp: + def test_basic_local_label(self): + sexp = generate_label_sexp("NET_A", 100.5, 200.25) + assert '(label "NET_A"' in sexp + assert "(at 100.5 200.25 0)" in sexp + assert "(uuid" in sexp + assert "(effects" in sexp + + def test_custom_uuid(self): + sexp = generate_label_sexp("X", 0, 0, uuid_str="test-uuid-123") + assert '(uuid "test-uuid-123")' in sexp + + def test_rotation(self): + sexp = generate_label_sexp("CLK", 50, 50, rotation=90) + assert "(at 50 50 90)" in sexp + + def test_auto_uuid_is_unique(self): + sexp1 = generate_label_sexp("A", 0, 0) + sexp2 = generate_label_sexp("A", 0, 0) + # Extract UUIDs + import re + + uuids = re.findall(r'\(uuid "([^"]+)"\)', sexp1 + sexp2) + assert len(uuids) == 2 + assert uuids[0] != uuids[1] + + def test_quote_escaping(self): + sexp = generate_label_sexp('NET"SPECIAL', 0, 0) + assert r'(label "NET\"SPECIAL"' in sexp + + def test_backslash_escaping(self): + sexp = generate_label_sexp("NET\\PATH", 0, 0) + assert r'(label "NET\\PATH"' in sexp + + +class TestGenerateGlobalLabelSexp: + def test_basic_global_label(self): + sexp = generate_global_label_sexp("VBUS_OUT", 187.96, 114.3) + assert '(global_label "VBUS_OUT"' in sexp + assert "(shape bidirectional)" in sexp + assert "(at 187.96 114.3 0)" in sexp + assert "Intersheetrefs" in sexp + assert "${INTERSHEET_REFS}" in sexp + + def test_custom_shape(self): + sexp = generate_global_label_sexp("CLK", 0, 0, shape="output") + assert "(shape output)" in sexp + + def test_rotation(self): + sexp = generate_global_label_sexp("SIG", 10, 20, rotation=180) + assert "(at 10 20 180)" in sexp + + def test_round_trip_parse(self, tmp_path): + """Generated global label should be parseable by parse_global_labels.""" + sexp = generate_global_label_sexp("TEST_NET", 123.45, 67.89) + # Wrap in a minimal schematic + content = f"(kicad_sch\n (version 20231120)\n{sexp})\n" + filepath = str(tmp_path / "test.kicad_sch") + with open(filepath, "w") as f: + f.write(content) + + labels = parse_global_labels(filepath) + assert len(labels) == 1 + assert labels[0]["text"] == "TEST_NET" + assert labels[0]["x"] == pytest.approx(123.45) + assert labels[0]["y"] == pytest.approx(67.89) + + +class TestInsertSexpBeforeClose: + def test_insert_into_minimal_schematic(self, tmp_path): + filepath = str(tmp_path / "test.kicad_sch") + with open(filepath, "w") as f: + f.write("(kicad_sch\n (version 20231120)\n)\n") + + insert_sexp_before_close(filepath, ' (label "X"\n (at 0 0 0)\n )\n') + + with open(filepath) as f: + content = f.read() + + assert '(label "X"' in content + assert content.strip().endswith(")") + assert content.startswith("(kicad_sch") + + def test_preserves_existing_content(self, tmp_path): + filepath = str(tmp_path / "test.kicad_sch") + original = '(kicad_sch\n (version 20231120)\n (uuid "abc")\n)\n' + with open(filepath, "w") as f: + f.write(original) + + insert_sexp_before_close(filepath, ' (label "Y"\n (at 1 2 0)\n )\n') + + with open(filepath) as f: + content = f.read() + + assert '(uuid "abc")' in content + assert '(label "Y"' in content + + def test_rejects_non_kicad_file(self, tmp_path): + filepath = str(tmp_path / "bad.kicad_sch") + with open(filepath, "w") as f: + f.write("not a kicad file") + + with pytest.raises(ValueError, match="Not a KiCad schematic"): + insert_sexp_before_close(filepath, "(label)") + + def test_multiple_insertions(self, tmp_path): + filepath = str(tmp_path / "multi.kicad_sch") + with open(filepath, "w") as f: + f.write("(kicad_sch\n (version 20231120)\n)\n") + + insert_sexp_before_close(filepath, ' (label "A"\n (at 0 0 0)\n )\n') + insert_sexp_before_close(filepath, ' (label "B"\n (at 10 10 0)\n )\n') + + with open(filepath) as f: + content = f.read() + + assert '(label "A"' in content + assert '(label "B"' in content + + def test_file_not_found(self, tmp_path): + with pytest.raises(FileNotFoundError): + insert_sexp_before_close(str(tmp_path / "missing.kicad_sch"), "(label)") + + +class TestResolvePinPosition: + """Tests for resolve_pin_position (requires mocking sch object).""" + + def test_returns_api_result_when_available(self): + """When the API returns a position, use it directly.""" + from unittest.mock import MagicMock + + from mckicad.utils.sexp_parser import resolve_pin_position + + sch = MagicMock() + pos = MagicMock() + pos.x = 100.0 + pos.y = 200.0 + sch.get_component_pin_position.return_value = pos + + result = resolve_pin_position(sch, "/fake/path.kicad_sch", "R1", "1") + assert result == (100.0, 200.0) + + def test_returns_none_when_api_returns_none_and_no_component(self): + from unittest.mock import MagicMock + + from mckicad.utils.sexp_parser import resolve_pin_position + + sch = MagicMock() + sch.get_component_pin_position.return_value = None + sch.components.get.return_value = None + + result = resolve_pin_position(sch, "/fake/path.kicad_sch", "U99", "1") + assert result is None + + def test_falls_back_to_sexp(self, sample_schematic_file): + """When API returns None, use sexp parsing for pin resolution.""" + from unittest.mock import MagicMock + + from mckicad.utils.sexp_parser import resolve_pin_position + + sch = MagicMock() + sch.get_component_pin_position.return_value = None + + # Mock a component at position (100, 100) with lib_id "Device:R" + comp = MagicMock() + comp.lib_id = "Device:R" + comp.position = MagicMock() + comp.position.x = 100.0 + comp.position.y = 100.0 + comp.rotation = 0 + comp.mirror = None + sch.components.get.return_value = comp + + result = resolve_pin_position(sch, sample_schematic_file, "R1", "1") + assert result is not None + # Pin 1 of Device:R is at (0, 3.81) in local coords + # At component position (100, 100) with 0 rotation: (100, 103.81) + assert result[0] == pytest.approx(100.0) + assert result[1] == pytest.approx(103.81)