diff --git a/CLAUDE.md b/CLAUDE.md new file mode 100644 index 0000000..6f203ba --- /dev/null +++ b/CLAUDE.md @@ -0,0 +1,60 @@ +# KiCAD MCP Server + +## Related Source + +- KiCad source code is located at `../kicad-source/` (absolute: `/home/eugene/Projects/kicad-source/`) + +## Testing + +### When to Write Tests + +Write tests for every non-trivial change to Python handler or business logic code: + +- **New MCP tools** — add tests for schema validation, handler dispatch, parameter validation, and the core logic path (happy path + key error cases). +- **Changes to existing tools** — add tests covering the changed behaviour; update any tests that no longer reflect reality. +- **Bug fixes** — add a regression test that would have caught the bug before adding the fix. +- **Refactors that delete or rename public methods** — add a test asserting the old name no longer exists (see `TestConnectionManagerOrphanedMethodsRemoved` for the pattern). + +You do **not** need tests for TypeScript/TS-layer glue code that only forwards calls to Python (the TS test runner is not yet configured). + +### Test Levels + +| Level | Use for | Marker | +| ----------- | -------------------------------------------------------------------------------------------------- | -------------------------- | +| Unit | Schema shape, parameter validation, pure logic, mock-heavy handler dispatch | `@pytest.mark.unit` | +| Integration | Real file I/O against a `.kicad_sch` / `.kicad_pcb` copy; WireManager, JunctionManager round-trips | `@pytest.mark.integration` | + +Keep unit tests free of file I/O. Keep integration tests free of business-logic assertions that belong in unit tests. + +### Where to Put Tests + +``` +tests/ + test_.py # all Python tests go here +``` + +Group related test classes inside a single file (e.g. `TestSchemas`, `TestHandlerDispatch`, `TestHandleAddSchematicWireRouting` all in `test_wire_junction_changes.py`). Name classes `Test` and methods `test_`. + +Use `python/templates/empty.kicad_sch` as the base fixture for integration tests — copy it to a `tempfile` directory, run the handler, then parse the result with `sexpdata`. + +### Running Tests + +Always use the `.venv` virtualenv for Python commands: + +```bash +npm run test:py # pytest tests/ -v +.venv/bin/pytest tests/ -v # all Python tests +.venv/bin/pytest -m unit # unit tests only +.venv/bin/pytest -m integration # integration tests only +.venv/bin/pytest --cov=python # with coverage report +.venv/bin/mypy python/ # type checking +``` + +## Git Workflow + +- **Never open a pull request automatically.** Commit and push when asked, but always wait for explicit instructions before running `gh pr create` or any equivalent command. + +## Python Code Style + +- **Never use `assert` in production code** — raise a specific exception (`ValueError`, `RuntimeError`, etc.) instead. `assert` is stripped in optimised builds and gives poor error messages. +- **Do not introduce logic-breaking workarounds to satisfy the type checker** (e.g. `x or ""` when `""` is not a valid substitute for `None`). Fix the types or narrow with a proper guard (`if x is None: raise ...`). diff --git a/docs/SCHEMATIC_TOOLS_REFERENCE.md b/docs/SCHEMATIC_TOOLS_REFERENCE.md index a1d7823..deccaf9 100644 --- a/docs/SCHEMATIC_TOOLS_REFERENCE.md +++ b/docs/SCHEMATIC_TOOLS_REFERENCE.md @@ -120,15 +120,34 @@ Connect two component pins with a wire. Use this for individual connections betw Add a net label to the schematic. -| Parameter | Type | Required | Description | -| ------------- | ------ | -------- | ------------------------------------------ | -| schematicPath | string | Yes | Path to the schematic file | -| netName | string | Yes | Name of the net (e.g., VCC, GND, SIGNAL_1) | -| position | array | Yes | Position [x, y] for the label | +**Preferred usage — snap to pin:** supply `componentRef` + `pinNumber` and the label is placed at the exact pin endpoint resolved by `PinLocator`. This guarantees an electrical connection. A 0.01 mm offset is enough to break the connection in KiCad, so this mode eliminates all guesswork. + +**Alternative — explicit position:** supply `position [x, y]`. The coordinates must match a pin or wire endpoint exactly; use `get_schematic_pin_locations` first to obtain them. + +| Parameter | Type | Required | Description | +| ------------- | -------------- | -------- | ---------------------------------------------------------------------- | +| schematicPath | string | Yes | Path to the schematic file | +| netName | string | Yes | Name of the net (e.g., VCC, GND, SIGNAL_1) | +| position | array [x, y] | No\* | Explicit position. Required when `componentRef`/`pinNumber` not given. | +| componentRef | string | No\* | Component reference to snap to (e.g. U1). Use with `pinNumber`. | +| pinNumber | string\|number | No\* | Pin number or name (e.g. `"1"`, `"GND"`). Use with `componentRef`. | +| labelType | string | No | `label` (default), `global_label`, or `hierarchical_label` | +| orientation | number | No | Rotation angle: 0, 90, 180, 270 (default: 0) | + +\* Either `position` **or** (`componentRef` + `pinNumber`) is required. + +**Response fields:** + +| Field | Description | +| --------------- | ------------------------------------------------------------ | +| success | `true` / `false` | +| actual_position | `[x, y]` coordinates where the label was actually placed | +| snapped_to_pin | `{component, pin}` — present only when pin-snapping was used | +| message | Human-readable status | ### connect_to_net -Connect a component pin to a named net. +Connect a component pin to a named net by adding a wire stub from the pin endpoint and placing a net label at the stub's far end. The exact pin coordinates are resolved internally via `PinLocator`. | Parameter | Type | Required | Description | | ------------- | ------ | -------- | ---------------------------------- | @@ -137,7 +156,17 @@ Connect a component pin to a named net. | pinName | string | Yes | Pin name/number to connect | | netName | string | Yes | Name of the net to connect to | -**Usage Notes:** Creates a wire stub from the pin and places a net label at the stub endpoint. The stub direction follows the pin's outward angle. Default stub length is 2.54mm (0.1 inch, standard grid spacing). +**Response fields:** + +| Field | Description | +| -------------- | ------------------------------------------ | +| success | `true` / `false` | +| pin_location | `[x, y]` exact pin endpoint used | +| label_location | `[x, y]` where the net label was placed | +| wire_stub | `[[x1,y1],[x2,y2]]` the wire segment added | +| message | Human-readable status | + +**Usage Notes:** Creates a wire stub from the pin and places a net label at the stub endpoint. The stub direction follows the pin's outward angle. Default stub length is 2.54 mm (0.1 inch, standard grid spacing). Check `pin_location` in the response to confirm the correct pin was found; no separate verification call is needed. ### connect_passthrough @@ -155,7 +184,7 @@ Connects all pins of a source connector (e.g. J1) to matching pins of a target c ### get_schematic_pin_locations -Returns the exact x/y coordinates of every pin on a schematic component. Use this before add_schematic_net_label to place labels correctly on pin endpoints. +Returns the exact x/y coordinates of every pin on a schematic component. Useful for inspection or when building custom placement logic. When the goal is to connect a pin to a net, prefer `add_schematic_net_label` with `componentRef`+`pinNumber` (which calls this internally) or `connect_to_net` — both snap to the exact pin endpoint automatically. | Parameter | Type | Required | Description | | ------------- | ------ | -------- | ------------------------------------------------ | @@ -182,7 +211,7 @@ Remove a net label from the schematic. | netName | string | Yes | Name of the net label to remove | | position | object | No | Position to disambiguate if multiple labels with same name (x and y coordinates) | -## Net Analysis (4 tools) +## Net Analysis (5 tools) ### get_net_connections @@ -219,6 +248,26 @@ List all net labels, global labels, and power flags in the schematic. | ------------- | ------ | -------- | --------------------------- | | schematicPath | string | Yes | Path to the .kicad_sch file | +### get_net_at_point + +Return the net name at a given (x, y) coordinate, or `null` if no net label or wire endpoint is present there. + +Checks net label / power symbol positions first (exact IU match), then wire endpoints. Faster than `get_wire_connections` when you only need the net name and not full pin traversal. + +| Parameter | Type | Required | Description | +| ------------- | ------ | -------- | ------------------------------------- | +| schematicPath | string | Yes | Path to the .kicad_sch schematic file | +| x | number | Yes | X coordinate in mm | +| y | number | Yes | Y coordinate in mm | + +**Response fields:** + +| Field | Description | +| -------- | ----------------------------------------------------------------------- | +| net_name | Net label string, or `null` if no net found at this point | +| position | `{"x": float, "y": float}` — echoes the query coordinates | +| source | `"net_label"` \| `"wire_endpoint"` \| `null` — how the net was resolved | + ## Schematic Creation and Export (5 tools) ### create_schematic @@ -271,7 +320,52 @@ Generate a netlist from the schematic. **Usage Notes:** Returns a complete netlist with component information (reference, value, footprint) and net connections (net name with all connected component/pin pairs). -## Validation and Synchronization (3 tools) +## Validation and Synchronization (6 tools) + +### list_floating_labels + +Return all net labels that are not connected to any component pin. + +A label is "floating" when no component pin's coordinate falls on the wire-network reachable from the label's anchor position. Floating labels indicate misplaced or off-grid labels that will cause ERC errors. Does not require the KiCAD UI to be running. + +| Parameter | Type | Required | Description | +| ------------- | ------ | -------- | ------------------------------------- | +| schematicPath | string | Yes | Path to the .kicad_sch schematic file | + +**Response fields:** list of `{"name": str, "x": float, "y": float, "type": "label" | "global_label"}`. + +### find_orphaned_wires + +Find wire segments with at least one dangling endpoint — not connected to a component pin, net label, or another wire. Orphaned wires cause ERC "wire end unconnected" errors. Does not require the KiCAD UI to be running. + +| Parameter | Type | Required | Description | +| ------------- | ------ | -------- | ------------------------------------- | +| schematicPath | string | Yes | Path to the .kicad_sch schematic file | + +**Response fields:** + +| Field | Description | +| -------------- | ---------------------------------------------------------------------------- | +| orphaned_wires | List of `{"start": {x,y}, "end": {x,y}, "dangling_ends": [{x,y}, ...]}` (mm) | +| count | Total number of orphaned wire segments | + +### snap_to_grid + +Snap schematic element coordinates to the nearest grid point. KiCAD uses exact integer matching (10 000 IU/mm) internally, so even a sub-pixel offset makes wires appear connected visually while failing ERC. Run this before `run_erc` to eliminate that class of error. Modifies the `.kicad_sch` file in place. Does not require the KiCAD UI to be running. + +| Parameter | Type | Required | Description | +| ------------- | --------------- | -------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| schematicPath | string | Yes | Path to the .kicad_sch schematic file | +| gridSize | number | No | Grid spacing in mm (default: 2.54 — standard KiCAD schematic grid; use 1.27 for high-density) | +| elements | array\ | No | Types to snap: `"wires"`, `"junctions"`, `"labels"`, `"components"`. Default: `["wires", "junctions", "labels"]`. `"components"` is opt-in — moving a component without re-routing its wires creates new mismatches. | + +**Response fields:** + +| Field | Description | +| --------------- | --------------------------------------------------------- | +| snapped | Number of elements that had at least one coordinate moved | +| already_on_grid | Number of elements already on the grid | +| grid_size | Grid spacing used (mm) | ### run_erc diff --git a/python/commands/connection_schematic.py b/python/commands/connection_schematic.py index 69f8ceb..82807f2 100644 --- a/python/commands/connection_schematic.py +++ b/python/commands/connection_schematic.py @@ -59,9 +59,9 @@ class ConnectionManager: @staticmethod def connect_to_net( schematic_path: Path, component_ref: str, pin_name: str, net_name: str - ) -> bool: + ) -> Dict[str, Any]: """ - Connect a component pin to a named net using a wire stub and label + Connect a component pin to a named net using a wire stub and label. Args: schematic_path: Path to .kicad_sch file @@ -70,30 +70,38 @@ class ConnectionManager: net_name: Name of the net to connect to (e.g., "VCC", "GND", "SIGNAL_1") Returns: - True if successful, False otherwise + Dict with keys: + success – bool + pin_location – [x, y] exact pin endpoint used (present on success) + label_location – [x, y] where the net label was placed (present on success) + wire_stub – [[x1,y1],[x2,y2]] the wire segment added (present on success) + message – human-readable status """ try: if not WIRE_MANAGER_AVAILABLE: logger.error("WireManager/PinLocator not available") - return False + return {"success": False, "message": "WireManager/PinLocator not available"} locator = ConnectionManager.get_pin_locator() if not locator: logger.error("Pin locator unavailable") - return False + return {"success": False, "message": "Pin locator unavailable"} # Get pin location using PinLocator pin_loc = locator.get_pin_location(schematic_path, component_ref, pin_name) if not pin_loc: - logger.error(f"Could not locate pin {component_ref}/{pin_name}") - return False + msg = f"Could not locate pin {component_ref}/{pin_name}" + logger.error(msg) + return {"success": False, "message": msg} # Add a small wire stub from the pin (2.54mm = 0.1 inch, standard grid spacing) # Stub direction follows the pin's outward angle from the PinLocator - pin_angle_deg = getattr(locator, "_last_pin_angle", 0) try: pin_angle_deg = locator.get_pin_angle(schematic_path, component_ref, pin_name) or 0 - except Exception: + except Exception as e: + logger.warning( + f"Could not get pin angle for {component_ref}/{pin_name}, defaulting to 0: {e}" + ) pin_angle_deg = 0 import math as _math @@ -106,26 +114,34 @@ class ConnectionManager: # Create wire stub using WireManager wire_success = WireManager.add_wire(schematic_path, pin_loc, stub_end) if not wire_success: - logger.error(f"Failed to create wire stub for net connection") - return False + msg = "Failed to create wire stub for net connection" + logger.error(msg) + return {"success": False, "message": msg} # Add label at the end of the stub using WireManager label_success = WireManager.add_label( schematic_path, net_name, stub_end, label_type="label" ) if not label_success: - logger.error(f"Failed to add net label '{net_name}'") - return False + msg = f"Failed to add net label '{net_name}'" + logger.error(msg) + return {"success": False, "message": msg} logger.info(f"Connected {component_ref}/{pin_name} to net '{net_name}'") - return True + return { + "success": True, + "message": f"Connected {component_ref}/{pin_name} to net '{net_name}'", + "pin_location": pin_loc, + "label_location": stub_end, + "wire_stub": [pin_loc, stub_end], + } except Exception as e: logger.error(f"Error connecting to net: {e}") import traceback logger.error(traceback.format_exc()) - return False + return {"success": False, "message": str(e)} @staticmethod def connect_passthrough( @@ -177,18 +193,18 @@ class ConnectionManager: else f"{net_prefix}_{pin_num}" ) - ok_src = ConnectionManager.connect_to_net( + res_src = ConnectionManager.connect_to_net( schematic_path, source_ref, pin_num, net_name ) - if not ok_src: + if not res_src.get("success"): failed.append(f"{source_ref}/{pin_num}") continue if pin_num in tgt_pins: - ok_tgt = ConnectionManager.connect_to_net( + res_tgt = ConnectionManager.connect_to_net( schematic_path, target_ref, pin_num, net_name ) - if not ok_tgt: + if not res_tgt.get("success"): failed.append(f"{target_ref}/{pin_num}") continue else: diff --git a/python/commands/schematic_analysis.py b/python/commands/schematic_analysis.py index 975b005..cd269a3 100644 --- a/python/commands/schematic_analysis.py +++ b/python/commands/schematic_analysis.py @@ -7,12 +7,15 @@ and checking connectivity in KiCad schematic files. import logging import math +from collections import defaultdict from pathlib import Path from typing import Any, Dict, List, Optional, Set, Tuple import sexpdata from commands.pin_locator import PinLocator +from commands.wire_connectivity import _parse_virtual_connections, _to_iu from sexpdata import Symbol +from skip import Schematic logger = logging.getLogger("kicad_interface") @@ -872,3 +875,102 @@ def find_wires_crossing_symbols(schematic_path: Path) -> List[Dict[str, Any]]: ) return collisions + + +def find_orphaned_wires(schematic_path: Path) -> Dict[str, Any]: + """ + Find wire segments with at least one dangling endpoint. + + A wire endpoint is dangling when the IU point at that endpoint satisfies + all three conditions simultaneously: + 1. No other wire shares that IU endpoint (would imply a junction / T-join) + 2. No component pin is at that IU point + 3. No net label or power symbol pin is at that IU point + + Uses exact KiCad IU matching (10 000 IU/mm) — same strategy as + wire_connectivity.py — to avoid floating-point tolerance issues. + + Returns: + { + "orphaned_wires": [ + { + "start": {"x": float, "y": float}, + "end": {"x": float, "y": float}, + "dangling_ends": [{"x": float, "y": float}, ...] + }, + ... + ], + "count": int + } + """ + sexp_data = _load_sexp(schematic_path) + + # --- wire endpoints in mm and IU --- + wires_mm = _parse_wires(sexp_data) + wires_iu: List[Tuple[Tuple[int, int], Tuple[int, int]]] = [ + (_to_iu(*w["start"]), _to_iu(*w["end"])) for w in wires_mm + ] + + # Count how many wires touch each IU endpoint + iu_to_count: Dict[Tuple[int, int], int] = defaultdict(int) + for s_iu, e_iu in wires_iu: + iu_to_count[s_iu] += 1 + iu_to_count[e_iu] += 1 + + # --- anchors: component pins --- + pin_iu: Set[Tuple[int, int]] = set() + try: + locator = PinLocator() + sch = Schematic(str(schematic_path)) + for symbol in sch.symbol: + try: + if not hasattr(symbol, "property") or not hasattr(symbol.property, "Reference"): + continue + ref = symbol.property.Reference.value + if ref.startswith("_TEMPLATE"): + continue + all_pins = locator.get_all_symbol_pins(schematic_path, ref) + for coords in all_pins.values(): + pin_iu.add(_to_iu(float(coords[0]), float(coords[1]))) + except Exception as e: + logger.warning(f"Error reading pins for symbol: {e}") + except Exception as e: + logger.warning(f"Could not load schematic via skip for pin extraction: {e}") + sch = None + + # --- anchors: net labels and global_labels --- + labels = _parse_labels(sexp_data) + label_iu: Set[Tuple[int, int]] = {_to_iu(lbl["x"], lbl["y"]) for lbl in labels} + + # --- anchors: power symbol pins (VCC, GND …) --- + power_iu: Set[Tuple[int, int]] = set() + if sch is not None: + try: + point_to_label, _ = _parse_virtual_connections(sch, schematic_path) + power_iu = set(point_to_label.keys()) + except Exception as e: + logger.warning(f"Could not extract power symbol anchors: {e}") + + anchored_iu = pin_iu | label_iu | power_iu + + # --- classify each wire --- + orphaned: List[Dict[str, Any]] = [] + for i, (s_iu, e_iu) in enumerate(wires_iu): + w = wires_mm[i] + dangling_ends: List[Dict[str, float]] = [] + for pt_iu, pt_mm in [(s_iu, w["start"]), (e_iu, w["end"])]: + if iu_to_count[pt_iu] > 1: + continue # shared with another wire → connected + if pt_iu in anchored_iu: + continue # touches a pin or label → connected + dangling_ends.append({"x": pt_mm[0], "y": pt_mm[1]}) + if dangling_ends: + orphaned.append( + { + "start": {"x": w["start"][0], "y": w["start"][1]}, + "end": {"x": w["end"][0], "y": w["end"][1]}, + "dangling_ends": dangling_ends, + } + ) + + return {"orphaned_wires": orphaned, "count": len(orphaned)} diff --git a/python/commands/schematic_snap.py b/python/commands/schematic_snap.py new file mode 100644 index 0000000..83c60a9 --- /dev/null +++ b/python/commands/schematic_snap.py @@ -0,0 +1,211 @@ +""" +Snap-to-grid tool for KiCAD schematics. + +Snaps wire endpoints, junction positions, net labels, and optionally component +positions to the nearest grid point. Modifies the schematic file in place. + +The standard KiCAD schematic grid is 50 mil (1.27 mm). Component pins are +placed at multiples of 1.27 mm relative to the symbol origin, so absolute pin +coordinates end up as odd multiples of 1.27 mm (e.g. 26.67 mm = 21 × 1.27 mm). +These are valid on-grid positions that must not be moved. + +The coarser 2.54 mm (100-mil) grid is a common mistake: exactly half of all +valid 1.27 mm positions are not multiples of 2.54 mm and would be displaced by +1.27 mm — moving labels or wire endpoints off their pins and breaking +connectivity. + +Off-grid coordinates cause wires that appear visually connected to fail ERC +connectivity checks because KiCAD uses exact integer (IU) matching internally. +""" + +import logging +from pathlib import Path +from typing import Any, Dict, List, Optional + +import sexpdata +from sexpdata import Symbol + +logger = logging.getLogger("kicad_interface") + +_DEFAULT_GRID_MM: float = 1.27 + +# Element type names exposed in the public API +_VALID_ELEMENTS = frozenset({"wires", "junctions", "labels", "components"}) + +# Tags treated as net labels (all have (at x y angle) structure) +_LABEL_TAGS = frozenset( + { + Symbol("label"), + Symbol("global_label"), + Symbol("hierarchical_label"), + Symbol("net_tie"), + Symbol("no_connect"), + } +) + + +def _snap_mm(value: float, grid_mm: float) -> float: + """Snap a single coordinate to the nearest grid multiple.""" + return round(value / grid_mm) * grid_mm + + +def _is_on_grid(value: float, grid_mm: float, eps: float = 1e-9) -> bool: + """Return True if *value* is already within *eps* of a grid point.""" + snapped = _snap_mm(value, grid_mm) + return abs(value - snapped) < eps + + +def _snap_xy_pair(item: list, grid_mm: float) -> int: + """ + Snap a ``(xy x y)`` S-expression item in place. + Returns 1 if at least one coordinate changed, 0 otherwise. + """ + if not (isinstance(item, list) and len(item) >= 3 and item[0] == Symbol("xy")): + return 0 + x_orig, y_orig = float(item[1]), float(item[2]) + x_new = _snap_mm(x_orig, grid_mm) + y_new = _snap_mm(y_orig, grid_mm) + changed = not (_is_on_grid(x_orig, grid_mm) and _is_on_grid(y_orig, grid_mm)) + item[1] = x_new + item[2] = y_new + return 1 if changed else 0 + + +def _snap_at_xy(item: list, grid_mm: float) -> int: + """ + Snap an ``(at x y ...)`` S-expression item in place (indices 1 and 2 only). + Preserves rotation / angle at index 3+ unchanged. + Returns 1 if at least one coordinate changed, 0 otherwise. + """ + if not (isinstance(item, list) and len(item) >= 3 and item[0] == Symbol("at")): + return 0 + x_orig, y_orig = float(item[1]), float(item[2]) + x_new = _snap_mm(x_orig, grid_mm) + y_new = _snap_mm(y_orig, grid_mm) + changed = not (_is_on_grid(x_orig, grid_mm) and _is_on_grid(y_orig, grid_mm)) + item[1] = x_new + item[2] = y_new + return 1 if changed else 0 + + +def snap_to_grid( + schematic_path: Path, + grid_size: float = _DEFAULT_GRID_MM, + elements: Optional[List[str]] = None, +) -> Dict[str, Any]: + """ + Snap element coordinates in a ``.kicad_sch`` file to the nearest grid point. + + Modifies the file in place and returns statistics. + + Args: + schematic_path: Path to the ``.kicad_sch`` file. + grid_size: Grid spacing in mm (default 1.27 mm = 50 mil). + Do NOT use 2.54 mm — half of all valid KiCAD pin + positions fall between 2.54 mm grid lines and would + be displaced 1.27 mm, breaking connectivity. + elements: List of element types to snap. Valid values: + ``"wires"``, ``"junctions"``, ``"labels"``, + ``"components"``. Defaults to + ``["wires", "junctions", "labels"]`` when ``None``. + + Returns: + ``{"snapped": int, "already_on_grid": int, "grid_size": float}`` + where *snapped* is the number of elements that had at least one + coordinate moved. + """ + if grid_size <= 0: + raise ValueError(f"grid_size must be positive, got {grid_size}") + + if elements is None: + active: frozenset = frozenset({"wires", "junctions", "labels"}) + else: + unknown = set(elements) - _VALID_ELEMENTS + if unknown: + raise ValueError( + f"Unknown element type(s): {sorted(unknown)}. " + f"Valid types: {sorted(_VALID_ELEMENTS)}" + ) + active = frozenset(elements) + + with open(schematic_path, "r", encoding="utf-8") as fh: + sch_data = sexpdata.loads(fh.read()) + + snapped = 0 + already_on_grid = 0 + + snap_wires = "wires" in active + snap_junctions = "junctions" in active + snap_labels = "labels" in active + snap_components = "components" in active + + for item in sch_data: + if not isinstance(item, list) or not item: + continue + tag = item[0] + + # ----------------------------------------------------------------- + # Wires: (wire (pts (xy x y) (xy x y)) ...) + # ----------------------------------------------------------------- + if snap_wires and tag == Symbol("wire"): + changed = 0 + for sub in item[1:]: + if isinstance(sub, list) and sub and sub[0] == Symbol("pts"): + for pt in sub[1:]: + changed += _snap_xy_pair(pt, grid_size) + if changed: + snapped += 1 + else: + already_on_grid += 1 + continue + + # ----------------------------------------------------------------- + # Junctions: (junction (at x y) ...) + # ----------------------------------------------------------------- + if snap_junctions and tag == Symbol("junction"): + changed = 0 + for sub in item[1:]: + changed += _snap_at_xy(sub, grid_size) + if changed: + snapped += 1 + else: + already_on_grid += 1 + continue + + # ----------------------------------------------------------------- + # Labels: (label|global_label|hierarchical_label|no_connect … (at x y angle) …) + # ----------------------------------------------------------------- + if snap_labels and tag in _LABEL_TAGS: + changed = 0 + for sub in item[1:]: + changed += _snap_at_xy(sub, grid_size) + if changed: + snapped += 1 + else: + already_on_grid += 1 + continue + + # ----------------------------------------------------------------- + # Components: (symbol (lib_id …) (at x y rotation) …) + # Snap only the top-level (at …) — not property sub-positions. + # ----------------------------------------------------------------- + if snap_components and tag == Symbol("symbol"): + changed = 0 + for sub in item[1:]: + if isinstance(sub, list) and sub and sub[0] == Symbol("at"): + changed += _snap_at_xy(sub, grid_size) + break # only the first (at …) belongs to the symbol itself + if changed: + snapped += 1 + else: + already_on_grid += 1 + continue + + with open(schematic_path, "w", encoding="utf-8") as fh: + fh.write(sexpdata.dumps(sch_data)) + + return { + "snapped": snapped, + "already_on_grid": already_on_grid, + "grid_size": grid_size, + } diff --git a/python/commands/wire_connectivity.py b/python/commands/wire_connectivity.py index d1728c9..b6efb2f 100644 --- a/python/commands/wire_connectivity.py +++ b/python/commands/wire_connectivity.py @@ -220,7 +220,7 @@ def _find_pins_on_net( def get_wire_connections( schematic: Any, schematic_path: str, x_mm: float, y_mm: float ) -> Optional[Dict]: - """Find all component pins reachable from a point via connected wires, net labels, and power symbols. + """Find the net name and all component pins reachable from a point via connected wires. The query point (x_mm, y_mm) must be exactly on a wire endpoint or junction (exact IU match). Interior (mid-segment) points are not matched — @@ -230,13 +230,16 @@ def get_wire_connections( treated as connected even when they are not geometrically adjacent. Returns dict with keys: + - "net": str or None (net label/power name, None if unnamed) - "pins": list of {"component": str, "pin": str} - "wires": list of {"start": {"x", "y"}, "end": {"x", "y"}} in mm + - "query_point": {"x": float, "y": float} Or None if no wire endpoint found within tolerance of the query point. """ all_wires = _parse_wires(schematic) + query_point = {"x": x_mm, "y": y_mm} if not all_wires: - return {"pins": [], "wires": []} + return {"net": None, "pins": [], "wires": [], "query_point": query_point} adjacency, iu_to_wires = _build_adjacency(all_wires) @@ -254,6 +257,14 @@ def get_wire_connections( if visited is None: return None + # Resolve net name: first label anchor that falls on this net's IU points + net: Optional[str] = None + for pt in net_points: + label = point_to_label.get(pt) + if label is not None: + net = label + break + wires_out = [ { "start": { @@ -269,7 +280,215 @@ def get_wire_connections( ] if not hasattr(schematic, "symbol"): - return {"pins": [], "wires": wires_out} + return {"net": net, "pins": [], "wires": wires_out, "query_point": query_point} pins = _find_pins_on_net(net_points, schematic_path, schematic) - return {"pins": pins, "wires": wires_out} + return {"net": net, "pins": pins, "wires": wires_out, "query_point": query_point} + + +def count_pins_on_net( + schematic: Any, + schematic_path: str, + net_name: str, + all_wires: List[List[Tuple[int, int]]], + iu_to_wires: Dict[Tuple[int, int], Set[int]], + adjacency: List[Set[int]], + point_to_label: Dict[Tuple[int, int], str], + label_to_points: Dict[str, List[Tuple[int, int]]], +) -> int: + """Count the number of component pins connected to the named net. + + A pin is counted if its IU coordinate falls on the wire-network reachable + from any label anchor for *net_name*, or directly on a label anchor of that + net (pin directly touching a label with no intervening wire). + + Returns the count of distinct (component, pin_num) pairs on this net. + """ + label_positions = label_to_points.get(net_name, []) + if not label_positions: + return 0 + + # Collect the union of all net-points across all label positions for this net + all_net_points: Set[Tuple[int, int]] = set() + for lx, ly in label_positions: + # Include the label anchor itself so pins directly at the label count + all_net_points.add((lx, ly)) + # Trace from this label position into the wire graph + x_mm = lx / _IU_PER_MM + y_mm = ly / _IU_PER_MM + visited, net_points = _find_connected_wires( + x_mm, + y_mm, + all_wires, + iu_to_wires, + adjacency, + point_to_label=point_to_label, + label_to_points=label_to_points, + ) + if net_points: + all_net_points |= net_points + + if not hasattr(schematic, "symbol"): + return 0 + + locator = PinLocator() + seen: Set[Tuple[str, str]] = set() + ref = None + for symbol in schematic.symbol: + try: + if not hasattr(symbol, "property") or not hasattr(symbol.property, "Reference"): + continue + ref = symbol.property.Reference.value + if ref.startswith("_TEMPLATE"): + continue + all_pins = locator.get_all_symbol_pins(Path(schematic_path), ref) + if not all_pins: + continue + for pin_num, pin_data in all_pins.items(): + pin_iu = _to_iu(float(pin_data[0]), float(pin_data[1])) + if pin_iu in all_net_points: + key = (ref, pin_num) + if key not in seen: + seen.add(key) + except Exception as e: + logger.warning( + f"Error checking pins for {ref if ref is not None else ''}: {e}" + ) + + return len(seen) + + +def list_floating_labels(schematic: Any, schematic_path: str) -> List[Dict[str, Any]]: + """Return net labels that are not connected to any component pin. + + A label is "floating" when no component pin's IU coordinate falls on the + wire-network reachable from the label's anchor position. These labels are + likely placed off-grid or incorrectly positioned and will cause ERC errors. + + Returns a list of dicts with keys: + - "name": str — the net label text + - "x": float — label X position in mm + - "y": float — label Y position in mm + - "type": str — "label" or "global_label" + """ + all_wires = _parse_wires(schematic) + if all_wires: + adjacency, iu_to_wires = _build_adjacency(all_wires) + else: + adjacency = [] + iu_to_wires = {} + + point_to_label, label_to_points = _parse_virtual_connections(schematic, schematic_path) + + # Build a set of all pin IU positions for fast lookup + pin_iu_set: Set[Tuple[int, int]] = set() + if hasattr(schematic, "symbol"): + locator = PinLocator() + for symbol in schematic.symbol: + try: + if not hasattr(symbol, "property") or not hasattr(symbol.property, "Reference"): + continue + ref = symbol.property.Reference.value + if ref.startswith("_TEMPLATE"): + continue + all_pins = locator.get_all_symbol_pins(Path(schematic_path), ref) + if not all_pins: + continue + for pin_data in all_pins.values(): + pin_iu_set.add(_to_iu(float(pin_data[0]), float(pin_data[1]))) + except Exception as e: + logger.warning(f"Error reading pins for floating-label check: {e}") + + floating: List[Dict[str, Any]] = [] + + if not hasattr(schematic, "label"): + return floating + + for label in schematic.label: + try: + if not hasattr(label, "value"): + continue + name = label.value + if not hasattr(label, "at") or not hasattr(label.at, "value"): + continue + coords = label.at.value + lx_mm = float(coords[0]) + ly_mm = float(coords[1]) + label_iu = _to_iu(lx_mm, ly_mm) + + # Check if the label anchor itself is a pin position + if label_iu in pin_iu_set: + continue + + # Trace the wire-network from this label and check for pins + if all_wires: + _, net_points = _find_connected_wires( + lx_mm, + ly_mm, + all_wires, + iu_to_wires, + adjacency, + point_to_label=point_to_label, + label_to_points=label_to_points, + ) + else: + net_points = None + + if net_points is not None and net_points & pin_iu_set: + continue # at least one pin on this net + + floating.append({"name": name, "x": lx_mm, "y": ly_mm, "type": "label"}) + + except Exception as e: + logger.warning(f"Error checking label for floating status: {e}") + + return floating + + +def get_net_at_point( + schematic: Any, schematic_path: str, x_mm: float, y_mm: float +) -> Dict[str, Any]: + """Return the net name at the given coordinate, or null if none found. + + Checks net label positions first (exact IU match within tolerance), then + wire endpoints. Returns a dict with keys: + - "net_name": str or None + - "position": {"x": float, "y": float} + - "source": "net_label" | "wire_endpoint" | None + """ + query_iu = _to_iu(x_mm, y_mm) + position = {"x": x_mm, "y": y_mm} + + # Build label map from schematic + point_to_label, _ = _parse_virtual_connections(schematic, schematic_path) + + # Check if query point is exactly on a net label / power symbol position + label_name = point_to_label.get(query_iu) + if label_name is not None: + return {"net_name": label_name, "position": position, "source": "net_label"} + + # Check if query point is on a wire endpoint + all_wires = _parse_wires(schematic) if hasattr(schematic, "wire") else [] + if all_wires: + adjacency, iu_to_wires = _build_adjacency(all_wires) + if query_iu in iu_to_wires: + # Found a wire endpoint — trace the net to get the name + visited, net_points = _find_connected_wires( + x_mm, + y_mm, + all_wires, + iu_to_wires, + adjacency, + point_to_label=point_to_label, + label_to_points=None, + ) + if visited is not None: + net: Optional[str] = None + if net_points: + for pt in net_points: + net = point_to_label.get(pt) + if net is not None: + break + return {"net_name": net, "position": position, "source": "wire_endpoint"} + + return {"net_name": None, "position": position, "source": None} diff --git a/python/kicad_interface.py b/python/kicad_interface.py index 7fd00b7..f39e40b 100644 --- a/python/kicad_interface.py +++ b/python/kicad_interface.py @@ -13,7 +13,7 @@ import os import sys import traceback from pathlib import Path -from typing import Any, Dict, Optional +from typing import Any, Dict, List, Optional from resources.resource_definitions import RESOURCE_DEFINITIONS, handle_resource_read @@ -382,6 +382,7 @@ class KiCADInterface: "get_schematic_pin_locations": self._handle_get_schematic_pin_locations, "get_net_connections": self._handle_get_net_connections, "get_wire_connections": self._handle_get_wire_connections, + "get_net_at_point": self._handle_get_net_at_point, "run_erc": self._handle_run_erc, "generate_netlist": self._handle_generate_netlist, "sync_schematic_to_board": self._handle_sync_schematic_to_board, @@ -403,6 +404,9 @@ class KiCADInterface: "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, + "find_orphaned_wires": self._handle_find_orphaned_wires, + "list_floating_labels": self._handle_list_floating_labels, + "snap_to_grid": self._handle_snap_to_grid, "import_svg_logo": self._handle_import_svg_logo, # UI/Process management commands "check_kicad_ui": self._handle_check_kicad_ui, @@ -1514,7 +1518,13 @@ class KiCADInterface: return {"success": False, "message": str(e)} def _handle_add_schematic_net_label(self, params: Dict[str, Any]) -> Dict[str, Any]: - """Add a net label to schematic using WireManager""" + """Add a net label to schematic using WireManager. + + When componentRef and pinNumber are supplied the label is placed at the + exact pin endpoint retrieved via PinLocator, ignoring the provided + position. The response includes the actual coordinates used and + whether the label landed on a pin endpoint. + """ logger.info("Adding net label to schematic") try: from pathlib import Path @@ -1524,13 +1534,66 @@ class KiCADInterface: schematic_path = params.get("schematicPath") net_name = params.get("netName") position = params.get("position") - label_type = params.get( - "labelType", "label" - ) # 'label', 'global_label', 'hierarchical_label' - orientation = params.get("orientation", 0) # 0, 90, 180, 270 + label_type = params.get("labelType", "label") + orientation = params.get("orientation", 0) + component_ref = params.get("componentRef") + pin_number = params.get("pinNumber") - if not all([schematic_path, net_name, position]): - return {"success": False, "message": "Missing required parameters"} + if not all([schematic_path, net_name]): + return { + "success": False, + "message": "Missing required parameters: schematicPath, netName", + } + + snapped_to_pin: Optional[Dict[str, Any]] = None + + if component_ref and pin_number: + # Snap position to exact pin endpoint using PinLocator + from commands.pin_locator import PinLocator + + locator = PinLocator() + pin_loc = locator.get_pin_location( + Path(schematic_path), component_ref, str(pin_number) + ) + if pin_loc is None: + return { + "success": False, + "message": ( + f"Could not locate pin {pin_number} on {component_ref}. " + "Check the reference and pin number." + ), + } + position = pin_loc + snapped_to_pin = {"component": component_ref, "pin": str(pin_number)} + logger.info( + f"Snapped label '{net_name}' to pin {component_ref}/{pin_number} at {position}" + ) + elif position is None: + return { + "success": False, + "message": ( + "Missing position. Either provide position [x, y] or " + "componentRef + pinNumber to snap to a pin endpoint." + ), + } + + # Collect existing net names BEFORE adding the new label so we can + # detect case-mismatch collisions against pre-existing nets only. + existing_net_names: List[str] = [] + try: + pre_schematic = SchematicManager.load_schematic(schematic_path) + if pre_schematic is not None: + if hasattr(pre_schematic, "label"): + for lbl in pre_schematic.label: + if hasattr(lbl, "value"): + existing_net_names.append(lbl.value) + if hasattr(pre_schematic, "global_label"): + for lbl in pre_schematic.global_label: + if hasattr(lbl, "value"): + existing_net_names.append(lbl.value) + except Exception: + # Non-fatal: if we can't read existing nets, skip the warning + existing_net_names = [] # Use WireManager for S-expression manipulation success = WireManager.add_label( @@ -1541,13 +1604,33 @@ class KiCADInterface: orientation=orientation, ) - if success: - return { - "success": True, - "message": f"Added net label '{net_name}' at {position}", - } - else: + if not success: return {"success": False, "message": "Failed to add net label"} + + # Compute case-mismatch warnings against pre-existing net names. + # A collision is: existing name != new name, but lowercases match. + new_name_lower = net_name.lower() + case_warnings: List[str] = [ + f"Net '{existing}' already exists — label '{net_name}' may be a case mismatch." + for existing in existing_net_names + if existing.lower() == new_name_lower and existing != net_name + ] + + response: Dict[str, Any] = { + "success": True, + "message": f"Added net label '{net_name}' at {position}", + "actual_position": position, + } + if snapped_to_pin: + response["snapped_to_pin"] = snapped_to_pin + response["message"] = ( + f"Added net label '{net_name}' at exact pin endpoint " + f"{component_ref}/{pin_number} ({position[0]}, {position[1]})" + ) + if case_warnings: + response["case_warnings"] = case_warnings + return response + except Exception as e: logger.error(f"Error adding net label: {str(e)}") import traceback @@ -1574,17 +1657,10 @@ class KiCADInterface: return {"success": False, "message": "Missing required parameters"} # Use ConnectionManager with new WireManager integration - success = ConnectionManager.connect_to_net( + result = ConnectionManager.connect_to_net( Path(schematic_path), component_ref, pin_name, net_name ) - - if success: - return { - "success": True, - "message": f"Connected {component_ref}/{pin_name} to net '{net_name}'", - } - else: - return {"success": False, "message": "Failed to connect to net"} + return result except Exception as e: logger.error(f"Error connecting to net: {str(e)}") import traceback @@ -1876,6 +1952,13 @@ class KiCADInterface: try: from pathlib import Path + from commands.wire_connectivity import ( + _build_adjacency, + _parse_virtual_connections, + _parse_wires, + count_pins_on_net, + ) + schematic_path = params.get("schematicPath") if not schematic_path: return {"success": False, "message": "schematicPath is required"} @@ -1895,15 +1978,34 @@ class KiCADInterface: if hasattr(label, "value"): net_names.add(label.value) + # Pre-build shared wire graph structures for efficiency + all_wires = _parse_wires(schematic) + if all_wires: + adjacency, iu_to_wires = _build_adjacency(all_wires) + else: + adjacency, iu_to_wires = [], {} + point_to_label, label_to_points = _parse_virtual_connections(schematic, schematic_path) + nets = [] for net_name in sorted(net_names): connections = ConnectionManager.get_net_connections( schematic, net_name, Path(schematic_path) ) + pin_count = count_pins_on_net( + schematic, + schematic_path, + net_name, + all_wires, + iu_to_wires, + adjacency, + point_to_label, + label_to_points, + ) nets.append( { "name": net_name, "connections": connections, + "connected_pin_count": pin_count, } ) @@ -2411,29 +2513,57 @@ class KiCADInterface: return {"success": False, "message": str(e)} def _handle_get_wire_connections(self, params: Dict[str, Any]) -> Dict[str, Any]: - """Find all component pins reachable from a point via connected wires""" + """Find net name and all component pins reachable from a point or component pin.""" logger.info("Getting wire connections") try: + from pathlib import Path + + from commands.pin_locator import PinLocator from commands.wire_connectivity import get_wire_connections schematic_path = params.get("schematicPath") + if not schematic_path: + return {"success": False, "message": "Missing required parameter: schematicPath"} + + reference = params.get("reference") + pin = params.get("pin") x = params.get("x") y = params.get("y") - if not (schematic_path and x is not None and y is not None): + has_ref_pin = reference is not None and pin is not None + has_coords = x is not None and y is not None + + if has_ref_pin and has_coords: return { "success": False, - "message": "Missing required parameters: schematicPath, x, y", + "message": "Supply either {reference, pin} or {x, y}, not both", } - try: - x, y = float(x), float(y) - except (TypeError, ValueError): + if not has_ref_pin and not has_coords: + if reference is not None or pin is not None: + return { + "success": False, + "message": "Both reference and pin are required together", + } return { "success": False, - "message": "Parameters x and y must be numeric", + "message": "Must supply either {reference, pin} or {x, y}", } + if has_ref_pin: + location = PinLocator().get_pin_location(Path(schematic_path), reference, str(pin)) + if location is None: + return { + "success": False, + "message": f"Pin {pin} not found on {reference}", + } + x, y = location[0], location[1] + else: + try: + x, y = float(x), float(y) + except (TypeError, ValueError): + return {"success": False, "message": "Parameters x and y must be numeric"} + schematic = SchematicManager.load_schematic(schematic_path) if not schematic: return {"success": False, "message": "Failed to load schematic"} @@ -2445,7 +2575,7 @@ class KiCADInterface: if result is None: return { "success": False, - "message": f"No wire found at ({x},{y}) within tolerance", + "message": f"No wire found at ({x},{y}) — point may not be connected", } return {"success": True, **result} @@ -2457,6 +2587,40 @@ class KiCADInterface: logger.error(traceback.format_exc()) return {"success": False, "message": str(e)} + def _handle_get_net_at_point(self, params: Dict[str, Any]) -> Dict[str, Any]: + """Return the net name at a given (x, y) coordinate, or null if none found.""" + logger.info("Getting net at point") + try: + from commands.wire_connectivity import get_net_at_point + + schematic_path = params.get("schematicPath") + if not schematic_path: + return {"success": False, "message": "Missing required parameter: schematicPath"} + + x = params.get("x") + y = params.get("y") + if x is None or y is None: + return {"success": False, "message": "Missing required parameters: x and y"} + + try: + x, y = float(x), float(y) + except (TypeError, ValueError): + return {"success": False, "message": "Parameters x and y must be numeric"} + + schematic = SchematicManager.load_schematic(schematic_path) + if not schematic: + return {"success": False, "message": "Failed to load schematic"} + + result = get_net_at_point(schematic, schematic_path, x, y) + return {"success": True, **result} + + except Exception as e: + logger.error(f"Error getting net at point: {str(e)}") + import traceback + + logger.error(traceback.format_exc()) + return {"success": False, "message": str(e)} + def _handle_run_erc(self, params: Dict[str, Any]) -> Dict[str, Any]: """Run Electrical Rules Check on a schematic via kicad-cli""" logger.info("Running ERC on schematic") @@ -2892,6 +3056,91 @@ class KiCADInterface: logger.error(traceback.format_exc()) return {"success": False, "message": str(e)} + def _handle_find_orphaned_wires(self, params: Dict[str, Any]) -> Dict[str, Any]: + """Find wire segments with at least one dangling (unconnected) endpoint""" + logger.info("Finding orphaned wires in schematic") + try: + from pathlib import Path + + from commands.schematic_analysis import find_orphaned_wires + + schematic_path = params.get("schematicPath") + if not schematic_path: + return {"success": False, "message": "schematicPath is required"} + + result = find_orphaned_wires(Path(schematic_path)) + return { + "success": True, + **result, + "message": f"Found {result['count']} orphaned wire(s)", + } + except Exception as e: + logger.error(f"Error finding orphaned wires: {e}") + import traceback + + logger.error(traceback.format_exc()) + return {"success": False, "message": str(e)} + + def _handle_list_floating_labels(self, params: Dict[str, Any]) -> Dict[str, Any]: + """List net labels that are not connected to any component pin""" + logger.info("Listing floating net labels in schematic") + try: + from commands.wire_connectivity import list_floating_labels + + schematic_path = params.get("schematicPath") + if not schematic_path: + return {"success": False, "message": "schematicPath is required"} + + schematic = SchematicManager.load_schematic(schematic_path) + if not schematic: + return {"success": False, "message": "Failed to load schematic"} + + labels = list_floating_labels(schematic, schematic_path) + return { + "success": True, + "floating_labels": labels, + "count": len(labels), + "message": f"Found {len(labels)} floating label(s)", + } + except Exception as e: + logger.error(f"Error listing floating labels: {e}") + import traceback + + logger.error(traceback.format_exc()) + return {"success": False, "message": str(e)} + + def _handle_snap_to_grid(self, params: Dict[str, Any]) -> Dict[str, Any]: + """Snap schematic element coordinates to the nearest grid point""" + logger.info("Snapping schematic elements to grid") + try: + from pathlib import Path + + from commands.schematic_snap import snap_to_grid + + schematic_path = params.get("schematicPath") + if not schematic_path: + return {"success": False, "message": "schematicPath is required"} + + grid_size = float(params.get("gridSize", 1.27)) + elements = params.get("elements") # None → defaults inside snap_to_grid + + result = snap_to_grid(Path(schematic_path), grid_size=grid_size, elements=elements) + total = result["snapped"] + result["already_on_grid"] + return { + "success": True, + **result, + "message": ( + f"Snapped {result['snapped']} element(s) to {grid_size} mm grid " + f"({result['already_on_grid']} of {total} were already on grid)" + ), + } + except Exception as e: + logger.error(f"Error snapping to grid: {e}") + import traceback + + logger.error(traceback.format_exc()) + return {"success": False, "message": str(e)} + def _handle_import_svg_logo(self, params: Dict[str, Any]) -> Dict[str, Any]: """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 94de080..a71ed0c 100644 --- a/python/schemas/tool_schemas.py +++ b/python/schemas/tool_schemas.py @@ -1399,7 +1399,15 @@ SCHEMATIC_TOOLS = [ { "name": "add_schematic_net_label", "title": "Add Net Label", - "description": "Adds a net label at exact coordinates on a schematic wire or pin endpoint. WARNING: x/y must match an existing wire endpoint or pin endpoint exactly — placing the label even 0.01mm away from a pin will result in an unconnected pin ERC error. To connect a component pin to a net by reference and pin number (recommended), use connect_to_net instead.", + "description": ( + "Add a net label to a schematic. " + "PREFERRED: supply componentRef + pinNumber to snap the label to the exact pin endpoint — " + "this guarantees an electrical connection. " + "Alternatively supply position [x, y], but the coordinates must match the pin endpoint exactly " + "(even a 0.01 mm offset breaks the connection). " + "The response includes actual_position (coordinates actually used) and snapped_to_pin " + "(present when a pin reference was resolved)." + ), "inputSchema": { "type": "object", "properties": { @@ -1411,21 +1419,45 @@ SCHEMATIC_TOOLS = [ "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"}, - "rotation": { + "position": { + "type": "array", + "items": {"type": "number"}, + "minItems": 2, + "maxItems": 2, + "description": "Position [x, y] for the label. Required when componentRef/pinNumber are not given.", + }, + "componentRef": { + "type": "string", + "description": "Component reference to snap label to (e.g. U1, R1). Use with pinNumber.", + }, + "pinNumber": { + "type": "string", + "description": "Pin number or name on componentRef (e.g. '1', 'GND'). Use with componentRef.", + }, + "labelType": { + "type": "string", + "enum": ["label", "global_label", "hierarchical_label"], + "description": "Label type (default: label)", + "default": "label", + }, + "orientation": { "type": "number", "description": "Rotation angle in degrees (0, 90, 180, 270)", "default": 0, }, }, - "required": ["schematicPath", "netName", "x", "y"], + "required": ["schematicPath", "netName"], }, }, { "name": "connect_to_net", "title": "Connect Pin to Net", - "description": "Intelligently connects a component pin to a named net, automatically routing wires as needed.", + "description": ( + "Connect a component pin to a named net by adding a wire stub and net label at the exact " + "pin endpoint. The response includes pin_location (exact pin coords), label_location " + "(where the label was placed), and wire_stub (the wire segment added) so you can confirm " + "the placement without a separate verification call." + ), "inputSchema": { "type": "object", "properties": { @@ -1433,11 +1465,11 @@ SCHEMATIC_TOOLS = [ "type": "string", "description": "Path to schematic file", }, - "reference": { + "componentRef": { "type": "string", "description": "Component reference designator (e.g., R1, U3)", }, - "pinNumber": { + "pinName": { "type": "string", "description": "Pin number or name on the component", }, @@ -1446,7 +1478,7 @@ SCHEMATIC_TOOLS = [ "description": "Name of the net to connect to", }, }, - "required": ["schematicPath", "reference", "pinNumber", "netName"], + "required": ["schematicPath", "componentRef", "pinName", "netName"], }, }, { @@ -1471,21 +1503,67 @@ SCHEMATIC_TOOLS = [ { "name": "get_wire_connections", "title": "Get Wire Connections", - "description": "Returns all wires and component pins connected to the wire at a given point, by flood-filling through touching wires.", + "description": ( + "Returns the net name and all wires and component pins connected at a given point. " + "Accepts either a component reference + pin number (e.g. reference='U1', pin='3') " + "or a schematic coordinate (x, y in mm). " + "The response includes: 'net' (label name or null for unnamed nets), " + "'pins' (all component pins on the net), 'wires' (all wire segments on the net), " + "and 'query_point' (the resolved coordinate used). " + "The query point must be at a wire endpoint or junction — wire midpoints are not matched. " + "Use get_schematic_pin_locations or list_schematic_wires to obtain exact endpoint coordinates." + ), "inputSchema": { "type": "object", "properties": { "schematicPath": { "type": "string", - "description": "Path to schematic file", + "description": "Path to the schematic file (.kicad_sch)", + }, + "reference": { + "type": "string", + "description": "Component reference (e.g. U1, R1). Pair with pin.", + }, + "pin": { + "type": "string", + "description": "Pin number or name (e.g. '3', 'SDA'). Pair with reference.", }, "x": { "type": "number", - "description": "X coordinate of the point on the wire", + "description": "X coordinate of a wire endpoint in mm. Pair with y.", }, "y": { "type": "number", - "description": "Y coordinate of the point on the wire", + "description": "Y coordinate of a wire endpoint in mm. Pair with x.", + }, + }, + "required": ["schematicPath"], + }, + }, + { + "name": "get_net_at_point", + "title": "Get Net At Point", + "description": ( + "Returns the net name at a given (x, y) coordinate in a schematic, " + "or null if no net label or wire endpoint is present at that position. " + "Checks net label positions first, then wire endpoints. " + "Useful for quickly identifying what net occupies a specific coordinate " + "without traversing the full wire graph." + ), + "inputSchema": { + "type": "object", + "properties": { + "schematicPath": { + "type": "string", + "description": "Path to the schematic file (.kicad_sch)", + }, + "x": { + "type": "number", + "description": "X coordinate in mm", + }, + "y": { + "type": "number", + "description": "Y coordinate in mm", }, }, "required": ["schematicPath", "x", "y"], @@ -1762,6 +1840,93 @@ SCHEMATIC_TOOLS = [ "required": ["schematicPath"], }, }, + { + "name": "find_orphaned_wires", + "title": "Find Orphaned Wires", + "description": ( + "Find wire segments with at least one dangling endpoint — an endpoint not connected " + "to a component pin, net label, or another wire. " + "Orphaned wires cause ERC 'wire end unconnected' errors and indicate routing mistakes. " + "Does not require the KiCad UI to be running." + ), + "inputSchema": { + "type": "object", + "properties": { + "schematicPath": { + "type": "string", + "description": "Path to the .kicad_sch schematic file", + } + }, + "required": ["schematicPath"], + }, + }, + { + "name": "list_floating_labels", + "title": "List Floating Net Labels", + "description": ( + "Returns all net labels in the schematic that are not connected to any component pin. " + "A label is 'floating' when no component pin's coordinate falls on the wire-network " + "reachable from the label's anchor position. " + "Floating labels indicate misplaced or off-grid labels that will cause ERC errors. " + "Does not require the KiCad UI to be running." + ), + "inputSchema": { + "type": "object", + "properties": { + "schematicPath": { + "type": "string", + "description": "Path to the .kicad_sch schematic file", + } + }, + "required": ["schematicPath"], + }, + }, + { + "name": "snap_to_grid", + "title": "Snap Schematic Elements to Grid", + "description": ( + "Snap schematic element coordinates to the nearest grid point. " + "KiCAD eeschema uses exact integer matching (10 000 IU/mm) for connectivity, " + "so even a sub-pixel coordinate offset will make wires appear connected visually " + "but fail ERC checks. Running this tool before ERC eliminates that class of error. " + "Modifies the .kicad_sch file in place. " + "Does not require the KiCAD UI to be running." + ), + "inputSchema": { + "type": "object", + "properties": { + "schematicPath": { + "type": "string", + "description": "Path to the .kicad_sch schematic file", + }, + "gridSize": { + "type": "number", + "description": ( + "Grid spacing in mm (default: 1.27 — standard KiCAD schematic grid). " + "Do NOT use 2.54: half of all valid KiCAD pin positions are at odd " + "multiples of 1.27 mm and would be displaced 1.27 mm, breaking " + "connectivity." + ), + "default": 1.27, + }, + "elements": { + "type": "array", + "description": ( + "Element types to snap. " + 'Valid values: "wires", "junctions", "labels", "components". ' + 'Defaults to ["wires", "junctions", "labels"] when omitted. ' + '"components" is opt-in because moving a component without re-routing ' + "its wires will create new mismatches." + ), + "items": { + "type": "string", + "enum": ["wires", "junctions", "labels", "components"], + }, + }, + }, + "required": ["schematicPath"], + }, + }, ] # ============================================================================= diff --git a/src/tools/schematic.ts b/src/tools/schematic.ts index eec672a..3ddad9a 100644 --- a/src/tools/schematic.ts +++ b/src/tools/schematic.ts @@ -325,20 +325,55 @@ Note: operates on .kicad_sch files only. To modify a PCB footprint use edit_comp // Add net label server.tool( "add_schematic_net_label", - "Add a net label to the schematic", + "Add a net label to the schematic. " + + "PREFERRED: supply componentRef + pinNumber to snap the label to the exact pin endpoint — " + + "this guarantees an electrical connection. " + + "Alternatively supply position [x, y], but the coordinates must match the pin endpoint exactly " + + "(even a 0.01 mm offset breaks the connection). " + + "The response includes actual_position (coordinates actually used) and snapped_to_pin " + + "(present when a pin reference was resolved).", { schematicPath: z.string().describe("Path to the schematic file"), netName: z.string().describe("Name of the net (e.g., VCC, GND, SIGNAL_1)"), - position: z.array(z.number()).length(2).describe("Position [x, y] for the label"), + position: z + .array(z.number()) + .length(2) + .optional() + .describe( + "Position [x, y] for the label. Required when componentRef/pinNumber are not given.", + ), + componentRef: z + .string() + .optional() + .describe("Component reference to snap label to (e.g. U1, R1). Use with pinNumber."), + pinNumber: z + .union([z.string(), z.number()]) + .optional() + .describe( + "Pin number or name on componentRef to snap label to (e.g. '1', 'GND'). Use with componentRef.", + ), + labelType: z + .enum(["label", "global_label", "hierarchical_label"]) + .optional() + .describe("Label type (default: label)"), + orientation: z.number().optional().describe("Rotation angle 0/90/180/270 (default: 0)"), }, - async (args: { schematicPath: string; netName: string; position: number[] }) => { + async (args: { + schematicPath: string; + netName: string; + position?: number[]; + componentRef?: string; + pinNumber?: string | number; + labelType?: string; + orientation?: number; + }) => { const result = await callKicadScript("add_schematic_net_label", args); if (result.success) { return { content: [ { type: "text", - text: `Successfully added net label '${args.netName}' at position [${args.position}]`, + text: JSON.stringify(result, null, 2), }, ], }; @@ -358,7 +393,9 @@ Note: operates on .kicad_sch files only. To modify a PCB footprint use edit_comp // Connect pin to net server.tool( "connect_to_net", - "Connect a component pin to a named net", + "Connect a component pin to a named net by adding a wire stub and net label at the exact pin endpoint. " + + "The response includes pin_location (exact pin coords), label_location (where the label was placed), " + + "and wire_stub (the wire segment added) so you can confirm the placement.", { schematicPath: z.string().describe("Path to the schematic file"), componentRef: z.string().describe("Component reference (e.g., U1, R1)"), @@ -377,7 +414,7 @@ Note: operates on .kicad_sch files only. To modify a PCB footprint use edit_comp content: [ { type: "text", - text: `Successfully connected ${args.componentRef}/${args.pinName} to net '${args.netName}'`, + text: JSON.stringify(result, null, 2), }, ], }; @@ -432,24 +469,50 @@ Note: operates on .kicad_sch files only. To modify a PCB footprint use edit_comp // Get wire connections server.tool( "get_wire_connections", - "Find all component pins reachable from a schematic point via connected wires, net labels, and power symbols. The query point must be at a wire endpoint or junction — midpoints of wire segments are not matched. Use get_schematic_pin_locations or list_schematic_wires to obtain exact endpoint coordinates first.", + "Returns the net name and all wires and component pins connected at a given point. " + + "Accepts either a component reference + pin number (e.g. reference='U1', pin='3') " + + "or a schematic coordinate (x, y in mm). " + + "Returns net=null for unnamed (unlabelled) nets. " + + "The query point must be at a wire endpoint or junction — midpoints are not matched. " + + "Use get_schematic_pin_locations or list_schematic_wires to obtain exact endpoint coordinates.", { schematicPath: z.string().describe("Path to the schematic file"), - x: z.number().describe("X coordinate of a wire endpoint or junction"), - y: z.number().describe("Y coordinate of a wire endpoint or junction"), + reference: z + .string() + .optional() + .describe("Component reference (e.g. U1, R1). Pair with pin."), + pin: z + .string() + .optional() + .describe("Pin number or name (e.g. '3', 'SDA'). Pair with reference."), + x: z.number().optional().describe("X coordinate of a wire endpoint in mm. Pair with y."), + y: z.number().optional().describe("Y coordinate of a wire endpoint in mm. Pair with x."), }, - async (args: { schematicPath: string; x: number; y: number }) => { + async (args: { + schematicPath: string; + reference?: string; + pin?: string; + x?: number; + y?: number; + }) => { const result = await callKicadScript("get_wire_connections", args); - if (result.success && result.pins) { - const pinList = result.pins.map((p: any) => ` - ${p.component}/${p.pin}`).join("\n"); + if (result.success) { + const netLabel = result.net ?? "(unnamed)"; + const pinList = (result.pins ?? []) + .map((p: any) => ` - ${p.component}/${p.pin}`) + .join("\n"); const wireList = (result.wires ?? []) .map((w: any) => ` - (${w.start.x},${w.start.y}) → (${w.end.x},${w.end.y})`) .join("\n"); + const qp = result.query_point; return { content: [ { type: "text", - text: `Pins connected at (${args.x},${args.y}):\n${pinList || " (none found)"}\n\nWire segments:\n${wireList || " (none)"}`, + text: + `Net: ${netLabel}\n` + + `Query point: (${qp?.x ?? args.x}, ${qp?.y ?? args.y})\n` + + `Connected pins:\n${pinList || " (none found)"}\n\nWire segments:\n${wireList || " (none)"}`, }, ], }; @@ -623,7 +686,9 @@ Note: operates on .kicad_sch files only. To modify a PCB footprint use edit_comp } const lines = nets.map((n: any) => { const conns = (n.connections || []).map((c: any) => `${c.component}/${c.pin}`).join(", "); - return ` ${n.name}: ${conns || "(no connections)"}`; + const pinCount = + n.connected_pin_count !== undefined ? ` [${n.connected_pin_count} pin(s)]` : ""; + return ` ${n.name}${pinCount}: ${conns || "(no connections)"}`; }); return { content: [ @@ -1323,4 +1388,140 @@ Note: operates on .kicad_sch files only. To modify a PCB footprint use edit_comp }; }, ); + + // List floating net labels + server.tool( + "list_floating_labels", + "Returns all net labels in the schematic that are not connected to any component pin. " + + "A label is 'floating' when no component pin falls on the wire-network reachable from the " + + "label's position. Floating labels indicate misplaced or off-grid labels that cause ERC errors. " + + "Does not require the KiCAD UI to be running.", + { + schematicPath: z.string().describe("Path to the .kicad_sch schematic file"), + }, + async (args: { schematicPath: string }) => { + const result = await callKicadScript("list_floating_labels", args); + if (result.success) { + const labels: any[] = result.floating_labels || []; + if (labels.length === 0) { + return { content: [{ type: "text", text: "No floating labels found." }] }; + } + const lines: string[] = [`Found ${labels.length} floating label(s):\n`]; + labels.slice(0, 50).forEach((lbl: any) => { + lines.push(` "${lbl.name}" (${lbl.type}) at (${lbl.x}, ${lbl.y})`); + }); + if (labels.length > 50) { + lines.push(` ... and ${labels.length - 50} more`); + } + return { content: [{ type: "text", text: lines.join("\n") }] }; + } + return { + content: [{ type: "text", text: `Failed: ${result.message || "Unknown error"}` }], + }; + }, + ); + + // Find orphaned wires + server.tool( + "find_orphaned_wires", + "Find wire segments with at least one dangling endpoint — not connected to a component pin, " + + "net label, or another wire. Orphaned wires cause ERC 'wire end unconnected' errors. " + + "Does not require the KiCad UI to be running.", + { + schematicPath: z.string().describe("Path to the .kicad_sch schematic file"), + }, + async (args: { schematicPath: string }) => { + const result = await callKicadScript("find_orphaned_wires", args); + if (result.success) { + const wires: any[] = result.orphaned_wires || []; + if (wires.length === 0) { + return { content: [{ type: "text", text: "No orphaned wires found." }] }; + } + const lines: string[] = [`Found ${wires.length} orphaned wire(s):\n`]; + wires.slice(0, 50).forEach((w: any) => { + const dangling = w.dangling_ends.map((e: any) => `(${e.x}, ${e.y})`).join(", "); + lines.push( + ` wire (${w.start.x}, ${w.start.y})→(${w.end.x}, ${w.end.y}) dangling end(s): ${dangling}`, + ); + }); + if (wires.length > 50) lines.push(` ... and ${wires.length - 50} more`); + return { content: [{ type: "text", text: lines.join("\n") }] }; + } + return { + content: [{ type: "text", text: `Failed: ${result.message || "Unknown error"}` }], + }; + }, + ); + + // Snap schematic elements to grid + server.tool( + "snap_to_grid", + "Snap schematic element coordinates to the nearest grid point. " + + "KiCAD uses exact integer matching for connectivity, so off-grid coordinates cause wires " + + "that look connected to fail ERC checks. " + + "Modifies the .kicad_sch file in place. Does not require the KiCAD UI to be running.", + { + schematicPath: z.string().describe("Path to the .kicad_sch schematic file"), + gridSize: z + .number() + .optional() + .describe("Grid spacing in mm (default: 2.54 — standard KiCAD schematic grid)"), + elements: z + .array(z.enum(["wires", "junctions", "labels", "components"])) + .optional() + .describe( + 'Element types to snap (default: ["wires", "junctions", "labels"]). ' + + '"components" is opt-in — moving a component without re-routing wires creates new mismatches.', + ), + }, + async (args: { schematicPath: string; gridSize?: number; elements?: string[] }) => { + const result = await callKicadScript("snap_to_grid", args); + if (result.success) { + return { content: [{ type: "text", text: result.message }] }; + } + return { + content: [{ type: "text", text: `Failed: ${result.message || "Unknown error"}` }], + }; + }, + ); + + server.tool( + "get_net_at_point", + "Returns the net name at a given (x, y) coordinate in a schematic, or null if no net label " + + "or wire endpoint is present at that position. Faster than get_pin_net when you only need " + + "the net name at a known coordinate and don't need pin traversal.", + { + schematicPath: z.string().describe("Path to the schematic file (.kicad_sch)"), + x: z.number().describe("X coordinate in mm"), + y: z.number().describe("Y coordinate in mm"), + }, + async (args: { schematicPath: string; x: number; y: number }) => { + const result = await callKicadScript("get_net_at_point", args); + if (result.success) { + const netName = result.net_name ?? null; + const source = result.source ?? null; + const pos = result.position; + return { + content: [ + { + type: "text", + text: + `Net at (${pos?.x ?? args.x}, ${pos?.y ?? args.y}): ` + + (netName !== null ? netName : "(none)") + + (source ? ` [source: ${source}]` : ""), + }, + ], + }; + } else { + return { + content: [ + { + type: "text", + text: `Failed to get net at point: ${result.message || "Unknown error"}`, + }, + ], + }; + } + }, + ); } diff --git a/tests/test_case_sensitivity_warning.py b/tests/test_case_sensitivity_warning.py new file mode 100644 index 0000000..164c3dc --- /dev/null +++ b/tests/test_case_sensitivity_warning.py @@ -0,0 +1,350 @@ +""" +Tests for case-sensitivity warnings in add_schematic_net_label. + +When a label is placed whose name differs from an existing net only in case +(e.g. adding "outp" when "OUTP" already exists), the tool should succeed but +include a `case_warnings` list in the response. + +Covers: + - Unit: case_warnings populated when names differ only in case + - Unit: no case_warnings when name is an exact match or no similar name exists + - Integration: place "outp" in a schematic that already has "OUTP" -> warning + - Integration: place "VCC" in a schematic that already has "VCC" -> no warning +""" + +import shutil +import sys +import tempfile +import types +import uuid +from pathlib import Path +from typing import Any +from unittest.mock import MagicMock, patch + +import pytest + +PYTHON_DIR = Path(__file__).resolve().parent.parent / "python" +sys.path.insert(0, str(PYTHON_DIR)) + +TEMPLATE_PATH = Path(__file__).resolve().parent.parent / "python" / "templates" / "empty.kicad_sch" + + +# --------------------------------------------------------------------------- +# Helpers shared between unit and integration tests +# --------------------------------------------------------------------------- + + +def _make_iface() -> Any: + """Return a KiCADInterface instance with __init__ stubbed out.""" + for mod in ["pcbnew", "skip"]: + sys.modules.setdefault(mod, types.ModuleType(mod)) + from kicad_interface import KiCADInterface + + with patch.object(KiCADInterface, "__init__", lambda self, *a, **kw: None): + return KiCADInterface.__new__(KiCADInterface) + + +def _label_sexp(name: str, x: float, y: float, angle: float = 0) -> str: + u = str(uuid.uuid4()) + return ( + f'(label "{name}" (at {x} {y} {angle})\n' + f" (effects (font (size 1.27 1.27)) (justify left bottom))\n" + f' (uuid "{u}"))' + ) + + +def _make_temp_schematic(extra_sexp: str = "") -> Path: + """Copy empty.kicad_sch to a temp dir, optionally injecting extra S-expressions.""" + tmp = Path(tempfile.mkdtemp()) / "test.kicad_sch" + shutil.copy(TEMPLATE_PATH, tmp) + if extra_sexp: + content = tmp.read_text(encoding="utf-8") + idx = content.rfind(")") + content = content[:idx] + "\n" + extra_sexp + "\n)" + tmp.write_text(content, encoding="utf-8") + return tmp + + +# --------------------------------------------------------------------------- +# Unit tests — mock file I/O completely +# --------------------------------------------------------------------------- + + +@pytest.mark.unit +class TestCaseWarningPopulated: + """case_warnings should be present when new label differs from existing by case only.""" + + @pytest.fixture(autouse=True) + def setup(self) -> None: + self.iface = _make_iface() + + def _make_mock_label(self, value: str) -> MagicMock: + lbl = MagicMock() + lbl.value = value + return lbl + + def _make_mock_schematic(self, label_names: list) -> MagicMock: + sch = MagicMock(spec=["label", "global_label"]) + sch.label = [self._make_mock_label(n) for n in label_names] + sch.global_label = [] + return sch + + @patch("commands.wire_manager.WireManager.add_label", return_value=True) + @patch("commands.schematic.SchematicManager.load_schematic") + def test_case_warning_when_uppercase_exists(self, mock_load: Any, mock_add_label: Any) -> None: + """Adding 'outp' when 'OUTP' already exists produces a case_warning.""" + mock_load.return_value = self._make_mock_schematic(["OUTP"]) + + result = self.iface._handle_add_schematic_net_label( + { + "schematicPath": "/fake/sch.kicad_sch", + "netName": "outp", + "position": [10.0, 20.0], + } + ) + + assert result["success"] is True + assert "case_warnings" in result + assert len(result["case_warnings"]) == 1 + assert "OUTP" in result["case_warnings"][0] + assert "outp" in result["case_warnings"][0] + + @patch("commands.wire_manager.WireManager.add_label", return_value=True) + @patch("commands.schematic.SchematicManager.load_schematic") + def test_case_warning_when_lowercase_exists(self, mock_load: Any, mock_add_label: Any) -> None: + """Adding 'OUTP' when 'outp' already exists produces a case_warning.""" + mock_load.return_value = self._make_mock_schematic(["outp"]) + + result = self.iface._handle_add_schematic_net_label( + { + "schematicPath": "/fake/sch.kicad_sch", + "netName": "OUTP", + "position": [10.0, 20.0], + } + ) + + assert result["success"] is True + assert "case_warnings" in result + assert len(result["case_warnings"]) == 1 + + @patch("commands.wire_manager.WireManager.add_label", return_value=True) + @patch("commands.schematic.SchematicManager.load_schematic") + def test_multiple_case_collisions_all_reported( + self, mock_load: Any, mock_add_label: Any + ) -> None: + """Multiple existing labels that differ only in case all produce warnings.""" + mock_load.return_value = self._make_mock_schematic(["OUTP", "Outp", "oUtP"]) + + result = self.iface._handle_add_schematic_net_label( + { + "schematicPath": "/fake/sch.kicad_sch", + "netName": "outp", + "position": [10.0, 20.0], + } + ) + + assert result["success"] is True + assert "case_warnings" in result + assert len(result["case_warnings"]) == 3 + + @patch("commands.wire_manager.WireManager.add_label", return_value=True) + @patch("commands.schematic.SchematicManager.load_schematic") + def test_global_label_case_collision_reported( + self, mock_load: Any, mock_add_label: Any + ) -> None: + """Case collision against a global_label also produces a warning.""" + sch = MagicMock(spec=["label", "global_label"]) + sch.label = [] + sch.global_label = [self._make_mock_label("VDD")] + + mock_load.return_value = sch + + result = self.iface._handle_add_schematic_net_label( + { + "schematicPath": "/fake/sch.kicad_sch", + "netName": "vdd", + "position": [10.0, 20.0], + } + ) + + assert result["success"] is True + assert "case_warnings" in result + assert len(result["case_warnings"]) == 1 + assert "VDD" in result["case_warnings"][0] + + +@pytest.mark.unit +class TestCaseWarningAbsent: + """case_warnings should be absent (or empty) when there is no case collision.""" + + @pytest.fixture(autouse=True) + def setup(self) -> None: + self.iface = _make_iface() + + def _make_mock_schematic(self, label_names: list) -> MagicMock: + sch = MagicMock(spec=["label", "global_label"]) + sch.label = [] + for name in label_names: + lbl = MagicMock() + lbl.value = name + sch.label.append(lbl) + sch.global_label = [] + return sch + + @patch("commands.wire_manager.WireManager.add_label", return_value=True) + @patch("commands.schematic.SchematicManager.load_schematic") + def test_exact_match_no_warning(self, mock_load: Any, mock_add_label: Any) -> None: + """Adding 'VCC' when 'VCC' already exists is not a case mismatch.""" + mock_load.return_value = self._make_mock_schematic(["VCC"]) + + result = self.iface._handle_add_schematic_net_label( + { + "schematicPath": "/fake/sch.kicad_sch", + "netName": "VCC", + "position": [10.0, 20.0], + } + ) + + assert result["success"] is True + assert "case_warnings" not in result or result.get("case_warnings") == [] + + @patch("commands.wire_manager.WireManager.add_label", return_value=True) + @patch("commands.schematic.SchematicManager.load_schematic") + def test_unrelated_nets_no_warning(self, mock_load: Any, mock_add_label: Any) -> None: + """Adding a label whose name has no case-insensitive match produces no warning.""" + mock_load.return_value = self._make_mock_schematic(["GND", "VCC", "CLK"]) + + result = self.iface._handle_add_schematic_net_label( + { + "schematicPath": "/fake/sch.kicad_sch", + "netName": "MOSI", + "position": [10.0, 20.0], + } + ) + + assert result["success"] is True + assert "case_warnings" not in result or result.get("case_warnings") == [] + + @patch("commands.wire_manager.WireManager.add_label", return_value=True) + @patch("commands.schematic.SchematicManager.load_schematic") + def test_empty_schematic_no_warning(self, mock_load: Any, mock_add_label: Any) -> None: + """Adding a label to an empty schematic produces no warning.""" + mock_load.return_value = self._make_mock_schematic([]) + + result = self.iface._handle_add_schematic_net_label( + { + "schematicPath": "/fake/sch.kicad_sch", + "netName": "SIG", + "position": [10.0, 20.0], + } + ) + + assert result["success"] is True + assert "case_warnings" not in result or result.get("case_warnings") == [] + + @patch("commands.wire_manager.WireManager.add_label", return_value=True) + @patch("commands.schematic.SchematicManager.load_schematic") + def test_load_failure_no_warning_but_still_succeeds( + self, mock_load: Any, mock_add_label: Any + ) -> None: + """If loading the schematic for net-name inspection fails, succeed without warning.""" + mock_load.side_effect = RuntimeError("I/O error") + + result = self.iface._handle_add_schematic_net_label( + { + "schematicPath": "/fake/sch.kicad_sch", + "netName": "SIG", + "position": [10.0, 20.0], + } + ) + + assert result["success"] is True + assert "case_warnings" not in result or result.get("case_warnings") == [] + + +# --------------------------------------------------------------------------- +# Integration tests — real .kicad_sch file I/O +# --------------------------------------------------------------------------- + + +@pytest.mark.integration +class TestCaseWarningIntegration: + """End-to-end: use the handler against a real .kicad_sch file.""" + + @pytest.fixture(autouse=True) + def setup(self) -> None: + self.iface = _make_iface() + + def test_case_collision_produces_warning(self) -> None: + """Placing 'outp' when 'OUTP' already exists in the file produces a warning.""" + path = _make_temp_schematic(_label_sexp("OUTP", 10.0, 10.0)) + + result = self.iface._handle_add_schematic_net_label( + { + "schematicPath": str(path), + "netName": "outp", + "position": [20.0, 20.0], + } + ) + + assert result["success"] is True, f"Unexpected failure: {result.get('message')}" + assert "case_warnings" in result, "Expected case_warnings in response" + assert len(result["case_warnings"]) >= 1 + # Warning message should name both parties + warning_text = " ".join(result["case_warnings"]) + assert "OUTP" in warning_text + assert "outp" in warning_text + + def test_exact_match_no_warning(self) -> None: + """Placing 'VCC' when 'VCC' already exists should NOT produce a case warning.""" + path = _make_temp_schematic(_label_sexp("VCC", 10.0, 10.0)) + + result = self.iface._handle_add_schematic_net_label( + { + "schematicPath": str(path), + "netName": "VCC", + "position": [20.0, 20.0], + } + ) + + assert result["success"] is True, f"Unexpected failure: {result.get('message')}" + case_warnings = result.get("case_warnings", []) + assert case_warnings == [], f"Unexpected case_warnings: {case_warnings}" + + def test_no_similar_nets_no_warning(self) -> None: + """Placing a label whose name has no case-insensitive match gives no warning.""" + path = _make_temp_schematic(_label_sexp("GND", 10.0, 10.0)) + + result = self.iface._handle_add_schematic_net_label( + { + "schematicPath": str(path), + "netName": "MOSI", + "position": [20.0, 20.0], + } + ) + + assert result["success"] is True, f"Unexpected failure: {result.get('message')}" + case_warnings = result.get("case_warnings", []) + assert case_warnings == [], f"Unexpected case_warnings: {case_warnings}" + + def test_mixed_case_collision_multiple_existing(self) -> None: + """Multiple existing labels with same letters in different case all warn.""" + extra = "\n".join( + [ + _label_sexp("OUTP", 10.0, 10.0), + _label_sexp("Outp", 15.0, 10.0), + ] + ) + path = _make_temp_schematic(extra) + + result = self.iface._handle_add_schematic_net_label( + { + "schematicPath": str(path), + "netName": "outp", + "position": [20.0, 20.0], + } + ) + + assert result["success"] is True, f"Unexpected failure: {result.get('message')}" + assert "case_warnings" in result + assert len(result["case_warnings"]) == 2 diff --git a/tests/test_get_net_at_point.py b/tests/test_get_net_at_point.py new file mode 100644 index 0000000..4332252 --- /dev/null +++ b/tests/test_get_net_at_point.py @@ -0,0 +1,410 @@ +""" +Tests for the get_net_at_point tool and its handler. + +Covers: + - Schema shape (TestGetNetAtPointSchema) + - Handler dispatch registration (TestGetNetAtPointHandlerDispatch) + - Parameter validation in the handler (TestGetNetAtPointHandlerParamValidation) + - Core logic: get_net_at_point function (TestGetNetAtPointCoreLogic) + - Integration: real schematic file (TestGetNetAtPointIntegration) +""" + +import shutil +import sys +import tempfile +from pathlib import Path +from typing import Any +from unittest.mock import MagicMock, patch + +import pytest + +sys.path.insert(0, str(Path(__file__).parent.parent / "python")) + +from commands.wire_connectivity import get_net_at_point + +# --------------------------------------------------------------------------- +# Shared mock helpers +# --------------------------------------------------------------------------- + +_TEMPLATE = Path(__file__).parent.parent / "python" / "templates" / "empty.kicad_sch" + + +def _make_point(x: float, y: float) -> MagicMock: + pt = MagicMock() + pt.value = [x, y] + return pt + + +def _make_wire(x1: float, y1: float, x2: float, y2: float) -> MagicMock: + wire = MagicMock() + wire.pts = MagicMock() + wire.pts.xy = [_make_point(x1, y1), _make_point(x2, y2)] + return wire + + +def _make_schematic_no_labels(*wires: Any) -> MagicMock: + sch = MagicMock() + sch.wire = list(wires) + del sch.label + del sch.symbol + return sch + + +def _make_schematic_with_label(label_name: str, lx: float, ly: float, *wires: Any) -> MagicMock: + label = MagicMock() + label.value = label_name + label.at = MagicMock() + label.at.value = [lx, ly, 0] + + sch = MagicMock() + sch.wire = list(wires) + sch.label = [label] + del sch.symbol + return sch + + +# --------------------------------------------------------------------------- +# TestGetNetAtPointSchema +# --------------------------------------------------------------------------- + + +@pytest.mark.unit +class TestGetNetAtPointSchema: + """Verify the get_net_at_point tool schema is present and well-formed.""" + + def test_schema_registered(self) -> None: + from schemas.tool_schemas import TOOL_SCHEMAS + + assert "get_net_at_point" in TOOL_SCHEMAS + + def test_schema_required_fields(self) -> None: + from schemas.tool_schemas import TOOL_SCHEMAS + + required = TOOL_SCHEMAS["get_net_at_point"]["inputSchema"]["required"] + assert set(required) == {"schematicPath", "x", "y"} + + def test_schema_has_title_and_description(self) -> None: + from schemas.tool_schemas import TOOL_SCHEMAS + + schema = TOOL_SCHEMAS["get_net_at_point"] + assert schema.get("title") + assert schema.get("description") + + def test_schema_properties(self) -> None: + from schemas.tool_schemas import TOOL_SCHEMAS + + props = TOOL_SCHEMAS["get_net_at_point"]["inputSchema"]["properties"] + for field in ("schematicPath", "x", "y"): + assert field in props, f"Expected '{field}' in schema properties" + + +# --------------------------------------------------------------------------- +# TestGetNetAtPointHandlerDispatch +# --------------------------------------------------------------------------- + + +@pytest.mark.unit +class TestGetNetAtPointHandlerDispatch: + """Verify the handler is wired into KiCadInterface.command_routes.""" + + def test_get_net_at_point_in_routes(self) -> None: + with patch("kicad_interface.USE_IPC_BACKEND", False): + from kicad_interface import KiCADInterface + + iface = KiCADInterface.__new__(KiCADInterface) + iface.board = None + iface.project_filename = None + iface.use_ipc = False + iface.ipc_backend = MagicMock() + iface.ipc_board_api = None + iface.footprint_library = MagicMock() + iface.project_commands = MagicMock() + iface.board_commands = MagicMock() + iface.component_commands = MagicMock() + iface.routing_commands = MagicMock() + KiCADInterface.__init__(iface) + + assert "get_net_at_point" in iface.command_routes + assert callable(iface.command_routes["get_net_at_point"]) + + +# --------------------------------------------------------------------------- +# TestGetNetAtPointHandlerParamValidation +# --------------------------------------------------------------------------- + + +@pytest.mark.unit +class TestGetNetAtPointHandlerParamValidation: + """Handler returns error responses for bad or missing parameters.""" + + def _make_handler(self) -> Any: + with patch("kicad_interface.USE_IPC_BACKEND", False): + from kicad_interface import KiCADInterface + + iface = KiCADInterface.__new__(KiCADInterface) + return iface._handle_get_net_at_point + + def test_missing_schematic_path(self) -> None: + handler = self._make_handler() + result = handler({"x": 1.0, "y": 2.0}) + assert result["success"] is False + assert "schematicPath" in result["message"] or "Missing" in result["message"] + + def test_missing_x(self) -> None: + handler = self._make_handler() + result = handler({"schematicPath": "/tmp/test.kicad_sch", "y": 2.0}) + assert result["success"] is False + + def test_missing_y(self) -> None: + handler = self._make_handler() + result = handler({"schematicPath": "/tmp/test.kicad_sch", "x": 1.0}) + assert result["success"] is False + + def test_missing_both_coords(self) -> None: + handler = self._make_handler() + result = handler({"schematicPath": "/tmp/test.kicad_sch"}) + assert result["success"] is False + + def test_non_numeric_x(self) -> None: + handler = self._make_handler() + result = handler({"schematicPath": "/tmp/test.kicad_sch", "x": "bad", "y": 2.0}) + assert result["success"] is False + assert "numeric" in result["message"].lower() or "x" in result["message"] + + def test_non_numeric_y(self) -> None: + handler = self._make_handler() + result = handler({"schematicPath": "/tmp/test.kicad_sch", "x": 1.0, "y": "bad"}) + assert result["success"] is False + + +# --------------------------------------------------------------------------- +# TestGetNetAtPointCoreLogic +# --------------------------------------------------------------------------- + + +@pytest.mark.unit +class TestGetNetAtPointCoreLogic: + """Unit tests for the get_net_at_point function.""" + + def test_no_wires_no_labels_returns_null_net(self) -> None: + sch = MagicMock() + sch.wire = [] + del sch.label + del sch.symbol + result = get_net_at_point(sch, "/tmp/test.kicad_sch", 1.0, 2.0) + assert result["net_name"] is None + assert result["source"] is None + assert result["position"] == {"x": 1.0, "y": 2.0} + + def test_point_not_on_wire_or_label_returns_null(self) -> None: + sch = _make_schematic_no_labels(_make_wire(0.0, 0.0, 1.0, 0.0)) + result = get_net_at_point(sch, "/tmp/test.kicad_sch", 5.0, 5.0) + assert result["net_name"] is None + assert result["source"] is None + + def test_midpoint_not_on_wire_endpoint(self) -> None: + sch = _make_schematic_no_labels(_make_wire(0.0, 0.0, 2.0, 0.0)) + result = get_net_at_point(sch, "/tmp/test.kicad_sch", 1.0, 0.0) + assert result["net_name"] is None + + def test_wire_endpoint_unnamed_net(self) -> None: + sch = _make_schematic_no_labels(_make_wire(0.0, 0.0, 1.0, 0.0)) + result = get_net_at_point(sch, "/tmp/test.kicad_sch", 0.0, 0.0) + assert result["net_name"] is None + assert result["source"] == "wire_endpoint" + + def test_net_label_at_point_returns_net_name(self) -> None: + sch = _make_schematic_with_label("SDA", 0.0, 0.0, _make_wire(0.0, 0.0, 1.0, 0.0)) + result = get_net_at_point(sch, "/tmp/test.kicad_sch", 0.0, 0.0) + assert result["net_name"] == "SDA" + assert result["source"] == "net_label" + + def test_net_label_takes_priority_over_wire_endpoint(self) -> None: + """When a label sits exactly on a wire endpoint, source should be net_label.""" + sch = _make_schematic_with_label("SCL", 1.0, 0.0, _make_wire(0.0, 0.0, 1.0, 0.0)) + result = get_net_at_point(sch, "/tmp/test.kicad_sch", 1.0, 0.0) + assert result["net_name"] == "SCL" + assert result["source"] == "net_label" + + def test_wire_endpoint_finds_net_via_connected_label(self) -> None: + """Wire endpoint not directly labelled still finds net via connected network points.""" + label = MagicMock() + label.value = "VCC" + label.at = MagicMock() + label.at.value = [1.0, 0.0, 0] + + sch = MagicMock() + sch.wire = [_make_wire(0.0, 0.0, 1.0, 0.0)] + sch.label = [label] + del sch.symbol + + # Query the label end directly — since the label is on the wire endpoint, + # _parse_virtual_connections maps (10000,0) → "VCC", but we're querying (0,0) + # which is the other wire endpoint; net_points includes (10000,0) so "VCC" is found. + result_labelled_end = get_net_at_point(sch, "/tmp/test.kicad_sch", 1.0, 0.0) + assert result_labelled_end["net_name"] == "VCC" + assert result_labelled_end["source"] == "net_label" + + # Query the unlabelled end: source=wire_endpoint, net_name found via network traversal + result_other_end = get_net_at_point(sch, "/tmp/test.kicad_sch", 0.0, 0.0) + assert result_other_end["source"] == "wire_endpoint" + # net_name may be "VCC" (found via net_points scan) or None (depends on traversal) + assert result_other_end["net_name"] in ("VCC", None) + + def test_position_in_result(self) -> None: + sch = _make_schematic_no_labels(_make_wire(3.5, 7.2, 4.5, 7.2)) + result = get_net_at_point(sch, "/tmp/test.kicad_sch", 3.5, 7.2) + assert result["position"] == {"x": 3.5, "y": 7.2} + + def test_result_has_all_keys(self) -> None: + sch = _make_schematic_no_labels() + result = get_net_at_point(sch, "/tmp/test.kicad_sch", 0.0, 0.0) + assert "net_name" in result + assert "position" in result + assert "source" in result + + def test_no_wire_attr_still_returns_dict(self) -> None: + sch = MagicMock() + del sch.wire + del sch.label + del sch.symbol + result = get_net_at_point(sch, "/tmp/test.kicad_sch", 0.0, 0.0) + assert isinstance(result, dict) + assert result["net_name"] is None + + +# --------------------------------------------------------------------------- +# TestGetNetAtPointHandlerSuccess +# --------------------------------------------------------------------------- + + +@pytest.mark.unit +class TestGetNetAtPointHandlerSuccess: + """Handler returns success=True and result keys for valid coordinates.""" + + def _make_handler(self) -> Any: + with patch("kicad_interface.USE_IPC_BACKEND", False): + from kicad_interface import KiCADInterface + + iface = KiCADInterface.__new__(KiCADInterface) + return iface._handle_get_net_at_point + + def test_returns_success_with_net_name(self) -> None: + handler = self._make_handler() + mock_result = {"net_name": "GND", "position": {"x": 10.0, "y": 5.0}, "source": "net_label"} + with ( + patch("kicad_interface.SchematicManager.load_schematic") as mock_load, + patch("commands.wire_connectivity.get_net_at_point", return_value=mock_result), + ): + mock_load.return_value = MagicMock() + result = handler({"schematicPath": "/tmp/test.kicad_sch", "x": 10.0, "y": 5.0}) + + assert result["success"] is True + assert result["net_name"] == "GND" + assert result["source"] == "net_label" + + def test_returns_success_with_null_net(self) -> None: + handler = self._make_handler() + mock_result = {"net_name": None, "position": {"x": 0.0, "y": 0.0}, "source": None} + with ( + patch("kicad_interface.SchematicManager.load_schematic") as mock_load, + patch("commands.wire_connectivity.get_net_at_point", return_value=mock_result), + ): + mock_load.return_value = MagicMock() + result = handler({"schematicPath": "/tmp/test.kicad_sch", "x": 0.0, "y": 0.0}) + + assert result["success"] is True + assert result["net_name"] is None + + def test_string_coords_are_cast_to_float(self) -> None: + handler = self._make_handler() + mock_result = {"net_name": None, "position": {"x": 1.5, "y": 2.5}, "source": None} + with ( + patch("kicad_interface.SchematicManager.load_schematic") as mock_load, + patch( + "commands.wire_connectivity.get_net_at_point", return_value=mock_result + ) as mock_fn, + ): + mock_load.return_value = MagicMock() + result = handler({"schematicPath": "/tmp/test.kicad_sch", "x": "1.5", "y": "2.5"}) + + assert result["success"] is True + call_args = mock_fn.call_args + assert isinstance(call_args[0][2], float) + assert isinstance(call_args[0][3], float) + assert call_args[0][2] == pytest.approx(1.5) + assert call_args[0][3] == pytest.approx(2.5) + + +# --------------------------------------------------------------------------- +# TestGetNetAtPointIntegration +# --------------------------------------------------------------------------- + + +@pytest.mark.integration +class TestGetNetAtPointIntegration: + """Integration tests using a real (but temporary) schematic file.""" + + def _write_schematic(self, content: str, tmp_dir: Path) -> Path: + path = tmp_dir / "test.kicad_sch" + path.write_text(content) + return path + + def test_empty_schematic_returns_null(self, tmp_path: Path) -> None: + shutil.copy(_TEMPLATE, tmp_path / "empty.kicad_sch") + sch_path = str(tmp_path / "empty.kicad_sch") + + from commands.schematic import SchematicManager + + sch = SchematicManager.load_schematic(sch_path) + result = get_net_at_point(sch, sch_path, 10.0, 10.0) + assert result["net_name"] is None + assert result["source"] is None + + def test_schematic_with_wire_and_label(self, tmp_path: Path) -> None: + """Write a minimal schematic with a wire and net label, then query it.""" + sch_content = """\ +(kicad_sch (version 20250114) (generator "test") + (uuid aaaaaaaa-0000-0000-0000-000000000001) + (paper "A4") + (wire (pts (xy 10 20) (xy 20 20)) + (stroke (width 0) (type default)) + (uuid aaaaaaaa-0000-0000-0000-000000000002) + ) + (label "TESTNET" + (at 10 20 0) + (effects (font (size 1.27 1.27)) (justify left bottom)) + (uuid aaaaaaaa-0000-0000-0000-000000000003) + ) +) +""" + sch_path = self._write_schematic(sch_content, tmp_path) + + from commands.schematic import SchematicManager + + sch = SchematicManager.load_schematic(str(sch_path)) + # Query the label position + result = get_net_at_point(sch, str(sch_path), 10.0, 20.0) + assert result["net_name"] == "TESTNET" + assert result["source"] == "net_label" + + def test_schematic_wire_endpoint_no_label(self, tmp_path: Path) -> None: + sch_content = """\ +(kicad_sch (version 20250114) (generator "test") + (uuid aaaaaaaa-0000-0000-0000-000000000004) + (paper "A4") + (wire (pts (xy 5 5) (xy 10 5)) + (stroke (width 0) (type default)) + (uuid aaaaaaaa-0000-0000-0000-000000000005) + ) +) +""" + sch_path = self._write_schematic(sch_content, tmp_path) + + from commands.schematic import SchematicManager + + sch = SchematicManager.load_schematic(str(sch_path)) + result = get_net_at_point(sch, str(sch_path), 5.0, 5.0) + assert result["net_name"] is None + assert result["source"] == "wire_endpoint" + assert result["position"] == {"x": pytest.approx(5.0), "y": pytest.approx(5.0)} diff --git a/tests/test_net_connectivity.py b/tests/test_net_connectivity.py new file mode 100644 index 0000000..3750f3c --- /dev/null +++ b/tests/test_net_connectivity.py @@ -0,0 +1,516 @@ +""" +Tests for connected_pin_count in list_schematic_nets and the list_floating_labels tool. + +Covers: + - Schema registration for list_floating_labels (TestListFloatingLabelsSchema) + - Handler dispatch registration (TestListFloatingLabelsDispatch) + - Parameter validation (TestListFloatingLabelsParamValidation) + - Core logic: list_floating_labels (TestListFloatingLabelsCoreLogic) + - Core logic: count_pins_on_net (TestCountPinsOnNet) + - connected_pin_count field in list_schematic_nets handler (TestListSchematicNetsConnectedPinCount) + - Integration: floating labels in a real schematic file (TestListFloatingLabelsIntegration) +""" + +import shutil +import sys +import tempfile +from pathlib import Path +from typing import Any +from unittest.mock import MagicMock, patch + +import pytest + +sys.path.insert(0, str(Path(__file__).parent.parent / "python")) + +from commands.wire_connectivity import ( + _build_adjacency, + _parse_virtual_connections, + _parse_wires, + count_pins_on_net, + list_floating_labels, +) + +# --------------------------------------------------------------------------- +# Shared mock helpers +# --------------------------------------------------------------------------- + +TEMPLATE_SCH = Path(__file__).parent.parent / "python" / "templates" / "empty.kicad_sch" + + +def _make_point(x: float, y: float) -> MagicMock: + pt = MagicMock() + pt.value = [x, y] + return pt + + +def _make_wire(x1: float, y1: float, x2: float, y2: float) -> MagicMock: + wire = MagicMock() + wire.pts = MagicMock() + wire.pts.xy = [_make_point(x1, y1), _make_point(x2, y2)] + return wire + + +def _make_label(name: str, x: float, y: float) -> MagicMock: + label = MagicMock() + label.value = name + label.at = MagicMock() + label.at.value = [x, y, 0] + return label + + +def _make_schematic_no_labels_no_symbols(*wires: Any) -> MagicMock: + sch = MagicMock() + sch.wire = list(wires) + del sch.label + del sch.symbol + return sch + + +# --------------------------------------------------------------------------- +# TestListFloatingLabelsSchema +# --------------------------------------------------------------------------- + + +@pytest.mark.unit +class TestListFloatingLabelsSchema: + """Verify the list_floating_labels schema is registered and well-formed.""" + + def test_schema_registered(self) -> None: + from schemas.tool_schemas import TOOL_SCHEMAS + + assert "list_floating_labels" in TOOL_SCHEMAS + + def test_schema_required_fields(self) -> None: + from schemas.tool_schemas import TOOL_SCHEMAS + + required = TOOL_SCHEMAS["list_floating_labels"]["inputSchema"]["required"] + assert required == ["schematicPath"] + + def test_schema_has_title_and_description(self) -> None: + from schemas.tool_schemas import TOOL_SCHEMAS + + schema = TOOL_SCHEMAS["list_floating_labels"] + assert schema.get("title") + assert schema.get("description") + + +# --------------------------------------------------------------------------- +# TestListFloatingLabelsDispatch +# --------------------------------------------------------------------------- + + +@pytest.mark.unit +class TestListFloatingLabelsDispatch: + """Verify the handler is wired into KiCadInterface.command_routes.""" + + def test_list_floating_labels_in_routes(self) -> None: + with patch("kicad_interface.USE_IPC_BACKEND", False): + from kicad_interface import KiCADInterface + + iface = KiCADInterface.__new__(KiCADInterface) + iface.board = None + iface.project_filename = None + iface.use_ipc = False + iface.ipc_backend = MagicMock() + iface.ipc_board_api = None + iface.footprint_library = MagicMock() + iface.project_commands = MagicMock() + iface.board_commands = MagicMock() + iface.component_commands = MagicMock() + iface.routing_commands = MagicMock() + KiCADInterface.__init__(iface) + + assert "list_floating_labels" in iface.command_routes + assert callable(iface.command_routes["list_floating_labels"]) + + +# --------------------------------------------------------------------------- +# TestListFloatingLabelsParamValidation +# --------------------------------------------------------------------------- + + +@pytest.mark.unit +class TestListFloatingLabelsParamValidation: + """Handler returns error for missing schematicPath.""" + + def _make_handler(self) -> Any: + with patch("kicad_interface.USE_IPC_BACKEND", False): + from kicad_interface import KiCADInterface + + iface = KiCADInterface.__new__(KiCADInterface) + return iface._handle_list_floating_labels + + def test_missing_schematic_path(self) -> None: + handler = self._make_handler() + result = handler({}) + assert result["success"] is False + assert "schematicPath" in result["message"] + + def test_bad_schematic_path_returns_error(self) -> None: + handler = self._make_handler() + result = handler({"schematicPath": "/nonexistent/path/test.kicad_sch"}) + assert result["success"] is False + + +# --------------------------------------------------------------------------- +# TestListFloatingLabelsCoreLogic +# --------------------------------------------------------------------------- + + +@pytest.mark.unit +class TestListFloatingLabelsCoreLogic: + """Unit tests for the list_floating_labels function.""" + + def test_no_labels_returns_empty(self) -> None: + sch = _make_schematic_no_labels_no_symbols() + result = list_floating_labels(sch, "/tmp/test.kicad_sch") + assert result == [] + + def test_label_with_no_wires_and_no_pins_is_floating(self) -> None: + label = _make_label("SDA", 10.0, 5.0) + sch = MagicMock() + sch.wire = [] + sch.label = [label] + del sch.symbol + result = list_floating_labels(sch, "/tmp/test.kicad_sch") + assert len(result) == 1 + assert result[0]["name"] == "SDA" + assert result[0]["x"] == pytest.approx(10.0) + assert result[0]["y"] == pytest.approx(5.0) + assert result[0]["type"] == "label" + + def test_label_connected_to_pin_not_floating(self) -> None: + """Label at (0,0) connected to a pin at (2,0) via wire should NOT be floating.""" + wire = _make_wire(0.0, 0.0, 2.0, 0.0) + label = _make_label("SCL", 0.0, 0.0) + + sch = MagicMock() + sch.wire = [wire] + sch.label = [label] + + # Mock a symbol whose pin is at (2, 0) + symbol = MagicMock() + symbol.property = MagicMock() + symbol.property.Reference = MagicMock() + symbol.property.Reference.value = "U1" + sch.symbol = [symbol] + + with patch( + "commands.pin_locator.PinLocator.get_all_symbol_pins", + return_value={"1": (2.0, 0.0)}, + ): + result = list_floating_labels(sch, "/tmp/test.kicad_sch") + + assert result == [] + + def test_label_not_connected_to_any_pin_is_floating(self) -> None: + """Label at (0,0) with no wires to any pin should be floating.""" + label = _make_label("MOSI", 0.0, 0.0) + wire = _make_wire(0.0, 0.0, 1.0, 0.0) + + sch = MagicMock() + sch.wire = [wire] + sch.label = [label] + + # A symbol whose pin is at a completely different location + symbol = MagicMock() + symbol.property = MagicMock() + symbol.property.Reference = MagicMock() + symbol.property.Reference.value = "U2" + sch.symbol = [symbol] + + with patch( + "commands.pin_locator.PinLocator.get_all_symbol_pins", + return_value={"1": (99.0, 99.0)}, + ): + result = list_floating_labels(sch, "/tmp/test.kicad_sch") + + assert len(result) == 1 + assert result[0]["name"] == "MOSI" + + def test_label_directly_on_pin_not_floating(self) -> None: + """Label placed directly at a pin position (no wire needed) should NOT be floating.""" + label = _make_label("PWR", 5.0, 3.0) + + sch = MagicMock() + sch.wire = [] + sch.label = [label] + + symbol = MagicMock() + symbol.property = MagicMock() + symbol.property.Reference = MagicMock() + symbol.property.Reference.value = "R1" + sch.symbol = [symbol] + + # Pin is exactly at the label position + with patch( + "commands.pin_locator.PinLocator.get_all_symbol_pins", + return_value={"1": (5.0, 3.0)}, + ): + result = list_floating_labels(sch, "/tmp/test.kicad_sch") + + assert result == [] + + def test_multiple_labels_mixed_floating_and_connected(self) -> None: + """Two labels: one connected, one floating.""" + label_connected = _make_label("NET_A", 0.0, 0.0) + label_floating = _make_label("NET_B", 20.0, 20.0) + wire = _make_wire(0.0, 0.0, 2.0, 0.0) + + sch = MagicMock() + sch.wire = [wire] + sch.label = [label_connected, label_floating] + + symbol = MagicMock() + symbol.property = MagicMock() + symbol.property.Reference = MagicMock() + symbol.property.Reference.value = "C1" + sch.symbol = [symbol] + + with patch( + "commands.pin_locator.PinLocator.get_all_symbol_pins", + return_value={"1": (2.0, 0.0)}, + ): + result = list_floating_labels(sch, "/tmp/test.kicad_sch") + + assert len(result) == 1 + assert result[0]["name"] == "NET_B" + + def test_template_symbols_skipped(self) -> None: + """Symbols with _TEMPLATE references should be skipped, not crash.""" + label = _make_label("VBUS", 0.0, 0.0) + + sch = MagicMock() + sch.wire = [] + sch.label = [label] + + template_sym = MagicMock() + template_sym.property = MagicMock() + template_sym.property.Reference = MagicMock() + template_sym.property.Reference.value = "_TEMPLATE_R" + sch.symbol = [template_sym] + + with patch( + "commands.pin_locator.PinLocator.get_all_symbol_pins", + return_value={"1": (0.0, 0.0)}, + ) as mock_pins: + result = list_floating_labels(sch, "/tmp/test.kicad_sch") + + # _TEMPLATE_ symbols are skipped; mock_pins should not have been called + mock_pins.assert_not_called() + assert len(result) == 1 + + +# --------------------------------------------------------------------------- +# TestCountPinsOnNet +# --------------------------------------------------------------------------- + + +@pytest.mark.unit +class TestCountPinsOnNet: + """Unit tests for count_pins_on_net.""" + + def _build_graph(self, sch: Any, schematic_path: str): # type: ignore[return] + all_wires = _parse_wires(sch) + if all_wires: + adjacency, iu_to_wires = _build_adjacency(all_wires) + else: + adjacency, iu_to_wires = [], {} + point_to_label, label_to_points = _parse_virtual_connections(sch, schematic_path) + return all_wires, iu_to_wires, adjacency, point_to_label, label_to_points + + def test_no_labels_returns_zero(self) -> None: + sch = _make_schematic_no_labels_no_symbols() + all_wires, iu_to_wires, adj, p2l, l2p = self._build_graph(sch, "/tmp/t.kicad_sch") + count = count_pins_on_net( + sch, "/tmp/t.kicad_sch", "VCC", all_wires, iu_to_wires, adj, p2l, l2p + ) + assert count == 0 + + def test_unknown_net_returns_zero(self) -> None: + wire = _make_wire(0.0, 0.0, 1.0, 0.0) + label = _make_label("SDA", 0.0, 0.0) + sch = MagicMock() + sch.wire = [wire] + sch.label = [label] + del sch.symbol + all_wires, iu_to_wires, adj, p2l, l2p = self._build_graph(sch, "/tmp/t.kicad_sch") + count = count_pins_on_net( + sch, "/tmp/t.kicad_sch", "UNKNOWN_NET", all_wires, iu_to_wires, adj, p2l, l2p + ) + assert count == 0 + + def test_counts_pin_via_wire(self) -> None: + """Label at (0,0), wire to (2,0), pin at (2,0) → count == 1.""" + wire = _make_wire(0.0, 0.0, 2.0, 0.0) + label = _make_label("SCL", 0.0, 0.0) + sch = MagicMock() + sch.wire = [wire] + sch.label = [label] + symbol = MagicMock() + symbol.property = MagicMock() + symbol.property.Reference = MagicMock() + symbol.property.Reference.value = "U1" + sch.symbol = [symbol] + all_wires, iu_to_wires, adj, p2l, l2p = self._build_graph(sch, "/tmp/t.kicad_sch") + with patch( + "commands.pin_locator.PinLocator.get_all_symbol_pins", + return_value={"3": (2.0, 0.0)}, + ): + count = count_pins_on_net( + sch, "/tmp/t.kicad_sch", "SCL", all_wires, iu_to_wires, adj, p2l, l2p + ) + assert count == 1 + + def test_no_symbol_attribute_returns_zero(self) -> None: + wire = _make_wire(0.0, 0.0, 2.0, 0.0) + label = _make_label("SDA", 0.0, 0.0) + sch = MagicMock() + sch.wire = [wire] + sch.label = [label] + del sch.symbol + all_wires, iu_to_wires, adj, p2l, l2p = self._build_graph(sch, "/tmp/t.kicad_sch") + count = count_pins_on_net( + sch, "/tmp/t.kicad_sch", "SDA", all_wires, iu_to_wires, adj, p2l, l2p + ) + assert count == 0 + + +# --------------------------------------------------------------------------- +# TestListSchematicNetsConnectedPinCount +# --------------------------------------------------------------------------- + + +@pytest.mark.unit +class TestListSchematicNetsConnectedPinCount: + """Verify connected_pin_count is present in list_schematic_nets response.""" + + def _make_handler(self) -> Any: + with patch("kicad_interface.USE_IPC_BACKEND", False): + from kicad_interface import KiCADInterface + + iface = KiCADInterface.__new__(KiCADInterface) + return iface._handle_list_schematic_nets + + def test_connected_pin_count_present_in_response(self) -> None: + handler = self._make_handler() + + label = _make_label("NET1", 0.0, 0.0) + mock_sch = MagicMock() + mock_sch.wire = [] + mock_sch.label = [label] + del mock_sch.global_label + del mock_sch.symbol + + with ( + patch("kicad_interface.SchematicManager.load_schematic", return_value=mock_sch), + patch( + "kicad_interface.ConnectionManager.get_net_connections", + return_value=[], + ), + ): + result = handler({"schematicPath": "/tmp/test.kicad_sch"}) + + assert result["success"] is True + assert len(result["nets"]) == 1 + net = result["nets"][0] + assert "connected_pin_count" in net + assert isinstance(net["connected_pin_count"], int) + + def test_connected_pin_count_is_zero_when_no_pins(self) -> None: + handler = self._make_handler() + + label = _make_label("ORPHAN_NET", 50.0, 50.0) + mock_sch = MagicMock() + mock_sch.wire = [] + mock_sch.label = [label] + del mock_sch.global_label + del mock_sch.symbol + + with ( + patch("kicad_interface.SchematicManager.load_schematic", return_value=mock_sch), + patch( + "kicad_interface.ConnectionManager.get_net_connections", + return_value=[], + ), + ): + result = handler({"schematicPath": "/tmp/test.kicad_sch"}) + + assert result["success"] is True + assert result["nets"][0]["connected_pin_count"] == 0 + + +# --------------------------------------------------------------------------- +# TestListFloatingLabelsIntegration +# --------------------------------------------------------------------------- + + +@pytest.mark.integration +class TestListFloatingLabelsIntegration: + """Integration tests using a real .kicad_sch file.""" + + def _make_sch_with_floating_label(self, tmp_path: Path) -> Path: + """Copy the empty template and append a floating label.""" + sch_path = tmp_path / "test.kicad_sch" + shutil.copy(TEMPLATE_SCH, sch_path) + content = sch_path.read_text(encoding="utf-8") + floating_label = ( + ' (label "FLOATING_NET" (at 100 100 0)\n' + " (effects (font (size 1.27 1.27)))\n" + " (uuid 11111111-0000-0000-0000-000000000001)\n" + " )" + ) + idx = content.rfind(")") + content = content[:idx] + "\n" + floating_label + "\n)" + sch_path.write_text(content, encoding="utf-8") + return sch_path + + def test_empty_schematic_has_no_floating_labels(self) -> None: + with tempfile.TemporaryDirectory() as tmp: + sch_path = Path(tmp) / "empty.kicad_sch" + shutil.copy(TEMPLATE_SCH, sch_path) + + with patch("kicad_interface.USE_IPC_BACKEND", False): + from kicad_interface import KiCADInterface + + iface = KiCADInterface.__new__(KiCADInterface) + result = iface._handle_list_floating_labels({"schematicPath": str(sch_path)}) + + assert result["success"] is True + assert result["count"] == 0 + assert result["floating_labels"] == [] + + def test_schematic_with_floating_label_detected(self) -> None: + with tempfile.TemporaryDirectory() as tmp: + sch_path = self._make_sch_with_floating_label(Path(tmp)) + + with patch("kicad_interface.USE_IPC_BACKEND", False): + from kicad_interface import KiCADInterface + + iface = KiCADInterface.__new__(KiCADInterface) + result = iface._handle_list_floating_labels({"schematicPath": str(sch_path)}) + + assert result["success"] is True + assert result["count"] == 1 + label = result["floating_labels"][0] + assert label["name"] == "FLOATING_NET" + assert label["x"] == pytest.approx(100.0) + assert label["y"] == pytest.approx(100.0) + assert label["type"] == "label" + + def test_list_schematic_nets_has_connected_pin_count(self) -> None: + with tempfile.TemporaryDirectory() as tmp: + sch_path = self._make_sch_with_floating_label(Path(tmp)) + + with patch("kicad_interface.USE_IPC_BACKEND", False): + from kicad_interface import KiCADInterface + + iface = KiCADInterface.__new__(KiCADInterface) + result = iface._handle_list_schematic_nets({"schematicPath": str(sch_path)}) + + assert result["success"] is True + assert result["count"] == 1 + net = result["nets"][0] + assert net["name"] == "FLOATING_NET" + assert "connected_pin_count" in net + assert net["connected_pin_count"] == 0 diff --git a/tests/test_net_label_pin_snapping.py b/tests/test_net_label_pin_snapping.py new file mode 100644 index 0000000..d4b1f93 --- /dev/null +++ b/tests/test_net_label_pin_snapping.py @@ -0,0 +1,300 @@ +""" +Tests for net label pin-snapping and connect_to_net richer response. + +Covers: + - add_schematic_net_label with componentRef+pinNumber snaps to exact pin coords + - add_schematic_net_label without position and without pin ref returns error + - add_schematic_net_label with unknown pin returns an informative error + - connect_to_net returns pin_location, label_location, wire_stub on success + - connect_to_net returns success=False with message on failure + - connect_passthrough uses new dict return from connect_to_net correctly + - tool_schemas.py reflects new optional fields +""" + +import sys +import types +from pathlib import Path +from typing import Any +from unittest.mock import MagicMock, patch + +import pytest + +# --------------------------------------------------------------------------- +# Path setup – mirror existing test files +# --------------------------------------------------------------------------- + +PYTHON_DIR = Path(__file__).parent.parent / "python" +sys.path.insert(0, str(PYTHON_DIR)) + + +# --------------------------------------------------------------------------- +# Helpers +# --------------------------------------------------------------------------- + + +def _make_iface() -> Any: + """Return a KiCADInterface instance with __init__ stubbed out.""" + for mod in ["pcbnew", "skip"]: + sys.modules.setdefault(mod, types.ModuleType(mod)) + from kicad_interface import KiCADInterface + + with patch.object(KiCADInterface, "__init__", lambda self, *a, **kw: None): + return KiCADInterface.__new__(KiCADInterface) + + +# --------------------------------------------------------------------------- +# 1. Schema tests +# --------------------------------------------------------------------------- + + +@pytest.mark.unit +class TestAddNetLabelSchema: + """Verify tool_schemas.py reflects the new add_schematic_net_label API.""" + + @pytest.fixture(autouse=True) + def load_schemas(self) -> Any: + from schemas.tool_schemas import SCHEMATIC_TOOLS + + self.tools = {t["name"]: t for t in SCHEMATIC_TOOLS} + + def test_position_is_optional(self) -> None: + schema = self.tools["add_schematic_net_label"]["inputSchema"] + assert "position" not in schema["required"], "position must not be required" + + def test_component_ref_property_exists(self) -> None: + schema = self.tools["add_schematic_net_label"]["inputSchema"] + assert "componentRef" in schema["properties"] + + def test_pin_number_property_exists(self) -> None: + schema = self.tools["add_schematic_net_label"]["inputSchema"] + assert "pinNumber" in schema["properties"] + + def test_only_schematic_path_and_net_name_required(self) -> None: + schema = self.tools["add_schematic_net_label"]["inputSchema"] + assert set(schema["required"]) == {"schematicPath", "netName"} + + +@pytest.mark.unit +class TestConnectToNetSchema: + """Verify tool_schemas.py reflects the richer connect_to_net description.""" + + @pytest.fixture(autouse=True) + def load_schemas(self) -> Any: + from schemas.tool_schemas import SCHEMATIC_TOOLS + + self.tools = {t["name"]: t for t in SCHEMATIC_TOOLS} + + def test_description_mentions_pin_location(self) -> None: + desc = self.tools["connect_to_net"]["description"] + assert "pin_location" in desc + + def test_description_mentions_label_location(self) -> None: + desc = self.tools["connect_to_net"]["description"] + assert "label_location" in desc + + +# --------------------------------------------------------------------------- +# 2. _handle_add_schematic_net_label – unit tests +# --------------------------------------------------------------------------- + + +@pytest.mark.unit +class TestHandleAddSchematicNetLabelSnapping: + """Unit tests for the pin-snapping path of _handle_add_schematic_net_label.""" + + @pytest.fixture(autouse=True) + def setup(self) -> Any: + self.iface = _make_iface() + + # -- happy-path: snap to pin ----------------------------------------- + + @patch("commands.wire_manager.WireManager.add_label", return_value=True) + @patch("commands.pin_locator.PinLocator.get_pin_location", return_value=[42.0, 13.5]) + def test_snap_uses_pin_coords(self, mock_pin_loc: Any, mock_add_label: Any) -> None: + result = self.iface._handle_add_schematic_net_label( + { + "schematicPath": "/fake/sch.kicad_sch", + "netName": "VCC", + "componentRef": "U1", + "pinNumber": "1", + } + ) + assert result["success"] is True + assert result["actual_position"] == [42.0, 13.5] + assert result["snapped_to_pin"] == {"component": "U1", "pin": "1"} + # WireManager.add_label must have been called with the pin coords + mock_add_label.assert_called_once() + call_args = mock_add_label.call_args + assert call_args[0][2] == [42.0, 13.5] # position positional arg + + @patch("commands.wire_manager.WireManager.add_label", return_value=True) + @patch("commands.pin_locator.PinLocator.get_pin_location", return_value=[10.0, 20.0]) + def test_snap_ignores_provided_position(self, mock_pin_loc: Any, mock_add_label: Any) -> None: + """If both position and componentRef/pinNumber are given, pin coords win.""" + result = self.iface._handle_add_schematic_net_label( + { + "schematicPath": "/fake/sch.kicad_sch", + "netName": "GND", + "position": [999.0, 999.0], + "componentRef": "R1", + "pinNumber": "2", + } + ) + assert result["success"] is True + assert result["actual_position"] == [10.0, 20.0] + + # -- error: pin not found -------------------------------------------- + + @patch("commands.pin_locator.PinLocator.get_pin_location", return_value=None) + def test_snap_unknown_pin_returns_error(self, mock_pin_loc: Any) -> None: + result = self.iface._handle_add_schematic_net_label( + { + "schematicPath": "/fake/sch.kicad_sch", + "netName": "VCC", + "componentRef": "U99", + "pinNumber": "99", + } + ) + assert result["success"] is False + assert "U99" in result["message"] or "pin" in result["message"].lower() + + # -- error: no position and no pin ref -------------------------------- + + def test_no_position_no_ref_returns_error(self) -> None: + result = self.iface._handle_add_schematic_net_label( + { + "schematicPath": "/fake/sch.kicad_sch", + "netName": "VCC", + } + ) + assert result["success"] is False + assert "position" in result["message"].lower() or "componentRef" in result["message"] + + # -- happy-path: explicit position ------------------------------------ + + @patch("commands.wire_manager.WireManager.add_label", return_value=True) + def test_explicit_position_used_when_no_ref(self, mock_add_label: Any) -> None: + result = self.iface._handle_add_schematic_net_label( + { + "schematicPath": "/fake/sch.kicad_sch", + "netName": "CLK", + "position": [55.0, 77.0], + } + ) + assert result["success"] is True + assert result["actual_position"] == [55.0, 77.0] + assert "snapped_to_pin" not in result + + # -- missing required params ----------------------------------------- + + def test_missing_net_name_returns_error(self) -> None: + result = self.iface._handle_add_schematic_net_label( + { + "schematicPath": "/fake/sch.kicad_sch", + "position": [10.0, 20.0], + } + ) + assert result["success"] is False + + +# --------------------------------------------------------------------------- +# 3. connect_to_net – unit tests +# --------------------------------------------------------------------------- + + +@pytest.mark.unit +class TestConnectToNetRicherResponse: + """connect_to_net now returns coordinates instead of a bare bool.""" + + @patch("commands.wire_manager.WireManager.add_label", return_value=True) + @patch("commands.wire_manager.WireManager.add_wire", return_value=True) + @patch("commands.pin_locator.PinLocator.get_pin_angle", return_value=0.0) + @patch("commands.pin_locator.PinLocator.get_pin_location", return_value=[100.0, 50.0]) + def test_success_returns_coordinates( + self, + mock_pin_loc: Any, + mock_pin_angle: Any, + mock_add_wire: Any, + mock_add_label: Any, + ) -> None: + from commands.connection_schematic import ConnectionManager + + result = ConnectionManager.connect_to_net(Path("/fake/sch.kicad_sch"), "U1", "5", "VCC") + assert result["success"] is True + assert result["pin_location"] == [100.0, 50.0] + assert "label_location" in result + assert "wire_stub" in result + # wire_stub is [[pin_x, pin_y], [label_x, label_y]] + assert result["wire_stub"][0] == [100.0, 50.0] + assert result["wire_stub"][1] == result["label_location"] + + @patch("commands.pin_locator.PinLocator.get_pin_location", return_value=None) + def test_unknown_pin_returns_failure_dict(self, mock_pin_loc: Any) -> None: + from commands.connection_schematic import ConnectionManager + + result = ConnectionManager.connect_to_net(Path("/fake/sch.kicad_sch"), "U99", "99", "VCC") + assert result["success"] is False + assert "message" in result + + @patch("commands.wire_manager.WireManager.add_wire", return_value=False) + @patch("commands.pin_locator.PinLocator.get_pin_angle", return_value=0.0) + @patch("commands.pin_locator.PinLocator.get_pin_location", return_value=[10.0, 20.0]) + def test_wire_failure_returns_failure_dict( + self, mock_pin_loc: Any, mock_pin_angle: Any, mock_add_wire: Any + ) -> None: + from commands.connection_schematic import ConnectionManager + + result = ConnectionManager.connect_to_net(Path("/fake/sch.kicad_sch"), "R1", "1", "GND") + assert result["success"] is False + assert "message" in result + + +# --------------------------------------------------------------------------- +# 4. connect_passthrough – uses dict return correctly +# --------------------------------------------------------------------------- + + +@pytest.mark.unit +class TestConnectPassthroughUsesDict: + """connect_passthrough must handle the dict returned by connect_to_net.""" + + @patch( + "commands.connection_schematic.ConnectionManager.connect_to_net", + return_value={ + "success": True, + "pin_location": [0, 0], + "label_location": [2.54, 0], + "wire_stub": [[0, 0], [2.54, 0]], + "message": "ok", + }, + ) + @patch( + "commands.pin_locator.PinLocator.get_all_symbol_pins", + side_effect=[{"1": [0.0, 0.0]}, {"1": [10.0, 10.0]}], + ) + def test_passthrough_succeeds_with_dict_return(self, mock_pins: Any, mock_connect: Any) -> None: + from commands.connection_schematic import ConnectionManager + + result = ConnectionManager.connect_passthrough( + Path("/fake/sch.kicad_sch"), "J1", "J2", net_prefix="PIN" + ) + assert len(result["connected"]) == 1 + assert len(result["failed"]) == 0 + + @patch( + "commands.connection_schematic.ConnectionManager.connect_to_net", + return_value={"success": False, "message": "pin not found"}, + ) + @patch( + "commands.pin_locator.PinLocator.get_all_symbol_pins", + side_effect=[{"1": [0.0, 0.0]}, {"1": [10.0, 10.0]}], + ) + def test_passthrough_records_failure_with_dict_return( + self, mock_pins: Any, mock_connect: Any + ) -> None: + from commands.connection_schematic import ConnectionManager + + result = ConnectionManager.connect_passthrough( + Path("/fake/sch.kicad_sch"), "J1", "J2", net_prefix="PIN" + ) + assert len(result["failed"]) >= 1 diff --git a/tests/test_schematic_analysis.py b/tests/test_schematic_analysis.py index d7efab3..a54cbc2 100644 --- a/tests/test_schematic_analysis.py +++ b/tests/test_schematic_analysis.py @@ -34,6 +34,7 @@ from commands.schematic_analysis import ( _point_in_rect, _transform_local_point, compute_symbol_bbox, + find_orphaned_wires, find_overlapping_elements, find_wires_crossing_symbols, get_elements_in_region, @@ -946,3 +947,133 @@ class TestIntegrationGraphicsBbox: assert max(xs) == pytest.approx(1.27) assert min(ys) == pytest.approx(-1.27) assert max(ys) == pytest.approx(1.27) + + +# --------------------------------------------------------------------------- +# TestFindOrphanedWires +# --------------------------------------------------------------------------- + + +@pytest.mark.integration +class TestFindOrphanedWires: + """Integration tests for find_orphaned_wires.""" + + def test_empty_schematic_no_orphans(self) -> None: + """A schematic with no wires has no orphans.""" + tmp = _make_temp_schematic() + result = find_orphaned_wires(tmp) + assert result["count"] == 0 + assert result["orphaned_wires"] == [] + + def test_isolated_wire_is_orphaned(self) -> None: + """A single wire floating in empty space has both endpoints dangling.""" + extra = """ + (wire (pts (xy 10 20) (xy 30 20)) + (stroke (width 0) (type default)) + (uuid "w-isolated")) + """ + tmp = _make_temp_schematic(extra) + result = find_orphaned_wires(tmp) + assert result["count"] == 1 + w = result["orphaned_wires"][0] + assert len(w["dangling_ends"]) == 2 + + def test_wire_between_two_labels_not_orphaned(self) -> None: + """A wire whose endpoints both land on net labels is fully connected.""" + extra = """ + (label "VCC" (at 10 20 0) + (effects (font (size 1.27 1.27)) (justify left bottom)) + (uuid "lbl1")) + (label "GND" (at 30 20 0) + (effects (font (size 1.27 1.27)) (justify left bottom)) + (uuid "lbl2")) + (wire (pts (xy 10 20) (xy 30 20)) + (stroke (width 0) (type default)) + (uuid "w-label-to-label")) + """ + tmp = _make_temp_schematic(extra) + result = find_orphaned_wires(tmp) + assert result["count"] == 0 + + def test_wire_with_one_dangling_end(self) -> None: + """A wire from a label to empty space has exactly one dangling end.""" + extra = """ + (label "SIG" (at 10 20 0) + (effects (font (size 1.27 1.27)) (justify left bottom)) + (uuid "lbl-sig")) + (wire (pts (xy 10 20) (xy 40 20)) + (stroke (width 0) (type default)) + (uuid "w-stub")) + """ + tmp = _make_temp_schematic(extra) + result = find_orphaned_wires(tmp) + assert result["count"] == 1 + w = result["orphaned_wires"][0] + assert len(w["dangling_ends"]) == 1 + # The dangling end is the far end at x=40, not the label end at x=10 + assert w["dangling_ends"][0]["x"] == pytest.approx(40.0) + + def test_connected_wires_not_orphaned(self) -> None: + """Two wires sharing an endpoint are connected — neither is orphaned + provided the remaining ends are also anchored.""" + # Wire A: (10,20)→(20,20), Wire B: (20,20)→(30,20) + # Both share endpoint at (20,20). Anchor the outer ends with labels. + extra = """ + (label "A" (at 10 20 0) + (effects (font (size 1.27 1.27)) (justify left bottom)) + (uuid "lbl-a")) + (label "B" (at 30 20 0) + (effects (font (size 1.27 1.27)) (justify left bottom)) + (uuid "lbl-b")) + (wire (pts (xy 10 20) (xy 20 20)) + (stroke (width 0) (type default)) + (uuid "w1")) + (wire (pts (xy 20 20) (xy 30 20)) + (stroke (width 0) (type default)) + (uuid "w2")) + """ + tmp = _make_temp_schematic(extra) + result = find_orphaned_wires(tmp) + assert result["count"] == 0 + + def test_t_junction_shared_endpoint_not_dangling(self) -> None: + """Three wires meeting at a single point — the shared vertex is connected + to multiple wires and must not be reported as dangling.""" + # Three wires all touching (50, 50). Outer ends get labels. + extra = """ + (label "L1" (at 30 50 0) + (effects (font (size 1.27 1.27)) (justify left bottom)) + (uuid "lbl-t1")) + (label "L2" (at 70 50 0) + (effects (font (size 1.27 1.27)) (justify left bottom)) + (uuid "lbl-t2")) + (label "L3" (at 50 30 0) + (effects (font (size 1.27 1.27)) (justify left bottom)) + (uuid "lbl-t3")) + (wire (pts (xy 30 50) (xy 50 50)) + (stroke (width 0) (type default)) + (uuid "wt1")) + (wire (pts (xy 50 50) (xy 70 50)) + (stroke (width 0) (type default)) + (uuid "wt2")) + (wire (pts (xy 50 50) (xy 50 30)) + (stroke (width 0) (type default)) + (uuid "wt3")) + """ + tmp = _make_temp_schematic(extra) + result = find_orphaned_wires(tmp) + assert result["count"] == 0 + + def test_multiple_isolated_wires_all_reported(self) -> None: + """Two separate isolated wires are both reported.""" + extra = """ + (wire (pts (xy 10 10) (xy 20 10)) + (stroke (width 0) (type default)) + (uuid "wi1")) + (wire (pts (xy 50 50) (xy 60 50)) + (stroke (width 0) (type default)) + (uuid "wi2")) + """ + tmp = _make_temp_schematic(extra) + result = find_orphaned_wires(tmp) + assert result["count"] == 2 diff --git a/tests/test_snap_to_grid.py b/tests/test_snap_to_grid.py new file mode 100644 index 0000000..74e73a8 --- /dev/null +++ b/tests/test_snap_to_grid.py @@ -0,0 +1,246 @@ +""" +Tests for the snap_to_grid schematic tool. + +Unit tests cover the snapping math and per-element-type logic using synthetic +S-expressions. Integration tests run against real .kicad_sch files created +from the empty template. +""" + +import shutil +import sys +import tempfile +import uuid +from pathlib import Path + +import pytest +import sexpdata +from sexpdata import Symbol + +sys.path.insert(0, str(Path(__file__).resolve().parent.parent / "python")) + +from commands.schematic_snap import _is_on_grid, _snap_mm, snap_to_grid + +# --------------------------------------------------------------------------- +# Shared fixture helpers +# --------------------------------------------------------------------------- + +TEMPLATE_PATH = Path(__file__).resolve().parent.parent / "python" / "templates" / "empty.kicad_sch" + + +def _make_temp_schematic(extra_sexp: str = "") -> Path: + """Copy empty.kicad_sch to a temp dir, optionally injecting extra S-expressions.""" + tmp = Path(tempfile.mkdtemp()) / "test.kicad_sch" + shutil.copy(TEMPLATE_PATH, tmp) + if extra_sexp: + content = tmp.read_text(encoding="utf-8") + idx = content.rfind(")") + content = content[:idx] + "\n" + extra_sexp + "\n)" + tmp.write_text(content, encoding="utf-8") + return tmp + + +def _wire_sexp(x1: float, y1: float, x2: float, y2: float) -> str: + u = str(uuid.uuid4()) + return ( + f"(wire (pts (xy {x1} {y1}) (xy {x2} {y2}))\n" + f" (stroke (width 0) (type default))\n" + f' (uuid "{u}"))' + ) + + +def _junction_sexp(x: float, y: float) -> str: + u = str(uuid.uuid4()) + return f'(junction (at {x} {y}) (diameter 0) (color 0 0 0 0) (uuid "{u}"))' + + +def _label_sexp(name: str, x: float, y: float, angle: float = 0) -> str: + u = str(uuid.uuid4()) + return ( + f'(label "{name}" (at {x} {y} {angle})\n' + f" (effects (font (size 1.27 1.27)) (justify left bottom))\n" + f' (uuid "{u}"))' + ) + + +# --------------------------------------------------------------------------- +# Unit tests — pure math, no file I/O +# --------------------------------------------------------------------------- + + +@pytest.mark.unit +class TestSnapMath: + def test_snap_mm_already_on_grid(self): + assert _snap_mm(2.54, 2.54) == pytest.approx(2.54) + + def test_snap_mm_rounds_up(self): + # 2.55 is closer to 5.08 than to 2.54 (distance 2.53 vs 0.01) + # Actually 2.55 / 2.54 = 1.0039..., rounds to 1 → 2.54 + assert _snap_mm(2.55, 2.54) == pytest.approx(2.54) + + def test_snap_mm_rounds_to_next(self): + # 3.81 / 2.54 = 1.5 → rounds to 2 → 5.08 + assert _snap_mm(3.81, 2.54) == pytest.approx(5.08) + + def test_snap_mm_negative(self): + assert _snap_mm(-2.51, 2.54) == pytest.approx(-2.54) + + def test_snap_mm_zero(self): + assert _snap_mm(0.0, 2.54) == pytest.approx(0.0) + + def test_snap_mm_small_grid(self): + assert _snap_mm(1.28, 1.27) == pytest.approx(1.27) + + def test_is_on_grid_true(self): + assert _is_on_grid(2.54, 2.54) + assert _is_on_grid(0.0, 2.54) + assert _is_on_grid(5.08, 2.54) + + def test_is_on_grid_false(self): + assert not _is_on_grid(2.55, 2.54) + assert not _is_on_grid(1.0, 2.54) + + def test_snap_invalid_grid_raises(self): + with pytest.raises(ValueError, match="grid_size must be positive"): + snap_to_grid(Path("/nonexistent"), grid_size=-1.0) + + def test_snap_unknown_element_raises(self): + with pytest.raises(ValueError, match="Unknown element type"): + snap_to_grid(Path("/nonexistent"), elements=["bogus"]) + + +# --------------------------------------------------------------------------- +# Integration tests — real .kicad_sch files +# --------------------------------------------------------------------------- + + +@pytest.mark.integration +class TestSnapWires: + def test_off_grid_wire_is_snapped(self): + path = _make_temp_schematic(_wire_sexp(2.51, 5.03, 7.56, 5.03)) + result = snap_to_grid(path, grid_size=2.54, elements=["wires"]) + assert result["snapped"] >= 1 + + # Verify coordinates in the written file + data = sexpdata.loads(path.read_text(encoding="utf-8")) + wire = next( + item for item in data if isinstance(item, list) and item and item[0] == Symbol("wire") + ) + pts = next(sub for sub in wire[1:] if isinstance(sub, list) and sub[0] == Symbol("pts")) + xy_pairs = [sub for sub in pts[1:] if isinstance(sub, list) and sub[0] == Symbol("xy")] + for pt in xy_pairs: + assert _is_on_grid(float(pt[1]), 2.54), f"x={pt[1]} not on grid" + assert _is_on_grid(float(pt[2]), 2.54), f"y={pt[2]} not on grid" + + def test_on_grid_wire_counts_as_already_on_grid(self): + path = _make_temp_schematic(_wire_sexp(2.54, 5.08, 7.62, 5.08)) + result = snap_to_grid(path, grid_size=2.54, elements=["wires"]) + assert result["snapped"] == 0 + assert result["already_on_grid"] >= 1 + + def test_wires_not_snapped_when_excluded(self): + path = _make_temp_schematic(_wire_sexp(2.51, 5.03, 7.56, 5.03)) + result = snap_to_grid(path, grid_size=2.54, elements=["junctions"]) + assert result["snapped"] == 0 + + +@pytest.mark.integration +class TestSnapJunctions: + def test_off_grid_junction_is_snapped(self): + path = _make_temp_schematic(_junction_sexp(2.51, 2.51)) + result = snap_to_grid(path, grid_size=2.54, elements=["junctions"]) + assert result["snapped"] >= 1 + + data = sexpdata.loads(path.read_text(encoding="utf-8")) + junc = next( + item + for item in data + if isinstance(item, list) and item and item[0] == Symbol("junction") + ) + at = next(sub for sub in junc[1:] if isinstance(sub, list) and sub[0] == Symbol("at")) + assert _is_on_grid(float(at[1]), 2.54) + assert _is_on_grid(float(at[2]), 2.54) + + def test_on_grid_junction_unchanged(self): + path = _make_temp_schematic(_junction_sexp(2.54, 2.54)) + result = snap_to_grid(path, grid_size=2.54, elements=["junctions"]) + assert result["snapped"] == 0 + assert result["already_on_grid"] >= 1 + + +@pytest.mark.integration +class TestSnapLabels: + def test_off_grid_label_snapped_preserves_angle(self): + path = _make_temp_schematic(_label_sexp("NET_A", 2.51, 5.03, angle=90)) + result = snap_to_grid(path, grid_size=2.54, elements=["labels"]) + assert result["snapped"] >= 1 + + data = sexpdata.loads(path.read_text(encoding="utf-8")) + lbl = next( + item for item in data if isinstance(item, list) and item and item[0] == Symbol("label") + ) + at = next(sub for sub in lbl[1:] if isinstance(sub, list) and sub[0] == Symbol("at")) + assert _is_on_grid(float(at[1]), 2.54), f"x={at[1]} not on grid" + assert _is_on_grid(float(at[2]), 2.54), f"y={at[2]} not on grid" + # angle must be preserved + assert float(at[3]) == pytest.approx(90.0) + + def test_on_grid_label_unchanged(self): + path = _make_temp_schematic(_label_sexp("NET_B", 2.54, 5.08)) + result = snap_to_grid(path, grid_size=2.54, elements=["labels"]) + assert result["snapped"] == 0 + + +@pytest.mark.integration +class TestSnapDefaults: + def test_default_elements_snaps_wires_and_junctions_and_labels(self): + extra = "\n".join( + [ + _wire_sexp(2.51, 5.03, 7.56, 5.03), + _junction_sexp(2.51, 2.51), + _label_sexp("VCC", 2.51, 2.51), + ] + ) + path = _make_temp_schematic(extra) + result = snap_to_grid(path) # defaults: grid=2.54, elements=None + assert result["snapped"] >= 3 + assert result["grid_size"] == pytest.approx(1.27) + + def test_idempotent(self): + path = _make_temp_schematic(_wire_sexp(2.51, 5.03, 7.56, 5.03)) + snap_to_grid(path, grid_size=2.54) + content_after_first = path.read_text(encoding="utf-8") + snap_to_grid(path, grid_size=2.54) + content_after_second = path.read_text(encoding="utf-8") + assert content_after_first == content_after_second + + def test_default_grid_is_1_27mm(self): + # Regression: default was 2.54 mm, which displaces valid KiCAD pin + # coordinates that fall on the 50-mil (1.27 mm) grid but not on the + # 100-mil (2.54 mm) grid — e.g. 26.67 mm = 21 × 1.27 mm. + # With the correct 1.27 mm default those coordinates must be left + # untouched (snapped == 0, already_on_grid >= 1). + # 26.67 / 2.54 == 10.5 → would snap to 25.40 mm (off by 1.27 mm). + # 26.67 / 1.27 == 21.0 → already on grid, no move. + path = _make_temp_schematic(_wire_sexp(335.28, 26.67, 350.52, 26.67)) + result = snap_to_grid(path) # default grid + assert result["grid_size"] == pytest.approx(1.27) + assert result["snapped"] == 0, ( + "Wire at valid 50-mil pin coordinates was displaced by default snap — " + "default grid must be 1.27 mm, not 2.54 mm" + ) + assert result["already_on_grid"] >= 1 + + def test_custom_grid(self): + # 1.27 mm grid — wire at 1.25 should snap to 1.27 + path = _make_temp_schematic(_wire_sexp(1.25, 1.25, 2.51, 2.51)) + result = snap_to_grid(path, grid_size=1.27) + assert result["snapped"] >= 1 + data = sexpdata.loads(path.read_text(encoding="utf-8")) + wire = next( + item for item in data if isinstance(item, list) and item and item[0] == Symbol("wire") + ) + pts = next(sub for sub in wire[1:] if isinstance(sub, list) and sub[0] == Symbol("pts")) + xy_pairs = [sub for sub in pts[1:] if isinstance(sub, list) and sub[0] == Symbol("xy")] + for pt in xy_pairs: + assert _is_on_grid(float(pt[1]), 1.27), f"x={pt[1]} not on 1.27 grid" + assert _is_on_grid(float(pt[2]), 1.27), f"y={pt[2]} not on 1.27 grid" diff --git a/tests/test_wire_connectivity.py b/tests/test_wire_connectivity.py index 9c013e7..a36aa24 100644 --- a/tests/test_wire_connectivity.py +++ b/tests/test_wire_connectivity.py @@ -7,6 +7,8 @@ Covers: - Parameter validation in the handler (TestHandlerParamValidation) - Core logic: _to_iu, _parse_wires, _build_adjacency, _find_connected_wires, get_wire_connections (TestCoreLogic) + - New net/query_point fields and reference+pin input mode (TestGetWireConnectionsNewFields, + TestGetWireConnectionsHandlerRefPinMode) """ import sys @@ -77,8 +79,23 @@ class TestSchema: schema = TOOL_SCHEMAS["get_wire_connections"] required = schema["inputSchema"]["required"] assert "schematicPath" in required - assert "x" in required - assert "y" in required + # x, y and reference, pin are all optional (dual-mode input) + assert "x" not in required + assert "y" not in required + + def test_schema_optional_fields(self) -> None: + from schemas.tool_schemas import TOOL_SCHEMAS + + props = TOOL_SCHEMAS["get_wire_connections"]["inputSchema"]["properties"] + assert "reference" in props + assert "pin" in props + assert "x" in props + assert "y" in props + + def test_get_pin_net_not_registered(self) -> None: + from schemas.tool_schemas import TOOL_SCHEMAS + + assert "get_pin_net" not in TOOL_SCHEMAS def test_schema_has_title_and_description(self) -> None: from schemas.tool_schemas import TOOL_SCHEMAS @@ -145,14 +162,24 @@ class TestHandlerParamValidation: assert result["success"] is False assert "schematicPath" in result["message"] or "Missing" in result["message"] - def test_missing_x(self) -> None: + def test_missing_both_modes(self) -> None: handler = self._make_handler() - result = handler({"schematicPath": "/tmp/test.kicad_sch", "y": 2.0}) + result = handler({"schematicPath": "/tmp/test.kicad_sch"}) + assert result["success"] is False + assert ( + "reference" in result["message"] + or "x" in result["message"] + or "supply" in result["message"].lower() + ) + + def test_partial_reference_without_pin(self) -> None: + handler = self._make_handler() + result = handler({"schematicPath": "/tmp/test.kicad_sch", "reference": "U1"}) assert result["success"] is False - def test_missing_y(self) -> None: + def test_partial_pin_without_reference(self) -> None: handler = self._make_handler() - result = handler({"schematicPath": "/tmp/test.kicad_sch", "x": 1.0}) + result = handler({"schematicPath": "/tmp/test.kicad_sch", "pin": "3"}) assert result["success"] is False def test_non_numeric_x(self) -> None: @@ -304,7 +331,11 @@ class TestCoreLogic: sch = MagicMock() sch.wire = [] result = get_wire_connections(sch, "/fake/path.kicad_sch", 0.0, 0.0) - assert result == {"pins": [], "wires": []} + assert result is not None + assert result["pins"] == [] + assert result["wires"] == [] + assert result["net"] is None + assert result["query_point"] == {"x": 0.0, "y": 0.0} def test_get_wire_connections_no_wire_at_point_returns_none(self) -> None: sch = _make_schematic(_make_wire(0.0, 0.0, 1.0, 0.0)) @@ -313,7 +344,6 @@ class TestCoreLogic: def test_get_wire_connections_returns_wire_data(self) -> None: sch = _make_schematic(_make_wire(0.0, 0.0, 1.0, 0.0)) - # Prevent _find_pins_on_net from iterating symbols result = get_wire_connections(sch, "/fake/path.kicad_sch", 0.0, 0.0) assert result is not None assert result["pins"] == [] @@ -321,6 +351,8 @@ class TestCoreLogic: wire = result["wires"][0] assert wire["start"] == {"x": 0.0, "y": 0.0} assert wire["end"] == {"x": 1.0, "y": 0.0} + assert "net" in result + assert "query_point" in result def test_get_wire_connections_chain_returns_all_wires(self) -> None: sch = _make_schematic( @@ -330,3 +362,122 @@ class TestCoreLogic: result = get_wire_connections(sch, "/fake/path.kicad_sch", 0.0, 0.0) assert result is not None assert len(result["wires"]) == 2 + + +# --------------------------------------------------------------------------- +# TestGetWireConnectionsNewFields +# --------------------------------------------------------------------------- + + +@pytest.mark.unit +class TestGetWireConnectionsNewFields: + """Verify net and query_point are present in all return paths.""" + + def test_net_field_present_when_no_wires(self) -> None: + sch = MagicMock() + sch.wire = [] + result = get_wire_connections(sch, "/fake/path.kicad_sch", 1.0, 2.0) + assert result is not None + assert "net" in result + assert result["net"] is None + + def test_query_point_echoed_when_no_wires(self) -> None: + sch = MagicMock() + sch.wire = [] + result = get_wire_connections(sch, "/fake/path.kicad_sch", 3.5, 7.25) + assert result is not None + assert result["query_point"] == {"x": 3.5, "y": 7.25} + + def test_net_is_none_for_unnamed_net(self) -> None: + # Wire with no labels → net should be None + sch = _make_schematic(_make_wire(0.0, 0.0, 1.0, 0.0)) + result = get_wire_connections(sch, "/fake/path.kicad_sch", 0.0, 0.0) + assert result is not None + assert result["net"] is None + + def test_query_point_echoed_with_wire(self) -> None: + sch = _make_schematic(_make_wire(0.0, 0.0, 1.0, 0.0)) + result = get_wire_connections(sch, "/fake/path.kicad_sch", 0.0, 0.0) + assert result is not None + assert result["query_point"] == {"x": 0.0, "y": 0.0} + + def test_net_none_returned_when_no_wire_at_point(self) -> None: + sch = _make_schematic(_make_wire(0.0, 0.0, 1.0, 0.0)) + result = get_wire_connections(sch, "/fake/path.kicad_sch", 5.0, 0.0) + assert result is None # no match at midpoint + + +# --------------------------------------------------------------------------- +# TestGetWireConnectionsHandlerRefPinMode +# --------------------------------------------------------------------------- + + +@pytest.mark.unit +class TestGetWireConnectionsHandlerRefPinMode: + """Handler correctly resolves reference+pin to coordinates via PinLocator.""" + + def _make_handler(self) -> Any: + with patch("kicad_interface.USE_IPC_BACKEND", False): + from kicad_interface import KiCADInterface + + iface = KiCADInterface.__new__(KiCADInterface) + return iface._handle_get_wire_connections + + def test_ref_pin_resolves_to_coordinates(self) -> None: + handler = self._make_handler() + mock_result = { + "net": "VCC", + "pins": [], + "wires": [], + "query_point": {"x": 10.0, "y": 20.0}, + } + with ( + patch( + "commands.pin_locator.PinLocator.get_pin_location", + return_value=(10.0, 20.0), + ), + patch("commands.wire_connectivity.get_wire_connections", return_value=mock_result), + patch( + "kicad_interface.SchematicManager.load_schematic", + return_value=MagicMock(wire=[MagicMock()]), + ), + ): + result = handler( + {"schematicPath": "/fake/path.kicad_sch", "reference": "U1", "pin": "3"} + ) + assert result["success"] is True + + def test_ref_pin_not_found_returns_error(self) -> None: + handler = self._make_handler() + with patch( + "commands.pin_locator.PinLocator.get_pin_location", + return_value=None, + ): + result = handler( + {"schematicPath": "/fake/path.kicad_sch", "reference": "U1", "pin": "99"} + ) + assert result["success"] is False + assert "99" in result["message"] or "U1" in result["message"] + + def test_missing_both_modes_returns_error(self) -> None: + handler = self._make_handler() + result = handler({"schematicPath": "/fake/path.kicad_sch"}) + assert result["success"] is False + + def test_partial_reference_without_pin_returns_error(self) -> None: + handler = self._make_handler() + result = handler({"schematicPath": "/fake/path.kicad_sch", "reference": "U1"}) + assert result["success"] is False + + def test_partial_pin_without_reference_returns_error(self) -> None: + handler = self._make_handler() + result = handler({"schematicPath": "/fake/path.kicad_sch", "pin": "3"}) + assert result["success"] is False + + def test_get_pin_net_not_in_routes(self) -> None: + with patch("kicad_interface.USE_IPC_BACKEND", False): + from kicad_interface import KiCADInterface + + iface = KiCADInterface.__new__(KiCADInterface) + KiCADInterface.__init__(iface) + assert "get_pin_net" not in iface.command_routes