Fix add_label persistence and add_power_symbol custom library fallback
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.
This commit is contained in:
parent
52ff054f43
commit
7525f3dcdc
@ -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"],
|
||||
|
||||
@ -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,
|
||||
|
||||
@ -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)
|
||||
|
||||
@ -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)
|
||||
|
||||
@ -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
|
||||
|
||||
@ -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
|
||||
|
||||
@ -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
|
||||
|
||||
@ -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)
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user