diff --git a/python/commands/wire_dragger.py b/python/commands/wire_dragger.py new file mode 100644 index 0000000..910236b --- /dev/null +++ b/python/commands/wire_dragger.py @@ -0,0 +1,311 @@ +""" +WireDragger — drag connected wires when a schematic component is moved. + +All methods operate on in-memory sexpdata lists (no disk I/O). +""" + +import logging +import math +from typing import Dict, Optional, Tuple + +import sexpdata +from sexpdata import Symbol + +logger = logging.getLogger("kicad_interface") + +# Module-level Symbol constants +_K = { + name: Symbol(name) + for name in [ + "symbol", + "at", + "lib_id", + "mirror", + "lib_symbols", + "pts", + "xy", + "wire", + "junction", + "property", + ] +} + +EPS = 1e-4 # mm — coordinate match tolerance + + +def _rotate(x: float, y: float, angle_deg: float) -> Tuple[float, float]: + """Rotate (x, y) around the origin by angle_deg degrees (CCW).""" + if angle_deg == 0: + return x, y + rad = math.radians(angle_deg) + c, s = math.cos(rad), math.sin(rad) + return x * c - y * s, x * s + y * c + + +def _coords_match(ax: float, ay: float, bx: float, by: float, eps: float = EPS) -> bool: + return abs(ax - bx) < eps and abs(ay - by) < eps + + +class WireDragger: + """Pure-logic helpers for wire-endpoint dragging during component moves.""" + + @staticmethod + def find_symbol(sch_data: list, reference: str): + """ + Find a placed symbol by reference designator. + + Returns (symbol_item, old_x, old_y, rotation, lib_id, mirror_x, mirror_y) + or None if the reference is not found. + + mirror_x=True means the symbol has (mirror x) — flips the X local axis. + mirror_y=True means the symbol has (mirror y) — flips the Y local axis. + """ + sym_k = _K["symbol"] + prop_k = _K["property"] + at_k = _K["at"] + lib_id_k = _K["lib_id"] + mirror_k = _K["mirror"] + + for item in sch_data: + if not (isinstance(item, list) and item and item[0] == sym_k): + continue + + # Check Reference property + 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: + continue + + old_x = old_y = rotation = 0.0 + lib_id = "" + mirror_x = mirror_y = False + + for sub in item[1:]: + if not isinstance(sub, list) or not sub: + continue + tag = sub[0] + if tag == at_k: + if len(sub) >= 3: + old_x = float(sub[1]) + old_y = float(sub[2]) + if len(sub) >= 4: + rotation = float(sub[3]) + elif tag == lib_id_k and len(sub) >= 2: + lib_id = str(sub[1]).strip('"') + elif tag == mirror_k and len(sub) >= 2: + mv = str(sub[1]) + if mv == "x": + mirror_x = True + elif mv == "y": + mirror_y = True + + return item, old_x, old_y, rotation, lib_id, mirror_x, mirror_y + + return None + + @staticmethod + def get_pin_defs(sch_data: list, lib_id: str) -> Dict: + """ + Get pin definitions from lib_symbols for the given lib_id. + + Returns the same dict format as PinLocator.parse_symbol_definition: + {pin_num: {"x": ..., "y": ..., ...}}. + """ + from commands.pin_locator import PinLocator + + lib_sym_k = _K["lib_symbols"] + symbol_k = _K["symbol"] + + for item in sch_data: + if not (isinstance(item, list) and item and item[0] == lib_sym_k): + continue + for sym_def in item[1:]: + if not (isinstance(sym_def, list) and sym_def and sym_def[0] == symbol_k): + continue + if len(sym_def) < 2: + continue + name = str(sym_def[1]).strip('"') + if name == lib_id: + return PinLocator.parse_symbol_definition(sym_def) + break # only one lib_symbols section + return {} + + @staticmethod + def pin_world_xy( + px: float, + py: float, + sym_x: float, + sym_y: float, + rotation: float, + mirror_x: bool, + mirror_y: bool, + ) -> Tuple[float, float]: + """ + Compute the world coordinate of a pin given the symbol transform. + + KiCAD applies mirror first (in local space), then rotation, then translation. + mirror_x negates the local X axis; mirror_y negates the local Y axis. + """ + lx, ly = px, py + if mirror_x: + lx = -lx + if mirror_y: + ly = -ly + rx, ry = _rotate(lx, ly, rotation) + return sym_x + rx, sym_y + ry + + @staticmethod + def compute_pin_positions( + sch_data: list, + reference: str, + new_x: float, + new_y: float, + ) -> Dict[str, Tuple[Tuple[float, float], Tuple[float, float]]]: + """ + Compute world pin positions before and after a component move. + + Returns {pin_num: (old_world_xy, new_world_xy)}. + old_world_xy uses the symbol's current position; new_world_xy uses (new_x, new_y). + """ + found = WireDragger.find_symbol(sch_data, reference) + if found is None: + return {} + _, old_x, old_y, rotation, lib_id, mirror_x, mirror_y = found + + pins = WireDragger.get_pin_defs(sch_data, lib_id) + result: Dict[str, Tuple] = {} + for pin_num, pin in pins.items(): + px, py = pin["x"], pin["y"] + old_wx, old_wy = WireDragger.pin_world_xy( + px, py, old_x, old_y, rotation, mirror_x, mirror_y + ) + new_wx, new_wy = WireDragger.pin_world_xy( + px, py, new_x, new_y, rotation, mirror_x, mirror_y + ) + result[pin_num] = ( + (round(old_wx, 6), round(old_wy, 6)), + (round(new_wx, 6), round(new_wy, 6)), + ) + return result + + @staticmethod + def drag_wires( + sch_data: list, + old_to_new: Dict[Tuple[float, float], Tuple[float, float]], + eps: float = EPS, + ) -> Dict: + """ + Move wire endpoints and junctions from old positions to new positions. + Removes zero-length wires that result from the move. + Modifies sch_data in place. + + old_to_new: {(old_x, old_y): (new_x, new_y)} + + Returns {'endpoints_moved': N, 'wires_removed': M}. + """ + wire_k = _K["wire"] + pts_k = _K["pts"] + xy_k = _K["xy"] + junction_k = _K["junction"] + at_k = _K["at"] + + def find_new(x: float, y: float): + for (ox, oy), (nx, ny) in old_to_new.items(): + if _coords_match(x, y, ox, oy, eps): + return nx, ny + return None + + endpoints_moved = 0 + zero_length_indices = [] + + # First pass: update wire endpoints + for idx, item in enumerate(sch_data): + if not (isinstance(item, list) and item and item[0] == wire_k): + continue + + pts_sub = None + for sub in item[1:]: + if isinstance(sub, list) and sub and sub[0] == pts_k: + pts_sub = sub + break + if pts_sub is None: + continue + + xy_items = [ + p for p in pts_sub[1:] if isinstance(p, list) and len(p) >= 3 and p[0] == xy_k + ] + for xy_item in xy_items: + nc = find_new(float(xy_item[1]), float(xy_item[2])) + if nc is not None: + xy_item[1] = nc[0] + xy_item[2] = nc[1] + endpoints_moved += 1 + + # Check if this wire is now zero-length + if len(xy_items) >= 2: + x1, y1 = float(xy_items[0][1]), float(xy_items[0][2]) + x2, y2 = float(xy_items[-1][1]), float(xy_items[-1][2]) + if _coords_match(x1, y1, x2, y2, eps): + zero_length_indices.append(idx) + + # Remove zero-length wires (backwards to preserve indices) + for idx in reversed(zero_length_indices): + del sch_data[idx] + + # Second pass: update junctions + for item in sch_data: + if not (isinstance(item, list) and item and item[0] == junction_k): + continue + for sub in item[1:]: + if isinstance(sub, list) and sub and sub[0] == at_k and len(sub) >= 3: + nc = find_new(float(sub[1]), float(sub[2])) + if nc is not None: + sub[1] = nc[0] + sub[2] = nc[1] + break + + return { + "endpoints_moved": endpoints_moved, + "wires_removed": len(zero_length_indices), + } + + @staticmethod + def update_symbol_position(sch_data: list, reference: str, new_x: float, new_y: float) -> bool: + """ + Update the (at x y rot) of the named symbol in sch_data. + Returns True if the symbol was found and updated. + """ + found = WireDragger.find_symbol(sch_data, reference) + if found is None: + return False + item = found[0] + at_k = _K["at"] + prop_k = _K["property"] + + # Find current position and compute delta + old_x = old_y = None + for sub in item[1:]: + if isinstance(sub, list) and sub and sub[0] == at_k and len(sub) >= 3: + old_x, old_y = sub[1], sub[2] + sub[1] = new_x + sub[2] = new_y + break + if old_x is None or old_y is None: + return False + + dx = new_x - old_x + dy = new_y - old_y + + # Shift all property label positions by the same delta + for sub in item[1:]: + if isinstance(sub, list) and sub and sub[0] == prop_k: + for psub in sub[1:]: + if isinstance(psub, list) and psub and psub[0] == at_k and len(psub) >= 3: + psub[1] += dx + psub[2] += dy + break + return True diff --git a/python/kicad_interface.py b/python/kicad_interface.py index b3903ad..899744d 100644 --- a/python/kicad_interface.py +++ b/python/kicad_interface.py @@ -1283,6 +1283,46 @@ class KiCADInterface: logger.error(f"Error listing schematic libraries: {str(e)}") return {"success": False, "message": str(e)} + def _handle_find_unconnected_pins(self, params): + """List component pins with no wire/label/power symbol touching them""" + logger.info("Finding unconnected pins") + try: + from commands.schematic_analysis import find_unconnected_pins + + schematic_path = params.get("schematicPath") + if not schematic_path: + return {"success": False, "message": "schematicPath is required"} + result = find_unconnected_pins(schematic_path) + return {"success": True, **result} + except ImportError: + return { + "success": False, + "message": "schematic_analysis module not available", + } + except Exception as e: + logger.error(f"Error finding unconnected pins: {e}") + return {"success": False, "message": str(e)} + + def _handle_check_wire_collisions(self, params): + """Detect wires passing through component bodies without connecting to pins""" + logger.info("Checking wire collisions") + try: + from commands.schematic_analysis import check_wire_collisions + + schematic_path = params.get("schematicPath") + if not schematic_path: + return {"success": False, "message": "schematicPath is required"} + result = check_wire_collisions(schematic_path) + return {"success": True, **result} + except ImportError: + return { + "success": False, + "message": "schematic_analysis module not available", + } + except Exception as e: + logger.error(f"Error checking wire collisions: {e}") + return {"success": False, "message": str(e)} + # ------------------------------------------------------------------ # # Footprint handlers # # ------------------------------------------------------------------ # @@ -1998,14 +2038,18 @@ class KiCADInterface: return {"success": False, "message": str(e)} def _handle_move_schematic_component(self, params): - """Move a schematic component to a new position""" + """Move a schematic component to a new position, dragging connected wires.""" logger.info("Moving schematic component") try: + import sexpdata as _sexpdata + from commands.wire_dragger import WireDragger + schematic_path = params.get("schematicPath") reference = params.get("reference") position = params.get("position", {}) new_x = position.get("x") new_y = position.get("y") + preserve_wires = params.get("preserveWires", True) if not schematic_path or not reference: return { @@ -2018,30 +2062,42 @@ class KiCADInterface: "message": "position with x and y is required", } - schematic = SchematicManager.load_schematic(schematic_path) - if not schematic: - return {"success": False, "message": "Failed to load schematic"} + with open(schematic_path, "r", encoding="utf-8") as f: + sch_data = _sexpdata.loads(f.read()) - # Find the symbol - for symbol in schematic.symbol: - if not hasattr(symbol.property, "Reference"): - continue - if symbol.property.Reference.value == reference: - old_pos = list(symbol.at.value) - old_position = {"x": float(old_pos[0]), "y": float(old_pos[1])} + # Find symbol and record old position + found = WireDragger.find_symbol(sch_data, reference) + if found is None: + return {"success": False, "message": f"Component {reference} not found"} + _, old_x, old_y = found[0], found[1], found[2] + old_position = {"x": old_x, "y": old_y} - # Preserve rotation (third element) - rotation = float(old_pos[2]) if len(old_pos) > 2 else 0 - symbol.at.value = [new_x, new_y, rotation] + drag_summary = {} + if preserve_wires: + # Compute pin world positions before and after the move + pin_positions = WireDragger.compute_pin_positions( + sch_data, reference, float(new_x), float(new_y) + ) + # Build old→new coordinate map (deduplicate coincident pins) + old_to_new = {} + for _pin, (old_xy, new_xy) in pin_positions.items(): + old_to_new[old_xy] = new_xy - SchematicManager.save_schematic(schematic, schematic_path) - return { - "success": True, - "oldPosition": old_position, - "newPosition": {"x": new_x, "y": new_y}, - } + drag_summary = WireDragger.drag_wires(sch_data, old_to_new) - return {"success": False, "message": f"Component {reference} not found"} + # Update symbol position + WireDragger.update_symbol_position(sch_data, reference, float(new_x), float(new_y)) + + with open(schematic_path, "w", encoding="utf-8") as f: + f.write(_sexpdata.dumps(sch_data)) + + return { + "success": True, + "oldPosition": old_position, + "newPosition": {"x": new_x, "y": new_y}, + "wiresMoved": drag_summary.get("endpoints_moved", 0), + "wiresRemoved": drag_summary.get("wires_removed", 0), + } except Exception as e: logger.error(f"Error moving schematic component: {e}") @@ -2074,21 +2130,18 @@ class KiCADInterface: continue if symbol.property.Reference.value == reference: pos = list(symbol.at.value) - pos[2] = angle if len(pos) > 2 else angle while len(pos) < 3: pos.append(0) pos[2] = angle symbol.at.value = pos - # Handle mirror if specified if mirror: if hasattr(symbol, "mirror"): symbol.mirror.value = mirror else: logger.warning( f"Mirror '{mirror}' requested for {reference}, " - f"but symbol does not have a 'mirror' attribute; " - f"mirror not applied" + f"but symbol has no mirror attribute; skipped" ) SchematicManager.save_schematic(schematic, schematic_path) diff --git a/python/tests/test_move_with_wire_preservation.py b/python/tests/test_move_with_wire_preservation.py new file mode 100644 index 0000000..acdda76 --- /dev/null +++ b/python/tests/test_move_with_wire_preservation.py @@ -0,0 +1,661 @@ +""" +Tests for move_schematic_component with wire preservation (WireDragger). + +Unit tests use synthetic sexpdata lists — no disk I/O, no KiCAD install needed. +Integration tests copy empty.kicad_sch to a tempdir and exercise the full handler. +""" + +import shutil +import sys +import tempfile +from pathlib import Path +from unittest.mock import MagicMock, patch + +import pytest +import sexpdata +from sexpdata import Symbol + +# Make python/ importable +sys.path.insert(0, str(Path(__file__).resolve().parent.parent)) + +from commands.wire_dragger import EPS, WireDragger, _rotate + +TEMPLATE_PATH = Path(__file__).resolve().parent.parent / "templates" / "empty.kicad_sch" + + +# --------------------------------------------------------------------------- +# Helpers +# --------------------------------------------------------------------------- + + +def _sym(name: str) -> Symbol: + return Symbol(name) + + +def _make_wire(x1, y1, x2, y2): + return [ + _sym("wire"), + [_sym("pts"), [_sym("xy"), x1, y1], [_sym("xy"), x2, y2]], + [_sym("stroke"), [_sym("width"), 0], [_sym("type"), _sym("default")]], + [_sym("uuid"), "00000000-0000-0000-0000-000000000000"], + ] + + +def _make_junction(x, y): + return [ + _sym("junction"), + [_sym("at"), x, y], + [_sym("diameter"), 0], + [_sym("color"), 0, 0, 0, 0], + [_sym("uuid"), "00000000-0000-0000-0000-000000000001"], + ] + + +def _make_symbol(ref, x, y, rotation=0, lib_id="Device:R", mirror=None): + """Build a minimal placed-symbol s-expression.""" + item = [ + _sym("symbol"), + [_sym("lib_id"), lib_id], + [_sym("at"), x, y, rotation], + [_sym("unit"), 1], + [_sym("property"), "Reference", ref, [_sym("at"), x + 2, y, 0]], + [_sym("property"), "Value", "10k", [_sym("at"), x, y, 0]], + ] + if mirror: + item.append([_sym("mirror"), _sym(mirror)]) + return item + + +def _make_lib_symbol_r(): + """Minimal Device:R lib_symbols entry — pins at (0, 3.81) and (0, -3.81).""" + return [ + _sym("symbol"), + "Device:R", + [ + _sym("symbol"), + "R_1_1", + [ + _sym("pin"), + _sym("passive"), + _sym("line"), + [_sym("at"), 0, 3.81, 270], + [_sym("length"), 1.27], + [ + _sym("name"), + "~", + [_sym("effects"), [_sym("font"), [_sym("size"), 1.27, 1.27]]], + ], + [ + _sym("number"), + "1", + [_sym("effects"), [_sym("font"), [_sym("size"), 1.27, 1.27]]], + ], + ], + [ + _sym("pin"), + _sym("passive"), + _sym("line"), + [_sym("at"), 0, -3.81, 90], + [_sym("length"), 1.27], + [ + _sym("name"), + "~", + [_sym("effects"), [_sym("font"), [_sym("size"), 1.27, 1.27]]], + ], + [ + _sym("number"), + "2", + [_sym("effects"), [_sym("font"), [_sym("size"), 1.27, 1.27]]], + ], + ], + ], + ] + + +def _make_sch_data(extra_items=None): + """Build a minimal sch_data list with lib_symbols and sheet_instances.""" + data = [ + _sym("kicad_sch"), + [_sym("lib_symbols"), _make_lib_symbol_r()], + [_sym("sheet_instances"), [_sym("path"), "/", [_sym("page"), "1"]]], + ] + if extra_items: + # Insert before sheet_instances (last item) + for item in extra_items: + data.insert(len(data) - 1, item) + return data + + +# --------------------------------------------------------------------------- +# TestRotatePoint +# --------------------------------------------------------------------------- + + +@pytest.mark.unit +class TestRotatePoint: + def test_zero_rotation(self): + assert _rotate(1.0, 2.0, 0) == (1.0, 2.0) + + def test_90_degrees(self): + rx, ry = _rotate(1.0, 0.0, 90) + assert abs(rx - 0.0) < 1e-9 + assert abs(ry - 1.0) < 1e-9 + + def test_180_degrees(self): + rx, ry = _rotate(1.0, 0.0, 180) + assert abs(rx - (-1.0)) < 1e-9 + assert abs(ry - 0.0) < 1e-9 + + def test_270_degrees(self): + rx, ry = _rotate(0.0, 1.0, 270) + assert abs(rx - 1.0) < 1e-6 + assert abs(ry - 0.0) < 1e-6 + + +# --------------------------------------------------------------------------- +# TestFindSymbol +# --------------------------------------------------------------------------- + + +@pytest.mark.unit +class TestFindSymbol: + def test_returns_none_for_missing_reference(self): + sch = _make_sch_data([_make_symbol("R1", 10, 20)]) + assert WireDragger.find_symbol(sch, "R99") is None + + def test_returns_item_and_position(self): + sch = _make_sch_data([_make_symbol("R1", 10.5, 20.5, rotation=90)]) + result = WireDragger.find_symbol(sch, "R1") + assert result is not None + _, old_x, old_y, rotation, lib_id, mirror_x, mirror_y = result + assert abs(old_x - 10.5) < EPS + assert abs(old_y - 20.5) < EPS + assert abs(rotation - 90) < EPS + assert lib_id == "Device:R" + assert mirror_x is False + assert mirror_y is False + + def test_detects_mirror_x(self): + sch = _make_sch_data([_make_symbol("R1", 0, 0, mirror="x")]) + result = WireDragger.find_symbol(sch, "R1") + assert result is not None + assert result[5] is True # mirror_x + assert result[6] is False # mirror_y + + def test_detects_mirror_y(self): + sch = _make_sch_data([_make_symbol("R1", 0, 0, mirror="y")]) + result = WireDragger.find_symbol(sch, "R1") + assert result is not None + assert result[5] is False # mirror_x + assert result[6] is True # mirror_y + + +# --------------------------------------------------------------------------- +# TestComputePinPositions +# --------------------------------------------------------------------------- + + +@pytest.mark.unit +class TestComputePinPositions: + def test_resistor_at_origin_no_rotation(self): + """Device:R at (0, 0) rot=0 — pins at (0, 3.81) and (0, -3.81).""" + sch = _make_sch_data([_make_symbol("R1", 0, 0)]) + positions = WireDragger.compute_pin_positions(sch, "R1", 10, 20) + assert "1" in positions and "2" in positions + old1, new1 = positions["1"] + old2, new2 = positions["2"] + # Pin 1 old: (0 + 0, 0 + 3.81) + assert abs(old1[0] - 0) < 1e-4 + assert abs(old1[1] - 3.81) < 1e-4 + # Pin 2 old: (0 + 0, 0 - 3.81) + assert abs(old2[0] - 0) < 1e-4 + assert abs(old2[1] - (-3.81)) < 1e-4 + # New positions shifted by (10, 20) + assert abs(new1[0] - 10) < 1e-4 + assert abs(new1[1] - 23.81) < 1e-4 + assert abs(new2[0] - 10) < 1e-4 + assert abs(new2[1] - 16.19) < 1e-4 + + def test_resistor_rotated_90(self): + """Device:R at (100, 100) rot=90 — pins should be at (100+3.81, 100) and (100-3.81, 100).""" + 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"] + # rotate(0, 3.81, 90) = (0*cos90 - 3.81*sin90, 0*sin90 + 3.81*cos90) = (-3.81, 0) + # Wait — pin 1 is at local (0, 3.81), rotated 90° CCW: + # x' = 0*cos90 - 3.81*sin90 = -3.81, y' = 0*sin90 + 3.81*cos90 ≈ 0 + # world: (100 - 3.81, 100 + 0) = (96.19, 100) + assert abs(old1[0] - 96.19) < 1e-3 + assert abs(old1[1] - 100) < 1e-3 + + def test_returns_empty_for_missing_component(self): + sch = _make_sch_data() + result = WireDragger.compute_pin_positions(sch, "MISSING", 0, 0) + assert result == {} + + def test_delta_is_consistent(self): + """new_xy - old_xy should equal (new_x - old_x, new_y - old_y) for any rotation.""" + sch = _make_sch_data([_make_symbol("R1", 50, 50, rotation=45)]) + positions = WireDragger.compute_pin_positions(sch, "R1", 60, 70) + for pin_num, (old_xy, new_xy) in positions.items(): + dx = new_xy[0] - old_xy[0] + dy = new_xy[1] - old_xy[1] + assert abs(dx - 10) < 1e-4, f"Pin {pin_num}: dx={dx}" + assert abs(dy - 20) < 1e-4, f"Pin {pin_num}: dy={dy}" + + +# --------------------------------------------------------------------------- +# TestDragWires +# --------------------------------------------------------------------------- + + +@pytest.mark.unit +class TestDragWires: + def test_no_wires_returns_zero_counts(self): + sch = _make_sch_data() + result = WireDragger.drag_wires(sch, {(0.0, 0.0): (10.0, 10.0)}) + assert result["endpoints_moved"] == 0 + assert result["wires_removed"] == 0 + + def test_wire_start_endpoint_moved(self): + wire = _make_wire(0, 3.81, 0, 10) + sch = _make_sch_data([wire]) + result = WireDragger.drag_wires(sch, {(0.0, 3.81): (10.0, 23.81)}) + assert result["endpoints_moved"] == 1 + assert result["wires_removed"] == 0 + # Find the updated wire in sch_data + updated = next(i for i in sch if isinstance(i, list) and i and i[0] == Symbol("wire")) + pts = next(s for s in updated[1:] if isinstance(s, list) and s and s[0] == Symbol("pts")) + xy1 = pts[1] + assert abs(xy1[1] - 10.0) < EPS + assert abs(xy1[2] - 23.81) < EPS + + def test_wire_end_endpoint_moved(self): + wire = _make_wire(0, 10, 0, -3.81) + sch = _make_sch_data([wire]) + result = WireDragger.drag_wires(sch, {(0.0, -3.81): (10.0, 16.19)}) + assert result["endpoints_moved"] == 1 + updated = next(i for i in sch if isinstance(i, list) and i and i[0] == Symbol("wire")) + pts = next(s for s in updated[1:] if isinstance(s, list) and s and s[0] == Symbol("pts")) + xy2 = pts[2] + assert abs(xy2[1] - 10.0) < EPS + assert abs(xy2[2] - 16.19) < EPS + + def test_zero_length_wire_removed(self): + """When both endpoints of a wire are moved to the same point, wire is deleted.""" + wire = _make_wire(0, 3.81, 0, -3.81) + sch = _make_sch_data([wire]) + # Both pins land at same position (degenerate move) + result = WireDragger.drag_wires( + sch, + { + (0.0, 3.81): (5.0, 5.0), + (0.0, -3.81): (5.0, 5.0), + }, + ) + assert result["wires_removed"] == 1 + wires_remaining = [i for i in sch if isinstance(i, list) and i and i[0] == Symbol("wire")] + assert len(wires_remaining) == 0 + + def test_unrelated_wire_not_touched(self): + """A wire whose endpoints don't match any old pin is not changed.""" + wire = _make_wire(50, 50, 60, 50) + sch = _make_sch_data([wire]) + original_start = (50.0, 50.0) + result = WireDragger.drag_wires(sch, {(0.0, 3.81): (10.0, 23.81)}) + assert result["endpoints_moved"] == 0 + updated = next(i for i in sch if isinstance(i, list) and i and i[0] == Symbol("wire")) + pts = next(s for s in updated[1:] if isinstance(s, list) and s and s[0] == Symbol("pts")) + xy1 = pts[1] + assert abs(xy1[1] - 50.0) < EPS + assert abs(xy1[2] - 50.0) < EPS + + def test_both_endpoints_on_moved_component(self): + """Wire connecting two pins of same component — both endpoints shift together.""" + wire = _make_wire(0, 3.81, 0, -3.81) + sch = _make_sch_data([wire]) + result = WireDragger.drag_wires( + sch, + { + (0.0, 3.81): (10.0, 23.81), + (0.0, -3.81): (10.0, 16.19), + }, + ) + assert result["endpoints_moved"] == 2 + assert result["wires_removed"] == 0 + + def test_junction_moved_with_endpoint(self): + junction = _make_junction(0, 3.81) + sch = _make_sch_data([junction]) + WireDragger.drag_wires(sch, {(0.0, 3.81): (10.0, 23.81)}) + updated_j = next(i for i in sch if isinstance(i, list) and i and i[0] == Symbol("junction")) + at_sub = next( + s for s in updated_j[1:] if isinstance(s, list) and s and s[0] == Symbol("at") + ) + assert abs(at_sub[1] - 10.0) < EPS + assert abs(at_sub[2] - 23.81) < EPS + + def test_junction_at_unrelated_position_not_touched(self): + junction = _make_junction(99, 99) + sch = _make_sch_data([junction]) + WireDragger.drag_wires(sch, {(0.0, 3.81): (10.0, 23.81)}) + updated_j = next(i for i in sch if isinstance(i, list) and i and i[0] == Symbol("junction")) + at_sub = next( + s for s in updated_j[1:] if isinstance(s, list) and s and s[0] == Symbol("at") + ) + assert abs(at_sub[1] - 99.0) < EPS + assert abs(at_sub[2] - 99.0) < EPS + + +# --------------------------------------------------------------------------- +# TestUpdateSymbolPosition +# --------------------------------------------------------------------------- + + +@pytest.mark.unit +class TestUpdateSymbolPosition: + def test_updates_position(self): + sch = _make_sch_data([_make_symbol("R1", 10, 20)]) + result = WireDragger.update_symbol_position(sch, "R1", 30, 40) + assert result is True + found = WireDragger.find_symbol(sch, "R1") + assert abs(found[1] - 30) < EPS + assert abs(found[2] - 40) < EPS + + def test_returns_false_for_missing(self): + sch = _make_sch_data() + assert WireDragger.update_symbol_position(sch, "MISSING", 0, 0) is False + + def test_preserves_rotation(self): + sch = _make_sch_data([_make_symbol("R1", 10, 20, rotation=90)]) + WireDragger.update_symbol_position(sch, "R1", 30, 40) + found = WireDragger.find_symbol(sch, "R1") + assert abs(found[3] - 90) < EPS # rotation preserved + + def test_property_labels_follow_symbol_move(self): + """Property (at ...) positions must shift by the same delta as the symbol.""" + sym = _make_symbol("R1", 100, 80) + sch = _make_sch_data([sym]) + + # Record initial property positions + prop_k = _sym("property") + at_k = _sym("at") + initial_positions = {} + for sub in sym[1:]: + if isinstance(sub, list) and sub and sub[0] == prop_k: + name = sub[1] + for psub in sub[2:]: + if isinstance(psub, list) and psub and psub[0] == at_k: + initial_positions[name] = (psub[1], psub[2]) + break + assert len(initial_positions) >= 2 # Reference and Value at minimum + + # Move component from (100, 80) to (120, 100) — delta (20, 20) + result = WireDragger.update_symbol_position(sch, "R1", 120, 100) + assert result is True + + # Verify each property shifted by (20, 20) + for sub in sym[1:]: + if isinstance(sub, list) and sub and sub[0] == prop_k: + name = sub[1] + for psub in sub[2:]: + if isinstance(psub, list) and psub and psub[0] == at_k: + expected_x = initial_positions[name][0] + 20 + expected_y = initial_positions[name][1] + 20 + assert ( + abs(psub[1] - expected_x) < EPS + ), f"{name} x: expected {expected_x}, got {psub[1]}" + assert ( + abs(psub[2] - expected_y) < EPS + ), f"{name} y: expected {expected_y}, got {psub[2]}" + break + + +# --------------------------------------------------------------------------- +# Integration tests +# --------------------------------------------------------------------------- + + +@pytest.mark.integration +class TestMoveWithWirePreservation: + """Integration tests using a real .kicad_sch file.""" + + def _make_schematic(self, extra_sexp=""): + """Copy empty.kicad_sch to a temp file and optionally append content.""" + tmp = Path(tempfile.mkdtemp()) / "test.kicad_sch" + shutil.copy(TEMPLATE_PATH, tmp) + if extra_sexp: + content = tmp.read_text(encoding="utf-8") + idx = content.rfind(")") + content = content[:idx] + "\n" + extra_sexp + "\n)" + tmp.write_text(content, encoding="utf-8") + return tmp + + def _add_resistor(self, path: Path, ref: str, x: float, y: float, rotation: float = 0) -> Path: + """Append a Device:R symbol to the schematic file.""" + import uuid + + u = str(uuid.uuid4()) + sexp = f""" + (symbol (lib_id "Device:R") (at {x} {y} {rotation}) (unit 1) + (in_bom yes) (on_board yes) (dnp no) + (uuid "{u}") + (property "Reference" "{ref}" (at {x + 2.032} {y} 90) + (effects (font (size 1.27 1.27))) + ) + (property "Value" "10k" (at {x} {y} 90) + (effects (font (size 1.27 1.27))) + ) + (property "Footprint" "" (at {x - 1.778} {y} 90) + (effects (font (size 1.27 1.27)) hide) + ) + (property "Datasheet" "~" (at {x} {y} 0) + (effects (font (size 1.27 1.27)) hide) + ) + (pin "1" (uuid "{uuid.uuid4()}")) + (pin "2" (uuid "{uuid.uuid4()}")) + (instances (project "test" (path "/" (reference "{ref}") (unit 1)))) + )""" + content = path.read_text(encoding="utf-8") + idx = content.rfind(")") + path.write_text(content[:idx] + "\n" + sexp + "\n)", encoding="utf-8") + return path + + def _add_wire(self, path: Path, x1, y1, x2, y2) -> Path: + """Append a wire to the schematic file.""" + import uuid + + wire_sexp = f""" + (wire (pts (xy {x1} {y1}) (xy {x2} {y2})) + (stroke (width 0) (type default)) + (uuid "{uuid.uuid4()}") + )""" + content = path.read_text(encoding="utf-8") + idx = content.rfind(")") + path.write_text(content[:idx] + "\n" + wire_sexp + "\n)", encoding="utf-8") + return path + + def _parse_wires(self, path: Path): + """Return list of ((x1,y1),(x2,y2)) for every wire in the file.""" + content = path.read_text(encoding="utf-8") + data = sexpdata.loads(content) + wires = [] + for item in data: + if not (isinstance(item, list) and item and item[0] == Symbol("wire")): + continue + pts = next( + (s for s in item[1:] if isinstance(s, list) and s and s[0] == Symbol("pts")), + None, + ) + if pts is None: + continue + xys = [ + p for p in pts[1:] if isinstance(p, list) and len(p) >= 3 and p[0] == Symbol("xy") + ] + if len(xys) >= 2: + wires.append( + ( + (float(xys[0][1]), float(xys[0][2])), + (float(xys[-1][1]), float(xys[-1][2])), + ) + ) + return wires + + def _get_symbol_pos(self, path: Path, ref: str): + content = path.read_text(encoding="utf-8") + data = sexpdata.loads(content) + found = WireDragger.find_symbol(data, ref) + if found is None: + return None + return found[1], found[2] + + def test_symbol_position_updated(self): + sch = self._make_schematic() + self._add_resistor(sch, "R1", 100, 100) + # Call handler directly + sys.path.insert(0, str(Path(__file__).resolve().parent.parent)) + from kicad_interface import KiCADInterface + + iface = KiCADInterface() + result = iface.handle_command( + "move_schematic_component", + { + "schematicPath": str(sch), + "reference": "R1", + "position": {"x": 120, "y": 130}, + }, + ) + assert result["success"], result.get("message") + pos = self._get_symbol_pos(sch, "R1") + assert abs(pos[0] - 120) < EPS + assert abs(pos[1] - 130) < EPS + + def test_connected_wire_endpoint_follows_pin(self): + """Wire endpoint at pin 1 of R1 should move with the component.""" + sch = self._make_schematic() + # R1 at (100, 100) — pin 1 at (100, 103.81) + self._add_resistor(sch, "R1", 100, 100) + self._add_wire(sch, 100, 103.81, 100, 120) # wire from pin 1 upward + + sys.path.insert(0, str(Path(__file__).resolve().parent.parent)) + from kicad_interface import KiCADInterface + + iface = KiCADInterface() + result = iface.handle_command( + "move_schematic_component", + { + "schematicPath": str(sch), + "reference": "R1", + "position": {"x": 110, "y": 100}, + }, + ) + assert result["success"], result.get("message") + assert result["wiresMoved"] >= 1 + + wires = self._parse_wires(sch) + assert len(wires) == 1 + # Pin 1 new world position: (110 + 0, 100 + 3.81) = (110, 103.81) + w = wires[0] + endpoints = {w[0], w[1]} + new_pin1 = (110.0, 103.81) + assert any( + abs(ep[0] - new_pin1[0]) < 0.01 and abs(ep[1] - new_pin1[1]) < 0.01 for ep in endpoints + ), f"Expected pin endpoint near {new_pin1}, got {endpoints}" + + def test_unrelated_wire_unchanged(self): + """A wire not connected to R1 must not be modified.""" + sch = self._make_schematic() + self._add_resistor(sch, "R1", 100, 100) + self._add_wire(sch, 50, 50, 60, 50) # unrelated wire + + sys.path.insert(0, str(Path(__file__).resolve().parent.parent)) + from kicad_interface import KiCADInterface + + iface = KiCADInterface() + iface.handle_command( + "move_schematic_component", + { + "schematicPath": str(sch), + "reference": "R1", + "position": {"x": 110, "y": 110}, + }, + ) + + wires = self._parse_wires(sch) + unrelated = [(s, e) for s, e in wires if abs(s[0] - 50) < 0.01 and abs(s[1] - 50) < 0.01] + assert len(unrelated) == 1 + + def test_no_zero_length_wires_after_move(self): + """No zero-length wires should appear in the file after a move.""" + sch = self._make_schematic() + self._add_resistor(sch, "R1", 100, 100) + # Wire from pin 1 to pin 2 of same component (intra-component wire) + self._add_wire(sch, 100, 103.81, 100, 96.19) + + sys.path.insert(0, str(Path(__file__).resolve().parent.parent)) + from kicad_interface import KiCADInterface + + iface = KiCADInterface() + iface.handle_command( + "move_schematic_component", + { + "schematicPath": str(sch), + "reference": "R1", + "position": {"x": 110, "y": 100}, + }, + ) + + wires = self._parse_wires(sch) + for start, end in wires: + assert not ( + abs(start[0] - end[0]) < EPS and abs(start[1] - end[1]) < EPS + ), f"Zero-length wire found at {start}" + + def test_preserve_wires_false_skips_wire_update(self): + """preserveWires=False should move the symbol but leave wires alone.""" + sch = self._make_schematic() + self._add_resistor(sch, "R1", 100, 100) + self._add_wire(sch, 100, 103.81, 100, 120) + + sys.path.insert(0, str(Path(__file__).resolve().parent.parent)) + from kicad_interface import KiCADInterface + + iface = KiCADInterface() + result = iface.handle_command( + "move_schematic_component", + { + "schematicPath": str(sch), + "reference": "R1", + "position": {"x": 110, "y": 100}, + "preserveWires": False, + }, + ) + assert result["success"] + assert result["wiresMoved"] == 0 + + # Wire should still start at old pin position + wires = self._parse_wires(sch) + assert len(wires) == 1 + endpoints = {wires[0][0], wires[0][1]} + old_pin1 = (100.0, 103.81) + assert any( + abs(ep[0] - old_pin1[0]) < 0.01 and abs(ep[1] - old_pin1[1]) < 0.01 for ep in endpoints + ), f"Wire should still be at {old_pin1}, got {endpoints}" + + def test_missing_component_returns_error(self): + sch = self._make_schematic() + sys.path.insert(0, str(Path(__file__).resolve().parent.parent)) + from kicad_interface import KiCADInterface + + iface = KiCADInterface() + result = iface.handle_command( + "move_schematic_component", + { + "schematicPath": str(sch), + "reference": "NOTHERE", + "position": {"x": 0, "y": 0}, + }, + ) + assert not result["success"] + assert "not found" in result.get("message", "").lower() diff --git a/src/tools/schematic.ts b/src/tools/schematic.ts index ad75abd..08e4d28 100644 --- a/src/tools/schematic.ts +++ b/src/tools/schematic.ts @@ -1124,6 +1124,56 @@ Note: operates on .kicad_sch files only. To modify a PCB footprint use edit_comp }, ); + // Move a placed symbol, dragging connected wires + server.tool( + "move_schematic_component", + "Move a placed symbol to a new position in the schematic. By default (preserveWires=true) wire endpoints touching the component's pins are stretched to follow the new position.", + { + schematicPath: z.string().describe("Path to the .kicad_sch file"), + reference: z.string().describe("Reference designator (e.g., R1, U1)"), + position: z + .object({ x: z.number(), y: z.number() }) + .describe("New position in schematic mm coordinates"), + preserveWires: z + .boolean() + .optional() + .describe("Stretch connected wire endpoints to follow the move (default true)"), + }, + async (args: { + schematicPath: string; + reference: string; + position: { x: number; y: number }; + preserveWires?: boolean; + }) => { + const result = await callKicadScript("move_schematic_component", args); + if (result.success) { + const moved = result.wiresMoved ?? 0; + const removed = result.wiresRemoved ?? 0; + return { + content: [ + { + type: "text", + text: + `Moved ${args.reference} from (${result.oldPosition.x}, ${result.oldPosition.y}) ` + + `to (${result.newPosition.x}, ${result.newPosition.y})` + + (moved > 0 ? `, ${moved} wire endpoint(s) updated` : "") + + (removed > 0 ? `, ${removed} zero-length wire(s) removed` : ""), + }, + ], + }; + } + return { + content: [ + { + type: "text", + text: `Failed to move component: ${result.message || "Unknown error"}`, + }, + ], + isError: true, + }; + }, + ); + // Sync schematic to PCB board (equivalent to KiCAD F8 / "Update PCB from Schematic") server.tool( "sync_schematic_to_board",