diff --git a/python/commands/schematic_analysis.py b/python/commands/schematic_analysis.py index 8fa58e5..9d91730 100644 --- a/python/commands/schematic_analysis.py +++ b/python/commands/schematic_analysis.py @@ -85,7 +85,7 @@ def _parse_symbols(sexp_data: list) -> List[Dict[str, Any]]: """ Parse all placed symbol instances from the schematic S-expression. - Returns list of dicts: {reference, lib_id, x, y, rotation, is_power} + Returns list of dicts: {reference, lib_id, x, y, rotation, mirror_x, mirror_y, is_power} """ symbols = [] for item in sexp_data: @@ -98,6 +98,8 @@ def _parse_symbols(sexp_data: list) -> List[Dict[str, Any]]: x, y, rotation = 0.0, 0.0, 0.0 reference = "" is_power = False + mirror_x = False + mirror_y = False for sub in item: if isinstance(sub, list) and len(sub) >= 2: @@ -108,6 +110,12 @@ def _parse_symbols(sexp_data: list) -> List[Dict[str, Any]]: y = float(sub[2]) if len(sub) >= 4: rotation = float(sub[3]) + elif sub[0] == Symbol("mirror"): + m = str(sub[1]) + if m == "x": + mirror_x = True + elif m == "y": + mirror_y = True elif sub[0] == Symbol("property") and len(sub) >= 3: prop_name = str(sub[1]).strip('"') if prop_name == "Reference": @@ -120,6 +128,8 @@ def _parse_symbols(sexp_data: list) -> List[Dict[str, Any]]: "x": x, "y": y, "rotation": rotation, + "mirror_x": mirror_x, + "mirror_y": mirror_y, "is_power": is_power, }) return symbols @@ -518,6 +528,44 @@ def get_elements_in_region( # Tool 5: check_wire_collisions # --------------------------------------------------------------------------- +def _compute_pin_positions_direct( + sym: Dict[str, Any], pin_defs: Dict[str, Dict] +) -> Dict[str, List[float]]: + """ + Compute absolute schematic pin positions for a symbol instance directly from + its parsed position/rotation/mirror data and pin definitions in local coords. + + Unlike PinLocator.get_all_symbol_pins, this does NOT do a reference-name + lookup in the schematic, so it works correctly when multiple symbols share + the same reference designator (e.g. unannotated "Q?"). + + KiCad transform order: mirror (in local coords) → rotate → translate. + """ + sym_x = sym["x"] + sym_y = sym["y"] + rotation = sym["rotation"] + mirror_x = sym.get("mirror_x", False) + mirror_y = sym.get("mirror_y", False) + + result: Dict[str, List[float]] = {} + for pin_num, pin_data in pin_defs.items(): + rel_x = float(pin_data["x"]) + rel_y = float(pin_data["y"]) + + # Apply mirroring in local symbol coordinates + if mirror_x: + rel_y = -rel_y + if mirror_y: + rel_x = -rel_x + + # Apply symbol rotation + if rotation != 0: + rel_x, rel_y = PinLocator.rotate_point(rel_x, rel_y, rotation) + + result[pin_num] = [sym_x + rel_x, sym_y + rel_y] + return result + + def check_wire_collisions(schematic_path: Path) -> List[Dict[str, Any]]: """ Detect wires passing through component bodies without connecting to their pins. @@ -546,11 +594,22 @@ def check_wire_collisions(schematic_path: Path) -> List[Dict[str, Any]]: if sym["is_power"] or ref.startswith("_TEMPLATE") or not ref: continue - bbox = compute_symbol_bbox(schematic_path, ref, locator) - if bbox is None: + # Get pin definitions by lib_id (works regardless of reference designator, + # so unannotated components with duplicate "Q?" references are handled correctly). + pin_defs = locator.get_symbol_pins(schematic_path, sym["lib_id"]) + if not pin_defs: continue - min_x, min_y, max_x, max_y = bbox + # Compute absolute pin positions directly from this symbol's own position/rotation, + # bypassing the reference-name lookup in PinLocator (which always finds the first + # symbol with a given reference, breaking for unannotated duplicates like "Q?"). + pin_positions = _compute_pin_positions_direct(sym, pin_defs) + if not pin_positions: + continue + + xs = [p[0] for p in pin_positions.values()] + ys = [p[1] for p in pin_positions.values()] + min_x, min_y, max_x, max_y = min(xs), min(ys), max(xs), max(ys) # Expand degenerate dimensions (pins in a line) to approximate body size min_body = 1.5 # mm minimum half-extent for component body @@ -573,7 +632,6 @@ def check_wire_collisions(schematic_path: Path) -> List[Dict[str, Any]]: if max_x <= min_x or max_y <= min_y: continue - pin_positions = locator.get_all_symbol_pins(schematic_path, ref) pin_set = set() for pos in pin_positions.values(): pin_set.add((pos[0], pos[1])) diff --git a/python/tests/test_schematic_analysis.py b/python/tests/test_schematic_analysis.py index 95d89e7..35b6d1e 100644 --- a/python/tests/test_schematic_analysis.py +++ b/python/tests/test_schematic_analysis.py @@ -382,6 +382,39 @@ class TestIntegrationCheckWireCollisions: d1_collisions = [c for c in result if c["component"]["reference"] == "D1"] assert len(d1_collisions) >= 1 + def test_unannotated_duplicates_not_over_reported(self): + """ + Regression: two components with the same unannotated reference ("R?") at + different positions should each produce independent bounding boxes. + A wire crossing only one of them must produce exactly 1 collision, not 2. + + Before the fix, PinLocator.get_all_symbol_pins always resolved "R?" to + the first match, so both symbols got identical bboxes and the same wire + was counted against both. + """ + # R? at (100, 100): Device:R pins are at (100, 96.19) and (100, 103.81). + # Effective bbox (after expansion + margin) ≈ x=[99,101], y=[96.69,103.31]. + # R? at (200, 100): identical type but far away → no intersection with wire. + r_at_100 = _make_resistor_sexp("R?", 100, 100) + r_at_200 = _make_resistor_sexp("R?", 200, 100) + # Horizontal wire crossing the body of the first R? only + wire = """ + (wire (pts (xy 95 100) (xy 105 100)) + (stroke (width 0) (type default)) + (uuid "w-collision")) + """ + tmp = _make_temp_schematic(r_at_100 + r_at_200 + wire) + result = check_wire_collisions(tmp) + # The wire must not be reported against the far-away R? at (200, 100) + collisions_at_200 = [ + c for c in result + if abs(c["component"]["position"]["x"] - 200) < 0.5 + ] + assert len(collisions_at_200) == 0, ( + "Wire at x≈100 must not be flagged against the R? at x=200; " + "likely caused by reference-lookup always returning the first 'R?'" + ) + @pytest.mark.integration class TestIntegrationGetElementsInRegion: