From 764b8db3d396e54ebae2c58c5da561ee7c07a251 Mon Sep 17 00:00:00 2001 From: Eugene Mikhantyev Date: Sat, 14 Mar 2026 20:39:49 +0000 Subject: [PATCH 01/13] feat: add schematic analysis tools (read-only) Add five new read-only schematic analysis MCP tools: - get_schematic_view_region: export cropped schematic region as PNG/SVG - find_unconnected_pins: list pins with no wire/label/power connection - find_overlapping_elements: detect duplicate symbols, stacked labels, collinear wire overlaps - get_elements_in_region: list all symbols/wires/labels in a bounding box - check_wire_collisions: detect wires passing through component bodies Includes Python handler dispatch, tool schemas, TypeScript server bindings, the schematic_analysis command module, and a full test suite (28 tests passing). Co-Authored-By: Claude Sonnet 4.6 --- python/commands/schematic_analysis.py | 623 ++++++++++++++++++++++++ python/kicad_interface.py | 205 ++++++++ python/schemas/tool_schemas.py | 125 +++++ python/tests/__init__.py | 0 python/tests/conftest.py | 67 ++- python/tests/test_schematic_analysis.py | 398 +++++++++++++++ src/tools/schematic.ts | 183 +++++++ 7 files changed, 1575 insertions(+), 26 deletions(-) create mode 100644 python/commands/schematic_analysis.py create mode 100644 python/tests/__init__.py create mode 100644 python/tests/test_schematic_analysis.py diff --git a/python/commands/schematic_analysis.py b/python/commands/schematic_analysis.py new file mode 100644 index 0000000..8fa58e5 --- /dev/null +++ b/python/commands/schematic_analysis.py @@ -0,0 +1,623 @@ +""" +Schematic Analysis Tools for KiCad Schematics + +Read-only analysis tools for detecting spatial problems, querying regions, +and checking connectivity in KiCad schematic files. +""" + +import logging +import math +from pathlib import Path +from typing import Dict, List, Tuple, Optional, Any, Set + +import sexpdata +from sexpdata import Symbol + +from commands.pin_locator import PinLocator + +logger = logging.getLogger("kicad_interface") + + +# --------------------------------------------------------------------------- +# S-expression parsing helpers +# --------------------------------------------------------------------------- + +def _load_sexp(schematic_path: Path) -> list: + """Load schematic file and return parsed S-expression data.""" + with open(schematic_path, "r", encoding="utf-8") as f: + return sexpdata.loads(f.read()) + + +def _parse_wires(sexp_data: list) -> List[Dict[str, Any]]: + """ + Parse all wire segments from the schematic S-expression. + + Returns list of dicts: {start: (x_mm, y_mm), end: (x_mm, y_mm)} + """ + wires = [] + for item in sexp_data: + if not isinstance(item, list) or len(item) < 2: + continue + if item[0] != Symbol("wire"): + continue + pts = None + for sub in item: + if isinstance(sub, list) and len(sub) > 0 and sub[0] == Symbol("pts"): + pts = sub + break + if not pts: + continue + coords = [] + for sub in pts: + if isinstance(sub, list) and len(sub) >= 3 and sub[0] == Symbol("xy"): + coords.append((float(sub[1]), float(sub[2]))) + if len(coords) >= 2: + wires.append({"start": coords[0], "end": coords[1]}) + return wires + + +def _parse_labels(sexp_data: list) -> List[Dict[str, Any]]: + """ + Parse all labels (label and global_label) from the schematic S-expression. + + Returns list of dicts: {name, type ('label'|'global_label'), x, y} + """ + labels = [] + for item in sexp_data: + if not isinstance(item, list) or len(item) < 2: + continue + tag = item[0] + if tag not in (Symbol("label"), Symbol("global_label")): + continue + name = str(item[1]).strip('"') + label_type = str(tag) + x, y = 0.0, 0.0 + for sub in item: + if isinstance(sub, list) and len(sub) >= 3 and sub[0] == Symbol("at"): + x = float(sub[1]) + y = float(sub[2]) + break + labels.append({"name": name, "type": label_type, "x": x, "y": y}) + return labels + + +def _parse_symbols(sexp_data: list) -> List[Dict[str, Any]]: + """ + Parse all placed symbol instances from the schematic S-expression. + + Returns list of dicts: {reference, lib_id, x, y, rotation, is_power} + """ + symbols = [] + for item in sexp_data: + if not isinstance(item, list) or len(item) < 2: + continue + if item[0] != Symbol("symbol"): + continue + + lib_id = "" + x, y, rotation = 0.0, 0.0, 0.0 + reference = "" + is_power = False + + for sub in item: + if isinstance(sub, list) and len(sub) >= 2: + if sub[0] == Symbol("lib_id"): + lib_id = str(sub[1]).strip('"') + elif sub[0] == Symbol("at") and len(sub) >= 3: + x = float(sub[1]) + y = float(sub[2]) + if len(sub) >= 4: + rotation = float(sub[3]) + elif sub[0] == Symbol("property") and len(sub) >= 3: + prop_name = str(sub[1]).strip('"') + if prop_name == "Reference": + reference = str(sub[2]).strip('"') + + is_power = reference.startswith("#PWR") or reference.startswith("#FLG") + symbols.append({ + "reference": reference, + "lib_id": lib_id, + "x": x, + "y": y, + "rotation": rotation, + "is_power": is_power, + }) + return symbols + + +def _parse_no_connects(sexp_data: list) -> Set[Tuple[float, float]]: + """Parse all no_connect elements and return their positions as (x, y) tuples in mm.""" + positions: Set[Tuple[float, float]] = set() + for item in sexp_data: + if not isinstance(item, list) or len(item) < 2: + continue + if item[0] != Symbol("no_connect"): + continue + for sub in item: + if isinstance(sub, list) and len(sub) >= 3 and sub[0] == Symbol("at"): + positions.add((float(sub[1]), float(sub[2]))) + break + return positions + + +# --------------------------------------------------------------------------- +# Geometry helpers +# --------------------------------------------------------------------------- + +def compute_symbol_bbox( + schematic_path: Path, + reference: str, + locator: PinLocator, +) -> Optional[Tuple[float, float, float, float]]: + """ + Compute bounding box of a symbol from its pin positions. + + Returns (min_x, min_y, max_x, max_y) in mm, or None if no pins found. + """ + pins = locator.get_all_symbol_pins(schematic_path, reference) + if not pins: + return None + xs = [p[0] for p in pins.values()] + ys = [p[1] for p in pins.values()] + return (min(xs), min(ys), max(xs), max(ys)) + + +def _line_segment_intersects_aabb( + x1: float, y1: float, x2: float, y2: float, + box_min_x: float, box_min_y: float, box_max_x: float, box_max_y: float, +) -> bool: + """ + Test whether line segment (x1,y1)→(x2,y2) intersects an axis-aligned bounding box. + + Uses the Liang-Barsky clipping algorithm. + """ + dx = x2 - x1 + dy = y2 - y1 + + p = [-dx, dx, -dy, dy] + q = [x1 - box_min_x, box_max_x - x1, y1 - box_min_y, box_max_y - y1] + + t_min = 0.0 + t_max = 1.0 + + for i in range(4): + if abs(p[i]) < 1e-12: + # Parallel to this edge + if q[i] < 0: + return False + else: + t = q[i] / p[i] + if p[i] < 0: + t_min = max(t_min, t) + else: + t_max = min(t_max, t) + if t_min > t_max: + return False + + return True + + +def _point_in_rect( + px: float, py: float, + min_x: float, min_y: float, max_x: float, max_y: float, +) -> bool: + """Check if a point is within a rectangle.""" + return min_x <= px <= max_x and min_y <= py <= max_y + + +def _distance(p1: Tuple[float, float], p2: Tuple[float, float]) -> float: + """Euclidean distance between two points.""" + return math.sqrt((p1[0] - p2[0]) ** 2 + (p1[1] - p2[1]) ** 2) + + +# --------------------------------------------------------------------------- +# Tool 2: find_unconnected_pins +# --------------------------------------------------------------------------- + +def find_unconnected_pins(schematic_path: Path) -> List[Dict[str, Any]]: + """ + Find all component pins with no wire, label, or power symbol touching them. + + Returns list of dicts: {reference, libId, pinNumber, pinName, position: {x, y}} + """ + sexp_data = _load_sexp(schematic_path) + symbols = _parse_symbols(sexp_data) + wires = _parse_wires(sexp_data) + labels = _parse_labels(sexp_data) + no_connects = _parse_no_connects(sexp_data) + + # Build set of "connected" positions in mm + connected: Set[Tuple[float, float]] = set() + + # Wire endpoints + for w in wires: + connected.add(w["start"]) + connected.add(w["end"]) + + # Label positions + for lbl in labels: + connected.add((lbl["x"], lbl["y"])) + + # Power symbol positions (they implicitly connect) + for sym in symbols: + if sym["is_power"]: + connected.add((sym["x"], sym["y"])) + + tolerance = 0.05 # mm + + def _snap(v: float) -> int: + """Snap coordinate to grid for O(1) set lookup.""" + return round(v / tolerance) + + connected_grid: set = set() + for pos in connected: + connected_grid.add((_snap(pos[0]), _snap(pos[1]))) + + no_connect_grid: set = set() + for pos in no_connects: + no_connect_grid.add((_snap(pos[0]), _snap(pos[1]))) + + def is_connected(px: float, py: float) -> bool: + sx, sy = _snap(px), _snap(py) + # Check the snapped cell and immediate neighbors to handle edge cases + for dx in (-1, 0, 1): + for dy in (-1, 0, 1): + if (sx + dx, sy + dy) in connected_grid: + return True + return False + + def is_no_connect(px: float, py: float) -> bool: + sx, sy = _snap(px), _snap(py) + for dx in (-1, 0, 1): + for dy in (-1, 0, 1): + if (sx + dx, sy + dy) in no_connect_grid: + return True + return False + + locator = PinLocator() + unconnected = [] + + for sym in symbols: + ref = sym["reference"] + # Skip power symbols, templates, and empty references + if sym["is_power"] or ref.startswith("_TEMPLATE") or not ref: + continue + + pin_positions = locator.get_all_symbol_pins(schematic_path, ref) + if not pin_positions: + continue + + pin_defs = None + + for pin_num, pos in pin_positions.items(): + px, py = pos[0], pos[1] + + if is_no_connect(px, py): + continue + if is_connected(px, py): + continue + + if pin_defs is None: + pin_defs = locator.get_symbol_pins(schematic_path, sym["lib_id"]) + + pin_name = pin_defs.get(pin_num, {}).get("name", pin_num) + unconnected.append({ + "reference": ref, + "libId": sym["lib_id"], + "pinNumber": pin_num, + "pinName": pin_name, + "position": {"x": round(px, 4), "y": round(py, 4)}, + }) + + return unconnected + + +# --------------------------------------------------------------------------- +# Tool 3: find_overlapping_elements +# --------------------------------------------------------------------------- + +def find_overlapping_elements( + schematic_path: Path, tolerance: float = 0.5 +) -> Dict[str, Any]: + """ + Detect spatially overlapping symbols, wires, and labels. + + Args: + schematic_path: Path to .kicad_sch file + tolerance: Distance in mm below which elements are considered overlapping + + Returns dict: {overlappingSymbols, overlappingLabels, overlappingWires, totalOverlaps} + """ + sexp_data = _load_sexp(schematic_path) + symbols = _parse_symbols(sexp_data) + wires = _parse_wires(sexp_data) + labels = _parse_labels(sexp_data) + + overlapping_symbols = [] + overlapping_labels = [] + overlapping_wires = [] + + # --- Symbol-symbol overlap (O(n²)) --- + non_template_symbols = [s for s in symbols if not s["reference"].startswith("_TEMPLATE") and s["reference"]] + for i in range(len(non_template_symbols)): + for j in range(i + 1, len(non_template_symbols)): + s1 = non_template_symbols[i] + s2 = non_template_symbols[j] + dist = _distance((s1["x"], s1["y"]), (s2["x"], s2["y"])) + if dist < tolerance: + entry = { + "element1": {"reference": s1["reference"], "libId": s1["lib_id"], + "position": {"x": s1["x"], "y": s1["y"]}}, + "element2": {"reference": s2["reference"], "libId": s2["lib_id"], + "position": {"x": s2["x"], "y": s2["y"]}}, + "distance": round(dist, 4), + } + # Flag power symbol pairs specifically + if s1["is_power"] and s2["is_power"]: + entry["type"] = "power_symbol_overlap" + else: + entry["type"] = "symbol_overlap" + overlapping_symbols.append(entry) + + # --- Label-label overlap --- + for i in range(len(labels)): + for j in range(i + 1, len(labels)): + l1 = labels[i] + l2 = labels[j] + dist = _distance((l1["x"], l1["y"]), (l2["x"], l2["y"])) + if dist < tolerance: + overlapping_labels.append({ + "element1": {"name": l1["name"], "type": l1["type"], + "position": {"x": l1["x"], "y": l1["y"]}}, + "element2": {"name": l2["name"], "type": l2["type"], + "position": {"x": l2["x"], "y": l2["y"]}}, + "distance": round(dist, 4), + }) + + # --- Wire-wire collinear overlap --- + for i in range(len(wires)): + for j in range(i + 1, len(wires)): + w1 = wires[i] + w2 = wires[j] + overlap = _check_wire_overlap(w1, w2, tolerance) + if overlap: + overlapping_wires.append(overlap) + + total = len(overlapping_symbols) + len(overlapping_labels) + len(overlapping_wires) + + return { + "overlappingSymbols": overlapping_symbols, + "overlappingLabels": overlapping_labels, + "overlappingWires": overlapping_wires, + "totalOverlaps": total, + } + + +def _check_wire_overlap( + w1: Dict[str, Any], w2: Dict[str, Any], tolerance: float +) -> Optional[Dict[str, Any]]: + """ + Check if two wire segments are collinear and overlapping. + + Returns overlap info dict or None. + """ + s1, e1 = w1["start"], w1["end"] + s2, e2 = w2["start"], w2["end"] + + # Check horizontal collinearity + if abs(s1[1] - e1[1]) < tolerance and abs(s2[1] - e2[1]) < tolerance: + if abs(s1[1] - s2[1]) < tolerance: + # Both horizontal, same Y + min1, max1 = min(s1[0], e1[0]), max(s1[0], e1[0]) + min2, max2 = min(s2[0], e2[0]), max(s2[0], e2[0]) + if min1 < max2 and min2 < max1: + return { + "wire1": {"start": {"x": s1[0], "y": s1[1]}, "end": {"x": e1[0], "y": e1[1]}}, + "wire2": {"start": {"x": s2[0], "y": s2[1]}, "end": {"x": e2[0], "y": e2[1]}}, + "type": "collinear_overlap", + } + + # Check vertical collinearity + if abs(s1[0] - e1[0]) < tolerance and abs(s2[0] - e2[0]) < tolerance: + if abs(s1[0] - s2[0]) < tolerance: + # Both vertical, same X + min1, max1 = min(s1[1], e1[1]), max(s1[1], e1[1]) + min2, max2 = min(s2[1], e2[1]), max(s2[1], e2[1]) + if min1 < max2 and min2 < max1: + return { + "wire1": {"start": {"x": s1[0], "y": s1[1]}, "end": {"x": e1[0], "y": e1[1]}}, + "wire2": {"start": {"x": s2[0], "y": s2[1]}, "end": {"x": e2[0], "y": e2[1]}}, + "type": "collinear_overlap", + } + + return None + + +# --------------------------------------------------------------------------- +# Tool 4: get_elements_in_region +# --------------------------------------------------------------------------- + +def get_elements_in_region( + schematic_path: Path, + x1: float, y1: float, x2: float, y2: float, +) -> Dict[str, Any]: + """ + List all wires, labels, and symbols within a rectangular region. + + Args: + schematic_path: Path to .kicad_sch file + x1, y1, x2, y2: Bounding box corners in schematic mm + + Returns dict: {symbols, wires, labels, counts} + """ + min_x, max_x = min(x1, x2), max(x1, x2) + min_y, max_y = min(y1, y2), max(y1, y2) + + sexp_data = _load_sexp(schematic_path) + symbols = _parse_symbols(sexp_data) + wires = _parse_wires(sexp_data) + labels = _parse_labels(sexp_data) + + locator = PinLocator() + + # Symbols: include if position is within bounds + region_symbols = [] + for sym in symbols: + if not sym["reference"] or sym["reference"].startswith("_TEMPLATE"): + continue + if _point_in_rect(sym["x"], sym["y"], min_x, min_y, max_x, max_y): + entry = { + "reference": sym["reference"], + "libId": sym["lib_id"], + "position": {"x": sym["x"], "y": sym["y"]}, + "isPower": sym["is_power"], + } + # Include pin positions + pin_positions = locator.get_all_symbol_pins(schematic_path, sym["reference"]) + if pin_positions: + entry["pins"] = { + pn: {"x": round(pos[0], 4), "y": round(pos[1], 4)} + for pn, pos in pin_positions.items() + } + region_symbols.append(entry) + + # Wires: include if ANY endpoint is within bounds + region_wires = [] + for w in wires: + s, e = w["start"], w["end"] + if (_point_in_rect(s[0], s[1], min_x, min_y, max_x, max_y) or + _point_in_rect(e[0], e[1], min_x, min_y, max_x, max_y)): + region_wires.append({ + "start": {"x": s[0], "y": s[1]}, + "end": {"x": e[0], "y": e[1]}, + }) + + # Labels: include if position is within bounds + region_labels = [] + for lbl in labels: + if _point_in_rect(lbl["x"], lbl["y"], min_x, min_y, max_x, max_y): + region_labels.append({ + "name": lbl["name"], + "type": lbl["type"], + "position": {"x": lbl["x"], "y": lbl["y"]}, + }) + + return { + "symbols": region_symbols, + "wires": region_wires, + "labels": region_labels, + "counts": { + "symbols": len(region_symbols), + "wires": len(region_wires), + "labels": len(region_labels), + }, + } + + +# --------------------------------------------------------------------------- +# Tool 5: check_wire_collisions +# --------------------------------------------------------------------------- + +def check_wire_collisions(schematic_path: Path) -> List[Dict[str, Any]]: + """ + Detect wires passing through component bodies without connecting to their pins. + + For each non-power, non-template symbol: + 1. Compute bounding box from pin positions (shrunk by margin). + 2. For each wire segment, test intersection with the bbox. + 3. If intersects but no wire endpoint matches a pin → collision. + + Returns list of collision dicts. + """ + sexp_data = _load_sexp(schematic_path) + symbols = _parse_symbols(sexp_data) + wires = _parse_wires(sexp_data) + + locator = PinLocator() + margin = 0.5 # mm margin to shrink bbox (avoids false positives at pin tips) + pin_tolerance = 0.05 # mm + + collisions = [] + + # Pre-compute per-symbol data + symbol_data = [] + for sym in symbols: + ref = sym["reference"] + if sym["is_power"] or ref.startswith("_TEMPLATE") or not ref: + continue + + bbox = compute_symbol_bbox(schematic_path, ref, locator) + if bbox is None: + continue + + min_x, min_y, max_x, max_y = bbox + + # Expand degenerate dimensions (pins in a line) to approximate body size + min_body = 1.5 # mm minimum half-extent for component body + if max_x - min_x < 2 * min_body: + cx = (min_x + max_x) / 2 + min_x = cx - min_body + max_x = cx + min_body + if max_y - min_y < 2 * min_body: + cy = (min_y + max_y) / 2 + min_y = cy - min_body + max_y = cy + min_body + + # Shrink bbox by margin + min_x += margin + min_y += margin + max_x -= margin + max_y -= margin + + # Skip degenerate bboxes (single-pin or very small after shrink) + if max_x <= min_x or max_y <= min_y: + continue + + pin_positions = locator.get_all_symbol_pins(schematic_path, ref) + pin_set = set() + for pos in pin_positions.values(): + pin_set.add((pos[0], pos[1])) + + symbol_data.append({ + "sym": sym, + "bbox": (min_x, min_y, max_x, max_y), + "pin_set": pin_set, + }) + + # Test each wire against each symbol bbox + for w in wires: + sx, sy = w["start"] + ex, ey = w["end"] + + for sd in symbol_data: + bx1, by1, bx2, by2 = sd["bbox"] + + if not _line_segment_intersects_aabb(sx, sy, ex, ey, bx1, by1, bx2, by2): + continue + + # Check if either wire endpoint matches a pin of this symbol + endpoint_matches_pin = False + for px, py in sd["pin_set"]: + if (abs(sx - px) < pin_tolerance and abs(sy - py) < pin_tolerance): + endpoint_matches_pin = True + break + if (abs(ex - px) < pin_tolerance and abs(ey - py) < pin_tolerance): + endpoint_matches_pin = True + break + + if not endpoint_matches_pin: + sym = sd["sym"] + collisions.append({ + "wire": { + "start": {"x": sx, "y": sy}, + "end": {"x": ex, "y": ey}, + }, + "component": { + "reference": sym["reference"], + "libId": sym["lib_id"], + "position": {"x": sym["x"], "y": sym["y"]}, + }, + "intersectionType": "passes_through", + }) + + return collisions diff --git a/python/kicad_interface.py b/python/kicad_interface.py index e99654b..5166e8d 100644 --- a/python/kicad_interface.py +++ b/python/kicad_interface.py @@ -398,6 +398,12 @@ class KiCADInterface: "delete_schematic_net_label": self._handle_delete_schematic_net_label, "export_schematic_pdf": self._handle_export_schematic_pdf, "export_schematic_svg": self._handle_export_schematic_svg, + # Schematic analysis tools (read-only) + "get_schematic_view_region": self._handle_get_schematic_view_region, + "find_unconnected_pins": self._handle_find_unconnected_pins, + "find_overlapping_elements": self._handle_find_overlapping_elements, + "get_elements_in_region": self._handle_get_elements_in_region, + "check_wire_collisions": self._handle_check_wire_collisions, "import_svg_logo": self._handle_import_svg_logo, # UI/Process management commands "check_kicad_ui": self._handle_check_kicad_ui, @@ -2557,6 +2563,205 @@ class KiCADInterface: logger.error(traceback.format_exc()) return {"success": False, "message": str(e)} + # =================================================================== + # Schematic analysis tools (read-only) + # =================================================================== + + def _handle_get_schematic_view_region(self, params): + """Export a cropped region of the schematic as an image""" + logger.info("Exporting schematic view region") + import subprocess + import tempfile + import os + import base64 + + try: + schematic_path = params.get("schematicPath") + if not schematic_path or not os.path.exists(schematic_path): + return {"success": False, "message": "Schematic file not found"} + + x1 = float(params.get("x1", 0)) + y1 = float(params.get("y1", 0)) + x2 = float(params.get("x2", 297)) + y2 = float(params.get("y2", 210)) + out_format = params.get("format", "png") + width = int(params.get("width", 800)) + height = int(params.get("height", 600)) + + kicad_cli = self.design_rule_commands._find_kicad_cli() + if not kicad_cli: + return {"success": False, "message": "kicad-cli not found"} + + with tempfile.NamedTemporaryFile(suffix=".svg", delete=False) as tmp: + svg_output = tmp.name + + try: + cmd = [kicad_cli, "sch", "export", "svg", "--output", svg_output, schematic_path] + result = subprocess.run(cmd, capture_output=True, text=True, timeout=60) + + if result.returncode != 0: + return {"success": False, "message": f"SVG export failed: {result.stderr}"} + + import xml.etree.ElementTree as ET + tree = ET.parse(svg_output) + root = tree.getroot() + + # Read original viewBox to determine SVG coordinate mapping + ns = {"svg": "http://www.w3.org/2000/svg"} + vb = root.get("viewBox", "") + if vb: + parts = vb.split() + if len(parts) == 4: + # KiCad SVG viewBox is in mils (1 mil = 0.0254 mm) + # or internal units. Detect scale from original viewBox. + orig_vb_x = float(parts[0]) + orig_vb_y = float(parts[1]) + orig_vb_w = float(parts[2]) + orig_vb_h = float(parts[3]) + + # KiCad schematic SVGs use mils (1/1000 inch) as user units + # 1 mm = 39.3701 mils + mils_per_mm = 39.3701 + + new_x = orig_vb_x + x1 * mils_per_mm + new_y = orig_vb_y + y1 * mils_per_mm + new_w = (x2 - x1) * mils_per_mm + new_h = (y2 - y1) * mils_per_mm + + root.set("viewBox", f"{new_x} {new_y} {new_w} {new_h}") + root.set("width", str(width)) + root.set("height", str(height)) + + # Write modified SVG + cropped_svg_path = svg_output + ".cropped.svg" + tree.write(cropped_svg_path, xml_declaration=True, encoding="utf-8") + + if out_format == "svg": + with open(cropped_svg_path, "r", encoding="utf-8") as f: + svg_data = f.read() + return {"success": True, "imageData": svg_data, "format": "svg"} + else: + try: + from cairosvg import svg2png + except ImportError: + return {"success": False, "message": "PNG export requires the 'cairosvg' package. Install it with: pip install cairosvg"} + png_data = svg2png(url=cropped_svg_path, output_width=width, output_height=height) + return { + "success": True, + "imageData": base64.b64encode(png_data).decode("utf-8"), + "format": "png", + } + finally: + for f in [svg_output, svg_output + ".cropped.svg"]: + if os.path.exists(f): + os.unlink(f) + + except Exception as e: + logger.error(f"Error in get_schematic_view_region: {e}") + import traceback + logger.error(traceback.format_exc()) + return {"success": False, "message": str(e)} + + def _handle_find_unconnected_pins(self, params): + """Find all component pins with no wire, label, or power symbol touching them""" + logger.info("Finding unconnected pins in schematic") + try: + from pathlib import Path + 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(Path(schematic_path)) + return { + "success": True, + "unconnectedPins": result, + "count": len(result), + "message": f"Found {len(result)} unconnected pin(s)", + } + except Exception as e: + logger.error(f"Error finding unconnected pins: {e}") + import traceback + logger.error(traceback.format_exc()) + return {"success": False, "message": str(e)} + + def _handle_find_overlapping_elements(self, params): + """Detect spatially overlapping symbols, wires, and labels""" + logger.info("Finding overlapping elements in schematic") + try: + from pathlib import Path + from commands.schematic_analysis import find_overlapping_elements + + schematic_path = params.get("schematicPath") + if not schematic_path: + return {"success": False, "message": "schematicPath is required"} + + tolerance = float(params.get("tolerance", 0.5)) + result = find_overlapping_elements(Path(schematic_path), tolerance) + return { + "success": True, + **result, + "message": f"Found {result['totalOverlaps']} overlap(s)", + } + except Exception as e: + logger.error(f"Error finding overlapping elements: {e}") + import traceback + logger.error(traceback.format_exc()) + return {"success": False, "message": str(e)} + + def _handle_get_elements_in_region(self, params): + """List all wires, labels, and symbols within a rectangular region""" + logger.info("Getting elements in schematic region") + try: + from pathlib import Path + from commands.schematic_analysis import get_elements_in_region + + schematic_path = params.get("schematicPath") + if not schematic_path: + return {"success": False, "message": "schematicPath is required"} + + x1 = float(params.get("x1", 0)) + y1 = float(params.get("y1", 0)) + x2 = float(params.get("x2", 0)) + y2 = float(params.get("y2", 0)) + + result = get_elements_in_region(Path(schematic_path), x1, y1, x2, y2) + return { + "success": True, + **result, + "message": f"Found {result['counts']['symbols']} symbols, {result['counts']['wires']} wires, {result['counts']['labels']} labels in region", + } + except Exception as e: + logger.error(f"Error getting elements in region: {e}") + import traceback + logger.error(traceback.format_exc()) + return {"success": False, "message": str(e)} + + def _handle_check_wire_collisions(self, params): + """Detect wires passing through component bodies without connecting to their pins""" + logger.info("Checking wire collisions in schematic") + try: + from pathlib import Path + 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(Path(schematic_path)) + return { + "success": True, + "collisions": result, + "count": len(result), + "message": f"Found {len(result)} wire collision(s)", + } + except Exception as e: + logger.error(f"Error checking wire collisions: {e}") + import traceback + logger.error(traceback.format_exc()) + return {"success": False, "message": str(e)} + def _handle_import_svg_logo(self, params): """Import an SVG file as PCB graphic polygons on the silkscreen""" logger.info("Importing SVG logo into PCB") diff --git a/python/schemas/tool_schemas.py b/python/schemas/tool_schemas.py index dffe3d7..4e6290c 100644 --- a/python/schemas/tool_schemas.py +++ b/python/schemas/tool_schemas.py @@ -1680,6 +1680,131 @@ SCHEMATIC_TOOLS = [ }, "required": ["schematicPath", "outputPath"] } + }, + # --- Schematic Analysis Tools (read-only) --- + { + "name": "get_schematic_view_region", + "title": "Get Schematic View Region", + "description": "Exports a cropped region of the schematic as an image (PNG or SVG). Specify a bounding box in schematic mm coordinates to zoom into a specific area.", + "inputSchema": { + "type": "object", + "properties": { + "schematicPath": { + "type": "string", + "description": "Path to the .kicad_sch schematic file" + }, + "x1": { + "type": "number", + "description": "Left X coordinate of the region in mm" + }, + "y1": { + "type": "number", + "description": "Top Y coordinate of the region in mm" + }, + "x2": { + "type": "number", + "description": "Right X coordinate of the region in mm" + }, + "y2": { + "type": "number", + "description": "Bottom Y coordinate of the region in mm" + }, + "format": { + "type": "string", + "enum": ["png", "svg"], + "description": "Output image format (default: png)" + }, + "width": { + "type": "integer", + "description": "Output image width in pixels (default: 800)" + }, + "height": { + "type": "integer", + "description": "Output image height in pixels (default: 600)" + } + }, + "required": ["schematicPath", "x1", "y1", "x2", "y2"] + } + }, + { + "name": "find_unconnected_pins", + "title": "Find Unconnected Pins", + "description": "Lists all component pins in the schematic that have no wire, label, or power symbol touching them. Useful for checking connectivity before running ERC. Skips power symbols, template symbols, and pins with no_connect flags.", + "inputSchema": { + "type": "object", + "properties": { + "schematicPath": { + "type": "string", + "description": "Path to the .kicad_sch schematic file" + } + }, + "required": ["schematicPath"] + } + }, + { + "name": "find_overlapping_elements", + "title": "Find Overlapping Elements", + "description": "Detects spatially overlapping symbols, wires, and labels in the schematic. Finds: duplicate power symbols at the same position, collinear overlapping wire segments, and labels stacked on top of each other.", + "inputSchema": { + "type": "object", + "properties": { + "schematicPath": { + "type": "string", + "description": "Path to the .kicad_sch schematic file" + }, + "tolerance": { + "type": "number", + "description": "Distance in mm below which elements are considered overlapping (default: 0.5)" + } + }, + "required": ["schematicPath"] + } + }, + { + "name": "get_elements_in_region", + "title": "Get Elements in Region", + "description": "Lists all symbols, wires, and labels within a rectangular region of the schematic. Useful for understanding what is in a specific area before modifying it.", + "inputSchema": { + "type": "object", + "properties": { + "schematicPath": { + "type": "string", + "description": "Path to the .kicad_sch schematic file" + }, + "x1": { + "type": "number", + "description": "Left X coordinate of the region in mm" + }, + "y1": { + "type": "number", + "description": "Top Y coordinate of the region in mm" + }, + "x2": { + "type": "number", + "description": "Right X coordinate of the region in mm" + }, + "y2": { + "type": "number", + "description": "Bottom Y coordinate of the region in mm" + } + }, + "required": ["schematicPath", "x1", "y1", "x2", "y2"] + } + }, + { + "name": "check_wire_collisions", + "title": "Check Wire Collisions", + "description": "Detects wires that pass through component bodies without connecting to their pins. These are usually routing mistakes where a wire crosses over a symbol instead of connecting to it. Uses pin-based bounding boxes (approximate but effective for 80/20 detection).", + "inputSchema": { + "type": "object", + "properties": { + "schematicPath": { + "type": "string", + "description": "Path to the .kicad_sch schematic file" + } + }, + "required": ["schematicPath"] + } } ] diff --git a/python/tests/__init__.py b/python/tests/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/python/tests/conftest.py b/python/tests/conftest.py index a6c8380..81d3b93 100644 --- a/python/tests/conftest.py +++ b/python/tests/conftest.py @@ -1,26 +1,41 @@ -""" -Pytest configuration for python/tests. - -Sets up sys.path so that the python/ package root is importable without -installing the project, and provides shared fixtures. -""" -import sys -from pathlib import Path - -# Make the python/ package root importable -PYTHON_ROOT = Path(__file__).parent.parent -if str(PYTHON_ROOT) not in sys.path: - sys.path.insert(0, str(PYTHON_ROOT)) - -# Stub out heavy KiCAD C-extension modules so tests can run without a real -# KiCAD installation. Extend this list whenever a new import fails. -import types -from unittest.mock import MagicMock - -# Use MagicMock so any attribute access (e.g. pcbnew.BOARD, pcbnew.LoadBoard) -# returns another MagicMock rather than raising AttributeError. -for _stub_name in ("pcbnew", "skip"): - if _stub_name not in sys.modules: - _m = MagicMock(spec_set=None) - _m.__name__ = _stub_name - sys.modules[_stub_name] = _m +""" +Test configuration for python/tests. + +Sets up sys.modules stubs for heavy KiCAD modules (pcbnew, skip) before any +test module can trigger their import, preventing crashes on systems where the +real KiCAD environment is not fully initialised for testing. +""" + +import sys +import types +from unittest.mock import MagicMock + +# --------------------------------------------------------------------------- +# pcbnew stub — kicad_interface.py accesses pcbnew.__file__ and +# pcbnew.GetBuildVersion() at module level. Use MagicMock so that any +# attribute access (pcbnew.BOARD, pcbnew.PCB_TRACK, …) returns a mock +# rather than raising AttributeError. +# --------------------------------------------------------------------------- +_pcbnew = MagicMock(name="pcbnew") +_pcbnew.__file__ = "/fake/pcbnew.cpython-313-x86_64-linux-gnu.so" +_pcbnew.__name__ = "pcbnew" +_pcbnew.__spec__ = None +_pcbnew.GetBuildVersion.return_value = "9.0.0-stub" +sys.modules["pcbnew"] = _pcbnew + +# --------------------------------------------------------------------------- +# Stub: skip (kicad-skip — use real module if available, stub otherwise) +# --------------------------------------------------------------------------- +try: + import skip as _skip_test # noqa: F401 — try importing real skip +except ImportError: + skip_mod = types.ModuleType("skip") + + class _FakeSchematic: + """Minimal stand-in for skip.Schematic used in PinLocator cache.""" + def __init__(self, path: str): + self.path = path + self.symbol = [] + + skip_mod.Schematic = _FakeSchematic # type: ignore[attr-defined] + sys.modules["skip"] = skip_mod diff --git a/python/tests/test_schematic_analysis.py b/python/tests/test_schematic_analysis.py new file mode 100644 index 0000000..95d89e7 --- /dev/null +++ b/python/tests/test_schematic_analysis.py @@ -0,0 +1,398 @@ +""" +Tests for schematic analysis tools (Tools 2–5). + +Unit tests use mock data / synthetic S-expressions. +Integration tests parse real .kicad_sch files via sexpdata. +""" + +import os +import sys +import shutil +import tempfile +from pathlib import Path +from unittest.mock import patch, MagicMock + +import pytest +import sexpdata +from sexpdata import Symbol + +# Ensure the python/ package is importable +sys.path.insert(0, str(Path(__file__).resolve().parent.parent)) + +from commands.schematic_analysis import ( + _parse_wires, + _parse_labels, + _parse_symbols, + _parse_no_connects, + _load_sexp, + _line_segment_intersects_aabb, + _point_in_rect, + _distance, + compute_symbol_bbox, + find_unconnected_pins, + find_overlapping_elements, + get_elements_in_region, + check_wire_collisions, +) + + +# --------------------------------------------------------------------------- +# Helpers +# --------------------------------------------------------------------------- + +TEMPLATE_PATH = Path(__file__).resolve().parent.parent / "templates" / "empty.kicad_sch" + + +def _make_temp_schematic(extra_sexp: str = "") -> Path: + """Copy empty.kicad_sch to a temp file and optionally append S-expression content.""" + tmp = Path(tempfile.mkdtemp()) / "test.kicad_sch" + shutil.copy(TEMPLATE_PATH, tmp) + if extra_sexp: + content = tmp.read_text(encoding="utf-8") + # Insert before the final closing paren + idx = content.rfind(")") + content = content[:idx] + "\n" + extra_sexp + "\n)" + tmp.write_text(content, encoding="utf-8") + return tmp + + +import uuid as _uuid + + +def _make_resistor_sexp(ref: str, x: float, y: float, rotation: float = 0) -> str: + """Generate a proper Device:R symbol S-expression that skip can parse.""" + u = str(_uuid.uuid4()) + return 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)) + ) + ) + ) +""" + + +def _make_led_sexp(ref: str, x: float, y: float, rotation: float = 0) -> str: + """Generate a proper Device:LED symbol S-expression (horizontal pin spread).""" + u = str(_uuid.uuid4()) + return f""" + (symbol (lib_id "Device:LED") (at {x} {y} {rotation}) (unit 1) + (in_bom yes) (on_board yes) (dnp no) + (uuid "{u}") + (property "Reference" "{ref}" (at {x} {y - 2.54} 0) + (effects (font (size 1.27 1.27))) + ) + (property "Value" "LED" (at {x} {y + 2.54} 0) + (effects (font (size 1.27 1.27))) + ) + (property "Footprint" "" (at {x} {y} 0) + (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)) + ) + ) + ) +""" + + +# =================================================================== +# Unit tests — geometry helpers +# =================================================================== + +class TestGeometryHelpers: + """Test low-level geometry utilities.""" + + def test_point_in_rect_inside(self): + assert _point_in_rect(5, 5, 0, 0, 10, 10) is True + + def test_point_in_rect_outside(self): + assert _point_in_rect(15, 5, 0, 0, 10, 10) is False + + def test_point_in_rect_boundary(self): + assert _point_in_rect(0, 0, 0, 0, 10, 10) is True + + def test_distance_zero(self): + assert _distance((0, 0), (0, 0)) == 0 + + def test_distance_unit(self): + assert abs(_distance((0, 0), (3, 4)) - 5.0) < 1e-9 + + def test_aabb_intersection_crossing(self): + # Line from (0,5) to (10,5) should intersect box (2,2)-(8,8) + assert _line_segment_intersects_aabb(0, 5, 10, 5, 2, 2, 8, 8) is True + + def test_aabb_intersection_miss(self): + # Line from (0,0) to (10,0) should miss box (2,2)-(8,8) + assert _line_segment_intersects_aabb(0, 0, 10, 0, 2, 2, 8, 8) is False + + def test_aabb_intersection_inside(self): + # Line entirely inside the box + assert _line_segment_intersects_aabb(3, 3, 7, 7, 2, 2, 8, 8) is True + + def test_aabb_intersection_diagonal(self): + # Diagonal line crossing through box + assert _line_segment_intersects_aabb(0, 0, 10, 10, 2, 2, 8, 8) is True + + def test_aabb_intersection_parallel_outside(self): + # Horizontal line above the box + assert _line_segment_intersects_aabb(0, 9, 10, 9, 2, 2, 8, 8) is False + + def test_aabb_intersection_touching_edge(self): + # Line ending exactly at box edge + assert _line_segment_intersects_aabb(0, 2, 2, 2, 2, 2, 8, 8) is True + + +# =================================================================== +# Unit tests — S-expression parsers +# =================================================================== + +class TestSexpParsers: + """Test S-expression parsing functions with synthetic data.""" + + def test_parse_wires_basic(self): + sexp = sexpdata.loads("""(kicad_sch + (wire (pts (xy 10 20) (xy 30 40)) + (stroke (width 0) (type default)) + (uuid "abc")) + )""") + wires = _parse_wires(sexp) + assert len(wires) == 1 + assert wires[0]["start"] == (10.0, 20.0) + assert wires[0]["end"] == (30.0, 40.0) + + def test_parse_wires_empty(self): + sexp = sexpdata.loads("(kicad_sch)") + assert _parse_wires(sexp) == [] + + def test_parse_labels_both_types(self): + sexp = sexpdata.loads("""(kicad_sch + (label "VCC" (at 10 20 0)) + (global_label "GND" (at 30 40 0)) + )""") + labels = _parse_labels(sexp) + assert len(labels) == 2 + assert labels[0]["name"] == "VCC" + assert labels[0]["type"] == "label" + assert labels[1]["name"] == "GND" + assert labels[1]["type"] == "global_label" + + def test_parse_symbols(self): + sexp = sexpdata.loads("""(kicad_sch + (symbol (lib_id "Device:R") (at 100 100 0) + (property "Reference" "R1" (at 0 0 0))) + (symbol (lib_id "power:VCC") (at 50 50 0) + (property "Reference" "#PWR01" (at 0 0 0))) + )""") + symbols = _parse_symbols(sexp) + assert len(symbols) == 2 + assert symbols[0]["reference"] == "R1" + assert symbols[0]["is_power"] is False + assert symbols[1]["reference"] == "#PWR01" + assert symbols[1]["is_power"] is True + + def test_parse_no_connects(self): + sexp = sexpdata.loads("""(kicad_sch + (no_connect (at 10 20) (uuid "x")) + (no_connect (at 30 40) (uuid "y")) + )""") + nc = _parse_no_connects(sexp) + assert (10.0, 20.0) in nc + assert (30.0, 40.0) in nc + assert len(nc) == 2 + + +# =================================================================== +# Unit tests — analysis functions with mocked PinLocator +# =================================================================== + +class TestFindOverlappingElements: + """Test overlapping detection logic.""" + + def test_no_overlaps_in_empty_schematic(self): + tmp = _make_temp_schematic() + result = find_overlapping_elements(tmp, tolerance=0.5) + assert result["totalOverlaps"] == 0 + + def test_overlapping_symbols_detected(self): + # Two symbols at nearly the same position + extra = """ + (symbol (lib_id "Device:R") (at 100 100 0) + (property "Reference" "R1" (at 0 0 0)) + (property "Value" "10k" (at 0 0 0))) + (symbol (lib_id "Device:R") (at 100.1 100 0) + (property "Reference" "R2" (at 0 0 0)) + (property "Value" "10k" (at 0 0 0))) + """ + tmp = _make_temp_schematic(extra) + result = find_overlapping_elements(tmp, tolerance=0.5) + assert result["totalOverlaps"] >= 1 + assert len(result["overlappingSymbols"]) >= 1 + + def test_well_separated_symbols_not_flagged(self): + extra = """ + (symbol (lib_id "Device:R") (at 100 100 0) + (property "Reference" "R1" (at 0 0 0)) + (property "Value" "10k" (at 0 0 0))) + (symbol (lib_id "Device:R") (at 200 200 0) + (property "Reference" "R2" (at 0 0 0)) + (property "Value" "10k" (at 0 0 0))) + """ + tmp = _make_temp_schematic(extra) + result = find_overlapping_elements(tmp, tolerance=0.5) + assert result["totalOverlaps"] == 0 + + def test_collinear_wire_overlap(self): + extra = """ + (wire (pts (xy 10 50) (xy 30 50)) + (stroke (width 0) (type default)) + (uuid "w1")) + (wire (pts (xy 20 50) (xy 40 50)) + (stroke (width 0) (type default)) + (uuid "w2")) + """ + tmp = _make_temp_schematic(extra) + result = find_overlapping_elements(tmp, tolerance=0.5) + assert len(result["overlappingWires"]) >= 1 + + +class TestGetElementsInRegion: + """Test region query logic.""" + + def test_elements_inside_region_found(self): + extra = """ + (symbol (lib_id "Device:R") (at 50 50 0) + (property "Reference" "R1" (at 0 0 0)) + (property "Value" "10k" (at 0 0 0))) + (wire (pts (xy 45 50) (xy 55 50)) + (stroke (width 0) (type default)) + (uuid "w1")) + (label "NET1" (at 50 50 0)) + """ + tmp = _make_temp_schematic(extra) + result = get_elements_in_region(tmp, 40, 40, 60, 60) + assert result["counts"]["symbols"] >= 1 + assert result["counts"]["wires"] >= 1 + assert result["counts"]["labels"] >= 1 + + def test_elements_outside_region_excluded(self): + extra = """ + (symbol (lib_id "Device:R") (at 200 200 0) + (property "Reference" "R1" (at 0 0 0)) + (property "Value" "10k" (at 0 0 0))) + """ + tmp = _make_temp_schematic(extra) + result = get_elements_in_region(tmp, 0, 0, 50, 50) + assert result["counts"]["symbols"] == 0 + + +class TestComputeSymbolBbox: + """Test bounding box computation.""" + + def test_returns_none_for_unknown_symbol(self): + tmp = _make_temp_schematic() + from commands.pin_locator import PinLocator + locator = PinLocator() + result = compute_symbol_bbox(tmp, "NONEXISTENT", locator) + assert result is None + + +# =================================================================== +# Integration tests — full schematic parsing +# =================================================================== + +@pytest.mark.integration +class TestIntegrationFindUnconnectedPins: + """Integration test using real schematic files.""" + + def test_component_with_no_wires_has_unconnected_pins(self): + """A resistor placed with no wires should have 2 unconnected pins.""" + extra = _make_resistor_sexp("R1", 100, 100) + tmp = _make_temp_schematic(extra) + result = find_unconnected_pins(tmp) + r1_pins = [p for p in result if p["reference"] == "R1"] + assert len(r1_pins) == 2 + + def test_pin_with_wire_is_connected(self): + """A wire endpoint exactly at a pin position should mark it connected.""" + # R1 at (100,100), rotation 0 → pin 1 at (100, 103.81), pin 2 at (100, 96.19) + extra = _make_resistor_sexp("R1", 100, 100) + """ + (wire (pts (xy 100 103.81) (xy 100 120)) + (stroke (width 0) (type default)) + (uuid "w1")) + """ + tmp = _make_temp_schematic(extra) + result = find_unconnected_pins(tmp) + r1_pins = [p for p in result if p["reference"] == "R1"] + # Pin 1 should be connected (wire at 100, 103.81), pin 2 still unconnected + assert len(r1_pins) == 1 + assert r1_pins[0]["pinNumber"] == "2" + + def test_no_connect_suppresses_pin(self): + """A no_connect at a pin position should not report it as unconnected.""" + extra = _make_resistor_sexp("R1", 100, 100) + """ + (no_connect (at 100 96.19) (uuid "nc1")) + (no_connect (at 100 103.81) (uuid "nc2")) + """ + tmp = _make_temp_schematic(extra) + result = find_unconnected_pins(tmp) + r1_pins = [p for p in result if p["reference"] == "R1"] + assert len(r1_pins) == 0 + + +@pytest.mark.integration +class TestIntegrationCheckWireCollisions: + """Integration test for wire collision detection.""" + + def test_wire_not_touching_pins_is_collision(self): + """A wire passing through a component bbox without pin contact → collision.""" + # LED D1 at (100,100) → pin 1 at (96.19, 100), pin 2 at (103.81, 100) + # Vertical wire from (100, 95) to (100, 105) crosses through the body + # without touching either horizontal pin + extra = _make_led_sexp("D1", 100, 100) + """ + (wire (pts (xy 100 95) (xy 100 105)) + (stroke (width 0) (type default)) + (uuid "w1")) + """ + tmp = _make_temp_schematic(extra) + result = check_wire_collisions(tmp) + d1_collisions = [c for c in result if c["component"]["reference"] == "D1"] + assert len(d1_collisions) >= 1 + + +@pytest.mark.integration +class TestIntegrationGetElementsInRegion: + """Integration test for region query.""" + + def test_region_returns_pin_data(self): + """Symbols in region should include pin position data.""" + extra = _make_resistor_sexp("R1", 100, 100) + tmp = _make_temp_schematic(extra) + result = get_elements_in_region(tmp, 90, 90, 110, 110) + assert result["counts"]["symbols"] == 1 + sym = result["symbols"][0] + assert "pins" in sym + assert len(sym["pins"]) == 2 # Resistor has 2 pins diff --git a/src/tools/schematic.ts b/src/tools/schematic.ts index 92c3d4a..28f5b7e 100644 --- a/src/tools/schematic.ts +++ b/src/tools/schematic.ts @@ -1092,4 +1092,187 @@ Note: operates on .kicad_sch files only. To modify a PCB footprint use edit_comp }; }, ); + + // ============================================================ + // Schematic Analysis Tools (read-only) + // ============================================================ + + // Get a zoomed view of a schematic region + server.tool( + "get_schematic_view_region", + "Export a cropped region of the schematic as an image (PNG or SVG). Specify bounding box coordinates in schematic mm. Useful for zooming into a specific area to inspect wiring or layout.", + { + schematicPath: z.string().describe("Path to the .kicad_sch schematic file"), + x1: z.number().describe("Left X coordinate of the region in mm"), + y1: z.number().describe("Top Y coordinate of the region in mm"), + x2: z.number().describe("Right X coordinate of the region in mm"), + y2: z.number().describe("Bottom Y coordinate of the region in mm"), + format: z.enum(["png", "svg"]).optional().describe("Output image format (default: png)"), + width: z.number().optional().describe("Output image width in pixels (default: 800)"), + height: z.number().optional().describe("Output image height in pixels (default: 600)"), + }, + async (args: { + schematicPath: string; + x1: number; y1: number; x2: number; y2: number; + format?: string; width?: number; height?: number; + }) => { + const result = await callKicadScript("get_schematic_view_region", args); + if (result.success && result.imageData) { + if (result.format === "svg") { + return { content: [{ type: "text", text: result.imageData }] }; + } + return { + content: [{ + type: "image", + data: result.imageData, + mimeType: "image/png", + }], + }; + } + return { + content: [{ type: "text", text: `Failed: ${result.message || "Unknown error"}` }], + }; + }, + ); + + // Find unconnected pins + server.tool( + "find_unconnected_pins", + "List all component pins in the schematic that have no wire, label, or power symbol touching them. Useful for checking connectivity before running ERC.", + { + schematicPath: z.string().describe("Path to the .kicad_sch schematic file"), + }, + async (args: { schematicPath: string }) => { + const result = await callKicadScript("find_unconnected_pins", args); + if (result.success) { + const pins: any[] = result.unconnectedPins || []; + const lines = [`Found ${pins.length} unconnected pin(s):`]; + pins.slice(0, 50).forEach((p: any) => { + lines.push(` ${p.reference} pin ${p.pinNumber} (${p.pinName}) @ (${p.position.x}, ${p.position.y})`); + }); + if (pins.length > 50) lines.push(` ... and ${pins.length - 50} more`); + return { content: [{ type: "text", text: lines.join("\n") }] }; + } + return { + content: [{ type: "text", text: `Failed: ${result.message || "Unknown error"}` }], + }; + }, + ); + + // Find overlapping elements + server.tool( + "find_overlapping_elements", + "Detect spatially overlapping symbols, wires, and labels in the schematic. Finds duplicate power symbols at the same position, collinear overlapping wires, and labels stacked on top of each other.", + { + schematicPath: z.string().describe("Path to the .kicad_sch schematic file"), + tolerance: z.number().optional().describe("Distance in mm below which elements are considered overlapping (default: 0.5)"), + }, + async (args: { schematicPath: string; tolerance?: number }) => { + const result = await callKicadScript("find_overlapping_elements", args); + if (result.success) { + const lines = [`Found ${result.totalOverlaps} overlap(s):`]; + const syms: any[] = result.overlappingSymbols || []; + const lbls: any[] = result.overlappingLabels || []; + const wires: any[] = result.overlappingWires || []; + if (syms.length) { + lines.push(`\nOverlapping symbols (${syms.length}):`); + syms.slice(0, 20).forEach((o: any) => { + lines.push(` ${o.element1.reference} ↔ ${o.element2.reference} (${o.distance}mm) [${o.type}]`); + }); + } + if (lbls.length) { + lines.push(`\nOverlapping labels (${lbls.length}):`); + lbls.slice(0, 20).forEach((o: any) => { + lines.push(` "${o.element1.name}" ↔ "${o.element2.name}" (${o.distance}mm)`); + }); + } + if (wires.length) { + lines.push(`\nOverlapping wires (${wires.length}):`); + wires.slice(0, 20).forEach((o: any) => { + lines.push(` wire @ (${o.wire1.start.x},${o.wire1.start.y})→(${o.wire1.end.x},${o.wire1.end.y}) overlaps with another`); + }); + } + return { content: [{ type: "text", text: lines.join("\n") }] }; + } + return { + content: [{ type: "text", text: `Failed: ${result.message || "Unknown error"}` }], + }; + }, + ); + + // Get elements in a region + server.tool( + "get_elements_in_region", + "List all symbols, wires, and labels within a rectangular region of the schematic. Useful for understanding what is in a specific area before modifying it.", + { + schematicPath: z.string().describe("Path to the .kicad_sch schematic file"), + x1: z.number().describe("Left X coordinate of the region in mm"), + y1: z.number().describe("Top Y coordinate of the region in mm"), + x2: z.number().describe("Right X coordinate of the region in mm"), + y2: z.number().describe("Bottom Y coordinate of the region in mm"), + }, + async (args: { + schematicPath: string; + x1: number; y1: number; x2: number; y2: number; + }) => { + const result = await callKicadScript("get_elements_in_region", args); + if (result.success) { + const c = result.counts; + const lines = [`Region (${args.x1},${args.y1})→(${args.x2},${args.y2}): ${c.symbols} symbols, ${c.wires} wires, ${c.labels} labels`]; + const syms: any[] = result.symbols || []; + if (syms.length) { + lines.push("\nSymbols:"); + syms.forEach((s: any) => { + const pinCount = s.pins ? Object.keys(s.pins).length : 0; + lines.push(` ${s.reference} (${s.libId}) @ (${s.position.x}, ${s.position.y}) [${pinCount} pins]`); + }); + } + const wires: any[] = result.wires || []; + if (wires.length) { + lines.push(`\nWires (${wires.length}):`); + wires.slice(0, 30).forEach((w: any) => { + lines.push(` (${w.start.x},${w.start.y}) → (${w.end.x},${w.end.y})`); + }); + if (wires.length > 30) lines.push(` ... and ${wires.length - 30} more`); + } + const labels: any[] = result.labels || []; + if (labels.length) { + lines.push(`\nLabels (${labels.length}):`); + labels.forEach((l: any) => { + lines.push(` "${l.name}" [${l.type}] @ (${l.position.x}, ${l.position.y})`); + }); + } + return { content: [{ type: "text", text: lines.join("\n") }] }; + } + return { + content: [{ type: "text", text: `Failed: ${result.message || "Unknown error"}` }], + }; + }, + ); + + // Check wire collisions + server.tool( + "check_wire_collisions", + "Detect wires that pass through component bodies without connecting to their pins. These are usually routing mistakes where a wire crosses over a symbol instead of connecting to it.", + { + schematicPath: z.string().describe("Path to the .kicad_sch schematic file"), + }, + async (args: { schematicPath: string }) => { + const result = await callKicadScript("check_wire_collisions", args); + if (result.success) { + const collisions: any[] = result.collisions || []; + const lines = [`Found ${collisions.length} wire collision(s):`]; + collisions.slice(0, 30).forEach((c: any, i: number) => { + lines.push( + ` ${i + 1}. Wire (${c.wire.start.x},${c.wire.start.y})→(${c.wire.end.x},${c.wire.end.y}) passes through ${c.component.reference} (${c.component.libId})` + ); + }); + if (collisions.length > 30) lines.push(` ... and ${collisions.length - 30} more`); + return { content: [{ type: "text", text: lines.join("\n") }] }; + } + return { + content: [{ type: "text", text: `Failed: ${result.message || "Unknown error"}` }], + }; + }, + ); } From 9141e33b70f35550f0186dd935c99a9454742962 Mon Sep 17 00:00:00 2001 From: Eugene Mikhantyev Date: Sat, 14 Mar 2026 20:55:14 +0000 Subject: [PATCH 02/13] fix: get_schematic_view_region SVG export and coordinate system - Use a temp directory (not file) for kicad-cli svg output, which expects a directory path - Clean up temp dir with shutil.rmtree instead of individual file unlinks - Fix viewBox cropping: KiCad schematic SVGs use mm directly, not mils (removed erroneous 39.3701 multiplier) Co-Authored-By: Claude Sonnet 4.6 --- python/kicad_interface.py | 38 +++++++++++++++++--------------------- 1 file changed, 17 insertions(+), 21 deletions(-) diff --git a/python/kicad_interface.py b/python/kicad_interface.py index 5166e8d..723b00c 100644 --- a/python/kicad_interface.py +++ b/python/kicad_interface.py @@ -2592,48 +2592,45 @@ class KiCADInterface: if not kicad_cli: return {"success": False, "message": "kicad-cli not found"} - with tempfile.NamedTemporaryFile(suffix=".svg", delete=False) as tmp: - svg_output = tmp.name + tmp_dir = tempfile.mkdtemp() + svg_output = None try: - cmd = [kicad_cli, "sch", "export", "svg", "--output", svg_output, schematic_path] + cmd = [kicad_cli, "sch", "export", "svg", "--output", tmp_dir, schematic_path] result = subprocess.run(cmd, capture_output=True, text=True, timeout=60) if result.returncode != 0: return {"success": False, "message": f"SVG export failed: {result.stderr}"} + # kicad-cli names the file after the schematic + svg_files = [f for f in os.listdir(tmp_dir) if f.endswith(".svg")] + if not svg_files: + return {"success": False, "message": "kicad-cli produced no SVG output"} + svg_output = os.path.join(tmp_dir, svg_files[0]) + import xml.etree.ElementTree as ET tree = ET.parse(svg_output) root = tree.getroot() - # Read original viewBox to determine SVG coordinate mapping - ns = {"svg": "http://www.w3.org/2000/svg"} + # KiCad schematic SVGs use mm as viewBox units directly vb = root.get("viewBox", "") if vb: parts = vb.split() if len(parts) == 4: - # KiCad SVG viewBox is in mils (1 mil = 0.0254 mm) - # or internal units. Detect scale from original viewBox. orig_vb_x = float(parts[0]) orig_vb_y = float(parts[1]) - orig_vb_w = float(parts[2]) - orig_vb_h = float(parts[3]) - # KiCad schematic SVGs use mils (1/1000 inch) as user units - # 1 mm = 39.3701 mils - mils_per_mm = 39.3701 - - new_x = orig_vb_x + x1 * mils_per_mm - new_y = orig_vb_y + y1 * mils_per_mm - new_w = (x2 - x1) * mils_per_mm - new_h = (y2 - y1) * mils_per_mm + new_x = orig_vb_x + x1 + new_y = orig_vb_y + y1 + new_w = x2 - x1 + new_h = y2 - y1 root.set("viewBox", f"{new_x} {new_y} {new_w} {new_h}") root.set("width", str(width)) root.set("height", str(height)) # Write modified SVG - cropped_svg_path = svg_output + ".cropped.svg" + cropped_svg_path = os.path.join(tmp_dir, "cropped.svg") tree.write(cropped_svg_path, xml_declaration=True, encoding="utf-8") if out_format == "svg": @@ -2652,9 +2649,8 @@ class KiCADInterface: "format": "png", } finally: - for f in [svg_output, svg_output + ".cropped.svg"]: - if os.path.exists(f): - os.unlink(f) + import shutil + shutil.rmtree(tmp_dir, ignore_errors=True) except Exception as e: logger.error(f"Error in get_schematic_view_region: {e}") From 53564cbc58dd54f43dbb4a0f5af744c33d1576ff Mon Sep 17 00:00:00 2001 From: Eugene Mikhantyev Date: Sun, 15 Mar 2026 10:55:11 +0000 Subject: [PATCH 03/13] fix: check_wire_collisions reports false positives for unannotated components MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PinLocator.get_all_symbol_pins resolves symbols by reference designator, so when multiple components share the same unannotated reference (e.g. "Q?"), it always returned the first match's pin positions. Every duplicate then got an identical bounding box, causing a single wire to be flagged against all N instances instead of only the ones it actually crosses. Fix: add _compute_pin_positions_direct() that computes absolute pin positions directly from each symbol's own (at x y rotation) and (mirror ...) data plus pin definitions fetched by lib_id — no reference-name lookup involved. Also extend _parse_symbols to capture mirror_x/mirror_y flags. Add regression test: two "R?" at different positions, wire crossing only one → must produce 0 collisions against the far-away component. Co-Authored-By: Claude Sonnet 4.6 --- python/commands/schematic_analysis.py | 68 +++++++++++++++++++++++-- python/tests/test_schematic_analysis.py | 33 ++++++++++++ 2 files changed, 96 insertions(+), 5 deletions(-) diff --git a/python/commands/schematic_analysis.py b/python/commands/schematic_analysis.py index 8fa58e5..9d91730 100644 --- a/python/commands/schematic_analysis.py +++ b/python/commands/schematic_analysis.py @@ -85,7 +85,7 @@ def _parse_symbols(sexp_data: list) -> List[Dict[str, Any]]: """ Parse all placed symbol instances from the schematic S-expression. - Returns list of dicts: {reference, lib_id, x, y, rotation, is_power} + Returns list of dicts: {reference, lib_id, x, y, rotation, mirror_x, mirror_y, is_power} """ symbols = [] for item in sexp_data: @@ -98,6 +98,8 @@ def _parse_symbols(sexp_data: list) -> List[Dict[str, Any]]: x, y, rotation = 0.0, 0.0, 0.0 reference = "" is_power = False + mirror_x = False + mirror_y = False for sub in item: if isinstance(sub, list) and len(sub) >= 2: @@ -108,6 +110,12 @@ def _parse_symbols(sexp_data: list) -> List[Dict[str, Any]]: y = float(sub[2]) if len(sub) >= 4: rotation = float(sub[3]) + elif sub[0] == Symbol("mirror"): + m = str(sub[1]) + if m == "x": + mirror_x = True + elif m == "y": + mirror_y = True elif sub[0] == Symbol("property") and len(sub) >= 3: prop_name = str(sub[1]).strip('"') if prop_name == "Reference": @@ -120,6 +128,8 @@ def _parse_symbols(sexp_data: list) -> List[Dict[str, Any]]: "x": x, "y": y, "rotation": rotation, + "mirror_x": mirror_x, + "mirror_y": mirror_y, "is_power": is_power, }) return symbols @@ -518,6 +528,44 @@ def get_elements_in_region( # Tool 5: check_wire_collisions # --------------------------------------------------------------------------- +def _compute_pin_positions_direct( + sym: Dict[str, Any], pin_defs: Dict[str, Dict] +) -> Dict[str, List[float]]: + """ + Compute absolute schematic pin positions for a symbol instance directly from + its parsed position/rotation/mirror data and pin definitions in local coords. + + Unlike PinLocator.get_all_symbol_pins, this does NOT do a reference-name + lookup in the schematic, so it works correctly when multiple symbols share + the same reference designator (e.g. unannotated "Q?"). + + KiCad transform order: mirror (in local coords) → rotate → translate. + """ + sym_x = sym["x"] + sym_y = sym["y"] + rotation = sym["rotation"] + mirror_x = sym.get("mirror_x", False) + mirror_y = sym.get("mirror_y", False) + + result: Dict[str, List[float]] = {} + for pin_num, pin_data in pin_defs.items(): + rel_x = float(pin_data["x"]) + rel_y = float(pin_data["y"]) + + # Apply mirroring in local symbol coordinates + if mirror_x: + rel_y = -rel_y + if mirror_y: + rel_x = -rel_x + + # Apply symbol rotation + if rotation != 0: + rel_x, rel_y = PinLocator.rotate_point(rel_x, rel_y, rotation) + + result[pin_num] = [sym_x + rel_x, sym_y + rel_y] + return result + + def check_wire_collisions(schematic_path: Path) -> List[Dict[str, Any]]: """ Detect wires passing through component bodies without connecting to their pins. @@ -546,11 +594,22 @@ def check_wire_collisions(schematic_path: Path) -> List[Dict[str, Any]]: if sym["is_power"] or ref.startswith("_TEMPLATE") or not ref: continue - bbox = compute_symbol_bbox(schematic_path, ref, locator) - if bbox is None: + # Get pin definitions by lib_id (works regardless of reference designator, + # so unannotated components with duplicate "Q?" references are handled correctly). + pin_defs = locator.get_symbol_pins(schematic_path, sym["lib_id"]) + if not pin_defs: continue - min_x, min_y, max_x, max_y = bbox + # Compute absolute pin positions directly from this symbol's own position/rotation, + # bypassing the reference-name lookup in PinLocator (which always finds the first + # symbol with a given reference, breaking for unannotated duplicates like "Q?"). + pin_positions = _compute_pin_positions_direct(sym, pin_defs) + if not pin_positions: + continue + + xs = [p[0] for p in pin_positions.values()] + ys = [p[1] for p in pin_positions.values()] + min_x, min_y, max_x, max_y = min(xs), min(ys), max(xs), max(ys) # Expand degenerate dimensions (pins in a line) to approximate body size min_body = 1.5 # mm minimum half-extent for component body @@ -573,7 +632,6 @@ def check_wire_collisions(schematic_path: Path) -> List[Dict[str, Any]]: if max_x <= min_x or max_y <= min_y: continue - pin_positions = locator.get_all_symbol_pins(schematic_path, ref) pin_set = set() for pos in pin_positions.values(): pin_set.add((pos[0], pos[1])) diff --git a/python/tests/test_schematic_analysis.py b/python/tests/test_schematic_analysis.py index 95d89e7..35b6d1e 100644 --- a/python/tests/test_schematic_analysis.py +++ b/python/tests/test_schematic_analysis.py @@ -382,6 +382,39 @@ class TestIntegrationCheckWireCollisions: d1_collisions = [c for c in result if c["component"]["reference"] == "D1"] assert len(d1_collisions) >= 1 + def test_unannotated_duplicates_not_over_reported(self): + """ + Regression: two components with the same unannotated reference ("R?") at + different positions should each produce independent bounding boxes. + A wire crossing only one of them must produce exactly 1 collision, not 2. + + Before the fix, PinLocator.get_all_symbol_pins always resolved "R?" to + the first match, so both symbols got identical bboxes and the same wire + was counted against both. + """ + # R? at (100, 100): Device:R pins are at (100, 96.19) and (100, 103.81). + # Effective bbox (after expansion + margin) ≈ x=[99,101], y=[96.69,103.31]. + # R? at (200, 100): identical type but far away → no intersection with wire. + r_at_100 = _make_resistor_sexp("R?", 100, 100) + r_at_200 = _make_resistor_sexp("R?", 200, 100) + # Horizontal wire crossing the body of the first R? only + wire = """ + (wire (pts (xy 95 100) (xy 105 100)) + (stroke (width 0) (type default)) + (uuid "w-collision")) + """ + tmp = _make_temp_schematic(r_at_100 + r_at_200 + wire) + result = check_wire_collisions(tmp) + # The wire must not be reported against the far-away R? at (200, 100) + collisions_at_200 = [ + c for c in result + if abs(c["component"]["position"]["x"] - 200) < 0.5 + ] + assert len(collisions_at_200) == 0, ( + "Wire at x≈100 must not be flagged against the R? at x=200; " + "likely caused by reference-lookup always returning the first 'R?'" + ) + @pytest.mark.integration class TestIntegrationGetElementsInRegion: From 1bfa60872976735066ef07dfa44c378d72b21d9f Mon Sep 17 00:00:00 2001 From: Eugene Mikhantyev Date: Sun, 15 Mar 2026 11:06:08 +0000 Subject: [PATCH 04/13] fix: apply unannotated-reference fix to find_unconnected_pins and get_elements_in_region The same reference-lookup bug fixed in check_wire_collisions (where PinLocator.get_all_symbol_pins always resolves to the first symbol with a given reference) also affected: - find_unconnected_pins: would check wrong pin positions for all unannotated components after the first, producing false connected/ unconnected reports. - get_elements_in_region: would return wrong pin coordinates for unannotated components in the queried region. Both now use _compute_pin_positions_direct (fetching pin defs by lib_id and applying each symbol's own position/rotation/mirror), matching the fix already applied to check_wire_collisions. Co-Authored-By: Claude Sonnet 4.6 --- python/commands/schematic_analysis.py | 27 ++++++++++++++------------- 1 file changed, 14 insertions(+), 13 deletions(-) diff --git a/python/commands/schematic_analysis.py b/python/commands/schematic_analysis.py index 9d91730..b8a3b82 100644 --- a/python/commands/schematic_analysis.py +++ b/python/commands/schematic_analysis.py @@ -293,11 +293,13 @@ def find_unconnected_pins(schematic_path: Path) -> List[Dict[str, Any]]: if sym["is_power"] or ref.startswith("_TEMPLATE") or not ref: continue - pin_positions = locator.get_all_symbol_pins(schematic_path, ref) - if not pin_positions: + pin_defs = locator.get_symbol_pins(schematic_path, sym["lib_id"]) + if not pin_defs: continue - pin_defs = None + pin_positions = _compute_pin_positions_direct(sym, pin_defs) + if not pin_positions: + continue for pin_num, pos in pin_positions.items(): px, py = pos[0], pos[1] @@ -307,9 +309,6 @@ def find_unconnected_pins(schematic_path: Path) -> List[Dict[str, Any]]: if is_connected(px, py): continue - if pin_defs is None: - pin_defs = locator.get_symbol_pins(schematic_path, sym["lib_id"]) - pin_name = pin_defs.get(pin_num, {}).get("name", pin_num) unconnected.append({ "reference": ref, @@ -482,13 +481,15 @@ def get_elements_in_region( "position": {"x": sym["x"], "y": sym["y"]}, "isPower": sym["is_power"], } - # Include pin positions - pin_positions = locator.get_all_symbol_pins(schematic_path, sym["reference"]) - if pin_positions: - entry["pins"] = { - pn: {"x": round(pos[0], 4), "y": round(pos[1], 4)} - for pn, pos in pin_positions.items() - } + # Include pin positions (compute directly to handle unannotated duplicates) + pin_defs = locator.get_symbol_pins(schematic_path, sym["lib_id"]) + if pin_defs: + pin_positions = _compute_pin_positions_direct(sym, pin_defs) + if pin_positions: + entry["pins"] = { + pn: {"x": round(pos[0], 4), "y": round(pos[1], 4)} + for pn, pos in pin_positions.items() + } region_symbols.append(entry) # Wires: include if ANY endpoint is within bounds From 6b09d93df2755cbcd8dfb107612369c4f02d85eb Mon Sep 17 00:00:00 2001 From: Eugene Mikhantyev Date: Sun, 15 Mar 2026 12:46:50 +0000 Subject: [PATCH 05/13] fix: check_wire_collisions now detects wires that short two pins of the same component Previously, the endpoint suppression logic skipped any wire where at least one endpoint touched a pin, hiding the case where both endpoints are pins of the same component (a direct pin-to-pin short through the body). Replace the single endpoint_matches_pin loop with separate start_at_pin / end_at_pin checks; suppress only when exactly one endpoint is at a pin. Add regression test to cover this case. Co-Authored-By: Claude Sonnet 4.6 --- python/commands/schematic_analysis.py | 30 +++++++++++++++---------- python/tests/test_schematic_analysis.py | 17 ++++++++++++++ 2 files changed, 35 insertions(+), 12 deletions(-) diff --git a/python/commands/schematic_analysis.py b/python/commands/schematic_analysis.py index b8a3b82..5c7fe7f 100644 --- a/python/commands/schematic_analysis.py +++ b/python/commands/schematic_analysis.py @@ -654,19 +654,25 @@ def check_wire_collisions(schematic_path: Path) -> List[Dict[str, Any]]: if not _line_segment_intersects_aabb(sx, sy, ex, ey, bx1, by1, bx2, by2): continue - # Check if either wire endpoint matches a pin of this symbol - endpoint_matches_pin = False - for px, py in sd["pin_set"]: - if (abs(sx - px) < pin_tolerance and abs(sy - py) < pin_tolerance): - endpoint_matches_pin = True - break - if (abs(ex - px) < pin_tolerance and abs(ey - py) < pin_tolerance): - endpoint_matches_pin = True - break + # Check which endpoints land on a pin of this symbol + start_at_pin = any( + abs(sx - px) < pin_tolerance and abs(sy - py) < pin_tolerance + for px, py in sd["pin_set"] + ) + end_at_pin = any( + abs(ex - px) < pin_tolerance and abs(ey - py) < pin_tolerance + for px, py in sd["pin_set"] + ) - if not endpoint_matches_pin: - sym = sd["sym"] - collisions.append({ + # Suppress only when exactly ONE endpoint is at a pin: the wire arrives + # from elsewhere and terminates at this component (a valid connection). + # If BOTH endpoints match pins of this same component, the wire shorts + # two pins while traversing the body — that IS a collision. + if (start_at_pin or end_at_pin) and not (start_at_pin and end_at_pin): + continue + + sym = sd["sym"] + collisions.append({ "wire": { "start": {"x": sx, "y": sy}, "end": {"x": ex, "y": ey}, diff --git a/python/tests/test_schematic_analysis.py b/python/tests/test_schematic_analysis.py index 35b6d1e..d9cdc30 100644 --- a/python/tests/test_schematic_analysis.py +++ b/python/tests/test_schematic_analysis.py @@ -415,6 +415,23 @@ class TestIntegrationCheckWireCollisions: "likely caused by reference-lookup always returning the first 'R?'" ) + def test_wire_shorts_component_pins_detected_as_collision(self): + """Regression: a wire connecting pin1→pin2 of the same component + must be reported even though both endpoints land on pins.""" + r_sexp = _make_resistor_sexp("R_short", 100.0, 100.0) + wire_sexp = ( + '(wire (pts (xy 100 103.81) (xy 100 96.19))\n' + ' (stroke (width 0) (type default))\n' + ' (uuid "aaaaaaaa-0000-0000-0000-000000000001"))' + ) + sch = _make_temp_schematic(r_sexp + "\n" + wire_sexp) + collisions = check_wire_collisions(sch) + assert len(collisions) == 1 + w = collisions[0]["wire"] + assert w["start"]["x"] == pytest.approx(100.0) + assert w["start"]["y"] == pytest.approx(103.81) + assert collisions[0]["component"]["reference"] == "R_short" + @pytest.mark.integration class TestIntegrationGetElementsInRegion: From 3ab93b241f60378ba86055a9fb2f44781c7a827f Mon Sep 17 00:00:00 2001 From: Eugene Mikhantyev Date: Sun, 15 Mar 2026 13:29:37 +0000 Subject: [PATCH 06/13] fix: find_overlapping_elements uses bounding-box intersection instead of center distance The overlap detection was comparing center-to-center Euclidean distance with a 0.5mm tolerance, missing components whose bodies physically overlap but have different centers (e.g. a resistor placed inside an opamp triangle). Now uses AABB intersection on pin-derived bounding boxes, matching the approach already used by check_wire_collisions. Extracted shared bbox logic into _compute_symbol_bbox_direct and _aabb_overlap helpers. Co-Authored-By: Claude Opus 4.6 --- python/commands/schematic_analysis.py | 133 +++++++++++++++++------- python/tests/test_schematic_analysis.py | 82 ++++++++++++--- 2 files changed, 158 insertions(+), 57 deletions(-) diff --git a/python/commands/schematic_analysis.py b/python/commands/schematic_analysis.py index 5c7fe7f..1bb357c 100644 --- a/python/commands/schematic_analysis.py +++ b/python/commands/schematic_analysis.py @@ -220,6 +220,67 @@ def _distance(p1: Tuple[float, float], p2: Tuple[float, float]) -> float: return math.sqrt((p1[0] - p2[0]) ** 2 + (p1[1] - p2[1]) ** 2) +def _aabb_overlap( + a: Tuple[float, float, float, float], + b: Tuple[float, float, float, float], +) -> bool: + """Check if two axis-aligned bounding boxes overlap. + + Each bbox is (min_x, min_y, max_x, max_y). + """ + return a[0] < b[2] and b[0] < a[2] and a[1] < b[3] and b[1] < a[3] + + +def _compute_symbol_bbox_direct( + sym: Dict[str, Any], + pin_defs: Dict[str, Dict], + margin: float = 0.0, +) -> Optional[Tuple[float, float, float, float]]: + """ + Compute bounding box of a symbol from its pin definitions and placement. + + Uses _compute_pin_positions_direct to get absolute pin positions, then + expands degenerate dimensions (pins in a line) to approximate body size. + + Args: + sym: Parsed symbol dict with x, y, rotation, mirror_x, mirror_y. + pin_defs: Pin definitions from PinLocator.get_symbol_pins(). + margin: Shrink bbox by this amount on each side (mm). + + Returns (min_x, min_y, max_x, max_y) in mm, or None if no pins. + """ + pin_positions = _compute_pin_positions_direct(sym, pin_defs) + if not pin_positions: + return None + + xs = [p[0] for p in pin_positions.values()] + ys = [p[1] for p in pin_positions.values()] + min_x, min_y, max_x, max_y = min(xs), min(ys), max(xs), max(ys) + + # Expand degenerate dimensions (pins in a line) to approximate body size + min_body = 1.5 # mm minimum half-extent for component body + if max_x - min_x < 2 * min_body: + cx = (min_x + max_x) / 2 + min_x = cx - min_body + max_x = cx + min_body + if max_y - min_y < 2 * min_body: + cy = (min_y + max_y) / 2 + min_y = cy - min_body + max_y = cy + min_body + + # Shrink bbox by margin + min_x += margin + min_y += margin + max_x -= margin + max_y -= margin + + # Skip degenerate bboxes + if max_x <= min_x or max_y <= min_y: + return None + + return (min_x, min_y, max_x, max_y) + + # --------------------------------------------------------------------------- # Tool 2: find_unconnected_pins # --------------------------------------------------------------------------- @@ -346,14 +407,35 @@ def find_overlapping_elements( overlapping_labels = [] overlapping_wires = [] - # --- Symbol-symbol overlap (O(n²)) --- + locator = PinLocator() + + # --- Symbol-symbol overlap using bounding-box intersection (O(n²)) --- non_template_symbols = [s for s in symbols if not s["reference"].startswith("_TEMPLATE") and s["reference"]] - for i in range(len(non_template_symbols)): - for j in range(i + 1, len(non_template_symbols)): - s1 = non_template_symbols[i] - s2 = non_template_symbols[j] + + # Pre-compute bounding boxes for all non-template symbols + symbol_bboxes = [] + for sym in non_template_symbols: + pin_defs = locator.get_symbol_pins(schematic_path, sym["lib_id"]) + bbox = None + if pin_defs: + bbox = _compute_symbol_bbox_direct(sym, pin_defs) + symbol_bboxes.append((sym, bbox)) + + for i in range(len(symbol_bboxes)): + s1, bbox1 = symbol_bboxes[i] + for j in range(i + 1, len(symbol_bboxes)): + s2, bbox2 = symbol_bboxes[j] dist = _distance((s1["x"], s1["y"]), (s2["x"], s2["y"])) - if dist < tolerance: + + overlap_detected = False + if bbox1 is not None and bbox2 is not None: + # Use bounding box intersection + overlap_detected = _aabb_overlap(bbox1, bbox2) + else: + # Fallback to center distance when pin data is unavailable + overlap_detected = dist < tolerance + + if overlap_detected: entry = { "element1": {"reference": s1["reference"], "libId": s1["lib_id"], "position": {"x": s1["x"], "y": s1["y"]}}, @@ -595,51 +677,22 @@ def check_wire_collisions(schematic_path: Path) -> List[Dict[str, Any]]: if sym["is_power"] or ref.startswith("_TEMPLATE") or not ref: continue - # Get pin definitions by lib_id (works regardless of reference designator, - # so unannotated components with duplicate "Q?" references are handled correctly). pin_defs = locator.get_symbol_pins(schematic_path, sym["lib_id"]) if not pin_defs: continue - # Compute absolute pin positions directly from this symbol's own position/rotation, - # bypassing the reference-name lookup in PinLocator (which always finds the first - # symbol with a given reference, breaking for unannotated duplicates like "Q?"). + bbox = _compute_symbol_bbox_direct(sym, pin_defs, margin=margin) + if bbox is None: + continue + pin_positions = _compute_pin_positions_direct(sym, pin_defs) - if not pin_positions: - continue - - xs = [p[0] for p in pin_positions.values()] - ys = [p[1] for p in pin_positions.values()] - min_x, min_y, max_x, max_y = min(xs), min(ys), max(xs), max(ys) - - # Expand degenerate dimensions (pins in a line) to approximate body size - min_body = 1.5 # mm minimum half-extent for component body - if max_x - min_x < 2 * min_body: - cx = (min_x + max_x) / 2 - min_x = cx - min_body - max_x = cx + min_body - if max_y - min_y < 2 * min_body: - cy = (min_y + max_y) / 2 - min_y = cy - min_body - max_y = cy + min_body - - # Shrink bbox by margin - min_x += margin - min_y += margin - max_x -= margin - max_y -= margin - - # Skip degenerate bboxes (single-pin or very small after shrink) - if max_x <= min_x or max_y <= min_y: - continue - pin_set = set() for pos in pin_positions.values(): pin_set.add((pos[0], pos[1])) symbol_data.append({ "sym": sym, - "bbox": (min_x, min_y, max_x, max_y), + "bbox": bbox, "pin_set": pin_set, }) diff --git a/python/tests/test_schematic_analysis.py b/python/tests/test_schematic_analysis.py index d9cdc30..3b68d1c 100644 --- a/python/tests/test_schematic_analysis.py +++ b/python/tests/test_schematic_analysis.py @@ -28,6 +28,8 @@ from commands.schematic_analysis import ( _line_segment_intersects_aabb, _point_in_rect, _distance, + _aabb_overlap, + _compute_symbol_bbox_direct, compute_symbol_bbox, find_unconnected_pins, find_overlapping_elements, @@ -229,6 +231,27 @@ class TestSexpParsers: # Unit tests — analysis functions with mocked PinLocator # =================================================================== +class TestAABBOverlap: + """Test AABB overlap helper.""" + + def test_overlapping_boxes(self): + assert _aabb_overlap((0, 0, 10, 10), (5, 5, 15, 15)) is True + + def test_non_overlapping_boxes(self): + assert _aabb_overlap((0, 0, 10, 10), (20, 20, 30, 30)) is False + + def test_touching_boxes_no_overlap(self): + # Touching edges are not overlapping (strict inequality) + assert _aabb_overlap((0, 0, 10, 10), (10, 0, 20, 10)) is False + + def test_contained_box(self): + assert _aabb_overlap((0, 0, 20, 20), (5, 5, 15, 15)) is True + + def test_overlap_one_axis_only(self): + # Overlap in X but not Y + assert _aabb_overlap((0, 0, 10, 10), (5, 15, 15, 25)) is False + + class TestFindOverlappingElements: """Test overlapping detection logic.""" @@ -238,29 +261,15 @@ class TestFindOverlappingElements: assert result["totalOverlaps"] == 0 def test_overlapping_symbols_detected(self): - # Two symbols at nearly the same position - extra = """ - (symbol (lib_id "Device:R") (at 100 100 0) - (property "Reference" "R1" (at 0 0 0)) - (property "Value" "10k" (at 0 0 0))) - (symbol (lib_id "Device:R") (at 100.1 100 0) - (property "Reference" "R2" (at 0 0 0)) - (property "Value" "10k" (at 0 0 0))) - """ + # Two resistors at nearly the same position — bboxes fully overlap + extra = _make_resistor_sexp("R1", 100, 100) + _make_resistor_sexp("R2", 100.1, 100) tmp = _make_temp_schematic(extra) result = find_overlapping_elements(tmp, tolerance=0.5) assert result["totalOverlaps"] >= 1 assert len(result["overlappingSymbols"]) >= 1 def test_well_separated_symbols_not_flagged(self): - extra = """ - (symbol (lib_id "Device:R") (at 100 100 0) - (property "Reference" "R1" (at 0 0 0)) - (property "Value" "10k" (at 0 0 0))) - (symbol (lib_id "Device:R") (at 200 200 0) - (property "Reference" "R2" (at 0 0 0)) - (property "Value" "10k" (at 0 0 0))) - """ + extra = _make_resistor_sexp("R1", 100, 100) + _make_resistor_sexp("R2", 200, 200) tmp = _make_temp_schematic(extra) result = find_overlapping_elements(tmp, tolerance=0.5) assert result["totalOverlaps"] == 0 @@ -278,6 +287,45 @@ class TestFindOverlappingElements: result = find_overlapping_elements(tmp, tolerance=0.5) assert len(result["overlappingWires"]) >= 1 + def test_overlapping_bodies_different_centers(self): + """Two resistors whose bodies overlap even though centers are ~5mm apart. + + Device:R pins are at y ±3.81 relative to center, so the body spans + ~7.62mm vertically. Two resistors at the same X but 5mm apart in Y + have overlapping bodies — this is the bug the center-distance approach missed. + """ + # R1 at y=100, R2 at y=105 — pin spans [96.19, 103.81] and [101.19, 108.81] + # These overlap in Y from 101.19 to 103.81 + extra = _make_resistor_sexp("R1", 100, 100) + _make_resistor_sexp("R2", 100, 105) + tmp = _make_temp_schematic(extra) + result = find_overlapping_elements(tmp, tolerance=0.5) + assert result["totalOverlaps"] >= 1, ( + "Should detect overlap when component bodies intersect, " + "even if centers are far apart" + ) + assert len(result["overlappingSymbols"]) >= 1 + + def test_adjacent_resistors_no_overlap(self): + """Two vertical resistors side by side should not overlap. + + R pins at y ±3.81, but different X positions far enough apart. + """ + extra = _make_resistor_sexp("R1", 100, 100) + _make_resistor_sexp("R2", 110, 100) + tmp = _make_temp_schematic(extra) + result = find_overlapping_elements(tmp, tolerance=0.5) + assert result["totalOverlaps"] == 0 + + def test_resistor_and_led_overlapping_bodies(self): + """A resistor and an LED placed close enough that bodies overlap. + + LED pins at x ±3.81, R pins at y ±3.81. Place LED at same position + as R — bodies clearly overlap. + """ + extra = _make_resistor_sexp("R1", 100, 100) + _make_led_sexp("D1", 100, 100) + tmp = _make_temp_schematic(extra) + result = find_overlapping_elements(tmp, tolerance=0.5) + assert result["totalOverlaps"] >= 1 + class TestGetElementsInRegion: """Test region query logic.""" From 76ad44121c1aca505ac3d64c331f5ed95dd9da6b Mon Sep 17 00:00:00 2001 From: Eugene Mikhantyev Date: Sun, 15 Mar 2026 13:48:18 +0000 Subject: [PATCH 07/13] fix: rename check_wire_collisions to find_wires_crossing_symbols and detect pass-through wires MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Wires that start at a component pin but continue through the body were incorrectly suppressed as "valid connections." Now nudges the pin endpoint toward the other end and re-tests intersection — if the shortened segment still hits the bbox, the wire passes through and is flagged. Renamed the tool from check_wire_collisions to find_wires_crossing_symbols across all layers (Python, handler, schema, TypeScript) to clarify that it finds wires crossing over component symbols, which is unacceptable in schematics. Co-Authored-By: Claude Opus 4.6 --- python/commands/schematic_analysis.py | 41 ++++++++++++++++----- python/kicad_interface.py | 14 ++++---- python/schemas/tool_schemas.py | 6 ++-- python/tests/test_schematic_analysis.py | 47 +++++++++++++++++++++---- src/tools/schematic.ts | 12 +++---- 5 files changed, 89 insertions(+), 31 deletions(-) diff --git a/python/commands/schematic_analysis.py b/python/commands/schematic_analysis.py index 1bb357c..e2788dc 100644 --- a/python/commands/schematic_analysis.py +++ b/python/commands/schematic_analysis.py @@ -649,16 +649,21 @@ def _compute_pin_positions_direct( return result -def check_wire_collisions(schematic_path: Path) -> List[Dict[str, Any]]: +def find_wires_crossing_symbols(schematic_path: Path) -> List[Dict[str, Any]]: """ - Detect wires passing through component bodies without connecting to their pins. + Find all wires that cross over component symbol bodies. + + Wires passing over symbols are unacceptable in schematics — they indicate + routing mistakes where a wire was drawn across a component instead of + around it. For each non-power, non-template symbol: 1. Compute bounding box from pin positions (shrunk by margin). 2. For each wire segment, test intersection with the bbox. - 3. If intersects but no wire endpoint matches a pin → collision. + 3. If intersects and the wire is not simply terminating at a pin from + outside, report it as a crossing. - Returns list of collision dicts. + Returns list of crossing dicts. """ sexp_data = _load_sexp(schematic_path) symbols = _parse_symbols(sexp_data) @@ -717,12 +722,30 @@ def check_wire_collisions(schematic_path: Path) -> List[Dict[str, Any]]: for px, py in sd["pin_set"] ) - # Suppress only when exactly ONE endpoint is at a pin: the wire arrives - # from elsewhere and terminates at this component (a valid connection). - # If BOTH endpoints match pins of this same component, the wire shorts - # two pins while traversing the body — that IS a collision. + # When exactly one endpoint is at a pin, check whether the wire + # just terminates at the pin (valid connection) or continues through + # the component body (pass-through → collision). + # Nudge the pin endpoint slightly toward the other end; if the + # shortened segment still intersects the bbox, the wire extends + # into/through the body. if (start_at_pin or end_at_pin) and not (start_at_pin and end_at_pin): - continue + dx, dy = ex - sx, ey - sy + length = math.sqrt(dx * dx + dy * dy) + if length > 0: + nudge = min(0.2, length * 0.5) + ux, uy = dx / length, dy / length + if start_at_pin: + nsx, nsy = sx + ux * nudge, sy + uy * nudge + if not _line_segment_intersects_aabb( + nsx, nsy, ex, ey, bx1, by1, bx2, by2 + ): + continue # Wire terminates at pin from outside + else: + nex, ney = ex - ux * nudge, ey - uy * nudge + if not _line_segment_intersects_aabb( + sx, sy, nex, ney, bx1, by1, bx2, by2 + ): + continue # Wire terminates at pin from outside sym = sd["sym"] collisions.append({ diff --git a/python/kicad_interface.py b/python/kicad_interface.py index 723b00c..56dcc4c 100644 --- a/python/kicad_interface.py +++ b/python/kicad_interface.py @@ -403,7 +403,7 @@ class KiCADInterface: "find_unconnected_pins": self._handle_find_unconnected_pins, "find_overlapping_elements": self._handle_find_overlapping_elements, "get_elements_in_region": self._handle_get_elements_in_region, - "check_wire_collisions": self._handle_check_wire_collisions, + "find_wires_crossing_symbols": self._handle_find_wires_crossing_symbols, "import_svg_logo": self._handle_import_svg_logo, # UI/Process management commands "check_kicad_ui": self._handle_check_kicad_ui, @@ -2734,23 +2734,23 @@ class KiCADInterface: logger.error(traceback.format_exc()) return {"success": False, "message": str(e)} - def _handle_check_wire_collisions(self, params): - """Detect wires passing through component bodies without connecting to their pins""" - logger.info("Checking wire collisions in schematic") + def _handle_find_wires_crossing_symbols(self, params): + """Find wires that cross over component symbol bodies""" + logger.info("Finding wires crossing symbols in schematic") try: from pathlib import Path - from commands.schematic_analysis import check_wire_collisions + from commands.schematic_analysis import find_wires_crossing_symbols schematic_path = params.get("schematicPath") if not schematic_path: return {"success": False, "message": "schematicPath is required"} - result = check_wire_collisions(Path(schematic_path)) + result = find_wires_crossing_symbols(Path(schematic_path)) return { "success": True, "collisions": result, "count": len(result), - "message": f"Found {len(result)} wire collision(s)", + "message": f"Found {len(result)} wire(s) crossing symbols", } except Exception as e: logger.error(f"Error checking wire collisions: {e}") diff --git a/python/schemas/tool_schemas.py b/python/schemas/tool_schemas.py index 4e6290c..ec9d70e 100644 --- a/python/schemas/tool_schemas.py +++ b/python/schemas/tool_schemas.py @@ -1792,9 +1792,9 @@ SCHEMATIC_TOOLS = [ } }, { - "name": "check_wire_collisions", - "title": "Check Wire Collisions", - "description": "Detects wires that pass through component bodies without connecting to their pins. These are usually routing mistakes where a wire crosses over a symbol instead of connecting to it. Uses pin-based bounding boxes (approximate but effective for 80/20 detection).", + "name": "find_wires_crossing_symbols", + "title": "Find Wires Crossing Symbols", + "description": "Find all wires that cross over component symbol bodies. Wires passing over symbols are unacceptable in schematics — they indicate routing mistakes where a wire was drawn across a component instead of around it.", "inputSchema": { "type": "object", "properties": { diff --git a/python/tests/test_schematic_analysis.py b/python/tests/test_schematic_analysis.py index 3b68d1c..1f00297 100644 --- a/python/tests/test_schematic_analysis.py +++ b/python/tests/test_schematic_analysis.py @@ -34,7 +34,7 @@ from commands.schematic_analysis import ( find_unconnected_pins, find_overlapping_elements, get_elements_in_region, - check_wire_collisions, + find_wires_crossing_symbols, ) @@ -412,8 +412,8 @@ class TestIntegrationFindUnconnectedPins: @pytest.mark.integration -class TestIntegrationCheckWireCollisions: - """Integration test for wire collision detection.""" +class TestIntegrationFindWiresCrossingSymbols: + """Integration test for wire crossing symbol detection.""" def test_wire_not_touching_pins_is_collision(self): """A wire passing through a component bbox without pin contact → collision.""" @@ -426,7 +426,7 @@ class TestIntegrationCheckWireCollisions: (uuid "w1")) """ tmp = _make_temp_schematic(extra) - result = check_wire_collisions(tmp) + result = find_wires_crossing_symbols(tmp) d1_collisions = [c for c in result if c["component"]["reference"] == "D1"] assert len(d1_collisions) >= 1 @@ -452,7 +452,7 @@ class TestIntegrationCheckWireCollisions: (uuid "w-collision")) """ tmp = _make_temp_schematic(r_at_100 + r_at_200 + wire) - result = check_wire_collisions(tmp) + result = find_wires_crossing_symbols(tmp) # The wire must not be reported against the far-away R? at (200, 100) collisions_at_200 = [ c for c in result @@ -463,6 +463,41 @@ class TestIntegrationCheckWireCollisions: "likely caused by reference-lookup always returning the first 'R?'" ) + def test_wire_starting_at_pin_passing_through_body(self): + """A wire that starts at a pin but continues through the component body + must be flagged — this is the core bug where the old suppression logic + treated any wire touching a pin as a valid connection.""" + # LED D1 at (100,100) → pin 1 at (96.19, 100), pin 2 at (103.81, 100) + # Wire starts exactly at pin 1 and extends through the body to the right + extra = _make_led_sexp("D1", 100, 100) + """ + (wire (pts (xy 96.19 100) (xy 110 100)) + (stroke (width 0) (type default)) + (uuid "w-through")) + """ + tmp = _make_temp_schematic(extra) + result = find_wires_crossing_symbols(tmp) + d1_crossings = [c for c in result if c["component"]["reference"] == "D1"] + assert len(d1_crossings) >= 1, ( + "Wire starting at pin but passing through body must be detected" + ) + + def test_wire_terminating_at_pin_from_outside(self): + """A wire that arrives at a pin from outside the component body + is a valid connection and must NOT be flagged.""" + # LED D1 at (100,100) → pin 1 at (96.19, 100) + # Wire comes from the left and terminates at pin 1 + extra = _make_led_sexp("D1", 100, 100) + """ + (wire (pts (xy 80 100) (xy 96.19 100)) + (stroke (width 0) (type default)) + (uuid "w-valid")) + """ + tmp = _make_temp_schematic(extra) + result = find_wires_crossing_symbols(tmp) + d1_crossings = [c for c in result if c["component"]["reference"] == "D1"] + assert len(d1_crossings) == 0, ( + "Wire terminating at pin from outside should not be flagged" + ) + def test_wire_shorts_component_pins_detected_as_collision(self): """Regression: a wire connecting pin1→pin2 of the same component must be reported even though both endpoints land on pins.""" @@ -473,7 +508,7 @@ class TestIntegrationCheckWireCollisions: ' (uuid "aaaaaaaa-0000-0000-0000-000000000001"))' ) sch = _make_temp_schematic(r_sexp + "\n" + wire_sexp) - collisions = check_wire_collisions(sch) + collisions = find_wires_crossing_symbols(sch) assert len(collisions) == 1 w = collisions[0]["wire"] assert w["start"]["x"] == pytest.approx(100.0) diff --git a/src/tools/schematic.ts b/src/tools/schematic.ts index 28f5b7e..771dc57 100644 --- a/src/tools/schematic.ts +++ b/src/tools/schematic.ts @@ -1250,21 +1250,21 @@ Note: operates on .kicad_sch files only. To modify a PCB footprint use edit_comp }, ); - // Check wire collisions + // Find wires crossing symbols server.tool( - "check_wire_collisions", - "Detect wires that pass through component bodies without connecting to their pins. These are usually routing mistakes where a wire crosses over a symbol instead of connecting to it.", + "find_wires_crossing_symbols", + "Find all wires that cross over component symbol bodies. Wires passing over symbols are unacceptable in schematics — they indicate routing mistakes where a wire was drawn across a component instead of around it.", { schematicPath: z.string().describe("Path to the .kicad_sch schematic file"), }, async (args: { schematicPath: string }) => { - const result = await callKicadScript("check_wire_collisions", args); + const result = await callKicadScript("find_wires_crossing_symbols", args); if (result.success) { const collisions: any[] = result.collisions || []; - const lines = [`Found ${collisions.length} wire collision(s):`]; + const lines = [`Found ${collisions.length} wire(s) crossing symbols:`]; collisions.slice(0, 30).forEach((c: any, i: number) => { lines.push( - ` ${i + 1}. Wire (${c.wire.start.x},${c.wire.start.y})→(${c.wire.end.x},${c.wire.end.y}) passes through ${c.component.reference} (${c.component.libId})` + ` ${i + 1}. Wire (${c.wire.start.x},${c.wire.start.y})→(${c.wire.end.x},${c.wire.end.y}) crosses ${c.component.reference} (${c.component.libId})` ); }); if (collisions.length > 30) lines.push(` ... and ${collisions.length - 30} more`); From 0ab7428b84c6fca2ce3b524cbd5148553abd4aa7 Mon Sep 17 00:00:00 2001 From: Eugene Mikhantyev Date: Sun, 15 Mar 2026 14:05:58 +0000 Subject: [PATCH 08/13] fix: address PR review issues for schematic analysis tools Eliminate repeated file parsing by extracting _extract_lib_symbols helper that walks already-parsed sexp_data once instead of re-reading the file per symbol via PinLocator. Support diagonal wire overlap detection using cross-product parallelism and 1D projection. Fix wire region inclusion to use AABB intersection for pass-through wires. Normalize view region coordinates. Clarify tolerance docstrings across Python, TS, and schema. Co-Authored-By: Claude Opus 4.6 --- python/commands/schematic_analysis.py | 111 +++++++++++----- python/kicad_interface.py | 2 + python/schemas/tool_schemas.py | 2 +- python/tests/test_schematic_analysis.py | 166 ++++++++++++++++++++++++ src/tools/schematic.ts | 2 +- 5 files changed, 246 insertions(+), 37 deletions(-) diff --git a/python/commands/schematic_analysis.py b/python/commands/schematic_analysis.py index e2788dc..01d170f 100644 --- a/python/commands/schematic_analysis.py +++ b/python/commands/schematic_analysis.py @@ -150,6 +150,33 @@ def _parse_no_connects(sexp_data: list) -> Set[Tuple[float, float]]: return positions +def _extract_lib_symbols(sexp_data: list) -> Dict[str, Dict[str, Dict]]: + """ + Walk the lib_symbols section of already-parsed sexp_data and return + pin definitions for every symbol definition. + + Returns: + Dict mapping lib_id → pin definitions (pin_number → pin_data dict). + """ + lib_symbols_section = None + for item in sexp_data: + if (isinstance(item, list) and len(item) > 0 + and item[0] == Symbol("lib_symbols")): + lib_symbols_section = item + break + + if not lib_symbols_section: + return {} + + result: Dict[str, Dict[str, Dict]] = {} + for item in lib_symbols_section[1:]: + if (isinstance(item, list) and len(item) > 1 + and item[0] == Symbol("symbol")): + symbol_name = str(item[1]).strip('"') + result[symbol_name] = PinLocator.parse_symbol_definition(item) + return result + + # --------------------------------------------------------------------------- # Geometry helpers # --------------------------------------------------------------------------- @@ -345,7 +372,7 @@ def find_unconnected_pins(schematic_path: Path) -> List[Dict[str, Any]]: return True return False - locator = PinLocator() + lib_pin_defs = _extract_lib_symbols(sexp_data) unconnected = [] for sym in symbols: @@ -354,7 +381,7 @@ def find_unconnected_pins(schematic_path: Path) -> List[Dict[str, Any]]: if sym["is_power"] or ref.startswith("_TEMPLATE") or not ref: continue - pin_defs = locator.get_symbol_pins(schematic_path, sym["lib_id"]) + pin_defs = lib_pin_defs.get(sym["lib_id"], {}) if not pin_defs: continue @@ -394,7 +421,7 @@ def find_overlapping_elements( Args: schematic_path: Path to .kicad_sch file - tolerance: Distance in mm below which elements are considered overlapping + tolerance: Distance threshold in mm for label proximity and wire collinearity checks. Symbol overlap uses bounding-box intersection. Returns dict: {overlappingSymbols, overlappingLabels, overlappingWires, totalOverlaps} """ @@ -407,7 +434,7 @@ def find_overlapping_elements( overlapping_labels = [] overlapping_wires = [] - locator = PinLocator() + lib_pin_defs = _extract_lib_symbols(sexp_data) # --- Symbol-symbol overlap using bounding-box intersection (O(n²)) --- non_template_symbols = [s for s in symbols if not s["reference"].startswith("_TEMPLATE") and s["reference"]] @@ -415,7 +442,7 @@ def find_overlapping_elements( # Pre-compute bounding boxes for all non-template symbols symbol_bboxes = [] for sym in non_template_symbols: - pin_defs = locator.get_symbol_pins(schematic_path, sym["lib_id"]) + pin_defs = lib_pin_defs.get(sym["lib_id"], {}) bbox = None if pin_defs: bbox = _compute_symbol_bbox_direct(sym, pin_defs) @@ -490,36 +517,49 @@ def _check_wire_overlap( """ Check if two wire segments are collinear and overlapping. + Works for horizontal, vertical, and diagonal wires. Uses direction + vectors, cross-product parallelism, point-to-line distance for + collinearity, and 1D projection overlap. + Returns overlap info dict or None. """ s1, e1 = w1["start"], w1["end"] s2, e2 = w2["start"], w2["end"] - # Check horizontal collinearity - if abs(s1[1] - e1[1]) < tolerance and abs(s2[1] - e2[1]) < tolerance: - if abs(s1[1] - s2[1]) < tolerance: - # Both horizontal, same Y - min1, max1 = min(s1[0], e1[0]), max(s1[0], e1[0]) - min2, max2 = min(s2[0], e2[0]), max(s2[0], e2[0]) - if min1 < max2 and min2 < max1: - return { - "wire1": {"start": {"x": s1[0], "y": s1[1]}, "end": {"x": e1[0], "y": e1[1]}}, - "wire2": {"start": {"x": s2[0], "y": s2[1]}, "end": {"x": e2[0], "y": e2[1]}}, - "type": "collinear_overlap", - } + d1 = (e1[0] - s1[0], e1[1] - s1[1]) + d2 = (e2[0] - s2[0], e2[1] - s2[1]) - # Check vertical collinearity - if abs(s1[0] - e1[0]) < tolerance and abs(s2[0] - e2[0]) < tolerance: - if abs(s1[0] - s2[0]) < tolerance: - # Both vertical, same X - min1, max1 = min(s1[1], e1[1]), max(s1[1], e1[1]) - min2, max2 = min(s2[1], e2[1]), max(s2[1], e2[1]) - if min1 < max2 and min2 < max1: - return { - "wire1": {"start": {"x": s1[0], "y": s1[1]}, "end": {"x": e1[0], "y": e1[1]}}, - "wire2": {"start": {"x": s2[0], "y": s2[1]}, "end": {"x": e2[0], "y": e2[1]}}, - "type": "collinear_overlap", - } + len1 = math.sqrt(d1[0] ** 2 + d1[1] ** 2) + len2 = math.sqrt(d2[0] ** 2 + d2[1] ** 2) + if len1 < 1e-12 or len2 < 1e-12: + return None # degenerate zero-length segment + + # Cross product to check parallel + cross = d1[0] * d2[1] - d1[1] * d2[0] + if abs(cross) > tolerance * max(len1, len2): + return None # not parallel + + # Point-to-line distance: s2 relative to line through s1 along d1 + ds = (s2[0] - s1[0], s2[1] - s1[1]) + perp_dist = abs(ds[0] * d1[1] - ds[1] * d1[0]) / len1 + if perp_dist > tolerance: + return None # parallel but offset + + # Project onto d1 direction for 1D overlap check + u1 = (d1[0] / len1, d1[1] / len1) + proj_s1 = s1[0] * u1[0] + s1[1] * u1[1] + proj_e1 = e1[0] * u1[0] + e1[1] * u1[1] + proj_s2 = s2[0] * u1[0] + s2[1] * u1[1] + proj_e2 = e2[0] * u1[0] + e2[1] * u1[1] + + min1, max1 = min(proj_s1, proj_e1), max(proj_s1, proj_e1) + min2, max2 = min(proj_s2, proj_e2), max(proj_s2, proj_e2) + if min1 < max2 and min2 < max1: + return { + "wire1": {"start": {"x": s1[0], "y": s1[1]}, "end": {"x": e1[0], "y": e1[1]}}, + "wire2": {"start": {"x": s2[0], "y": s2[1]}, "end": {"x": e2[0], "y": e2[1]}}, + "type": "collinear_overlap", + } return None @@ -549,7 +589,7 @@ def get_elements_in_region( wires = _parse_wires(sexp_data) labels = _parse_labels(sexp_data) - locator = PinLocator() + lib_pin_defs = _extract_lib_symbols(sexp_data) # Symbols: include if position is within bounds region_symbols = [] @@ -564,7 +604,7 @@ def get_elements_in_region( "isPower": sym["is_power"], } # Include pin positions (compute directly to handle unannotated duplicates) - pin_defs = locator.get_symbol_pins(schematic_path, sym["lib_id"]) + pin_defs = lib_pin_defs.get(sym["lib_id"], {}) if pin_defs: pin_positions = _compute_pin_positions_direct(sym, pin_defs) if pin_positions: @@ -574,12 +614,13 @@ def get_elements_in_region( } region_symbols.append(entry) - # Wires: include if ANY endpoint is within bounds + # Wires: include if any part of the wire intersects the region region_wires = [] for w in wires: s, e = w["start"], w["end"] if (_point_in_rect(s[0], s[1], min_x, min_y, max_x, max_y) or - _point_in_rect(e[0], e[1], min_x, min_y, max_x, max_y)): + _point_in_rect(e[0], e[1], min_x, min_y, max_x, max_y) or + _line_segment_intersects_aabb(s[0], s[1], e[0], e[1], min_x, min_y, max_x, max_y)): region_wires.append({ "start": {"x": s[0], "y": s[1]}, "end": {"x": e[0], "y": e[1]}, @@ -669,7 +710,7 @@ def find_wires_crossing_symbols(schematic_path: Path) -> List[Dict[str, Any]]: symbols = _parse_symbols(sexp_data) wires = _parse_wires(sexp_data) - locator = PinLocator() + lib_pin_defs = _extract_lib_symbols(sexp_data) margin = 0.5 # mm margin to shrink bbox (avoids false positives at pin tips) pin_tolerance = 0.05 # mm @@ -682,7 +723,7 @@ def find_wires_crossing_symbols(schematic_path: Path) -> List[Dict[str, Any]]: if sym["is_power"] or ref.startswith("_TEMPLATE") or not ref: continue - pin_defs = locator.get_symbol_pins(schematic_path, sym["lib_id"]) + pin_defs = lib_pin_defs.get(sym["lib_id"], {}) if not pin_defs: continue diff --git a/python/kicad_interface.py b/python/kicad_interface.py index 56dcc4c..47d5f60 100644 --- a/python/kicad_interface.py +++ b/python/kicad_interface.py @@ -2584,6 +2584,8 @@ class KiCADInterface: y1 = float(params.get("y1", 0)) x2 = float(params.get("x2", 297)) y2 = float(params.get("y2", 210)) + x1, x2 = min(x1, x2), max(x1, x2) + y1, y2 = min(y1, y2), max(y1, y2) out_format = params.get("format", "png") width = int(params.get("width", 800)) height = int(params.get("height", 600)) diff --git a/python/schemas/tool_schemas.py b/python/schemas/tool_schemas.py index ec9d70e..98804b4 100644 --- a/python/schemas/tool_schemas.py +++ b/python/schemas/tool_schemas.py @@ -1754,7 +1754,7 @@ SCHEMATIC_TOOLS = [ }, "tolerance": { "type": "number", - "description": "Distance in mm below which elements are considered overlapping (default: 0.5)" + "description": "Distance threshold in mm for label proximity and wire collinearity checks. Symbol overlap uses bounding-box intersection. (default: 0.5)" } }, "required": ["schematicPath"] diff --git a/python/tests/test_schematic_analysis.py b/python/tests/test_schematic_analysis.py index 1f00297..9ec2740 100644 --- a/python/tests/test_schematic_analysis.py +++ b/python/tests/test_schematic_analysis.py @@ -25,10 +25,12 @@ from commands.schematic_analysis import ( _parse_symbols, _parse_no_connects, _load_sexp, + _extract_lib_symbols, _line_segment_intersects_aabb, _point_in_rect, _distance, _aabb_overlap, + _check_wire_overlap, _compute_symbol_bbox_direct, compute_symbol_bbox, find_unconnected_pins, @@ -529,3 +531,167 @@ class TestIntegrationGetElementsInRegion: sym = result["symbols"][0] assert "pins" in sym assert len(sym["pins"]) == 2 # Resistor has 2 pins + + def test_wire_passing_through_region_included(self): + """A wire that passes through a region (no endpoints inside) should be included.""" + extra = """ + (wire (pts (xy 0 50) (xy 100 50)) + (stroke (width 0) (type default)) + (uuid "w-through")) + """ + tmp = _make_temp_schematic(extra) + result = get_elements_in_region(tmp, 40, 40, 60, 60) + assert result["counts"]["wires"] == 1 + + def test_wire_outside_region_excluded(self): + """A wire entirely outside a region should not be included.""" + extra = """ + (wire (pts (xy 0 0) (xy 10 0)) + (stroke (width 0) (type default)) + (uuid "w-outside")) + """ + tmp = _make_temp_schematic(extra) + result = get_elements_in_region(tmp, 40, 40, 60, 60) + assert result["counts"]["wires"] == 0 + + +# =================================================================== +# Unit tests — _check_wire_overlap +# =================================================================== + +class TestCheckWireOverlap: + """Test wire overlap detection for horizontal, vertical, and diagonal cases.""" + + def test_horizontal_overlap(self): + w1 = {"start": (10, 50), "end": (30, 50)} + w2 = {"start": (20, 50), "end": (40, 50)} + result = _check_wire_overlap(w1, w2, 0.5) + assert result is not None + assert result["type"] == "collinear_overlap" + + def test_vertical_overlap(self): + w1 = {"start": (50, 10), "end": (50, 30)} + w2 = {"start": (50, 20), "end": (50, 40)} + result = _check_wire_overlap(w1, w2, 0.5) + assert result is not None + assert result["type"] == "collinear_overlap" + + def test_diagonal_overlap(self): + w1 = {"start": (0, 0), "end": (20, 20)} + w2 = {"start": (10, 10), "end": (30, 30)} + result = _check_wire_overlap(w1, w2, 0.5) + assert result is not None + assert result["type"] == "collinear_overlap" + + def test_horizontal_no_overlap(self): + w1 = {"start": (10, 50), "end": (20, 50)} + w2 = {"start": (30, 50), "end": (40, 50)} + result = _check_wire_overlap(w1, w2, 0.5) + assert result is None + + def test_parallel_offset_no_overlap(self): + """Two parallel wires offset perpendicularly should not overlap.""" + w1 = {"start": (0, 0), "end": (20, 20)} + w2 = {"start": (0, 5), "end": (20, 25)} + result = _check_wire_overlap(w1, w2, 0.5) + assert result is None + + def test_non_parallel_no_overlap(self): + """Two wires at different angles should not overlap.""" + w1 = {"start": (0, 0), "end": (10, 10)} + w2 = {"start": (0, 0), "end": (10, 0)} + result = _check_wire_overlap(w1, w2, 0.5) + assert result is None + + def test_zero_length_segment(self): + w1 = {"start": (10, 10), "end": (10, 10)} + w2 = {"start": (10, 10), "end": (20, 20)} + result = _check_wire_overlap(w1, w2, 0.5) + assert result is None + + +@pytest.mark.integration +class TestIntegrationDiagonalWireOverlap: + """Integration tests for diagonal collinear wire overlap detection.""" + + def test_diagonal_collinear_wire_overlap(self): + """Two 45-degree wires that overlap should be detected.""" + extra = """ + (wire (pts (xy 0 0) (xy 20 20)) + (stroke (width 0) (type default)) + (uuid "w-diag1")) + (wire (pts (xy 10 10) (xy 30 30)) + (stroke (width 0) (type default)) + (uuid "w-diag2")) + """ + tmp = _make_temp_schematic(extra) + result = find_overlapping_elements(tmp, tolerance=0.5) + assert len(result["overlappingWires"]) >= 1 + + def test_diagonal_parallel_no_overlap(self): + """Two parallel 45-degree wires that are offset should not overlap.""" + extra = """ + (wire (pts (xy 0 0) (xy 20 20)) + (stroke (width 0) (type default)) + (uuid "w-diag1")) + (wire (pts (xy 0 5) (xy 20 25)) + (stroke (width 0) (type default)) + (uuid "w-diag2")) + """ + tmp = _make_temp_schematic(extra) + result = find_overlapping_elements(tmp, tolerance=0.5) + assert len(result["overlappingWires"]) == 0 + + def test_diagonal_non_collinear_no_overlap(self): + """Two wires at different angles crossing should not be flagged as collinear overlap.""" + extra = """ + (wire (pts (xy 0 0) (xy 20 20)) + (stroke (width 0) (type default)) + (uuid "w-diag1")) + (wire (pts (xy 0 20) (xy 20 0)) + (stroke (width 0) (type default)) + (uuid "w-diag2")) + """ + tmp = _make_temp_schematic(extra) + result = find_overlapping_elements(tmp, tolerance=0.5) + assert len(result["overlappingWires"]) == 0 + + +# =================================================================== +# Unit tests — _extract_lib_symbols +# =================================================================== + +class TestExtractLibSymbols: + """Test _extract_lib_symbols helper.""" + + def test_extracts_pins_from_lib_symbols(self): + sexp = sexpdata.loads("""(kicad_sch + (lib_symbols + (symbol "Device:R" + (symbol "Device:R_0_1" + (pin passive (at 0 3.81 270) (length 1.27) + (name "~" (effects (font (size 1.27 1.27)))) + (number "1" (effects (font (size 1.27 1.27))))) + (pin passive (at 0 -3.81 90) (length 1.27) + (name "~" (effects (font (size 1.27 1.27)))) + (number "2" (effects (font (size 1.27 1.27))))))) + ) + )""") + result = _extract_lib_symbols(sexp) + assert "Device:R" in result + pins = result["Device:R"] + assert "1" in pins + assert "2" in pins + assert pins["1"]["y"] == pytest.approx(3.81) + + def test_empty_schematic_returns_empty(self): + sexp = sexpdata.loads("(kicad_sch)") + result = _extract_lib_symbols(sexp) + assert result == {} + + def test_no_lib_symbols_section(self): + sexp = sexpdata.loads("""(kicad_sch + (wire (pts (xy 0 0) (xy 10 10))) + )""") + result = _extract_lib_symbols(sexp) + assert result == {} diff --git a/src/tools/schematic.ts b/src/tools/schematic.ts index 771dc57..262da24 100644 --- a/src/tools/schematic.ts +++ b/src/tools/schematic.ts @@ -1165,7 +1165,7 @@ Note: operates on .kicad_sch files only. To modify a PCB footprint use edit_comp "Detect spatially overlapping symbols, wires, and labels in the schematic. Finds duplicate power symbols at the same position, collinear overlapping wires, and labels stacked on top of each other.", { schematicPath: z.string().describe("Path to the .kicad_sch schematic file"), - tolerance: z.number().optional().describe("Distance in mm below which elements are considered overlapping (default: 0.5)"), + tolerance: z.number().optional().describe("Distance threshold in mm for label proximity and wire collinearity checks. Symbol overlap uses bounding-box intersection. (default: 0.5)"), }, async (args: { schematicPath: string; tolerance?: number }) => { const result = await callKicadScript("find_overlapping_elements", args); From dc3dc06af170db8c6a77362fde19d4bbe7f2539b Mon Sep 17 00:00:00 2001 From: Eugene Mikhantyev Date: Sun, 15 Mar 2026 14:10:11 +0000 Subject: [PATCH 09/13] feat: use real symbol body graphics for bounding boxes Parse graphical elements (rectangle, polyline, circle, arc, bezier) from lib_symbols definitions to compute accurate symbol bounding boxes instead of relying on pin positions with hardcoded degenerate expansion. This fixes bbox accuracy for ICs (previously too small), tiny 2-pin passives (previously too large), and single-pin symbols. Co-Authored-By: Claude Opus 4.6 --- python/commands/schematic_analysis.py | 194 +++++++++++++++---- python/tests/test_schematic_analysis.py | 237 +++++++++++++++++++++++- 2 files changed, 399 insertions(+), 32 deletions(-) diff --git a/python/commands/schematic_analysis.py b/python/commands/schematic_analysis.py index 01d170f..b56f427 100644 --- a/python/commands/schematic_analysis.py +++ b/python/commands/schematic_analysis.py @@ -150,13 +150,90 @@ def _parse_no_connects(sexp_data: list) -> Set[Tuple[float, float]]: return positions -def _extract_lib_symbols(sexp_data: list) -> Dict[str, Dict[str, Dict]]: +def _parse_lib_symbol_graphics(symbol_def: list) -> List[Tuple[float, float]]: + """ + Parse graphical body elements from a lib_symbol definition and return + local-coordinate bounding points. + + Extracts points from rectangle, polyline, circle, arc, and bezier + elements found in sub-symbols (typically the ``_0_1`` layers that + contain body shapes). + + Returns a list of ``(x, y)`` points in local symbol coordinates. + """ + points: List[Tuple[float, float]] = [] + + def _extract_graphics_recursive(sexp: list) -> None: + if not isinstance(sexp, list) or len(sexp) == 0: + return + + tag = sexp[0] + + if tag == Symbol("rectangle"): + # (rectangle (start x y) (end x y) ...) + for sub in sexp[1:]: + if isinstance(sub, list) and len(sub) >= 3: + if sub[0] in (Symbol("start"), Symbol("end")): + points.append((float(sub[1]), float(sub[2]))) + + elif tag == Symbol("polyline"): + # (polyline (pts (xy x y) (xy x y) ...) ...) + for sub in sexp[1:]: + if isinstance(sub, list) and len(sub) > 0 and sub[0] == Symbol("pts"): + for pt in sub[1:]: + if isinstance(pt, list) and len(pt) >= 3 and pt[0] == Symbol("xy"): + points.append((float(pt[1]), float(pt[2]))) + + elif tag == Symbol("circle"): + # (circle (center x y) (radius r) ...) + cx, cy, r = 0.0, 0.0, 0.0 + for sub in sexp[1:]: + if isinstance(sub, list) and len(sub) >= 3 and sub[0] == Symbol("center"): + cx, cy = float(sub[1]), float(sub[2]) + elif isinstance(sub, list) and len(sub) >= 2 and sub[0] == Symbol("radius"): + r = float(sub[1]) + if r > 0: + points.extend([ + (cx - r, cy - r), + (cx + r, cy + r), + ]) + + elif tag == Symbol("arc"): + # (arc (start x y) (mid x y) (end x y) ...) + for sub in sexp[1:]: + if isinstance(sub, list) and len(sub) >= 3: + if sub[0] in (Symbol("start"), Symbol("mid"), Symbol("end")): + points.append((float(sub[1]), float(sub[2]))) + + elif tag == Symbol("bezier"): + # (bezier (pts (xy x y) ...) ...) + for sub in sexp[1:]: + if isinstance(sub, list) and len(sub) > 0 and sub[0] == Symbol("pts"): + for pt in sub[1:]: + if isinstance(pt, list) and len(pt) >= 3 and pt[0] == Symbol("xy"): + points.append((float(pt[1]), float(pt[2]))) + + else: + # Recurse into sub-symbols to find graphics in nested definitions + for sub in sexp[1:]: + if isinstance(sub, list): + _extract_graphics_recursive(sub) + + # Search the top-level symbol definition and its sub-symbols + for item in symbol_def[1:]: + if isinstance(item, list): + _extract_graphics_recursive(item) + + return points + + +def _extract_lib_symbols(sexp_data: list) -> Dict[str, Dict]: """ Walk the lib_symbols section of already-parsed sexp_data and return - pin definitions for every symbol definition. + pin definitions and graphics points for every symbol definition. Returns: - Dict mapping lib_id → pin definitions (pin_number → pin_data dict). + Dict mapping lib_id → {"pins": pin_defs, "graphics_points": [(x,y), ...]}. """ lib_symbols_section = None for item in sexp_data: @@ -168,12 +245,15 @@ def _extract_lib_symbols(sexp_data: list) -> Dict[str, Dict[str, Dict]]: if not lib_symbols_section: return {} - result: Dict[str, Dict[str, Dict]] = {} + result: Dict[str, Dict] = {} for item in lib_symbols_section[1:]: if (isinstance(item, list) and len(item) > 1 and item[0] == Symbol("symbol")): symbol_name = str(item[1]).strip('"') - result[symbol_name] = PinLocator.parse_symbol_definition(item) + result[symbol_name] = { + "pins": PinLocator.parse_symbol_definition(item), + "graphics_points": _parse_lib_symbol_graphics(item), + } return result @@ -258,21 +338,48 @@ def _aabb_overlap( return a[0] < b[2] and b[0] < a[2] and a[1] < b[3] and b[1] < a[3] +def _transform_local_point( + lx: float, ly: float, + sym_x: float, sym_y: float, + rotation: float, + mirror_x: bool, mirror_y: bool, +) -> Tuple[float, float]: + """ + Transform a point from local symbol coordinates to absolute schematic + coordinates using KiCad's transform order: mirror → rotate → translate. + """ + # Apply mirroring in local coords + if mirror_x: + ly = -ly + if mirror_y: + lx = -lx + + # Apply rotation + if rotation != 0: + lx, ly = PinLocator.rotate_point(lx, ly, rotation) + + return (sym_x + lx, sym_y + ly) + + def _compute_symbol_bbox_direct( sym: Dict[str, Any], pin_defs: Dict[str, Dict], margin: float = 0.0, + graphics_points: Optional[List[Tuple[float, float]]] = None, ) -> Optional[Tuple[float, float, float, float]]: """ - Compute bounding box of a symbol from its pin definitions and placement. + Compute bounding box of a symbol from its graphics and pin definitions. - Uses _compute_pin_positions_direct to get absolute pin positions, then - expands degenerate dimensions (pins in a line) to approximate body size. + When graphics_points are available (from lib_symbol body shapes), uses + those for the bbox and unions with pin positions. Falls back to + pin-only estimation with degenerate expansion when no graphics data + is available. Args: sym: Parsed symbol dict with x, y, rotation, mirror_x, mirror_y. pin_defs: Pin definitions from PinLocator.get_symbol_pins(). margin: Shrink bbox by this amount on each side (mm). + graphics_points: Local-coordinate points from symbol body graphics. Returns (min_x, min_y, max_x, max_y) in mm, or None if no pins. """ @@ -280,20 +387,39 @@ def _compute_symbol_bbox_direct( if not pin_positions: return None - xs = [p[0] for p in pin_positions.values()] - ys = [p[1] for p in pin_positions.values()] - min_x, min_y, max_x, max_y = min(xs), min(ys), max(xs), max(ys) + if graphics_points: + # Transform graphics points to absolute coordinates + sym_x, sym_y = sym["x"], sym["y"] + rotation = sym["rotation"] + mirror_x = sym.get("mirror_x", False) + mirror_y = sym.get("mirror_y", False) - # Expand degenerate dimensions (pins in a line) to approximate body size - min_body = 1.5 # mm minimum half-extent for component body - if max_x - min_x < 2 * min_body: - cx = (min_x + max_x) / 2 - min_x = cx - min_body - max_x = cx + min_body - if max_y - min_y < 2 * min_body: - cy = (min_y + max_y) / 2 - min_y = cy - min_body - max_y = cy + min_body + abs_points = [ + _transform_local_point(lx, ly, sym_x, sym_y, rotation, mirror_x, mirror_y) + for lx, ly in graphics_points + ] + + # Union with pin positions so pins extending beyond body are included + all_xs = [p[0] for p in abs_points] + [p[0] for p in pin_positions.values()] + all_ys = [p[1] for p in abs_points] + [p[1] for p in pin_positions.values()] + + min_x, min_y = min(all_xs), min(all_ys) + max_x, max_y = max(all_xs), max(all_ys) + else: + # Fallback: pin-only estimation with degenerate expansion + xs = [p[0] for p in pin_positions.values()] + ys = [p[1] for p in pin_positions.values()] + min_x, min_y, max_x, max_y = min(xs), min(ys), max(xs), max(ys) + + min_body = 1.5 # mm minimum half-extent for component body + if max_x - min_x < 2 * min_body: + cx = (min_x + max_x) / 2 + min_x = cx - min_body + max_x = cx + min_body + if max_y - min_y < 2 * min_body: + cy = (min_y + max_y) / 2 + min_y = cy - min_body + max_y = cy + min_body # Shrink bbox by margin min_x += margin @@ -372,7 +498,7 @@ def find_unconnected_pins(schematic_path: Path) -> List[Dict[str, Any]]: return True return False - lib_pin_defs = _extract_lib_symbols(sexp_data) + lib_defs = _extract_lib_symbols(sexp_data) unconnected = [] for sym in symbols: @@ -381,7 +507,8 @@ def find_unconnected_pins(schematic_path: Path) -> List[Dict[str, Any]]: if sym["is_power"] or ref.startswith("_TEMPLATE") or not ref: continue - pin_defs = lib_pin_defs.get(sym["lib_id"], {}) + lib_data = lib_defs.get(sym["lib_id"], {}) + pin_defs = lib_data.get("pins", {}) if not pin_defs: continue @@ -434,7 +561,7 @@ def find_overlapping_elements( overlapping_labels = [] overlapping_wires = [] - lib_pin_defs = _extract_lib_symbols(sexp_data) + lib_defs = _extract_lib_symbols(sexp_data) # --- Symbol-symbol overlap using bounding-box intersection (O(n²)) --- non_template_symbols = [s for s in symbols if not s["reference"].startswith("_TEMPLATE") and s["reference"]] @@ -442,10 +569,12 @@ def find_overlapping_elements( # Pre-compute bounding boxes for all non-template symbols symbol_bboxes = [] for sym in non_template_symbols: - pin_defs = lib_pin_defs.get(sym["lib_id"], {}) + lib_data = lib_defs.get(sym["lib_id"], {}) + pin_defs = lib_data.get("pins", {}) + graphics_points = lib_data.get("graphics_points", []) bbox = None if pin_defs: - bbox = _compute_symbol_bbox_direct(sym, pin_defs) + bbox = _compute_symbol_bbox_direct(sym, pin_defs, graphics_points=graphics_points) symbol_bboxes.append((sym, bbox)) for i in range(len(symbol_bboxes)): @@ -589,7 +718,7 @@ def get_elements_in_region( wires = _parse_wires(sexp_data) labels = _parse_labels(sexp_data) - lib_pin_defs = _extract_lib_symbols(sexp_data) + lib_defs = _extract_lib_symbols(sexp_data) # Symbols: include if position is within bounds region_symbols = [] @@ -604,7 +733,8 @@ def get_elements_in_region( "isPower": sym["is_power"], } # Include pin positions (compute directly to handle unannotated duplicates) - pin_defs = lib_pin_defs.get(sym["lib_id"], {}) + lib_data = lib_defs.get(sym["lib_id"], {}) + pin_defs = lib_data.get("pins", {}) if pin_defs: pin_positions = _compute_pin_positions_direct(sym, pin_defs) if pin_positions: @@ -710,7 +840,7 @@ def find_wires_crossing_symbols(schematic_path: Path) -> List[Dict[str, Any]]: symbols = _parse_symbols(sexp_data) wires = _parse_wires(sexp_data) - lib_pin_defs = _extract_lib_symbols(sexp_data) + lib_defs = _extract_lib_symbols(sexp_data) margin = 0.5 # mm margin to shrink bbox (avoids false positives at pin tips) pin_tolerance = 0.05 # mm @@ -723,11 +853,13 @@ def find_wires_crossing_symbols(schematic_path: Path) -> List[Dict[str, Any]]: if sym["is_power"] or ref.startswith("_TEMPLATE") or not ref: continue - pin_defs = lib_pin_defs.get(sym["lib_id"], {}) + lib_data = lib_defs.get(sym["lib_id"], {}) + pin_defs = lib_data.get("pins", {}) if not pin_defs: continue - bbox = _compute_symbol_bbox_direct(sym, pin_defs, margin=margin) + graphics_points = lib_data.get("graphics_points", []) + bbox = _compute_symbol_bbox_direct(sym, pin_defs, margin=margin, graphics_points=graphics_points) if bbox is None: continue diff --git a/python/tests/test_schematic_analysis.py b/python/tests/test_schematic_analysis.py index 9ec2740..4f31446 100644 --- a/python/tests/test_schematic_analysis.py +++ b/python/tests/test_schematic_analysis.py @@ -26,6 +26,8 @@ from commands.schematic_analysis import ( _parse_no_connects, _load_sexp, _extract_lib_symbols, + _parse_lib_symbol_graphics, + _transform_local_point, _line_segment_intersects_aabb, _point_in_rect, _distance, @@ -679,7 +681,7 @@ class TestExtractLibSymbols: )""") result = _extract_lib_symbols(sexp) assert "Device:R" in result - pins = result["Device:R"] + pins = result["Device:R"]["pins"] assert "1" in pins assert "2" in pins assert pins["1"]["y"] == pytest.approx(3.81) @@ -695,3 +697,236 @@ class TestExtractLibSymbols: )""") result = _extract_lib_symbols(sexp) assert result == {} + + def test_extract_includes_graphics_points(self): + """_extract_lib_symbols should return graphics_points from body shapes.""" + sexp = sexpdata.loads("""(kicad_sch + (lib_symbols + (symbol "Device:R" + (symbol "Device:R_0_1" + (rectangle (start -1.016 -2.54) (end 1.016 2.54) + (stroke (width 0.254) (type default)) + (fill (type none)))) + (symbol "Device:R_1_1" + (pin passive line (at 0 3.81 270) (length 1.27) + (name "~" (effects (font (size 1.27 1.27)))) + (number "1" (effects (font (size 1.27 1.27))))) + (pin passive line (at 0 -3.81 90) (length 1.27) + (name "~" (effects (font (size 1.27 1.27)))) + (number "2" (effects (font (size 1.27 1.27))))))) + ) + )""") + result = _extract_lib_symbols(sexp) + lib_data = result["Device:R"] + assert "graphics_points" in lib_data + gfx = lib_data["graphics_points"] + assert len(gfx) >= 2 + # Rectangle corners should be present + xs = [p[0] for p in gfx] + ys = [p[1] for p in gfx] + assert pytest.approx(-1.016) in xs + assert pytest.approx(1.016) in xs + assert pytest.approx(-2.54) in ys + assert pytest.approx(2.54) in ys + + +# =================================================================== +# Unit tests — _parse_lib_symbol_graphics +# =================================================================== + +class TestParseLibSymbolGraphics: + """Test graphics extraction from lib_symbol definitions.""" + + def test_rectangle(self): + sexp = sexpdata.loads("""(symbol "Device:R" + (symbol "Device:R_0_1" + (rectangle (start -1.016 -2.54) (end 1.016 2.54) + (stroke (width 0.254) (type default)) + (fill (type none)))))""") + pts = _parse_lib_symbol_graphics(sexp) + assert len(pts) == 2 + assert (-1.016, -2.54) in pts + assert (1.016, 2.54) in pts + + def test_polyline(self): + sexp = sexpdata.loads("""(symbol "Device:C" + (symbol "Device:C_0_1" + (polyline + (pts (xy -2.032 -0.762) (xy 2.032 -0.762)) + (stroke (width 0.508) (type default)) + (fill (type none)))))""") + pts = _parse_lib_symbol_graphics(sexp) + assert (-2.032, -0.762) in pts + assert (2.032, -0.762) in pts + + def test_circle(self): + sexp = sexpdata.loads("""(symbol "Test:Circle" + (symbol "Test:Circle_0_1" + (circle (center 0 0) (radius 5) + (stroke (width 0.254) (type default)) + (fill (type none)))))""") + pts = _parse_lib_symbol_graphics(sexp) + assert len(pts) == 2 + assert (-5.0, -5.0) in pts + assert (5.0, 5.0) in pts + + def test_arc(self): + sexp = sexpdata.loads("""(symbol "Test:Arc" + (symbol "Test:Arc_0_1" + (arc (start 1 0) (mid 0 1) (end -1 0) + (stroke (width 0.254) (type default)) + (fill (type none)))))""") + pts = _parse_lib_symbol_graphics(sexp) + assert (1.0, 0.0) in pts + assert (0.0, 1.0) in pts + assert (-1.0, 0.0) in pts + + def test_no_graphics(self): + sexp = sexpdata.loads("""(symbol "Test:Empty" + (symbol "Test:Empty_1_1" + (pin passive line (at 0 0 0) (length 1.27) + (name "~" (effects (font (size 1.27 1.27)))) + (number "1" (effects (font (size 1.27 1.27)))))))""") + pts = _parse_lib_symbol_graphics(sexp) + assert pts == [] + + +# =================================================================== +# Unit tests — _transform_local_point +# =================================================================== + +class TestTransformLocalPoint: + """Test local→absolute coordinate transform.""" + + def test_no_transform(self): + x, y = _transform_local_point(1.0, 2.0, 100.0, 200.0, 0, False, False) + assert x == pytest.approx(101.0) + assert y == pytest.approx(202.0) + + def test_mirror_x(self): + x, y = _transform_local_point(1.0, 2.0, 0.0, 0.0, 0, True, False) + assert x == pytest.approx(1.0) + assert y == pytest.approx(-2.0) + + def test_mirror_y(self): + x, y = _transform_local_point(1.0, 2.0, 0.0, 0.0, 0, False, True) + assert x == pytest.approx(-1.0) + assert y == pytest.approx(2.0) + + def test_rotation_90(self): + x, y = _transform_local_point(1.0, 0.0, 0.0, 0.0, 90, False, False) + assert x == pytest.approx(0.0, abs=1e-9) + assert y == pytest.approx(1.0, abs=1e-9) + + +# =================================================================== +# Unit tests — _compute_symbol_bbox_direct with graphics +# =================================================================== + +class TestComputeSymbolBboxWithGraphics: + """Test that bounding box computation uses graphics points when available.""" + + def test_resistor_bbox_from_graphics(self): + """Device:R rectangle is (-1.016, -2.54) to (1.016, 2.54) in local coords. + Pins at (0, ±3.81). Placed at (100, 100) with no rotation. + Bbox should span from pin-to-pin in Y and use rectangle width in X.""" + sym = {"x": 100.0, "y": 100.0, "rotation": 0, "mirror_x": False, "mirror_y": False} + pin_defs = { + "1": {"x": 0, "y": 3.81, "angle": 270, "length": 1.27, "name": "~", "type": "passive"}, + "2": {"x": 0, "y": -3.81, "angle": 90, "length": 1.27, "name": "~", "type": "passive"}, + } + graphics_points = [(-1.016, -2.54), (1.016, 2.54)] + + bbox = _compute_symbol_bbox_direct(sym, pin_defs, graphics_points=graphics_points) + assert bbox is not None + min_x, min_y, max_x, max_y = bbox + # X should come from rectangle: 100 ± 1.016 + assert min_x == pytest.approx(100 - 1.016) + assert max_x == pytest.approx(100 + 1.016) + # Y should come from pins (extending beyond rectangle): 100 ± 3.81 + assert min_y == pytest.approx(100 - 3.81) + assert max_y == pytest.approx(100 + 3.81) + + def test_fallback_without_graphics(self): + """Without graphics_points, should use the old degenerate expansion.""" + sym = {"x": 100.0, "y": 100.0, "rotation": 0, "mirror_x": False, "mirror_y": False} + pin_defs = { + "1": {"x": 0, "y": 3.81, "angle": 270, "length": 1.27, "name": "~", "type": "passive"}, + "2": {"x": 0, "y": -3.81, "angle": 90, "length": 1.27, "name": "~", "type": "passive"}, + } + + bbox = _compute_symbol_bbox_direct(sym, pin_defs) + assert bbox is not None + min_x, min_y, max_x, max_y = bbox + # X should be expanded with min_body=1.5: 100 ± 1.5 + assert min_x == pytest.approx(100 - 1.5) + assert max_x == pytest.approx(100 + 1.5) + + def test_rotated_symbol_graphics(self): + """Graphics points should be rotated along with the symbol.""" + sym = {"x": 100.0, "y": 100.0, "rotation": 90, "mirror_x": False, "mirror_y": False} + pin_defs = { + "1": {"x": 0, "y": 3.81, "angle": 270, "length": 1.27, "name": "~", "type": "passive"}, + "2": {"x": 0, "y": -3.81, "angle": 90, "length": 1.27, "name": "~", "type": "passive"}, + } + # Rectangle corners in local coords + graphics_points = [(-1.016, -2.54), (1.016, 2.54)] + + bbox = _compute_symbol_bbox_direct(sym, pin_defs, graphics_points=graphics_points) + assert bbox is not None + min_x, min_y, max_x, max_y = bbox + # After 90° rotation, X and Y swap roles + # Pins now extend along X: 100 ± 3.81 + # Rectangle now extends along Y: 100 ± 1.016 + assert min_x == pytest.approx(100 - 3.81, abs=0.01) + assert max_x == pytest.approx(100 + 3.81, abs=0.01) + + +@pytest.mark.integration +class TestIntegrationGraphicsBbox: + """Integration tests verifying graphics-based bbox from real template data.""" + + def test_resistor_bbox_uses_rectangle(self): + """The template's Device:R has a rectangle body. + Verify that the bbox for a placed resistor uses the actual + rectangle width rather than the degenerate 1.5mm expansion.""" + extra = _make_resistor_sexp("R1", 100, 100) + tmp = _make_temp_schematic(extra) + sexp_data = _load_sexp(tmp) + symbols = _parse_symbols(sexp_data) + lib_defs = _extract_lib_symbols(sexp_data) + + r1 = [s for s in symbols if s["reference"] == "R1"][0] + lib_data = lib_defs.get(r1["lib_id"], {}) + pin_defs = lib_data.get("pins", {}) + graphics_points = lib_data.get("graphics_points", []) + + assert len(graphics_points) >= 2, "Should have extracted rectangle points" + + bbox = _compute_symbol_bbox_direct(r1, pin_defs, graphics_points=graphics_points) + assert bbox is not None + min_x, min_y, max_x, max_y = bbox + # Rectangle is ±1.016 in X, NOT ±1.5 from degenerate expansion + assert max_x - min_x == pytest.approx(2 * 1.016, abs=0.01) + + def test_led_bbox_uses_polyline(self): + """The template's Device:LED uses polylines for its body. + Verify that the bbox uses polyline extents.""" + extra = _make_led_sexp("D1", 100, 100) + tmp = _make_temp_schematic(extra) + sexp_data = _load_sexp(tmp) + symbols = _parse_symbols(sexp_data) + lib_defs = _extract_lib_symbols(sexp_data) + + d1 = [s for s in symbols if s["reference"] == "D1"][0] + lib_data = lib_defs.get(d1["lib_id"], {}) + graphics_points = lib_data.get("graphics_points", []) + + assert len(graphics_points) >= 4, "Should have extracted polyline points" + # LED body polylines span from -1.27 to 1.27 in both X and Y + xs = [p[0] for p in graphics_points] + ys = [p[1] for p in graphics_points] + assert min(xs) == pytest.approx(-1.27) + assert max(xs) == pytest.approx(1.27) + assert min(ys) == pytest.approx(-1.27) + assert max(ys) == pytest.approx(1.27) From 1ef4ce5cab9541c6811bae095212c420f78c450f Mon Sep 17 00:00:00 2001 From: Eugene Mikhantyev Date: Sun, 15 Mar 2026 14:15:46 +0000 Subject: [PATCH 10/13] refactor: remove find_unconnected_pins tool (redundant with run_erc) KiCad's native ERC already checks for unconnected pins with better accuracy (hierarchical sheets, bus connections, custom rules). Remove the reimplemented version and its dead helper _parse_no_connects. Co-Authored-By: Claude Opus 4.6 --- python/commands/schematic_analysis.py | 115 ------------------------ python/kicad_interface.py | 24 ----- python/schemas/tool_schemas.py | 15 ---- python/tests/test_schematic_analysis.py | 50 ----------- src/tools/schematic.ts | 23 ----- 5 files changed, 227 deletions(-) diff --git a/python/commands/schematic_analysis.py b/python/commands/schematic_analysis.py index b56f427..7eab32b 100644 --- a/python/commands/schematic_analysis.py +++ b/python/commands/schematic_analysis.py @@ -135,20 +135,6 @@ def _parse_symbols(sexp_data: list) -> List[Dict[str, Any]]: return symbols -def _parse_no_connects(sexp_data: list) -> Set[Tuple[float, float]]: - """Parse all no_connect elements and return their positions as (x, y) tuples in mm.""" - positions: Set[Tuple[float, float]] = set() - for item in sexp_data: - if not isinstance(item, list) or len(item) < 2: - continue - if item[0] != Symbol("no_connect"): - continue - for sub in item: - if isinstance(sub, list) and len(sub) >= 3 and sub[0] == Symbol("at"): - positions.add((float(sub[1]), float(sub[2]))) - break - return positions - def _parse_lib_symbol_graphics(symbol_def: list) -> List[Tuple[float, float]]: """ @@ -434,107 +420,6 @@ def _compute_symbol_bbox_direct( return (min_x, min_y, max_x, max_y) -# --------------------------------------------------------------------------- -# Tool 2: find_unconnected_pins -# --------------------------------------------------------------------------- - -def find_unconnected_pins(schematic_path: Path) -> List[Dict[str, Any]]: - """ - Find all component pins with no wire, label, or power symbol touching them. - - Returns list of dicts: {reference, libId, pinNumber, pinName, position: {x, y}} - """ - sexp_data = _load_sexp(schematic_path) - symbols = _parse_symbols(sexp_data) - wires = _parse_wires(sexp_data) - labels = _parse_labels(sexp_data) - no_connects = _parse_no_connects(sexp_data) - - # Build set of "connected" positions in mm - connected: Set[Tuple[float, float]] = set() - - # Wire endpoints - for w in wires: - connected.add(w["start"]) - connected.add(w["end"]) - - # Label positions - for lbl in labels: - connected.add((lbl["x"], lbl["y"])) - - # Power symbol positions (they implicitly connect) - for sym in symbols: - if sym["is_power"]: - connected.add((sym["x"], sym["y"])) - - tolerance = 0.05 # mm - - def _snap(v: float) -> int: - """Snap coordinate to grid for O(1) set lookup.""" - return round(v / tolerance) - - connected_grid: set = set() - for pos in connected: - connected_grid.add((_snap(pos[0]), _snap(pos[1]))) - - no_connect_grid: set = set() - for pos in no_connects: - no_connect_grid.add((_snap(pos[0]), _snap(pos[1]))) - - def is_connected(px: float, py: float) -> bool: - sx, sy = _snap(px), _snap(py) - # Check the snapped cell and immediate neighbors to handle edge cases - for dx in (-1, 0, 1): - for dy in (-1, 0, 1): - if (sx + dx, sy + dy) in connected_grid: - return True - return False - - def is_no_connect(px: float, py: float) -> bool: - sx, sy = _snap(px), _snap(py) - for dx in (-1, 0, 1): - for dy in (-1, 0, 1): - if (sx + dx, sy + dy) in no_connect_grid: - return True - return False - - lib_defs = _extract_lib_symbols(sexp_data) - unconnected = [] - - for sym in symbols: - ref = sym["reference"] - # Skip power symbols, templates, and empty references - if sym["is_power"] or ref.startswith("_TEMPLATE") or not ref: - continue - - lib_data = lib_defs.get(sym["lib_id"], {}) - pin_defs = lib_data.get("pins", {}) - if not pin_defs: - continue - - pin_positions = _compute_pin_positions_direct(sym, pin_defs) - if not pin_positions: - continue - - for pin_num, pos in pin_positions.items(): - px, py = pos[0], pos[1] - - if is_no_connect(px, py): - continue - if is_connected(px, py): - continue - - pin_name = pin_defs.get(pin_num, {}).get("name", pin_num) - unconnected.append({ - "reference": ref, - "libId": sym["lib_id"], - "pinNumber": pin_num, - "pinName": pin_name, - "position": {"x": round(px, 4), "y": round(py, 4)}, - }) - - return unconnected - # --------------------------------------------------------------------------- # Tool 3: find_overlapping_elements diff --git a/python/kicad_interface.py b/python/kicad_interface.py index 47d5f60..2c5ef16 100644 --- a/python/kicad_interface.py +++ b/python/kicad_interface.py @@ -400,7 +400,6 @@ class KiCADInterface: "export_schematic_svg": self._handle_export_schematic_svg, # Schematic analysis tools (read-only) "get_schematic_view_region": self._handle_get_schematic_view_region, - "find_unconnected_pins": self._handle_find_unconnected_pins, "find_overlapping_elements": self._handle_find_overlapping_elements, "get_elements_in_region": self._handle_get_elements_in_region, "find_wires_crossing_symbols": self._handle_find_wires_crossing_symbols, @@ -2660,29 +2659,6 @@ class KiCADInterface: logger.error(traceback.format_exc()) return {"success": False, "message": str(e)} - def _handle_find_unconnected_pins(self, params): - """Find all component pins with no wire, label, or power symbol touching them""" - logger.info("Finding unconnected pins in schematic") - try: - from pathlib import Path - 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(Path(schematic_path)) - return { - "success": True, - "unconnectedPins": result, - "count": len(result), - "message": f"Found {len(result)} unconnected pin(s)", - } - except Exception as e: - logger.error(f"Error finding unconnected pins: {e}") - import traceback - logger.error(traceback.format_exc()) - return {"success": False, "message": str(e)} def _handle_find_overlapping_elements(self, params): """Detect spatially overlapping symbols, wires, and labels""" diff --git a/python/schemas/tool_schemas.py b/python/schemas/tool_schemas.py index 98804b4..6eb3c4c 100644 --- a/python/schemas/tool_schemas.py +++ b/python/schemas/tool_schemas.py @@ -1726,21 +1726,6 @@ SCHEMATIC_TOOLS = [ "required": ["schematicPath", "x1", "y1", "x2", "y2"] } }, - { - "name": "find_unconnected_pins", - "title": "Find Unconnected Pins", - "description": "Lists all component pins in the schematic that have no wire, label, or power symbol touching them. Useful for checking connectivity before running ERC. Skips power symbols, template symbols, and pins with no_connect flags.", - "inputSchema": { - "type": "object", - "properties": { - "schematicPath": { - "type": "string", - "description": "Path to the .kicad_sch schematic file" - } - }, - "required": ["schematicPath"] - } - }, { "name": "find_overlapping_elements", "title": "Find Overlapping Elements", diff --git a/python/tests/test_schematic_analysis.py b/python/tests/test_schematic_analysis.py index 4f31446..b287f07 100644 --- a/python/tests/test_schematic_analysis.py +++ b/python/tests/test_schematic_analysis.py @@ -23,7 +23,6 @@ from commands.schematic_analysis import ( _parse_wires, _parse_labels, _parse_symbols, - _parse_no_connects, _load_sexp, _extract_lib_symbols, _parse_lib_symbol_graphics, @@ -35,7 +34,6 @@ from commands.schematic_analysis import ( _check_wire_overlap, _compute_symbol_bbox_direct, compute_symbol_bbox, - find_unconnected_pins, find_overlapping_elements, get_elements_in_region, find_wires_crossing_symbols, @@ -220,16 +218,6 @@ class TestSexpParsers: assert symbols[1]["reference"] == "#PWR01" assert symbols[1]["is_power"] is True - def test_parse_no_connects(self): - sexp = sexpdata.loads("""(kicad_sch - (no_connect (at 10 20) (uuid "x")) - (no_connect (at 30 40) (uuid "y")) - )""") - nc = _parse_no_connects(sexp) - assert (10.0, 20.0) in nc - assert (30.0, 40.0) in nc - assert len(nc) == 2 - # =================================================================== # Unit tests — analysis functions with mocked PinLocator @@ -376,44 +364,6 @@ class TestComputeSymbolBbox: # Integration tests — full schematic parsing # =================================================================== -@pytest.mark.integration -class TestIntegrationFindUnconnectedPins: - """Integration test using real schematic files.""" - - def test_component_with_no_wires_has_unconnected_pins(self): - """A resistor placed with no wires should have 2 unconnected pins.""" - extra = _make_resistor_sexp("R1", 100, 100) - tmp = _make_temp_schematic(extra) - result = find_unconnected_pins(tmp) - r1_pins = [p for p in result if p["reference"] == "R1"] - assert len(r1_pins) == 2 - - def test_pin_with_wire_is_connected(self): - """A wire endpoint exactly at a pin position should mark it connected.""" - # R1 at (100,100), rotation 0 → pin 1 at (100, 103.81), pin 2 at (100, 96.19) - extra = _make_resistor_sexp("R1", 100, 100) + """ - (wire (pts (xy 100 103.81) (xy 100 120)) - (stroke (width 0) (type default)) - (uuid "w1")) - """ - tmp = _make_temp_schematic(extra) - result = find_unconnected_pins(tmp) - r1_pins = [p for p in result if p["reference"] == "R1"] - # Pin 1 should be connected (wire at 100, 103.81), pin 2 still unconnected - assert len(r1_pins) == 1 - assert r1_pins[0]["pinNumber"] == "2" - - def test_no_connect_suppresses_pin(self): - """A no_connect at a pin position should not report it as unconnected.""" - extra = _make_resistor_sexp("R1", 100, 100) + """ - (no_connect (at 100 96.19) (uuid "nc1")) - (no_connect (at 100 103.81) (uuid "nc2")) - """ - tmp = _make_temp_schematic(extra) - result = find_unconnected_pins(tmp) - r1_pins = [p for p in result if p["reference"] == "R1"] - assert len(r1_pins) == 0 - @pytest.mark.integration class TestIntegrationFindWiresCrossingSymbols: diff --git a/src/tools/schematic.ts b/src/tools/schematic.ts index 262da24..ef8fb67 100644 --- a/src/tools/schematic.ts +++ b/src/tools/schematic.ts @@ -1135,29 +1135,6 @@ Note: operates on .kicad_sch files only. To modify a PCB footprint use edit_comp }, ); - // Find unconnected pins - server.tool( - "find_unconnected_pins", - "List all component pins in the schematic that have no wire, label, or power symbol touching them. Useful for checking connectivity before running ERC.", - { - schematicPath: z.string().describe("Path to the .kicad_sch schematic file"), - }, - async (args: { schematicPath: string }) => { - const result = await callKicadScript("find_unconnected_pins", args); - if (result.success) { - const pins: any[] = result.unconnectedPins || []; - const lines = [`Found ${pins.length} unconnected pin(s):`]; - pins.slice(0, 50).forEach((p: any) => { - lines.push(` ${p.reference} pin ${p.pinNumber} (${p.pinName}) @ (${p.position.x}, ${p.position.y})`); - }); - if (pins.length > 50) lines.push(` ... and ${pins.length - 50} more`); - return { content: [{ type: "text", text: lines.join("\n") }] }; - } - return { - content: [{ type: "text", text: `Failed: ${result.message || "Unknown error"}` }], - }; - }, - ); // Find overlapping elements server.tool( From be11948a44ddd60c1f15869335520c6183dc3824 Mon Sep 17 00:00:00 2001 From: Eugene Mikhantyev Date: Sun, 15 Mar 2026 14:37:35 +0000 Subject: [PATCH 11/13] fix: negate y-axis in graphics transform for correct symbol bounding boxes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Library symbols use y-up coordinates while schematics use y-down. The _transform_local_point function was not negating y, causing asymmetric symbols (e.g. power:VEE) to have their bounding boxes computed in the wrong direction — missing overlaps with adjacent components. Co-Authored-By: Claude Opus 4.6 --- python/commands/schematic_analysis.py | 6 +++++- python/tests/test_schematic_analysis.py | 9 ++++++--- 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/python/commands/schematic_analysis.py b/python/commands/schematic_analysis.py index 7eab32b..487db15 100644 --- a/python/commands/schematic_analysis.py +++ b/python/commands/schematic_analysis.py @@ -332,8 +332,12 @@ def _transform_local_point( ) -> Tuple[float, float]: """ Transform a point from local symbol coordinates to absolute schematic - coordinates using KiCad's transform order: mirror → rotate → translate. + coordinates using KiCad's transform order: + negate-y (lib y-up → schematic y-down) → mirror → rotate → translate. """ + # Library symbols use y-up; schematic uses y-down + ly = -ly + # Apply mirroring in local coords if mirror_x: ly = -ly diff --git a/python/tests/test_schematic_analysis.py b/python/tests/test_schematic_analysis.py index b287f07..c7c4bcd 100644 --- a/python/tests/test_schematic_analysis.py +++ b/python/tests/test_schematic_analysis.py @@ -749,21 +749,24 @@ class TestTransformLocalPoint: """Test local→absolute coordinate transform.""" def test_no_transform(self): + # ly is negated (lib y-up → schematic y-down) x, y = _transform_local_point(1.0, 2.0, 100.0, 200.0, 0, False, False) assert x == pytest.approx(101.0) - assert y == pytest.approx(202.0) + assert y == pytest.approx(198.0) def test_mirror_x(self): + # y-negate then mirror_x cancel out → net ly unchanged x, y = _transform_local_point(1.0, 2.0, 0.0, 0.0, 0, True, False) assert x == pytest.approx(1.0) - assert y == pytest.approx(-2.0) + assert y == pytest.approx(2.0) def test_mirror_y(self): x, y = _transform_local_point(1.0, 2.0, 0.0, 0.0, 0, False, True) assert x == pytest.approx(-1.0) - assert y == pytest.approx(2.0) + assert y == pytest.approx(-2.0) def test_rotation_90(self): + # ly=0 negated is still 0, then rotate lx=1 by 90° x, y = _transform_local_point(1.0, 0.0, 0.0, 0.0, 90, False, False) assert x == pytest.approx(0.0, abs=1e-9) assert y == pytest.approx(1.0, abs=1e-9) From 59bd4c4acfd4545c1c365db3dcad47ac3ba1f949 Mon Sep 17 00:00:00 2001 From: Eugene Mikhantyev Date: Sun, 15 Mar 2026 23:17:25 +0000 Subject: [PATCH 12/13] style: apply Black formatting to changed files Co-Authored-By: Claude Sonnet 4.6 --- python/commands/schematic_analysis.py | 215 ++++-- python/kicad_interface.py | 36 +- python/schemas/tool_schemas.py | 954 +++++++++++------------- python/tests/conftest.py | 1 + python/tests/test_schematic_analysis.py | 135 +++- 5 files changed, 731 insertions(+), 610 deletions(-) diff --git a/python/commands/schematic_analysis.py b/python/commands/schematic_analysis.py index 487db15..09d284d 100644 --- a/python/commands/schematic_analysis.py +++ b/python/commands/schematic_analysis.py @@ -22,6 +22,7 @@ logger = logging.getLogger("kicad_interface") # S-expression parsing helpers # --------------------------------------------------------------------------- + def _load_sexp(schematic_path: Path) -> list: """Load schematic file and return parsed S-expression data.""" with open(schematic_path, "r", encoding="utf-8") as f: @@ -122,20 +123,21 @@ def _parse_symbols(sexp_data: list) -> List[Dict[str, Any]]: reference = str(sub[2]).strip('"') is_power = reference.startswith("#PWR") or reference.startswith("#FLG") - symbols.append({ - "reference": reference, - "lib_id": lib_id, - "x": x, - "y": y, - "rotation": rotation, - "mirror_x": mirror_x, - "mirror_y": mirror_y, - "is_power": is_power, - }) + symbols.append( + { + "reference": reference, + "lib_id": lib_id, + "x": x, + "y": y, + "rotation": rotation, + "mirror_x": mirror_x, + "mirror_y": mirror_y, + "is_power": is_power, + } + ) return symbols - def _parse_lib_symbol_graphics(symbol_def: list) -> List[Tuple[float, float]]: """ Parse graphical body elements from a lib_symbol definition and return @@ -167,22 +169,36 @@ def _parse_lib_symbol_graphics(symbol_def: list) -> List[Tuple[float, float]]: for sub in sexp[1:]: if isinstance(sub, list) and len(sub) > 0 and sub[0] == Symbol("pts"): for pt in sub[1:]: - if isinstance(pt, list) and len(pt) >= 3 and pt[0] == Symbol("xy"): + if ( + isinstance(pt, list) + and len(pt) >= 3 + and pt[0] == Symbol("xy") + ): points.append((float(pt[1]), float(pt[2]))) elif tag == Symbol("circle"): # (circle (center x y) (radius r) ...) cx, cy, r = 0.0, 0.0, 0.0 for sub in sexp[1:]: - if isinstance(sub, list) and len(sub) >= 3 and sub[0] == Symbol("center"): + if ( + isinstance(sub, list) + and len(sub) >= 3 + and sub[0] == Symbol("center") + ): cx, cy = float(sub[1]), float(sub[2]) - elif isinstance(sub, list) and len(sub) >= 2 and sub[0] == Symbol("radius"): + elif ( + isinstance(sub, list) + and len(sub) >= 2 + and sub[0] == Symbol("radius") + ): r = float(sub[1]) if r > 0: - points.extend([ - (cx - r, cy - r), - (cx + r, cy + r), - ]) + points.extend( + [ + (cx - r, cy - r), + (cx + r, cy + r), + ] + ) elif tag == Symbol("arc"): # (arc (start x y) (mid x y) (end x y) ...) @@ -196,7 +212,11 @@ def _parse_lib_symbol_graphics(symbol_def: list) -> List[Tuple[float, float]]: for sub in sexp[1:]: if isinstance(sub, list) and len(sub) > 0 and sub[0] == Symbol("pts"): for pt in sub[1:]: - if isinstance(pt, list) and len(pt) >= 3 and pt[0] == Symbol("xy"): + if ( + isinstance(pt, list) + and len(pt) >= 3 + and pt[0] == Symbol("xy") + ): points.append((float(pt[1]), float(pt[2]))) else: @@ -223,8 +243,11 @@ def _extract_lib_symbols(sexp_data: list) -> Dict[str, Dict]: """ lib_symbols_section = None for item in sexp_data: - if (isinstance(item, list) and len(item) > 0 - and item[0] == Symbol("lib_symbols")): + if ( + isinstance(item, list) + and len(item) > 0 + and item[0] == Symbol("lib_symbols") + ): lib_symbols_section = item break @@ -233,8 +256,7 @@ def _extract_lib_symbols(sexp_data: list) -> Dict[str, Dict]: result: Dict[str, Dict] = {} for item in lib_symbols_section[1:]: - if (isinstance(item, list) and len(item) > 1 - and item[0] == Symbol("symbol")): + if isinstance(item, list) and len(item) > 1 and item[0] == Symbol("symbol"): symbol_name = str(item[1]).strip('"') result[symbol_name] = { "pins": PinLocator.parse_symbol_definition(item), @@ -247,6 +269,7 @@ def _extract_lib_symbols(sexp_data: list) -> Dict[str, Dict]: # Geometry helpers # --------------------------------------------------------------------------- + def compute_symbol_bbox( schematic_path: Path, reference: str, @@ -266,8 +289,14 @@ def compute_symbol_bbox( def _line_segment_intersects_aabb( - x1: float, y1: float, x2: float, y2: float, - box_min_x: float, box_min_y: float, box_max_x: float, box_max_y: float, + x1: float, + y1: float, + x2: float, + y2: float, + box_min_x: float, + box_min_y: float, + box_max_x: float, + box_max_y: float, ) -> bool: """ Test whether line segment (x1,y1)→(x2,y2) intersects an axis-aligned bounding box. @@ -301,8 +330,12 @@ def _line_segment_intersects_aabb( def _point_in_rect( - px: float, py: float, - min_x: float, min_y: float, max_x: float, max_y: float, + px: float, + py: float, + min_x: float, + min_y: float, + max_x: float, + max_y: float, ) -> bool: """Check if a point is within a rectangle.""" return min_x <= px <= max_x and min_y <= py <= max_y @@ -325,10 +358,13 @@ def _aabb_overlap( def _transform_local_point( - lx: float, ly: float, - sym_x: float, sym_y: float, + lx: float, + ly: float, + sym_x: float, + sym_y: float, rotation: float, - mirror_x: bool, mirror_y: bool, + mirror_x: bool, + mirror_y: bool, ) -> Tuple[float, float]: """ Transform a point from local symbol coordinates to absolute schematic @@ -424,11 +460,11 @@ def _compute_symbol_bbox_direct( return (min_x, min_y, max_x, max_y) - # --------------------------------------------------------------------------- # Tool 3: find_overlapping_elements # --------------------------------------------------------------------------- + def find_overlapping_elements( schematic_path: Path, tolerance: float = 0.5 ) -> Dict[str, Any]: @@ -453,7 +489,11 @@ def find_overlapping_elements( lib_defs = _extract_lib_symbols(sexp_data) # --- Symbol-symbol overlap using bounding-box intersection (O(n²)) --- - non_template_symbols = [s for s in symbols if not s["reference"].startswith("_TEMPLATE") and s["reference"]] + non_template_symbols = [ + s + for s in symbols + if not s["reference"].startswith("_TEMPLATE") and s["reference"] + ] # Pre-compute bounding boxes for all non-template symbols symbol_bboxes = [] @@ -463,7 +503,9 @@ def find_overlapping_elements( graphics_points = lib_data.get("graphics_points", []) bbox = None if pin_defs: - bbox = _compute_symbol_bbox_direct(sym, pin_defs, graphics_points=graphics_points) + bbox = _compute_symbol_bbox_direct( + sym, pin_defs, graphics_points=graphics_points + ) symbol_bboxes.append((sym, bbox)) for i in range(len(symbol_bboxes)): @@ -482,10 +524,16 @@ def find_overlapping_elements( if overlap_detected: entry = { - "element1": {"reference": s1["reference"], "libId": s1["lib_id"], - "position": {"x": s1["x"], "y": s1["y"]}}, - "element2": {"reference": s2["reference"], "libId": s2["lib_id"], - "position": {"x": s2["x"], "y": s2["y"]}}, + "element1": { + "reference": s1["reference"], + "libId": s1["lib_id"], + "position": {"x": s1["x"], "y": s1["y"]}, + }, + "element2": { + "reference": s2["reference"], + "libId": s2["lib_id"], + "position": {"x": s2["x"], "y": s2["y"]}, + }, "distance": round(dist, 4), } # Flag power symbol pairs specifically @@ -502,13 +550,21 @@ def find_overlapping_elements( l2 = labels[j] dist = _distance((l1["x"], l1["y"]), (l2["x"], l2["y"])) if dist < tolerance: - overlapping_labels.append({ - "element1": {"name": l1["name"], "type": l1["type"], - "position": {"x": l1["x"], "y": l1["y"]}}, - "element2": {"name": l2["name"], "type": l2["type"], - "position": {"x": l2["x"], "y": l2["y"]}}, - "distance": round(dist, 4), - }) + overlapping_labels.append( + { + "element1": { + "name": l1["name"], + "type": l1["type"], + "position": {"x": l1["x"], "y": l1["y"]}, + }, + "element2": { + "name": l2["name"], + "type": l2["type"], + "position": {"x": l2["x"], "y": l2["y"]}, + }, + "distance": round(dist, 4), + } + ) # --- Wire-wire collinear overlap --- for i in range(len(wires)): @@ -574,8 +630,14 @@ def _check_wire_overlap( min2, max2 = min(proj_s2, proj_e2), max(proj_s2, proj_e2) if min1 < max2 and min2 < max1: return { - "wire1": {"start": {"x": s1[0], "y": s1[1]}, "end": {"x": e1[0], "y": e1[1]}}, - "wire2": {"start": {"x": s2[0], "y": s2[1]}, "end": {"x": e2[0], "y": e2[1]}}, + "wire1": { + "start": {"x": s1[0], "y": s1[1]}, + "end": {"x": e1[0], "y": e1[1]}, + }, + "wire2": { + "start": {"x": s2[0], "y": s2[1]}, + "end": {"x": e2[0], "y": e2[1]}, + }, "type": "collinear_overlap", } @@ -586,9 +648,13 @@ def _check_wire_overlap( # Tool 4: get_elements_in_region # --------------------------------------------------------------------------- + def get_elements_in_region( schematic_path: Path, - x1: float, y1: float, x2: float, y2: float, + x1: float, + y1: float, + x2: float, + y2: float, ) -> Dict[str, Any]: """ List all wires, labels, and symbols within a rectangular region. @@ -637,23 +703,31 @@ def get_elements_in_region( region_wires = [] for w in wires: s, e = w["start"], w["end"] - if (_point_in_rect(s[0], s[1], min_x, min_y, max_x, max_y) or - _point_in_rect(e[0], e[1], min_x, min_y, max_x, max_y) or - _line_segment_intersects_aabb(s[0], s[1], e[0], e[1], min_x, min_y, max_x, max_y)): - region_wires.append({ - "start": {"x": s[0], "y": s[1]}, - "end": {"x": e[0], "y": e[1]}, - }) + if ( + _point_in_rect(s[0], s[1], min_x, min_y, max_x, max_y) + or _point_in_rect(e[0], e[1], min_x, min_y, max_x, max_y) + or _line_segment_intersects_aabb( + s[0], s[1], e[0], e[1], min_x, min_y, max_x, max_y + ) + ): + region_wires.append( + { + "start": {"x": s[0], "y": s[1]}, + "end": {"x": e[0], "y": e[1]}, + } + ) # Labels: include if position is within bounds region_labels = [] for lbl in labels: if _point_in_rect(lbl["x"], lbl["y"], min_x, min_y, max_x, max_y): - region_labels.append({ - "name": lbl["name"], - "type": lbl["type"], - "position": {"x": lbl["x"], "y": lbl["y"]}, - }) + region_labels.append( + { + "name": lbl["name"], + "type": lbl["type"], + "position": {"x": lbl["x"], "y": lbl["y"]}, + } + ) return { "symbols": region_symbols, @@ -671,6 +745,7 @@ def get_elements_in_region( # Tool 5: check_wire_collisions # --------------------------------------------------------------------------- + def _compute_pin_positions_direct( sym: Dict[str, Any], pin_defs: Dict[str, Dict] ) -> Dict[str, List[float]]: @@ -748,7 +823,9 @@ def find_wires_crossing_symbols(schematic_path: Path) -> List[Dict[str, Any]]: continue graphics_points = lib_data.get("graphics_points", []) - bbox = _compute_symbol_bbox_direct(sym, pin_defs, margin=margin, graphics_points=graphics_points) + bbox = _compute_symbol_bbox_direct( + sym, pin_defs, margin=margin, graphics_points=graphics_points + ) if bbox is None: continue @@ -757,11 +834,13 @@ def find_wires_crossing_symbols(schematic_path: Path) -> List[Dict[str, Any]]: for pos in pin_positions.values(): pin_set.add((pos[0], pos[1])) - symbol_data.append({ - "sym": sym, - "bbox": bbox, - "pin_set": pin_set, - }) + symbol_data.append( + { + "sym": sym, + "bbox": bbox, + "pin_set": pin_set, + } + ) # Test each wire against each symbol bbox for w in wires: @@ -810,7 +889,8 @@ def find_wires_crossing_symbols(schematic_path: Path) -> List[Dict[str, Any]]: continue # Wire terminates at pin from outside sym = sd["sym"] - collisions.append({ + collisions.append( + { "wire": { "start": {"x": sx, "y": sy}, "end": {"x": ex, "y": ey}, @@ -821,6 +901,7 @@ def find_wires_crossing_symbols(schematic_path: Path) -> List[Dict[str, Any]]: "position": {"x": sym["x"], "y": sym["y"]}, }, "intersectionType": "passes_through", - }) + } + ) return collisions diff --git a/python/kicad_interface.py b/python/kicad_interface.py index 2c5ef16..7a92cbe 100644 --- a/python/kicad_interface.py +++ b/python/kicad_interface.py @@ -2597,19 +2597,34 @@ class KiCADInterface: svg_output = None try: - cmd = [kicad_cli, "sch", "export", "svg", "--output", tmp_dir, schematic_path] + cmd = [ + kicad_cli, + "sch", + "export", + "svg", + "--output", + tmp_dir, + schematic_path, + ] result = subprocess.run(cmd, capture_output=True, text=True, timeout=60) if result.returncode != 0: - return {"success": False, "message": f"SVG export failed: {result.stderr}"} + return { + "success": False, + "message": f"SVG export failed: {result.stderr}", + } # kicad-cli names the file after the schematic svg_files = [f for f in os.listdir(tmp_dir) if f.endswith(".svg")] if not svg_files: - return {"success": False, "message": "kicad-cli produced no SVG output"} + return { + "success": False, + "message": "kicad-cli produced no SVG output", + } svg_output = os.path.join(tmp_dir, svg_files[0]) import xml.etree.ElementTree as ET + tree = ET.parse(svg_output) root = tree.getroot() @@ -2642,8 +2657,13 @@ class KiCADInterface: try: from cairosvg import svg2png except ImportError: - return {"success": False, "message": "PNG export requires the 'cairosvg' package. Install it with: pip install cairosvg"} - png_data = svg2png(url=cropped_svg_path, output_width=width, output_height=height) + return { + "success": False, + "message": "PNG export requires the 'cairosvg' package. Install it with: pip install cairosvg", + } + png_data = svg2png( + url=cropped_svg_path, output_width=width, output_height=height + ) return { "success": True, "imageData": base64.b64encode(png_data).decode("utf-8"), @@ -2651,15 +2671,16 @@ class KiCADInterface: } finally: import shutil + shutil.rmtree(tmp_dir, ignore_errors=True) except Exception as e: logger.error(f"Error in get_schematic_view_region: {e}") import traceback + logger.error(traceback.format_exc()) return {"success": False, "message": str(e)} - def _handle_find_overlapping_elements(self, params): """Detect spatially overlapping symbols, wires, and labels""" logger.info("Finding overlapping elements in schematic") @@ -2681,6 +2702,7 @@ class KiCADInterface: except Exception as e: logger.error(f"Error finding overlapping elements: {e}") import traceback + logger.error(traceback.format_exc()) return {"success": False, "message": str(e)} @@ -2709,6 +2731,7 @@ class KiCADInterface: except Exception as e: logger.error(f"Error getting elements in region: {e}") import traceback + logger.error(traceback.format_exc()) return {"success": False, "message": str(e)} @@ -2733,6 +2756,7 @@ class KiCADInterface: except Exception as e: logger.error(f"Error checking wire collisions: {e}") import traceback + logger.error(traceback.format_exc()) return {"success": False, "message": str(e)} diff --git a/python/schemas/tool_schemas.py b/python/schemas/tool_schemas.py index 6eb3c4c..d71a50c 100644 --- a/python/schemas/tool_schemas.py +++ b/python/schemas/tool_schemas.py @@ -27,19 +27,19 @@ PROJECT_TOOLS = [ "projectName": { "type": "string", "description": "Name of the project (used for file naming)", - "minLength": 1 + "minLength": 1, }, "path": { "type": "string", - "description": "Directory path where project will be created (defaults to current working directory)" + "description": "Directory path where project will be created (defaults to current working directory)", }, "template": { "type": "string", - "description": "Optional path to template board file to copy settings from" - } + "description": "Optional path to template board file to copy settings from", + }, }, - "required": ["projectName"] - } + "required": ["projectName"], + }, }, { "name": "open_project", @@ -50,11 +50,11 @@ PROJECT_TOOLS = [ "properties": { "filename": { "type": "string", - "description": "Path to .kicad_pro or .kicad_pcb file" + "description": "Path to .kicad_pro or .kicad_pcb file", } }, - "required": ["filename"] - } + "required": ["filename"], + }, }, { "name": "save_project", @@ -65,10 +65,10 @@ PROJECT_TOOLS = [ "properties": { "filename": { "type": "string", - "description": "Optional new path to save the board (if not provided, saves to current location)" + "description": "Optional new path to save the board (if not provided, saves to current location)", } - } - } + }, + }, }, { "name": "snapshot_project", @@ -79,28 +79,25 @@ PROJECT_TOOLS = [ "properties": { "step": { "type": "string", - "description": "Step number or name to include in snapshot folder name, e.g. '1' or '2'" + "description": "Step number or name to include in snapshot folder name, e.g. '1' or '2'", }, "label": { "type": "string", - "description": "Optional short label, e.g. 'schematic_ok' or 'layout_ok'" + "description": "Optional short label, e.g. 'schematic_ok' or 'layout_ok'", }, "projectPath": { "type": "string", - "description": "Project directory path. Auto-detected from loaded board if omitted." - } - } - } + "description": "Project directory path. Auto-detected from loaded board if omitted.", + }, + }, + }, }, { "name": "get_project_info", "title": "Get Project Information", "description": "Retrieves metadata and properties of the currently open project including name, paths, and board status.", - "inputSchema": { - "type": "object", - "properties": {} - } - } + "inputSchema": {"type": "object", "properties": {}}, + }, ] # ============================================================================= @@ -118,16 +115,16 @@ BOARD_TOOLS = [ "width": { "type": "number", "description": "Board width in millimeters", - "minimum": 1 + "minimum": 1, }, "height": { "type": "number", "description": "Board height in millimeters", - "minimum": 1 - } + "minimum": 1, + }, }, - "required": ["width", "height"] - } + "required": ["width", "height"], + }, }, { "name": "add_board_outline", @@ -139,44 +136,47 @@ BOARD_TOOLS = [ "shape": { "type": "string", "enum": ["rectangle", "rounded_rectangle", "circle", "polygon"], - "description": "Shape type for the board outline" + "description": "Shape type for the board outline", }, "width": { "type": "number", "description": "Width in mm (for rectangle/rounded_rectangle)", - "minimum": 1 + "minimum": 1, }, "height": { "type": "number", "description": "Height in mm (for rectangle/rounded_rectangle)", - "minimum": 1 + "minimum": 1, }, "x": { "type": "number", - "description": "X coordinate of the top-left corner in mm (default: 0). Board extends from x to x+width." + "description": "X coordinate of the top-left corner in mm (default: 0). Board extends from x to x+width.", }, "y": { "type": "number", - "description": "Y coordinate of the top-left corner in mm (default: 0). Board extends from y to y+height." + "description": "Y coordinate of the top-left corner in mm (default: 0). Board extends from y to y+height.", }, "radius": { "type": "number", "description": "Corner radius in mm for rounded_rectangle, or radius for circle", - "minimum": 0 + "minimum": 0, }, "points": { "type": "array", "description": "Array of {x, y} point objects in mm (for polygon shape only)", "items": { "type": "object", - "properties": {"x": {"type": "number"}, "y": {"type": "number"}}, - "required": ["x", "y"] + "properties": { + "x": {"type": "number"}, + "y": {"type": "number"}, + }, + "required": ["x", "y"], }, - "minItems": 3 - } + "minItems": 3, + }, }, - "required": ["shape"] - } + "required": ["shape"], + }, }, { "name": "add_layer", @@ -187,16 +187,16 @@ BOARD_TOOLS = [ "properties": { "layerName": { "type": "string", - "description": "Name of the layer to add" + "description": "Name of the layer to add", }, "layerType": { "type": "string", "enum": ["signal", "power", "mixed", "jumper"], - "description": "Type of layer (for copper layers)" - } + "description": "Type of layer (for copper layers)", + }, }, - "required": ["layerName"] - } + "required": ["layerName"], + }, }, { "name": "set_active_layer", @@ -207,29 +207,23 @@ BOARD_TOOLS = [ "properties": { "layerName": { "type": "string", - "description": "Name of the layer to make active (e.g., F.Cu, B.Cu, Edge.Cuts)" + "description": "Name of the layer to make active (e.g., F.Cu, B.Cu, Edge.Cuts)", } }, - "required": ["layerName"] - } + "required": ["layerName"], + }, }, { "name": "get_layer_list", "title": "List Board Layers", "description": "Returns a list of all layers in the board with their properties.", - "inputSchema": { - "type": "object", - "properties": {} - } + "inputSchema": {"type": "object", "properties": {}}, }, { "name": "get_board_info", "title": "Get Board Information", "description": "Retrieves comprehensive board information including dimensions, layer count, component count, and design rules.", - "inputSchema": { - "type": "object", - "properties": {} - } + "inputSchema": {"type": "object", "properties": {}}, }, { "name": "get_board_2d_view", @@ -242,16 +236,16 @@ BOARD_TOOLS = [ "type": "number", "description": "Image width in pixels (default: 800)", "minimum": 100, - "default": 800 + "default": 800, }, "height": { "type": "number", "description": "Image height in pixels (default: 600)", "minimum": 100, - "default": 600 - } - } - } + "default": 600, + }, + }, + }, }, { "name": "get_board_extents", @@ -264,10 +258,10 @@ BOARD_TOOLS = [ "type": "string", "enum": ["mm", "inch"], "description": "Unit for returned coordinates (default: mm)", - "default": "mm" + "default": "mm", } - } - } + }, + }, }, { "name": "add_mounting_hole", @@ -276,22 +270,16 @@ BOARD_TOOLS = [ "inputSchema": { "type": "object", "properties": { - "x": { - "type": "number", - "description": "X coordinate in millimeters" - }, - "y": { - "type": "number", - "description": "Y coordinate in millimeters" - }, + "x": {"type": "number", "description": "X coordinate in millimeters"}, + "y": {"type": "number", "description": "Y coordinate in millimeters"}, "diameter": { "type": "number", "description": "Hole diameter in millimeters", - "minimum": 0.1 - } + "minimum": 0.1, + }, }, - "required": ["x", "y", "diameter"] - } + "required": ["x", "y", "diameter"], + }, }, { "name": "import_svg_logo", @@ -302,43 +290,43 @@ BOARD_TOOLS = [ "properties": { "pcbPath": { "type": "string", - "description": "Path to the .kicad_pcb file" + "description": "Path to the .kicad_pcb file", }, "svgPath": { "type": "string", - "description": "Path to the SVG logo file" + "description": "Path to the SVG logo file", }, "x": { "type": "number", - "description": "X position of the logo top-left corner in mm" + "description": "X position of the logo top-left corner in mm", }, "y": { "type": "number", - "description": "Y position of the logo top-left corner in mm" + "description": "Y position of the logo top-left corner in mm", }, "width": { "type": "number", "description": "Target width of the logo in mm (height scaled to preserve aspect ratio)", - "minimum": 0.1 + "minimum": 0.1, }, "layer": { "type": "string", "description": "PCB layer name, e.g. F.SilkS or B.SilkS (default: F.SilkS)", - "default": "F.SilkS" + "default": "F.SilkS", }, "strokeWidth": { "type": "number", "description": "Outline stroke width in mm (0 = no outline, default 0)", - "default": 0 + "default": 0, }, "filled": { "type": "boolean", "description": "Fill polygons with solid layer colour (default true)", - "default": True - } + "default": True, + }, }, - "required": ["pcbPath", "svgPath", "x", "y", "width"] - } + "required": ["pcbPath", "svgPath", "x", "y", "width"], + }, }, { "name": "add_board_text", @@ -350,37 +338,31 @@ BOARD_TOOLS = [ "text": { "type": "string", "description": "Text content to add", - "minLength": 1 - }, - "x": { - "type": "number", - "description": "X coordinate in millimeters" - }, - "y": { - "type": "number", - "description": "Y coordinate in millimeters" + "minLength": 1, }, + "x": {"type": "number", "description": "X coordinate in millimeters"}, + "y": {"type": "number", "description": "Y coordinate in millimeters"}, "layer": { "type": "string", "description": "Layer name (e.g., F.SilkS, B.SilkS, F.Cu)", - "default": "F.SilkS" + "default": "F.SilkS", }, "size": { "type": "number", "description": "Text size in millimeters", "minimum": 0.1, - "default": 1.0 + "default": 1.0, }, "thickness": { "type": "number", "description": "Text thickness in millimeters", "minimum": 0.01, - "default": 0.15 - } + "default": 0.15, + }, }, - "required": ["text", "x", "y"] - } - } + "required": ["text", "x", "y"], + }, + }, ] # ============================================================================= @@ -397,36 +379,30 @@ COMPONENT_TOOLS = [ "properties": { "reference": { "type": "string", - "description": "Component reference designator (e.g., R1, C2, U3)" + "description": "Component reference designator (e.g., R1, C2, U3)", }, "footprint": { "type": "string", - "description": "Footprint library:name (e.g., Resistor_SMD:R_0805_2012Metric)" - }, - "x": { - "type": "number", - "description": "X coordinate in millimeters" - }, - "y": { - "type": "number", - "description": "Y coordinate in millimeters" + "description": "Footprint library:name (e.g., Resistor_SMD:R_0805_2012Metric)", }, + "x": {"type": "number", "description": "X coordinate in millimeters"}, + "y": {"type": "number", "description": "Y coordinate in millimeters"}, "rotation": { "type": "number", "description": "Rotation angle in degrees (0-360)", "minimum": 0, "maximum": 360, - "default": 0 + "default": 0, }, "layer": { "type": "string", "enum": ["F.Cu", "B.Cu"], "description": "Board layer (top or bottom)", - "default": "F.Cu" - } + "default": "F.Cu", + }, }, - "required": ["reference", "footprint", "x", "y"] - } + "required": ["reference", "footprint", "x", "y"], + }, }, { "name": "move_component", @@ -437,19 +413,19 @@ COMPONENT_TOOLS = [ "properties": { "reference": { "type": "string", - "description": "Component reference designator" + "description": "Component reference designator", }, "x": { "type": "number", - "description": "New X coordinate in millimeters" + "description": "New X coordinate in millimeters", }, "y": { "type": "number", - "description": "New Y coordinate in millimeters" - } + "description": "New Y coordinate in millimeters", + }, }, - "required": ["reference", "x", "y"] - } + "required": ["reference", "x", "y"], + }, }, { "name": "rotate_component", @@ -460,15 +436,15 @@ COMPONENT_TOOLS = [ "properties": { "reference": { "type": "string", - "description": "Component reference designator" + "description": "Component reference designator", }, "angle": { "type": "number", - "description": "Rotation angle in degrees (positive = counterclockwise)" - } + "description": "Rotation angle in degrees (positive = counterclockwise)", + }, }, - "required": ["reference", "angle"] - } + "required": ["reference", "angle"], + }, }, { "name": "delete_component", @@ -479,11 +455,11 @@ COMPONENT_TOOLS = [ "properties": { "reference": { "type": "string", - "description": "Component reference designator" + "description": "Component reference designator", } }, - "required": ["reference"] - } + "required": ["reference"], + }, }, { "name": "edit_component", @@ -494,19 +470,16 @@ COMPONENT_TOOLS = [ "properties": { "reference": { "type": "string", - "description": "Component reference designator" - }, - "value": { - "type": "string", - "description": "New component value" + "description": "Component reference designator", }, + "value": {"type": "string", "description": "New component value"}, "footprint": { "type": "string", - "description": "New footprint library:name" - } + "description": "New footprint library:name", + }, }, - "required": ["reference"] - } + "required": ["reference"], + }, }, { "name": "get_component_properties", @@ -517,20 +490,17 @@ COMPONENT_TOOLS = [ "properties": { "reference": { "type": "string", - "description": "Component reference designator" + "description": "Component reference designator", } }, - "required": ["reference"] - } + "required": ["reference"], + }, }, { "name": "get_component_list", "title": "List All Components", "description": "Returns a list of all components on the board with their properties.", - "inputSchema": { - "type": "object", - "properties": {} - } + "inputSchema": {"type": "object", "properties": {}}, }, { "name": "find_component", @@ -541,18 +511,18 @@ COMPONENT_TOOLS = [ "properties": { "reference": { "type": "string", - "description": "Reference designator pattern to match (e.g., 'R1', 'U', 'C2')" + "description": "Reference designator pattern to match (e.g., 'R1', 'U', 'C2')", }, "value": { "type": "string", - "description": "Value pattern to match (e.g., '10k', '100nF')" + "description": "Value pattern to match (e.g., '10k', '100nF')", }, "footprint": { "type": "string", - "description": "Footprint pattern to match (e.g., '0805', 'SOIC')" - } - } - } + "description": "Footprint pattern to match (e.g., '0805', 'SOIC')", + }, + }, + }, }, { "name": "get_component_pads", @@ -563,11 +533,11 @@ COMPONENT_TOOLS = [ "properties": { "reference": { "type": "string", - "description": "Component reference designator (e.g., U1, R5)" + "description": "Component reference designator (e.g., U1, R5)", } }, - "required": ["reference"] - } + "required": ["reference"], + }, }, { "name": "get_pad_position", @@ -578,19 +548,19 @@ COMPONENT_TOOLS = [ "properties": { "reference": { "type": "string", - "description": "Component reference designator" + "description": "Component reference designator", }, "padName": { "type": "string", - "description": "Pad name or number (e.g., '1', '2', 'A1')" + "description": "Pad name or number (e.g., '1', '2', 'A1')", }, "padNumber": { "type": "string", - "description": "Alternative to padName - pad number" - } + "description": "Alternative to padName - pad number", + }, }, - "required": ["reference"] - } + "required": ["reference"], + }, }, { "name": "place_component_array", @@ -601,61 +571,68 @@ COMPONENT_TOOLS = [ "properties": { "referencePrefix": { "type": "string", - "description": "Reference prefix (e.g., 'R' for R1, R2, R3...)" + "description": "Reference prefix (e.g., 'R' for R1, R2, R3...)", }, "startNumber": { "type": "integer", "description": "Starting number for references", "minimum": 1, - "default": 1 + "default": 1, }, "footprint": { "type": "string", - "description": "Footprint library:name" + "description": "Footprint library:name", }, "pattern": { "type": "string", "enum": ["grid", "circular"], - "description": "Array pattern type" + "description": "Array pattern type", }, "count": { "type": "integer", "description": "Total number of components to place", - "minimum": 1 + "minimum": 1, }, "startX": { "type": "number", - "description": "Starting X coordinate in millimeters" + "description": "Starting X coordinate in millimeters", }, "startY": { "type": "number", - "description": "Starting Y coordinate in millimeters" + "description": "Starting Y coordinate in millimeters", }, "spacingX": { "type": "number", - "description": "Horizontal spacing in mm (for grid pattern)" + "description": "Horizontal spacing in mm (for grid pattern)", }, "spacingY": { "type": "number", - "description": "Vertical spacing in mm (for grid pattern)" + "description": "Vertical spacing in mm (for grid pattern)", }, "radius": { "type": "number", - "description": "Circle radius in mm (for circular pattern)" + "description": "Circle radius in mm (for circular pattern)", }, "rows": { "type": "integer", "description": "Number of rows (for grid pattern)", - "minimum": 1 + "minimum": 1, }, "columns": { "type": "integer", "description": "Number of columns (for grid pattern)", - "minimum": 1 - } + "minimum": 1, + }, }, - "required": ["referencePrefix", "footprint", "pattern", "count", "startX", "startY"] - } + "required": [ + "referencePrefix", + "footprint", + "pattern", + "count", + "startX", + "startY", + ], + }, }, { "name": "align_components", @@ -668,20 +645,20 @@ COMPONENT_TOOLS = [ "type": "array", "description": "Array of component reference designators to align", "items": {"type": "string"}, - "minItems": 2 + "minItems": 2, }, "direction": { "type": "string", "enum": ["horizontal", "vertical"], - "description": "Alignment direction" + "description": "Alignment direction", }, "spacing": { "type": "number", - "description": "Spacing between components in mm (optional, for even distribution)" - } + "description": "Spacing between components in mm (optional, for even distribution)", + }, }, - "required": ["references", "direction"] - } + "required": ["references", "direction"], + }, }, { "name": "duplicate_component", @@ -692,26 +669,26 @@ COMPONENT_TOOLS = [ "properties": { "sourceReference": { "type": "string", - "description": "Reference of component to duplicate" + "description": "Reference of component to duplicate", }, "newReference": { "type": "string", - "description": "Reference designator for the new component" + "description": "Reference designator for the new component", }, "offsetX": { "type": "number", "description": "X offset from original position in mm", - "default": 0 + "default": 0, }, "offsetY": { "type": "number", "description": "Y offset from original position in mm", - "default": 0 - } + "default": 0, + }, }, - "required": ["sourceReference", "newReference"] - } - } + "required": ["sourceReference", "newReference"], + }, + }, ] # ============================================================================= @@ -729,15 +706,15 @@ ROUTING_TOOLS = [ "netName": { "type": "string", "description": "Name of the net (e.g., VCC, GND, SDA)", - "minLength": 1 + "minLength": 1, }, "netClass": { "type": "string", - "description": "Optional net class to assign (must exist first)" - } + "description": "Optional net class to assign (must exist first)", + }, }, - "required": ["netName"] - } + "required": ["netName"], + }, }, { "name": "route_trace", @@ -746,19 +723,16 @@ ROUTING_TOOLS = [ "inputSchema": { "type": "object", "properties": { - "netName": { - "type": "string", - "description": "Net name for this trace" - }, + "netName": {"type": "string", "description": "Net name for this trace"}, "layer": { "type": "string", "description": "Layer to route on (e.g., F.Cu, B.Cu)", - "default": "F.Cu" + "default": "F.Cu", }, "width": { "type": "number", "description": "Trace width in millimeters", - "minimum": 0.1 + "minimum": 0.1, }, "points": { "type": "array", @@ -767,13 +741,13 @@ ROUTING_TOOLS = [ "type": "array", "items": {"type": "number"}, "minItems": 2, - "maxItems": 2 + "maxItems": 2, }, - "minItems": 2 - } + "minItems": 2, + }, }, - "required": ["points", "width"] - } + "required": ["points", "width"], + }, }, { "name": "add_via", @@ -782,31 +756,25 @@ ROUTING_TOOLS = [ "inputSchema": { "type": "object", "properties": { - "x": { - "type": "number", - "description": "X coordinate in millimeters" - }, - "y": { - "type": "number", - "description": "Y coordinate in millimeters" - }, + "x": {"type": "number", "description": "X coordinate in millimeters"}, + "y": {"type": "number", "description": "Y coordinate in millimeters"}, "diameter": { "type": "number", "description": "Via diameter in millimeters", - "minimum": 0.1 + "minimum": 0.1, }, "drill": { "type": "number", "description": "Drill diameter in millimeters", - "minimum": 0.1 + "minimum": 0.1, }, "netName": { "type": "string", - "description": "Net name to assign to this via" - } + "description": "Net name to assign to this via", + }, }, - "required": ["x", "y", "diameter", "drill"] - } + "required": ["x", "y", "diameter", "drill"], + }, }, { "name": "delete_trace", @@ -817,7 +785,7 @@ ROUTING_TOOLS = [ "properties": { "uuid": { "type": "string", - "description": "UUID of a specific trace to delete" + "description": "UUID of a specific trace to delete", }, "position": { "type": "object", @@ -825,25 +793,29 @@ ROUTING_TOOLS = [ "properties": { "x": {"type": "number", "description": "X coordinate"}, "y": {"type": "number", "description": "Y coordinate"}, - "unit": {"type": "string", "enum": ["mm", "inch"], "default": "mm"} + "unit": { + "type": "string", + "enum": ["mm", "inch"], + "default": "mm", + }, }, - "required": ["x", "y"] + "required": ["x", "y"], }, "net": { "type": "string", - "description": "Delete all traces on this net (bulk delete)" + "description": "Delete all traces on this net (bulk delete)", }, "layer": { "type": "string", - "description": "Filter by layer when using net-based deletion" + "description": "Filter by layer when using net-based deletion", }, "includeVias": { "type": "boolean", "description": "Include vias in net-based deletion", - "default": False - } - } - } + "default": False, + }, + }, + }, }, { "name": "query_traces", @@ -854,11 +826,11 @@ ROUTING_TOOLS = [ "properties": { "net": { "type": "string", - "description": "Filter by net name (e.g., 'GND', 'VCC')" + "description": "Filter by net name (e.g., 'GND', 'VCC')", }, "layer": { "type": "string", - "description": "Filter by layer name (e.g., 'F.Cu', 'B.Cu')" + "description": "Filter by layer name (e.g., 'F.Cu', 'B.Cu')", }, "boundingBox": { "type": "object", @@ -868,16 +840,20 @@ ROUTING_TOOLS = [ "y1": {"type": "number", "description": "Top Y coordinate"}, "x2": {"type": "number", "description": "Right X coordinate"}, "y2": {"type": "number", "description": "Bottom Y coordinate"}, - "unit": {"type": "string", "enum": ["mm", "inch"], "default": "mm"} - } + "unit": { + "type": "string", + "enum": ["mm", "inch"], + "default": "mm", + }, + }, }, "includeVias": { "type": "boolean", "description": "Include vias in the result", - "default": False - } - } - } + "default": False, + }, + }, + }, }, { "name": "modify_trace", @@ -888,7 +864,7 @@ ROUTING_TOOLS = [ "properties": { "uuid": { "type": "string", - "description": "UUID of the trace to modify" + "description": "UUID of the trace to modify", }, "position": { "type": "object", @@ -896,24 +872,22 @@ ROUTING_TOOLS = [ "properties": { "x": {"type": "number", "description": "X coordinate"}, "y": {"type": "number", "description": "Y coordinate"}, - "unit": {"type": "string", "enum": ["mm", "inch"], "default": "mm"} + "unit": { + "type": "string", + "enum": ["mm", "inch"], + "default": "mm", + }, }, - "required": ["x", "y"] - }, - "width": { - "type": "number", - "description": "New trace width in mm" + "required": ["x", "y"], }, + "width": {"type": "number", "description": "New trace width in mm"}, "layer": { "type": "string", - "description": "New layer name (e.g., 'F.Cu', 'B.Cu')" + "description": "New layer name (e.g., 'F.Cu', 'B.Cu')", }, - "net": { - "type": "string", - "description": "New net name to assign" - } - } - } + "net": {"type": "string", "description": "New net name to assign"}, + }, + }, }, { "name": "copy_routing_pattern", @@ -925,34 +899,31 @@ ROUTING_TOOLS = [ "sourceRefs": { "type": "array", "items": {"type": "string"}, - "description": "Source component references (e.g., ['U1', 'U2', 'U3'])" + "description": "Source component references (e.g., ['U1', 'U2', 'U3'])", }, "targetRefs": { "type": "array", "items": {"type": "string"}, - "description": "Target component references (e.g., ['U4', 'U5', 'U6'])" + "description": "Target component references (e.g., ['U4', 'U5', 'U6'])", }, "includeVias": { "type": "boolean", "description": "Include vias in the pattern copy", - "default": True + "default": True, }, "traceWidth": { "type": "number", - "description": "Override trace width in mm (uses original if not specified)" - } + "description": "Override trace width in mm (uses original if not specified)", + }, }, - "required": ["sourceRefs", "targetRefs"] - } + "required": ["sourceRefs", "targetRefs"], + }, }, { "name": "get_nets_list", "title": "List All Nets", "description": "Returns a list of all electrical nets defined on the board.", - "inputSchema": { - "type": "object", - "properties": {} - } + "inputSchema": {"type": "object", "properties": {}}, }, { "name": "create_netclass", @@ -964,29 +935,29 @@ ROUTING_TOOLS = [ "name": { "type": "string", "description": "Net class name", - "minLength": 1 + "minLength": 1, }, "traceWidth": { "type": "number", "description": "Default trace width in millimeters", - "minimum": 0.1 + "minimum": 0.1, }, "clearance": { "type": "number", "description": "Clearance in millimeters", - "minimum": 0.1 + "minimum": 0.1, }, "viaDiameter": { "type": "number", - "description": "Via diameter in millimeters" + "description": "Via diameter in millimeters", }, "viaDrill": { "type": "number", - "description": "Via drill diameter in millimeters" - } + "description": "Via drill diameter in millimeters", + }, }, - "required": ["name", "traceWidth", "clearance"] - } + "required": ["name", "traceWidth", "clearance"], + }, }, { "name": "add_copper_pour", @@ -997,22 +968,22 @@ ROUTING_TOOLS = [ "properties": { "netName": { "type": "string", - "description": "Net to connect this copper pour to (e.g., GND, VCC)" + "description": "Net to connect this copper pour to (e.g., GND, VCC)", }, "layer": { "type": "string", - "description": "Layer for the copper pour (e.g., F.Cu, B.Cu)" + "description": "Layer for the copper pour (e.g., F.Cu, B.Cu)", }, "priority": { "type": "integer", "description": "Pour priority (higher priorities fill first)", "minimum": 0, - "default": 0 + "default": 0, }, "clearance": { "type": "number", "description": "Clearance from other objects in millimeters", - "minimum": 0.1 + "minimum": 0.1, }, "outline": { "type": "array", @@ -1021,13 +992,13 @@ ROUTING_TOOLS = [ "type": "array", "items": {"type": "number"}, "minItems": 2, - "maxItems": 2 + "maxItems": 2, }, - "minItems": 3 - } + "minItems": 3, + }, }, - "required": ["netName", "layer", "outline"] - } + "required": ["netName", "layer", "outline"], + }, }, { "name": "route_differential_pair", @@ -1038,23 +1009,20 @@ ROUTING_TOOLS = [ "properties": { "positiveName": { "type": "string", - "description": "Positive signal net name" + "description": "Positive signal net name", }, "negativeName": { "type": "string", - "description": "Negative signal net name" - }, - "layer": { - "type": "string", - "description": "Layer to route on" + "description": "Negative signal net name", }, + "layer": {"type": "string", "description": "Layer to route on"}, "width": { "type": "number", - "description": "Trace width in millimeters" + "description": "Trace width in millimeters", }, "gap": { "type": "number", - "description": "Gap between traces in millimeters" + "description": "Gap between traces in millimeters", }, "points": { "type": "array", @@ -1063,14 +1031,14 @@ ROUTING_TOOLS = [ "type": "array", "items": {"type": "number"}, "minItems": 2, - "maxItems": 2 + "maxItems": 2, }, - "minItems": 2 - } + "minItems": 2, + }, }, - "required": ["positiveName", "negativeName", "width", "gap", "points"] - } - } + "required": ["positiveName", "negativeName", "width", "gap", "points"], + }, + }, ] # ============================================================================= @@ -1082,10 +1050,7 @@ LIBRARY_TOOLS = [ "name": "list_libraries", "title": "List Footprint Libraries", "description": "Lists all available footprint libraries accessible to KiCAD.", - "inputSchema": { - "type": "object", - "properties": {} - } + "inputSchema": {"type": "object", "properties": {}}, }, { "name": "search_footprints", @@ -1097,15 +1062,15 @@ LIBRARY_TOOLS = [ "query": { "type": "string", "description": "Search query (e.g., '0805', 'SOIC', 'QFP')", - "minLength": 1 + "minLength": 1, }, "library": { "type": "string", - "description": "Optional library to restrict search to" - } + "description": "Optional library to restrict search to", + }, }, - "required": ["query"] - } + "required": ["query"], + }, }, { "name": "list_library_footprints", @@ -1117,11 +1082,11 @@ LIBRARY_TOOLS = [ "library": { "type": "string", "description": "Library name (e.g., Resistor_SMD, Connector_PinHeader)", - "minLength": 1 + "minLength": 1, } }, - "required": ["library"] - } + "required": ["library"], + }, }, { "name": "get_footprint_info", @@ -1130,18 +1095,12 @@ LIBRARY_TOOLS = [ "inputSchema": { "type": "object", "properties": { - "library": { - "type": "string", - "description": "Library name" - }, - "footprint": { - "type": "string", - "description": "Footprint name" - } + "library": {"type": "string", "description": "Library name"}, + "footprint": {"type": "string", "description": "Footprint name"}, }, - "required": ["library", "footprint"] - } - } + "required": ["library", "footprint"], + }, + }, ] # ============================================================================= @@ -1159,36 +1118,33 @@ DESIGN_RULE_TOOLS = [ "clearance": { "type": "number", "description": "Minimum clearance between copper in millimeters", - "minimum": 0.1 + "minimum": 0.1, }, "trackWidth": { "type": "number", "description": "Minimum track width in millimeters", - "minimum": 0.1 + "minimum": 0.1, }, "viaDiameter": { "type": "number", - "description": "Minimum via diameter in millimeters" + "description": "Minimum via diameter in millimeters", }, "viaDrill": { "type": "number", - "description": "Minimum via drill diameter in millimeters" + "description": "Minimum via drill diameter in millimeters", }, "microViaD iameter": { "type": "number", - "description": "Minimum micro-via diameter in millimeters" - } - } - } + "description": "Minimum micro-via diameter in millimeters", + }, + }, + }, }, { "name": "get_design_rules", "title": "Get Current Design Rules", "description": "Retrieves the currently configured design rules from the board.", - "inputSchema": { - "type": "object", - "properties": {} - } + "inputSchema": {"type": "object", "properties": {}}, }, { "name": "run_drc", @@ -1200,20 +1156,17 @@ DESIGN_RULE_TOOLS = [ "includeWarnings": { "type": "boolean", "description": "Include warnings in addition to errors", - "default": True + "default": True, } - } - } + }, + }, }, { "name": "get_drc_violations", "title": "Get DRC Violations", "description": "Returns a list of design rule violations from the most recent DRC run.", - "inputSchema": { - "type": "object", - "properties": {} - } - } + "inputSchema": {"type": "object", "properties": {}}, + }, ] # ============================================================================= @@ -1230,21 +1183,21 @@ EXPORT_TOOLS = [ "properties": { "outputPath": { "type": "string", - "description": "Directory path for output files" + "description": "Directory path for output files", }, "layers": { "type": "array", "description": "List of layers to export (if not provided, exports all copper and mask layers)", - "items": {"type": "string"} + "items": {"type": "string"}, }, "includeDrillFiles": { "type": "boolean", "description": "Include drill files (Excellon format)", - "default": True - } + "default": True, + }, }, - "required": ["outputPath"] - } + "required": ["outputPath"], + }, }, { "name": "export_pdf", @@ -1255,22 +1208,22 @@ EXPORT_TOOLS = [ "properties": { "outputPath": { "type": "string", - "description": "Path for output PDF file" + "description": "Path for output PDF file", }, "layers": { "type": "array", "description": "Layers to include in PDF", - "items": {"type": "string"} + "items": {"type": "string"}, }, "colorMode": { "type": "string", "enum": ["color", "black_white"], "description": "Color mode for output", - "default": "color" - } + "default": "color", + }, }, - "required": ["outputPath"] - } + "required": ["outputPath"], + }, }, { "name": "export_svg", @@ -1281,16 +1234,16 @@ EXPORT_TOOLS = [ "properties": { "outputPath": { "type": "string", - "description": "Path for output SVG file" + "description": "Path for output SVG file", }, "layers": { "type": "array", "description": "Layers to include in SVG", - "items": {"type": "string"} - } + "items": {"type": "string"}, + }, }, - "required": ["outputPath"] - } + "required": ["outputPath"], + }, }, { "name": "export_3d", @@ -1301,22 +1254,22 @@ EXPORT_TOOLS = [ "properties": { "outputPath": { "type": "string", - "description": "Path for output 3D file" + "description": "Path for output 3D file", }, "format": { "type": "string", "enum": ["step", "vrml"], "description": "3D model format", - "default": "step" + "default": "step", }, "includeComponents": { "type": "boolean", "description": "Include 3D component models", - "default": True - } + "default": True, + }, }, - "required": ["outputPath"] - } + "required": ["outputPath"], + }, }, { "name": "export_bom", @@ -1327,23 +1280,23 @@ EXPORT_TOOLS = [ "properties": { "outputPath": { "type": "string", - "description": "Path for output BOM file" + "description": "Path for output BOM file", }, "format": { "type": "string", "enum": ["csv", "xml", "html"], "description": "BOM output format", - "default": "csv" + "default": "csv", }, "groupByValue": { "type": "boolean", "description": "Group components with same value together", - "default": True - } + "default": True, + }, }, - "required": ["outputPath"] - } - } + "required": ["outputPath"], + }, + }, ] # ============================================================================= @@ -1360,15 +1313,12 @@ SCHEMATIC_TOOLS = [ "properties": { "filename": { "type": "string", - "description": "Path for the new schematic file (.kicad_sch)" + "description": "Path for the new schematic file (.kicad_sch)", }, - "title": { - "type": "string", - "description": "Schematic title" - } + "title": {"type": "string", "description": "Schematic title"}, }, - "required": ["filename"] - } + "required": ["filename"], + }, }, { "name": "load_schematic", @@ -1379,11 +1329,11 @@ SCHEMATIC_TOOLS = [ "properties": { "filename": { "type": "string", - "description": "Path to schematic file (.kicad_sch)" + "description": "Path to schematic file (.kicad_sch)", } }, - "required": ["filename"] - } + "required": ["filename"], + }, }, { "name": "add_schematic_component", @@ -1394,27 +1344,21 @@ SCHEMATIC_TOOLS = [ "properties": { "reference": { "type": "string", - "description": "Reference designator (e.g., R1, C2, U3)" + "description": "Reference designator (e.g., R1, C2, U3)", }, "symbol": { "type": "string", - "description": "Symbol library:name (e.g., Device:R, Device:C)" + "description": "Symbol library:name (e.g., Device:R, Device:C)", }, "value": { "type": "string", - "description": "Component value (e.g., 10k, 0.1uF)" + "description": "Component value (e.g., 10k, 0.1uF)", }, - "x": { - "type": "number", - "description": "X coordinate on schematic" - }, - "y": { - "type": "number", - "description": "Y coordinate on schematic" - } + "x": {"type": "number", "description": "X coordinate on schematic"}, + "y": {"type": "number", "description": "Y coordinate on schematic"}, }, - "required": ["reference", "symbol", "x", "y"] - } + "required": ["reference", "symbol", "x", "y"], + }, }, { "name": "add_schematic_wire", @@ -1430,13 +1374,13 @@ SCHEMATIC_TOOLS = [ "type": "array", "items": {"type": "number"}, "minItems": 2, - "maxItems": 2 + "maxItems": 2, }, - "minItems": 2 + "minItems": 2, } }, - "required": ["points"] - } + "required": ["points"], + }, }, { "name": "add_schematic_connection", @@ -1447,19 +1391,13 @@ SCHEMATIC_TOOLS = [ "properties": { "schematicPath": { "type": "string", - "description": "Path to schematic file" + "description": "Path to schematic file", }, - "x": { - "type": "number", - "description": "X coordinate on schematic" - }, - "y": { - "type": "number", - "description": "Y coordinate on schematic" - } + "x": {"type": "number", "description": "X coordinate on schematic"}, + "y": {"type": "number", "description": "Y coordinate on schematic"}, }, - "required": ["schematicPath", "x", "y"] - } + "required": ["schematicPath", "x", "y"], + }, }, { "name": "add_schematic_net_label", @@ -1470,28 +1408,22 @@ SCHEMATIC_TOOLS = [ "properties": { "schematicPath": { "type": "string", - "description": "Path to schematic file" + "description": "Path to schematic file", }, "netName": { "type": "string", - "description": "Name of the net (e.g., VCC, GND, SDA)" - }, - "x": { - "type": "number", - "description": "X coordinate on schematic" - }, - "y": { - "type": "number", - "description": "Y coordinate on schematic" + "description": "Name of the net (e.g., VCC, GND, SDA)", }, + "x": {"type": "number", "description": "X coordinate on schematic"}, + "y": {"type": "number", "description": "Y coordinate on schematic"}, "rotation": { "type": "number", "description": "Rotation angle in degrees (0, 90, 180, 270)", - "default": 0 - } + "default": 0, + }, }, - "required": ["schematicPath", "netName", "x", "y"] - } + "required": ["schematicPath", "netName", "x", "y"], + }, }, { "name": "connect_to_net", @@ -1502,23 +1434,23 @@ SCHEMATIC_TOOLS = [ "properties": { "schematicPath": { "type": "string", - "description": "Path to schematic file" + "description": "Path to schematic file", }, "reference": { "type": "string", - "description": "Component reference designator (e.g., R1, U3)" + "description": "Component reference designator (e.g., R1, U3)", }, "pinNumber": { "type": "string", - "description": "Pin number or name on the component" + "description": "Pin number or name on the component", }, "netName": { "type": "string", - "description": "Name of the net to connect to" - } + "description": "Name of the net to connect to", + }, }, - "required": ["schematicPath", "reference", "pinNumber", "netName"] - } + "required": ["schematicPath", "reference", "pinNumber", "netName"], + }, }, { "name": "get_net_connections", @@ -1529,15 +1461,15 @@ SCHEMATIC_TOOLS = [ "properties": { "schematicPath": { "type": "string", - "description": "Path to schematic file" + "description": "Path to schematic file", }, "netName": { "type": "string", - "description": "Name of the net to query" - } + "description": "Name of the net to query", + }, }, - "required": ["schematicPath", "netName"] - } + "required": ["schematicPath", "netName"], + }, }, { "name": "get_schematic_pin_locations", @@ -1548,15 +1480,15 @@ SCHEMATIC_TOOLS = [ "properties": { "schematicPath": { "type": "string", - "description": "Path to the schematic file" + "description": "Path to the schematic file", }, "reference": { "type": "string", - "description": "Component reference designator (e.g., U1, R1, J2)" - } + "description": "Component reference designator (e.g., U1, R1, J2)", + }, }, - "required": ["schematicPath", "reference"] - } + "required": ["schematicPath", "reference"], + }, }, { "name": "connect_passthrough", @@ -1567,27 +1499,27 @@ SCHEMATIC_TOOLS = [ "properties": { "schematicPath": { "type": "string", - "description": "Path to the schematic file" + "description": "Path to the schematic file", }, "sourceRef": { "type": "string", - "description": "Reference of the source connector (e.g., J1)" + "description": "Reference of the source connector (e.g., J1)", }, "targetRef": { "type": "string", - "description": "Reference of the target connector (e.g., J2)" + "description": "Reference of the target connector (e.g., J2)", }, "netPrefix": { "type": "string", - "description": "Prefix for generated net names, e.g. 'CSI' produces CSI_1, CSI_2, ... (default: PIN)" + "description": "Prefix for generated net names, e.g. 'CSI' produces CSI_1, CSI_2, ... (default: PIN)", }, "pinOffset": { "type": "integer", - "description": "Add this value to the pin number when building net names (default: 0)" - } + "description": "Add this value to the pin number when building net names (default: 0)", + }, }, - "required": ["schematicPath", "sourceRef", "targetRef"] - } + "required": ["schematicPath", "sourceRef", "targetRef"], + }, }, { "name": "run_erc", @@ -1598,11 +1530,11 @@ SCHEMATIC_TOOLS = [ "properties": { "schematicPath": { "type": "string", - "description": "Path to the .kicad_sch schematic file" + "description": "Path to the .kicad_sch schematic file", } }, - "required": ["schematicPath"] - } + "required": ["schematicPath"], + }, }, { "name": "sync_schematic_to_board", @@ -1613,14 +1545,14 @@ SCHEMATIC_TOOLS = [ "properties": { "schematicPath": { "type": "string", - "description": "Path to .kicad_sch file. If omitted, auto-detected from current board path." + "description": "Path to .kicad_sch file. If omitted, auto-detected from current board path.", }, "boardPath": { "type": "string", - "description": "Path to .kicad_pcb file. If omitted, uses currently loaded board." - } - } - } + "description": "Path to .kicad_pcb file. If omitted, uses currently loaded board.", + }, + }, + }, }, { "name": "generate_netlist", @@ -1631,21 +1563,21 @@ SCHEMATIC_TOOLS = [ "properties": { "schematicPath": { "type": "string", - "description": "Path to schematic file" + "description": "Path to schematic file", }, "outputPath": { "type": "string", - "description": "Optional path to save netlist file" + "description": "Optional path to save netlist file", }, "format": { "type": "string", "enum": ["kicad", "json", "spice"], "description": "Netlist output format", - "default": "json" - } + "default": "json", + }, }, - "required": ["schematicPath"] - } + "required": ["schematicPath"], + }, }, { "name": "list_schematic_libraries", @@ -1657,10 +1589,10 @@ SCHEMATIC_TOOLS = [ "searchPaths": { "type": "array", "description": "Optional additional paths to search for libraries", - "items": {"type": "string"} + "items": {"type": "string"}, } - } - } + }, + }, }, { "name": "export_schematic_pdf", @@ -1671,15 +1603,12 @@ SCHEMATIC_TOOLS = [ "properties": { "schematicPath": { "type": "string", - "description": "Path to schematic file" + "description": "Path to schematic file", }, - "outputPath": { - "type": "string", - "description": "Path for output PDF" - } + "outputPath": {"type": "string", "description": "Path for output PDF"}, }, - "required": ["schematicPath", "outputPath"] - } + "required": ["schematicPath", "outputPath"], + }, }, # --- Schematic Analysis Tools (read-only) --- { @@ -1691,40 +1620,40 @@ SCHEMATIC_TOOLS = [ "properties": { "schematicPath": { "type": "string", - "description": "Path to the .kicad_sch schematic file" + "description": "Path to the .kicad_sch schematic file", }, "x1": { "type": "number", - "description": "Left X coordinate of the region in mm" + "description": "Left X coordinate of the region in mm", }, "y1": { "type": "number", - "description": "Top Y coordinate of the region in mm" + "description": "Top Y coordinate of the region in mm", }, "x2": { "type": "number", - "description": "Right X coordinate of the region in mm" + "description": "Right X coordinate of the region in mm", }, "y2": { "type": "number", - "description": "Bottom Y coordinate of the region in mm" + "description": "Bottom Y coordinate of the region in mm", }, "format": { "type": "string", "enum": ["png", "svg"], - "description": "Output image format (default: png)" + "description": "Output image format (default: png)", }, "width": { "type": "integer", - "description": "Output image width in pixels (default: 800)" + "description": "Output image width in pixels (default: 800)", }, "height": { "type": "integer", - "description": "Output image height in pixels (default: 600)" - } + "description": "Output image height in pixels (default: 600)", + }, }, - "required": ["schematicPath", "x1", "y1", "x2", "y2"] - } + "required": ["schematicPath", "x1", "y1", "x2", "y2"], + }, }, { "name": "find_overlapping_elements", @@ -1735,15 +1664,15 @@ SCHEMATIC_TOOLS = [ "properties": { "schematicPath": { "type": "string", - "description": "Path to the .kicad_sch schematic file" + "description": "Path to the .kicad_sch schematic file", }, "tolerance": { "type": "number", - "description": "Distance threshold in mm for label proximity and wire collinearity checks. Symbol overlap uses bounding-box intersection. (default: 0.5)" - } + "description": "Distance threshold in mm for label proximity and wire collinearity checks. Symbol overlap uses bounding-box intersection. (default: 0.5)", + }, }, - "required": ["schematicPath"] - } + "required": ["schematicPath"], + }, }, { "name": "get_elements_in_region", @@ -1754,27 +1683,27 @@ SCHEMATIC_TOOLS = [ "properties": { "schematicPath": { "type": "string", - "description": "Path to the .kicad_sch schematic file" + "description": "Path to the .kicad_sch schematic file", }, "x1": { "type": "number", - "description": "Left X coordinate of the region in mm" + "description": "Left X coordinate of the region in mm", }, "y1": { "type": "number", - "description": "Top Y coordinate of the region in mm" + "description": "Top Y coordinate of the region in mm", }, "x2": { "type": "number", - "description": "Right X coordinate of the region in mm" + "description": "Right X coordinate of the region in mm", }, "y2": { "type": "number", - "description": "Bottom Y coordinate of the region in mm" - } + "description": "Bottom Y coordinate of the region in mm", + }, }, - "required": ["schematicPath", "x1", "y1", "x2", "y2"] - } + "required": ["schematicPath", "x1", "y1", "x2", "y2"], + }, }, { "name": "find_wires_crossing_symbols", @@ -1785,12 +1714,12 @@ SCHEMATIC_TOOLS = [ "properties": { "schematicPath": { "type": "string", - "description": "Path to the .kicad_sch schematic file" + "description": "Path to the .kicad_sch schematic file", } }, - "required": ["schematicPath"] - } - } + "required": ["schematicPath"], + }, + }, ] # ============================================================================= @@ -1802,10 +1731,7 @@ UI_TOOLS = [ "name": "check_kicad_ui", "title": "Check KiCAD UI Status", "description": "Checks if KiCAD user interface is currently running and returns process information.", - "inputSchema": { - "type": "object", - "properties": {} - } + "inputSchema": {"type": "object", "properties": {}}, }, { "name": "launch_kicad_ui", @@ -1816,16 +1742,16 @@ UI_TOOLS = [ "properties": { "projectPath": { "type": "string", - "description": "Optional path to project file to open in UI" + "description": "Optional path to project file to open in UI", }, "autoLaunch": { "type": "boolean", "description": "Whether to automatically launch if not running", - "default": True - } - } - } - } + "default": True, + }, + }, + }, + }, ] # ============================================================================= @@ -1835,9 +1761,17 @@ UI_TOOLS = [ TOOL_SCHEMAS: Dict[str, Any] = {} # Combine all tool categories -for tool in (PROJECT_TOOLS + BOARD_TOOLS + COMPONENT_TOOLS + ROUTING_TOOLS + - LIBRARY_TOOLS + DESIGN_RULE_TOOLS + EXPORT_TOOLS + - SCHEMATIC_TOOLS + UI_TOOLS): +for tool in ( + PROJECT_TOOLS + + BOARD_TOOLS + + COMPONENT_TOOLS + + ROUTING_TOOLS + + LIBRARY_TOOLS + + DESIGN_RULE_TOOLS + + EXPORT_TOOLS + + SCHEMATIC_TOOLS + + UI_TOOLS +): TOOL_SCHEMAS[tool["name"]] = tool # Total: 46 tools with comprehensive schemas diff --git a/python/tests/conftest.py b/python/tests/conftest.py index 81d3b93..249d74f 100644 --- a/python/tests/conftest.py +++ b/python/tests/conftest.py @@ -33,6 +33,7 @@ except ImportError: class _FakeSchematic: """Minimal stand-in for skip.Schematic used in PinLocator cache.""" + def __init__(self, path: str): self.path = path self.symbol = [] diff --git a/python/tests/test_schematic_analysis.py b/python/tests/test_schematic_analysis.py index c7c4bcd..c36290a 100644 --- a/python/tests/test_schematic_analysis.py +++ b/python/tests/test_schematic_analysis.py @@ -39,7 +39,6 @@ from commands.schematic_analysis import ( find_wires_crossing_symbols, ) - # --------------------------------------------------------------------------- # Helpers # --------------------------------------------------------------------------- @@ -127,6 +126,7 @@ def _make_led_sexp(ref: str, x: float, y: float, rotation: float = 0) -> str: # Unit tests — geometry helpers # =================================================================== + class TestGeometryHelpers: """Test low-level geometry utilities.""" @@ -174,6 +174,7 @@ class TestGeometryHelpers: # Unit tests — S-expression parsers # =================================================================== + class TestSexpParsers: """Test S-expression parsing functions with synthetic data.""" @@ -223,6 +224,7 @@ class TestSexpParsers: # Unit tests — analysis functions with mocked PinLocator # =================================================================== + class TestAABBOverlap: """Test AABB overlap helper.""" @@ -254,14 +256,18 @@ class TestFindOverlappingElements: def test_overlapping_symbols_detected(self): # Two resistors at nearly the same position — bboxes fully overlap - extra = _make_resistor_sexp("R1", 100, 100) + _make_resistor_sexp("R2", 100.1, 100) + extra = _make_resistor_sexp("R1", 100, 100) + _make_resistor_sexp( + "R2", 100.1, 100 + ) tmp = _make_temp_schematic(extra) result = find_overlapping_elements(tmp, tolerance=0.5) assert result["totalOverlaps"] >= 1 assert len(result["overlappingSymbols"]) >= 1 def test_well_separated_symbols_not_flagged(self): - extra = _make_resistor_sexp("R1", 100, 100) + _make_resistor_sexp("R2", 200, 200) + extra = _make_resistor_sexp("R1", 100, 100) + _make_resistor_sexp( + "R2", 200, 200 + ) tmp = _make_temp_schematic(extra) result = find_overlapping_elements(tmp, tolerance=0.5) assert result["totalOverlaps"] == 0 @@ -288,7 +294,9 @@ class TestFindOverlappingElements: """ # R1 at y=100, R2 at y=105 — pin spans [96.19, 103.81] and [101.19, 108.81] # These overlap in Y from 101.19 to 103.81 - extra = _make_resistor_sexp("R1", 100, 100) + _make_resistor_sexp("R2", 100, 105) + extra = _make_resistor_sexp("R1", 100, 100) + _make_resistor_sexp( + "R2", 100, 105 + ) tmp = _make_temp_schematic(extra) result = find_overlapping_elements(tmp, tolerance=0.5) assert result["totalOverlaps"] >= 1, ( @@ -302,7 +310,9 @@ class TestFindOverlappingElements: R pins at y ±3.81, but different X positions far enough apart. """ - extra = _make_resistor_sexp("R1", 100, 100) + _make_resistor_sexp("R2", 110, 100) + extra = _make_resistor_sexp("R1", 100, 100) + _make_resistor_sexp( + "R2", 110, 100 + ) tmp = _make_temp_schematic(extra) result = find_overlapping_elements(tmp, tolerance=0.5) assert result["totalOverlaps"] == 0 @@ -355,6 +365,7 @@ class TestComputeSymbolBbox: def test_returns_none_for_unknown_symbol(self): tmp = _make_temp_schematic() from commands.pin_locator import PinLocator + locator = PinLocator() result = compute_symbol_bbox(tmp, "NONEXISTENT", locator) assert result is None @@ -409,8 +420,7 @@ class TestIntegrationFindWiresCrossingSymbols: result = find_wires_crossing_symbols(tmp) # The wire must not be reported against the far-away R? at (200, 100) collisions_at_200 = [ - c for c in result - if abs(c["component"]["position"]["x"] - 200) < 0.5 + c for c in result if abs(c["component"]["position"]["x"] - 200) < 0.5 ] assert len(collisions_at_200) == 0, ( "Wire at x≈100 must not be flagged against the R? at x=200; " @@ -431,9 +441,9 @@ class TestIntegrationFindWiresCrossingSymbols: tmp = _make_temp_schematic(extra) result = find_wires_crossing_symbols(tmp) d1_crossings = [c for c in result if c["component"]["reference"] == "D1"] - assert len(d1_crossings) >= 1, ( - "Wire starting at pin but passing through body must be detected" - ) + assert ( + len(d1_crossings) >= 1 + ), "Wire starting at pin but passing through body must be detected" def test_wire_terminating_at_pin_from_outside(self): """A wire that arrives at a pin from outside the component body @@ -448,17 +458,17 @@ class TestIntegrationFindWiresCrossingSymbols: tmp = _make_temp_schematic(extra) result = find_wires_crossing_symbols(tmp) d1_crossings = [c for c in result if c["component"]["reference"] == "D1"] - assert len(d1_crossings) == 0, ( - "Wire terminating at pin from outside should not be flagged" - ) + assert ( + len(d1_crossings) == 0 + ), "Wire terminating at pin from outside should not be flagged" def test_wire_shorts_component_pins_detected_as_collision(self): """Regression: a wire connecting pin1→pin2 of the same component must be reported even though both endpoints land on pins.""" r_sexp = _make_resistor_sexp("R_short", 100.0, 100.0) wire_sexp = ( - '(wire (pts (xy 100 103.81) (xy 100 96.19))\n' - ' (stroke (width 0) (type default))\n' + "(wire (pts (xy 100 103.81) (xy 100 96.19))\n" + " (stroke (width 0) (type default))\n" ' (uuid "aaaaaaaa-0000-0000-0000-000000000001"))' ) sch = _make_temp_schematic(r_sexp + "\n" + wire_sexp) @@ -511,6 +521,7 @@ class TestIntegrationGetElementsInRegion: # Unit tests — _check_wire_overlap # =================================================================== + class TestCheckWireOverlap: """Test wire overlap detection for horizontal, vertical, and diagonal cases.""" @@ -613,6 +624,7 @@ class TestIntegrationDiagonalWireOverlap: # Unit tests — _extract_lib_symbols # =================================================================== + class TestExtractLibSymbols: """Test _extract_lib_symbols helper.""" @@ -684,6 +696,7 @@ class TestExtractLibSymbols: # Unit tests — _parse_lib_symbol_graphics # =================================================================== + class TestParseLibSymbolGraphics: """Test graphics extraction from lib_symbol definitions.""" @@ -745,6 +758,7 @@ class TestParseLibSymbolGraphics: # Unit tests — _transform_local_point # =================================================================== + class TestTransformLocalPoint: """Test local→absolute coordinate transform.""" @@ -776,6 +790,7 @@ class TestTransformLocalPoint: # Unit tests — _compute_symbol_bbox_direct with graphics # =================================================================== + class TestComputeSymbolBboxWithGraphics: """Test that bounding box computation uses graphics points when available.""" @@ -783,14 +798,36 @@ class TestComputeSymbolBboxWithGraphics: """Device:R rectangle is (-1.016, -2.54) to (1.016, 2.54) in local coords. Pins at (0, ±3.81). Placed at (100, 100) with no rotation. Bbox should span from pin-to-pin in Y and use rectangle width in X.""" - sym = {"x": 100.0, "y": 100.0, "rotation": 0, "mirror_x": False, "mirror_y": False} + sym = { + "x": 100.0, + "y": 100.0, + "rotation": 0, + "mirror_x": False, + "mirror_y": False, + } pin_defs = { - "1": {"x": 0, "y": 3.81, "angle": 270, "length": 1.27, "name": "~", "type": "passive"}, - "2": {"x": 0, "y": -3.81, "angle": 90, "length": 1.27, "name": "~", "type": "passive"}, + "1": { + "x": 0, + "y": 3.81, + "angle": 270, + "length": 1.27, + "name": "~", + "type": "passive", + }, + "2": { + "x": 0, + "y": -3.81, + "angle": 90, + "length": 1.27, + "name": "~", + "type": "passive", + }, } graphics_points = [(-1.016, -2.54), (1.016, 2.54)] - bbox = _compute_symbol_bbox_direct(sym, pin_defs, graphics_points=graphics_points) + bbox = _compute_symbol_bbox_direct( + sym, pin_defs, graphics_points=graphics_points + ) assert bbox is not None min_x, min_y, max_x, max_y = bbox # X should come from rectangle: 100 ± 1.016 @@ -802,10 +839,30 @@ class TestComputeSymbolBboxWithGraphics: def test_fallback_without_graphics(self): """Without graphics_points, should use the old degenerate expansion.""" - sym = {"x": 100.0, "y": 100.0, "rotation": 0, "mirror_x": False, "mirror_y": False} + sym = { + "x": 100.0, + "y": 100.0, + "rotation": 0, + "mirror_x": False, + "mirror_y": False, + } pin_defs = { - "1": {"x": 0, "y": 3.81, "angle": 270, "length": 1.27, "name": "~", "type": "passive"}, - "2": {"x": 0, "y": -3.81, "angle": 90, "length": 1.27, "name": "~", "type": "passive"}, + "1": { + "x": 0, + "y": 3.81, + "angle": 270, + "length": 1.27, + "name": "~", + "type": "passive", + }, + "2": { + "x": 0, + "y": -3.81, + "angle": 90, + "length": 1.27, + "name": "~", + "type": "passive", + }, } bbox = _compute_symbol_bbox_direct(sym, pin_defs) @@ -817,15 +874,37 @@ class TestComputeSymbolBboxWithGraphics: def test_rotated_symbol_graphics(self): """Graphics points should be rotated along with the symbol.""" - sym = {"x": 100.0, "y": 100.0, "rotation": 90, "mirror_x": False, "mirror_y": False} + sym = { + "x": 100.0, + "y": 100.0, + "rotation": 90, + "mirror_x": False, + "mirror_y": False, + } pin_defs = { - "1": {"x": 0, "y": 3.81, "angle": 270, "length": 1.27, "name": "~", "type": "passive"}, - "2": {"x": 0, "y": -3.81, "angle": 90, "length": 1.27, "name": "~", "type": "passive"}, + "1": { + "x": 0, + "y": 3.81, + "angle": 270, + "length": 1.27, + "name": "~", + "type": "passive", + }, + "2": { + "x": 0, + "y": -3.81, + "angle": 90, + "length": 1.27, + "name": "~", + "type": "passive", + }, } # Rectangle corners in local coords graphics_points = [(-1.016, -2.54), (1.016, 2.54)] - bbox = _compute_symbol_bbox_direct(sym, pin_defs, graphics_points=graphics_points) + bbox = _compute_symbol_bbox_direct( + sym, pin_defs, graphics_points=graphics_points + ) assert bbox is not None min_x, min_y, max_x, max_y = bbox # After 90° rotation, X and Y swap roles @@ -856,7 +935,9 @@ class TestIntegrationGraphicsBbox: assert len(graphics_points) >= 2, "Should have extracted rectangle points" - bbox = _compute_symbol_bbox_direct(r1, pin_defs, graphics_points=graphics_points) + bbox = _compute_symbol_bbox_direct( + r1, pin_defs, graphics_points=graphics_points + ) assert bbox is not None min_x, min_y, max_x, max_y = bbox # Rectangle is ±1.016 in X, NOT ±1.5 from degenerate expansion From c5a0bc495c1916a888dd678c384bed50a3d74f17 Mon Sep 17 00:00:00 2001 From: Eugene Mikhantyev Date: Sun, 22 Mar 2026 21:47:49 +0000 Subject: [PATCH 13/13] style: apply Black formatting to kicad_interface.py Co-Authored-By: Claude Sonnet 4.6 --- python/kicad_interface.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/python/kicad_interface.py b/python/kicad_interface.py index 7a92cbe..3a9ef26 100644 --- a/python/kicad_interface.py +++ b/python/kicad_interface.py @@ -833,7 +833,7 @@ class KiCADInterface: trim_start -= 1 if trim_start > 0 and content[trim_start - 1] == "\n": trim_start -= 1 - content = content[:trim_start] + content[b_end + 1:] + content = content[:trim_start] + content[b_end + 1 :] with open(sch_file, "w", encoding="utf-8") as f: f.write(content)