diff --git a/python/commands/schematic_analysis.py b/python/commands/schematic_analysis.py index 1bb357c..e2788dc 100644 --- a/python/commands/schematic_analysis.py +++ b/python/commands/schematic_analysis.py @@ -649,16 +649,21 @@ def _compute_pin_positions_direct( return result -def check_wire_collisions(schematic_path: Path) -> List[Dict[str, Any]]: +def find_wires_crossing_symbols(schematic_path: Path) -> List[Dict[str, Any]]: """ - Detect wires passing through component bodies without connecting to their pins. + Find all wires that cross over component symbol bodies. + + Wires passing over symbols are unacceptable in schematics — they indicate + routing mistakes where a wire was drawn across a component instead of + around it. For each non-power, non-template symbol: 1. Compute bounding box from pin positions (shrunk by margin). 2. For each wire segment, test intersection with the bbox. - 3. If intersects but no wire endpoint matches a pin → collision. + 3. If intersects and the wire is not simply terminating at a pin from + outside, report it as a crossing. - Returns list of collision dicts. + Returns list of crossing dicts. """ sexp_data = _load_sexp(schematic_path) symbols = _parse_symbols(sexp_data) @@ -717,12 +722,30 @@ def check_wire_collisions(schematic_path: Path) -> List[Dict[str, Any]]: for px, py in sd["pin_set"] ) - # Suppress only when exactly ONE endpoint is at a pin: the wire arrives - # from elsewhere and terminates at this component (a valid connection). - # If BOTH endpoints match pins of this same component, the wire shorts - # two pins while traversing the body — that IS a collision. + # When exactly one endpoint is at a pin, check whether the wire + # just terminates at the pin (valid connection) or continues through + # the component body (pass-through → collision). + # Nudge the pin endpoint slightly toward the other end; if the + # shortened segment still intersects the bbox, the wire extends + # into/through the body. if (start_at_pin or end_at_pin) and not (start_at_pin and end_at_pin): - continue + dx, dy = ex - sx, ey - sy + length = math.sqrt(dx * dx + dy * dy) + if length > 0: + nudge = min(0.2, length * 0.5) + ux, uy = dx / length, dy / length + if start_at_pin: + nsx, nsy = sx + ux * nudge, sy + uy * nudge + if not _line_segment_intersects_aabb( + nsx, nsy, ex, ey, bx1, by1, bx2, by2 + ): + continue # Wire terminates at pin from outside + else: + nex, ney = ex - ux * nudge, ey - uy * nudge + if not _line_segment_intersects_aabb( + sx, sy, nex, ney, bx1, by1, bx2, by2 + ): + continue # Wire terminates at pin from outside sym = sd["sym"] collisions.append({ diff --git a/python/kicad_interface.py b/python/kicad_interface.py index 723b00c..56dcc4c 100644 --- a/python/kicad_interface.py +++ b/python/kicad_interface.py @@ -403,7 +403,7 @@ class KiCADInterface: "find_unconnected_pins": self._handle_find_unconnected_pins, "find_overlapping_elements": self._handle_find_overlapping_elements, "get_elements_in_region": self._handle_get_elements_in_region, - "check_wire_collisions": self._handle_check_wire_collisions, + "find_wires_crossing_symbols": self._handle_find_wires_crossing_symbols, "import_svg_logo": self._handle_import_svg_logo, # UI/Process management commands "check_kicad_ui": self._handle_check_kicad_ui, @@ -2734,23 +2734,23 @@ class KiCADInterface: logger.error(traceback.format_exc()) return {"success": False, "message": str(e)} - def _handle_check_wire_collisions(self, params): - """Detect wires passing through component bodies without connecting to their pins""" - logger.info("Checking wire collisions in schematic") + def _handle_find_wires_crossing_symbols(self, params): + """Find wires that cross over component symbol bodies""" + logger.info("Finding wires crossing symbols in schematic") try: from pathlib import Path - from commands.schematic_analysis import check_wire_collisions + from commands.schematic_analysis import find_wires_crossing_symbols schematic_path = params.get("schematicPath") if not schematic_path: return {"success": False, "message": "schematicPath is required"} - result = check_wire_collisions(Path(schematic_path)) + result = find_wires_crossing_symbols(Path(schematic_path)) return { "success": True, "collisions": result, "count": len(result), - "message": f"Found {len(result)} wire collision(s)", + "message": f"Found {len(result)} wire(s) crossing symbols", } except Exception as e: logger.error(f"Error checking wire collisions: {e}") diff --git a/python/schemas/tool_schemas.py b/python/schemas/tool_schemas.py index 4e6290c..ec9d70e 100644 --- a/python/schemas/tool_schemas.py +++ b/python/schemas/tool_schemas.py @@ -1792,9 +1792,9 @@ SCHEMATIC_TOOLS = [ } }, { - "name": "check_wire_collisions", - "title": "Check Wire Collisions", - "description": "Detects wires that pass through component bodies without connecting to their pins. These are usually routing mistakes where a wire crosses over a symbol instead of connecting to it. Uses pin-based bounding boxes (approximate but effective for 80/20 detection).", + "name": "find_wires_crossing_symbols", + "title": "Find Wires Crossing Symbols", + "description": "Find all wires that cross over component symbol bodies. Wires passing over symbols are unacceptable in schematics — they indicate routing mistakes where a wire was drawn across a component instead of around it.", "inputSchema": { "type": "object", "properties": { diff --git a/python/tests/test_schematic_analysis.py b/python/tests/test_schematic_analysis.py index 3b68d1c..1f00297 100644 --- a/python/tests/test_schematic_analysis.py +++ b/python/tests/test_schematic_analysis.py @@ -34,7 +34,7 @@ from commands.schematic_analysis import ( find_unconnected_pins, find_overlapping_elements, get_elements_in_region, - check_wire_collisions, + find_wires_crossing_symbols, ) @@ -412,8 +412,8 @@ class TestIntegrationFindUnconnectedPins: @pytest.mark.integration -class TestIntegrationCheckWireCollisions: - """Integration test for wire collision detection.""" +class TestIntegrationFindWiresCrossingSymbols: + """Integration test for wire crossing symbol detection.""" def test_wire_not_touching_pins_is_collision(self): """A wire passing through a component bbox without pin contact → collision.""" @@ -426,7 +426,7 @@ class TestIntegrationCheckWireCollisions: (uuid "w1")) """ tmp = _make_temp_schematic(extra) - result = check_wire_collisions(tmp) + result = find_wires_crossing_symbols(tmp) d1_collisions = [c for c in result if c["component"]["reference"] == "D1"] assert len(d1_collisions) >= 1 @@ -452,7 +452,7 @@ class TestIntegrationCheckWireCollisions: (uuid "w-collision")) """ tmp = _make_temp_schematic(r_at_100 + r_at_200 + wire) - result = check_wire_collisions(tmp) + result = find_wires_crossing_symbols(tmp) # The wire must not be reported against the far-away R? at (200, 100) collisions_at_200 = [ c for c in result @@ -463,6 +463,41 @@ class TestIntegrationCheckWireCollisions: "likely caused by reference-lookup always returning the first 'R?'" ) + def test_wire_starting_at_pin_passing_through_body(self): + """A wire that starts at a pin but continues through the component body + must be flagged — this is the core bug where the old suppression logic + treated any wire touching a pin as a valid connection.""" + # LED D1 at (100,100) → pin 1 at (96.19, 100), pin 2 at (103.81, 100) + # Wire starts exactly at pin 1 and extends through the body to the right + extra = _make_led_sexp("D1", 100, 100) + """ + (wire (pts (xy 96.19 100) (xy 110 100)) + (stroke (width 0) (type default)) + (uuid "w-through")) + """ + tmp = _make_temp_schematic(extra) + result = find_wires_crossing_symbols(tmp) + d1_crossings = [c for c in result if c["component"]["reference"] == "D1"] + assert len(d1_crossings) >= 1, ( + "Wire starting at pin but passing through body must be detected" + ) + + def test_wire_terminating_at_pin_from_outside(self): + """A wire that arrives at a pin from outside the component body + is a valid connection and must NOT be flagged.""" + # LED D1 at (100,100) → pin 1 at (96.19, 100) + # Wire comes from the left and terminates at pin 1 + extra = _make_led_sexp("D1", 100, 100) + """ + (wire (pts (xy 80 100) (xy 96.19 100)) + (stroke (width 0) (type default)) + (uuid "w-valid")) + """ + tmp = _make_temp_schematic(extra) + result = find_wires_crossing_symbols(tmp) + d1_crossings = [c for c in result if c["component"]["reference"] == "D1"] + assert len(d1_crossings) == 0, ( + "Wire terminating at pin from outside should not be flagged" + ) + def test_wire_shorts_component_pins_detected_as_collision(self): """Regression: a wire connecting pin1→pin2 of the same component must be reported even though both endpoints land on pins.""" @@ -473,7 +508,7 @@ class TestIntegrationCheckWireCollisions: ' (uuid "aaaaaaaa-0000-0000-0000-000000000001"))' ) sch = _make_temp_schematic(r_sexp + "\n" + wire_sexp) - collisions = check_wire_collisions(sch) + collisions = find_wires_crossing_symbols(sch) assert len(collisions) == 1 w = collisions[0]["wire"] assert w["start"]["x"] == pytest.approx(100.0) diff --git a/src/tools/schematic.ts b/src/tools/schematic.ts index 28f5b7e..771dc57 100644 --- a/src/tools/schematic.ts +++ b/src/tools/schematic.ts @@ -1250,21 +1250,21 @@ Note: operates on .kicad_sch files only. To modify a PCB footprint use edit_comp }, ); - // Check wire collisions + // Find wires crossing symbols server.tool( - "check_wire_collisions", - "Detect wires that pass through component bodies without connecting to their pins. These are usually routing mistakes where a wire crosses over a symbol instead of connecting to it.", + "find_wires_crossing_symbols", + "Find all wires that cross over component symbol bodies. Wires passing over symbols are unacceptable in schematics — they indicate routing mistakes where a wire was drawn across a component instead of around it.", { schematicPath: z.string().describe("Path to the .kicad_sch schematic file"), }, async (args: { schematicPath: string }) => { - const result = await callKicadScript("check_wire_collisions", args); + const result = await callKicadScript("find_wires_crossing_symbols", args); if (result.success) { const collisions: any[] = result.collisions || []; - const lines = [`Found ${collisions.length} wire collision(s):`]; + const lines = [`Found ${collisions.length} wire(s) crossing symbols:`]; collisions.slice(0, 30).forEach((c: any, i: number) => { lines.push( - ` ${i + 1}. Wire (${c.wire.start.x},${c.wire.start.y})→(${c.wire.end.x},${c.wire.end.y}) passes through ${c.component.reference} (${c.component.libId})` + ` ${i + 1}. Wire (${c.wire.start.x},${c.wire.start.y})→(${c.wire.end.x},${c.wire.end.y}) crosses ${c.component.reference} (${c.component.libId})` ); }); if (collisions.length > 30) lines.push(` ... and ${collisions.length - 30} more`);