From 4f733bb3db33cc60c39ce0346db5c6891dd48456 Mon Sep 17 00:00:00 2001 From: Eugene Mikhantyev Date: Sun, 12 Apr 2026 17:02:35 +0100 Subject: [PATCH] refactor: consolidate get_pin_net into get_wire_connections get_pin_net was a superset of get_wire_connections with the same coordinate-based flood-fill but two extra response fields (net, query_point) and a reference+pin input mode. Having both tools confused LLM tool selection. get_wire_connections now: - Returns net (label name or null) and query_point in all response paths - Accepts reference+pin input in addition to x/y coordinates, resolving the pin endpoint via PinLocator internally get_pin_net tool, handler, schema, TS registration, and tests removed. test_wire_connectivity.py updated with coverage for all new behaviour. Co-Authored-By: Claude Sonnet 4.6 --- python/commands/wire_connectivity.py | 65 +----- python/kicad_interface.py | 109 +++------ python/schemas/tool_schemas.py | 65 ++---- src/tools/schematic.ts | 97 +++----- tests/test_get_pin_net.py | 325 --------------------------- tests/test_wire_connectivity.py | 167 +++++++++++++- 6 files changed, 259 insertions(+), 569 deletions(-) delete mode 100644 tests/test_get_pin_net.py diff --git a/python/commands/wire_connectivity.py b/python/commands/wire_connectivity.py index 3e65dd0..3785dd5 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 — @@ -229,67 +229,20 @@ def get_wire_connections( Net labels and power symbols are traversed: wires on the same named net are treated as connected even when they are not geometrically adjacent. - Returns dict with keys: - - "pins": list of {"component": str, "pin": str} - - "wires": list of {"start": {"x", "y"}, "end": {"x", "y"}} in mm - Or None if no wire endpoint found within tolerance of the query point. - """ - all_wires = _parse_wires(schematic) - if not all_wires: - return {"pins": [], "wires": []} - - adjacency, iu_to_wires = _build_adjacency(all_wires) - - point_to_label, label_to_points = _parse_virtual_connections(schematic, schematic_path) - - 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 visited is None: - return None - - wires_out = [ - { - "start": { - "x": all_wires[i][0][0] / _IU_PER_MM, - "y": all_wires[i][0][1] / _IU_PER_MM, - }, - "end": { - "x": all_wires[i][-1][0] / _IU_PER_MM, - "y": all_wires[i][-1][1] / _IU_PER_MM, - }, - } - for i in visited - ] - - if not hasattr(schematic, "symbol"): - return {"pins": [], "wires": wires_out} - - pins = _find_pins_on_net(net_points, schematic_path, schematic) - return {"pins": pins, "wires": wires_out} - - -def get_pin_net(schematic: Any, schematic_path: str, x_mm: float, y_mm: float) -> Optional[Dict]: - """Return the net name and all connected pins for the wire network at (x_mm, y_mm). - 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 at the query point. + 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 {"net": None, "pins": [], "wires": [], "query_point": {"x": x_mm, "y": y_mm}} + return {"net": None, "pins": [], "wires": [], "query_point": query_point} adjacency, iu_to_wires = _build_adjacency(all_wires) + point_to_label, label_to_points = _parse_virtual_connections(schematic, schematic_path) visited, net_points = _find_connected_wires( @@ -326,11 +279,11 @@ def get_pin_net(schematic: Any, schematic_path: str, x_mm: float, y_mm: float) - for i in visited ] - pins: List[Dict] = [] - if hasattr(schematic, "symbol"): - pins = _find_pins_on_net(net_points, schematic_path, schematic) + if not hasattr(schematic, "symbol"): + return {"net": net, "pins": [], "wires": wires_out, "query_point": query_point} - return {"net": net, "pins": pins, "wires": wires_out, "query_point": {"x": x_mm, "y": y_mm}} + pins = _find_pins_on_net(net_points, schematic_path, schematic) + return {"net": net, "pins": pins, "wires": wires_out, "query_point": query_point} def count_pins_on_net( diff --git a/python/kicad_interface.py b/python/kicad_interface.py index 58ae67d..49cbfff 100644 --- a/python/kicad_interface.py +++ b/python/kicad_interface.py @@ -383,7 +383,6 @@ class KiCADInterface: "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, - "get_pin_net": self._handle_get_pin_net, "run_erc": self._handle_run_erc, "generate_netlist": self._handle_generate_netlist, "sync_schematic_to_board": self._handle_sync_schematic_to_board, @@ -2515,29 +2514,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"} @@ -2549,7 +2576,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} @@ -2595,70 +2622,6 @@ class KiCADInterface: logger.error(traceback.format_exc()) return {"success": False, "message": str(e)} - def _handle_get_pin_net(self, params: Dict[str, Any]) -> Dict[str, Any]: - """Find net name and all connected pins for a reference+pin or coordinate query""" - logger.info("Getting pin net") - try: - from pathlib import Path - - from commands.pin_locator import PinLocator - from commands.wire_connectivity import get_pin_net - - 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") - - has_ref_pin = reference is not None and pin is not None - has_coords = x is not None and y is not None - - if not has_ref_pin and not has_coords: - return { - "success": False, - "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"} - - if not hasattr(schematic, "wire"): - return {"success": False, "message": "Schematic has no wires"} - - result = get_pin_net(schematic, schematic_path, x, y) - if result is None: - return { - "success": False, - "message": f"No wire found at ({x},{y}) — pin may not be connected", - } - - return {"success": True, **result} - - except Exception as e: - logger.error(f"Error getting pin net: {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") diff --git a/python/schemas/tool_schemas.py b/python/schemas/tool_schemas.py index bf4b1fc..d4eeac0 100644 --- a/python/schemas/tool_schemas.py +++ b/python/schemas/tool_schemas.py @@ -1503,24 +1503,41 @@ 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", "x", "y"], + "required": ["schematicPath"], }, }, { @@ -1552,44 +1569,6 @@ SCHEMATIC_TOOLS = [ "required": ["schematicPath", "x", "y"], }, }, - { - "name": "get_pin_net", - "title": "Get Pin Net", - "description": ( - "Returns the net name and all connected component pins for a query. " - "Accepts either a component reference + pin number (e.g. reference='U1', pin='3') " - "or a schematic coordinate (x, y in mm). " - "If a net label or power symbol is reachable from the query point, its name is " - "returned as 'net'; otherwise 'net' is null (unnamed net). " - "The 'pins' list contains every component pin on that same net." - ), - "inputSchema": { - "type": "object", - "properties": { - "schematicPath": { - "type": "string", - "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 a wire endpoint in mm. Pair with y.", - }, - "y": { - "type": "number", - "description": "Y coordinate of a wire endpoint in mm. Pair with x.", - }, - }, - "required": ["schematicPath"], - }, - }, { "name": "get_schematic_pin_locations", "title": "Get Schematic Pin Locations", diff --git a/src/tools/schematic.ts b/src/tools/schematic.ts index f82bb8e..3ddad9a 100644 --- a/src/tools/schematic.ts +++ b/src/tools/schematic.ts @@ -469,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)"}`, }, ], }; @@ -1498,61 +1524,4 @@ Note: operates on .kicad_sch files only. To modify a PCB footprint use edit_comp } }, ); - - server.tool( - "get_pin_net", - "Returns the net name and all connected pins for a component pin (reference + pin number) " + - "or a schematic coordinate (x, y in mm). Use this instead of list_schematic_nets + " + - "get_wire_connections when you want to answer 'what net is pin 3 of U1 on?'. " + - "Returns net=null for unnamed (unlabelled) nets.", - { - schematicPath: z.string().describe("Path to the schematic file"), - 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; - reference?: string; - pin?: string; - x?: number; - y?: number; - }) => { - const result = await callKicadScript("get_pin_net", args); - if (result.success) { - const netLabel = result.net ?? "(unnamed)"; - const pinList = (result.pins ?? []) - .map((p: any) => ` - ${p.component}/${p.pin}`) - .join("\n"); - const qp = result.query_point; - return { - content: [ - { - type: "text", - text: - `Net: ${netLabel}\n` + - `Query point: (${qp?.x ?? args.x}, ${qp?.y ?? args.y})\n` + - `Connected pins:\n${pinList || " (none found)"}`, - }, - ], - }; - } else { - return { - content: [ - { - type: "text", - text: `Failed to get pin net: ${result.message || "Unknown error"}`, - }, - ], - }; - } - }, - ); } diff --git a/tests/test_get_pin_net.py b/tests/test_get_pin_net.py deleted file mode 100644 index f9d43aa..0000000 --- a/tests/test_get_pin_net.py +++ /dev/null @@ -1,325 +0,0 @@ -""" -Tests for the get_pin_net tool and its handler. - -Covers: - - Schema shape (TestGetPinNetSchema) - - Handler dispatch registration (TestGetPinNetHandlerDispatch) - - Parameter validation in the handler (TestGetPinNetHandlerParamValidation) - - Core logic: get_pin_net function (TestGetPinNetCoreLogic) - - Reference+pin resolution path (TestGetPinNetHandlerRefPinResolution) -""" - -import sys -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_pin_net - -# --------------------------------------------------------------------------- -# Shared mock helpers (mirrors test_wire_connectivity.py) -# --------------------------------------------------------------------------- - - -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(*wires: Any) -> MagicMock: - sch = MagicMock() - sch.wire = list(wires) - del sch.label - del sch.symbol - return sch - - -# --------------------------------------------------------------------------- -# TestGetPinNetSchema -# --------------------------------------------------------------------------- - - -@pytest.mark.unit -class TestGetPinNetSchema: - """Verify the get_pin_net tool schema is present and well-formed.""" - - def test_schema_registered(self) -> None: - from schemas.tool_schemas import TOOL_SCHEMAS - - assert "get_pin_net" in TOOL_SCHEMAS - - def test_schema_required_fields(self) -> None: - from schemas.tool_schemas import TOOL_SCHEMAS - - required = TOOL_SCHEMAS["get_pin_net"]["inputSchema"]["required"] - assert required == ["schematicPath"] - - def test_schema_has_title_and_description(self) -> None: - from schemas.tool_schemas import TOOL_SCHEMAS - - schema = TOOL_SCHEMAS["get_pin_net"] - assert schema.get("title") - assert schema.get("description") - - def test_schema_has_optional_fields(self) -> None: - from schemas.tool_schemas import TOOL_SCHEMAS - - props = TOOL_SCHEMAS["get_pin_net"]["inputSchema"]["properties"] - for field in ("reference", "pin", "x", "y"): - assert field in props, f"Expected '{field}' in schema properties" - - -# --------------------------------------------------------------------------- -# TestGetPinNetHandlerDispatch -# --------------------------------------------------------------------------- - - -@pytest.mark.unit -class TestGetPinNetHandlerDispatch: - """Verify the handler is wired into KiCadInterface.command_routes.""" - - def test_get_pin_net_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_pin_net" in iface.command_routes - assert callable(iface.command_routes["get_pin_net"]) - - -# --------------------------------------------------------------------------- -# TestGetPinNetHandlerParamValidation -# --------------------------------------------------------------------------- - - -@pytest.mark.unit -class TestGetPinNetHandlerParamValidation: - """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_pin_net - - 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_both_modes(self) -> None: - handler = self._make_handler() - result = handler({"schematicPath": "/tmp/test.kicad_sch"}) - assert result["success"] is False - - def test_partial_ref_only(self) -> None: - handler = self._make_handler() - result = handler({"schematicPath": "/tmp/test.kicad_sch", "reference": "U1"}) - assert result["success"] is False - - def test_partial_pin_only(self) -> None: - handler = self._make_handler() - result = handler({"schematicPath": "/tmp/test.kicad_sch", "pin": "3"}) - assert result["success"] is False - - def test_partial_x_only(self) -> None: - handler = self._make_handler() - result = handler({"schematicPath": "/tmp/test.kicad_sch", "x": 1.0}) - assert result["success"] is False - - def test_partial_y_only(self) -> None: - handler = self._make_handler() - result = handler({"schematicPath": "/tmp/test.kicad_sch", "y": 1.0}) - assert result["success"] is False - - def test_non_numeric_coords(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"] - - -# --------------------------------------------------------------------------- -# TestGetPinNetCoreLogic -# --------------------------------------------------------------------------- - - -@pytest.mark.unit -class TestGetPinNetCoreLogic: - """Unit tests for the get_pin_net function.""" - - def test_no_wires_returns_empty_dict(self) -> None: - sch = MagicMock() - sch.wire = [] - del sch.label - del sch.symbol - result = get_pin_net(sch, "/tmp/test.kicad_sch", 1.0, 2.0) - assert result is not None - assert result["net"] is None - assert result["pins"] == [] - assert result["wires"] == [] - assert result["query_point"] == {"x": 1.0, "y": 2.0} - - def test_no_wire_at_point_returns_none(self) -> None: - sch = _make_schematic(_make_wire(0.0, 0.0, 1.0, 0.0)) - # Query a midpoint — not on a wire endpoint - result = get_pin_net(sch, "/tmp/test.kicad_sch", 0.5, 0.0) - assert result is None - - def test_unnamed_net_no_labels(self) -> None: - sch = _make_schematic(_make_wire(0.0, 0.0, 1.0, 0.0)) - result = get_pin_net(sch, "/tmp/test.kicad_sch", 0.0, 0.0) - assert result is not None - assert result["net"] is None - - def test_net_name_from_label(self) -> None: - """Wire with a net label at one endpoint should yield that label as net name.""" - wire = _make_wire(0.0, 0.0, 1.0, 0.0) - - label = MagicMock() - label.value = "SDA" - label.at = MagicMock() - label.at.value = [0.0, 0.0, 0] # label placed at wire start - - sch = MagicMock() - sch.wire = [wire] - sch.label = [label] - del sch.symbol - - result = get_pin_net(sch, "/tmp/test.kicad_sch", 0.0, 0.0) - assert result is not None - assert result["net"] == "SDA" - - def test_query_point_in_result(self) -> None: - sch = _make_schematic(_make_wire(5.0, 3.0, 6.0, 3.0)) - result = get_pin_net(sch, "/tmp/test.kicad_sch", 5.0, 3.0) - assert result is not None - assert result["query_point"] == {"x": 5.0, "y": 3.0} - - def test_result_has_pins_and_wires_keys(self) -> None: - sch = _make_schematic(_make_wire(0.0, 0.0, 1.0, 0.0)) - result = get_pin_net(sch, "/tmp/test.kicad_sch", 0.0, 0.0) - assert result is not None - assert "pins" in result - assert "wires" in result - - def test_wires_returned_in_mm(self) -> None: - sch = _make_schematic(_make_wire(2.0, 3.0, 4.0, 3.0)) - result = get_pin_net(sch, "/tmp/test.kicad_sch", 2.0, 3.0) - assert result is not None - assert len(result["wires"]) == 1 - w = result["wires"][0] - assert w["start"]["x"] == pytest.approx(2.0) - assert w["start"]["y"] == pytest.approx(3.0) - assert w["end"]["x"] == pytest.approx(4.0) - assert w["end"]["y"] == pytest.approx(3.0) - - -# --------------------------------------------------------------------------- -# TestGetPinNetHandlerRefPinResolution -# --------------------------------------------------------------------------- - - -@pytest.mark.unit -class TestGetPinNetHandlerRefPinResolution: - """Test the reference+pin → coordinate resolution path in the handler.""" - - 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_pin_net - - def test_ref_pin_resolves_to_coordinates(self) -> None: - handler = self._make_handler() - mock_result = { - "net": "SDA", - "pins": [{"component": "U1", "pin": "3"}], - "wires": [], - "query_point": {"x": 10.5, "y": 15.2}, - } - with ( - patch( - "commands.pin_locator.PinLocator.get_pin_location", - return_value=[10.5, 15.2], - ), - patch("kicad_interface.SchematicManager.load_schematic") as mock_load, - patch("commands.wire_connectivity.get_pin_net", return_value=mock_result) as mock_gpn, - ): - mock_sch = MagicMock() - mock_sch.wire = [_make_wire(10.5, 15.2, 11.5, 15.2)] - mock_load.return_value = mock_sch - - result = handler( - {"schematicPath": "/tmp/test.kicad_sch", "reference": "U1", "pin": "3"} - ) - - assert result["success"] is True - mock_gpn.assert_called_once() - call_args = mock_gpn.call_args - assert call_args[0][2] == pytest.approx(10.5) - assert call_args[0][3] == pytest.approx(15.2) - - def test_ref_pin_not_found(self) -> None: - handler = self._make_handler() - with patch( - "commands.pin_locator.PinLocator.get_pin_location", - return_value=None, - ): - result = handler( - {"schematicPath": "/tmp/test.kicad_sch", "reference": "U1", "pin": "99"} - ) - assert result["success"] is False - assert "U1" in result["message"] or "99" in result["message"] - - def test_coordinate_mode_passes_floats(self) -> None: - handler = self._make_handler() - mock_result = { - "net": None, - "pins": [], - "wires": [], - "query_point": {"x": 10.5, "y": 15.2}, - } - with ( - patch("kicad_interface.SchematicManager.load_schematic") as mock_load, - patch("commands.wire_connectivity.get_pin_net", return_value=mock_result) as mock_gpn, - ): - mock_sch = MagicMock() - mock_sch.wire = [_make_wire(10.5, 15.2, 11.5, 15.2)] - mock_load.return_value = mock_sch - - result = handler({"schematicPath": "/tmp/test.kicad_sch", "x": "10.5", "y": "15.2"}) - - assert result["success"] is True - call_args = mock_gpn.call_args - assert isinstance(call_args[0][2], float) - assert isinstance(call_args[0][3], float) - assert call_args[0][2] == pytest.approx(10.5) - assert call_args[0][3] == pytest.approx(15.2) 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