fix: rename check_wire_collisions to find_wires_crossing_symbols and detect pass-through wires
Wires that start at a component pin but continue through the body were incorrectly suppressed as "valid connections." Now nudges the pin endpoint toward the other end and re-tests intersection — if the shortened segment still hits the bbox, the wire passes through and is flagged. Renamed the tool from check_wire_collisions to find_wires_crossing_symbols across all layers (Python, handler, schema, TypeScript) to clarify that it finds wires crossing over component symbols, which is unacceptable in schematics. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
@@ -649,16 +649,21 @@ def _compute_pin_positions_direct(
|
|||||||
return result
|
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:
|
For each non-power, non-template symbol:
|
||||||
1. Compute bounding box from pin positions (shrunk by margin).
|
1. Compute bounding box from pin positions (shrunk by margin).
|
||||||
2. For each wire segment, test intersection with the bbox.
|
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)
|
sexp_data = _load_sexp(schematic_path)
|
||||||
symbols = _parse_symbols(sexp_data)
|
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"]
|
for px, py in sd["pin_set"]
|
||||||
)
|
)
|
||||||
|
|
||||||
# Suppress only when exactly ONE endpoint is at a pin: the wire arrives
|
# When exactly one endpoint is at a pin, check whether the wire
|
||||||
# from elsewhere and terminates at this component (a valid connection).
|
# just terminates at the pin (valid connection) or continues through
|
||||||
# If BOTH endpoints match pins of this same component, the wire shorts
|
# the component body (pass-through → collision).
|
||||||
# two pins while traversing the body — that IS a 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):
|
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"]
|
sym = sd["sym"]
|
||||||
collisions.append({
|
collisions.append({
|
||||||
|
|||||||
@@ -403,7 +403,7 @@ class KiCADInterface:
|
|||||||
"find_unconnected_pins": self._handle_find_unconnected_pins,
|
"find_unconnected_pins": self._handle_find_unconnected_pins,
|
||||||
"find_overlapping_elements": self._handle_find_overlapping_elements,
|
"find_overlapping_elements": self._handle_find_overlapping_elements,
|
||||||
"get_elements_in_region": self._handle_get_elements_in_region,
|
"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,
|
"import_svg_logo": self._handle_import_svg_logo,
|
||||||
# UI/Process management commands
|
# UI/Process management commands
|
||||||
"check_kicad_ui": self._handle_check_kicad_ui,
|
"check_kicad_ui": self._handle_check_kicad_ui,
|
||||||
@@ -2734,23 +2734,23 @@ class KiCADInterface:
|
|||||||
logger.error(traceback.format_exc())
|
logger.error(traceback.format_exc())
|
||||||
return {"success": False, "message": str(e)}
|
return {"success": False, "message": str(e)}
|
||||||
|
|
||||||
def _handle_check_wire_collisions(self, params):
|
def _handle_find_wires_crossing_symbols(self, params):
|
||||||
"""Detect wires passing through component bodies without connecting to their pins"""
|
"""Find wires that cross over component symbol bodies"""
|
||||||
logger.info("Checking wire collisions in schematic")
|
logger.info("Finding wires crossing symbols in schematic")
|
||||||
try:
|
try:
|
||||||
from pathlib import Path
|
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")
|
schematic_path = params.get("schematicPath")
|
||||||
if not schematic_path:
|
if not schematic_path:
|
||||||
return {"success": False, "message": "schematicPath is required"}
|
return {"success": False, "message": "schematicPath is required"}
|
||||||
|
|
||||||
result = check_wire_collisions(Path(schematic_path))
|
result = find_wires_crossing_symbols(Path(schematic_path))
|
||||||
return {
|
return {
|
||||||
"success": True,
|
"success": True,
|
||||||
"collisions": result,
|
"collisions": result,
|
||||||
"count": len(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:
|
except Exception as e:
|
||||||
logger.error(f"Error checking wire collisions: {e}")
|
logger.error(f"Error checking wire collisions: {e}")
|
||||||
|
|||||||
@@ -1792,9 +1792,9 @@ SCHEMATIC_TOOLS = [
|
|||||||
}
|
}
|
||||||
},
|
},
|
||||||
{
|
{
|
||||||
"name": "check_wire_collisions",
|
"name": "find_wires_crossing_symbols",
|
||||||
"title": "Check Wire Collisions",
|
"title": "Find Wires Crossing Symbols",
|
||||||
"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).",
|
"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": {
|
"inputSchema": {
|
||||||
"type": "object",
|
"type": "object",
|
||||||
"properties": {
|
"properties": {
|
||||||
|
|||||||
@@ -34,7 +34,7 @@ from commands.schematic_analysis import (
|
|||||||
find_unconnected_pins,
|
find_unconnected_pins,
|
||||||
find_overlapping_elements,
|
find_overlapping_elements,
|
||||||
get_elements_in_region,
|
get_elements_in_region,
|
||||||
check_wire_collisions,
|
find_wires_crossing_symbols,
|
||||||
)
|
)
|
||||||
|
|
||||||
|
|
||||||
@@ -412,8 +412,8 @@ class TestIntegrationFindUnconnectedPins:
|
|||||||
|
|
||||||
|
|
||||||
@pytest.mark.integration
|
@pytest.mark.integration
|
||||||
class TestIntegrationCheckWireCollisions:
|
class TestIntegrationFindWiresCrossingSymbols:
|
||||||
"""Integration test for wire collision detection."""
|
"""Integration test for wire crossing symbol detection."""
|
||||||
|
|
||||||
def test_wire_not_touching_pins_is_collision(self):
|
def test_wire_not_touching_pins_is_collision(self):
|
||||||
"""A wire passing through a component bbox without pin contact → collision."""
|
"""A wire passing through a component bbox without pin contact → collision."""
|
||||||
@@ -426,7 +426,7 @@ class TestIntegrationCheckWireCollisions:
|
|||||||
(uuid "w1"))
|
(uuid "w1"))
|
||||||
"""
|
"""
|
||||||
tmp = _make_temp_schematic(extra)
|
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"]
|
d1_collisions = [c for c in result if c["component"]["reference"] == "D1"]
|
||||||
assert len(d1_collisions) >= 1
|
assert len(d1_collisions) >= 1
|
||||||
|
|
||||||
@@ -452,7 +452,7 @@ class TestIntegrationCheckWireCollisions:
|
|||||||
(uuid "w-collision"))
|
(uuid "w-collision"))
|
||||||
"""
|
"""
|
||||||
tmp = _make_temp_schematic(r_at_100 + r_at_200 + wire)
|
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)
|
# The wire must not be reported against the far-away R? at (200, 100)
|
||||||
collisions_at_200 = [
|
collisions_at_200 = [
|
||||||
c for c in result
|
c for c in result
|
||||||
@@ -463,6 +463,41 @@ class TestIntegrationCheckWireCollisions:
|
|||||||
"likely caused by reference-lookup always returning the first 'R?'"
|
"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):
|
def test_wire_shorts_component_pins_detected_as_collision(self):
|
||||||
"""Regression: a wire connecting pin1→pin2 of the same component
|
"""Regression: a wire connecting pin1→pin2 of the same component
|
||||||
must be reported even though both endpoints land on pins."""
|
must be reported even though both endpoints land on pins."""
|
||||||
@@ -473,7 +508,7 @@ class TestIntegrationCheckWireCollisions:
|
|||||||
' (uuid "aaaaaaaa-0000-0000-0000-000000000001"))'
|
' (uuid "aaaaaaaa-0000-0000-0000-000000000001"))'
|
||||||
)
|
)
|
||||||
sch = _make_temp_schematic(r_sexp + "\n" + wire_sexp)
|
sch = _make_temp_schematic(r_sexp + "\n" + wire_sexp)
|
||||||
collisions = check_wire_collisions(sch)
|
collisions = find_wires_crossing_symbols(sch)
|
||||||
assert len(collisions) == 1
|
assert len(collisions) == 1
|
||||||
w = collisions[0]["wire"]
|
w = collisions[0]["wire"]
|
||||||
assert w["start"]["x"] == pytest.approx(100.0)
|
assert w["start"]["x"] == pytest.approx(100.0)
|
||||||
|
|||||||
@@ -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(
|
server.tool(
|
||||||
"check_wire_collisions",
|
"find_wires_crossing_symbols",
|
||||||
"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 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"),
|
schematicPath: z.string().describe("Path to the .kicad_sch schematic file"),
|
||||||
},
|
},
|
||||||
async (args: { schematicPath: string }) => {
|
async (args: { schematicPath: string }) => {
|
||||||
const result = await callKicadScript("check_wire_collisions", args);
|
const result = await callKicadScript("find_wires_crossing_symbols", args);
|
||||||
if (result.success) {
|
if (result.success) {
|
||||||
const collisions: any[] = result.collisions || [];
|
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) => {
|
collisions.slice(0, 30).forEach((c: any, i: number) => {
|
||||||
lines.push(
|
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`);
|
if (collisions.length > 30) lines.push(` ... and ${collisions.length - 30} more`);
|
||||||
|
|||||||
Reference in New Issue
Block a user