From ad21d00cd055457d4a6349c096a988cf2c37ffb6 Mon Sep 17 00:00:00 2001 From: Michele Bigi Date: Sun, 20 Sep 2026 16:06:10 +0200 Subject: [PATCH] Fix PE-BOM-011: classify footprint vias as vias, not pads. KiCad stitching/thermal vias inside a footprint are (via) or copper-only thru_hole (pad) objects. parse_kicad_pcb was appending every (pad) to LayoutFootprint.pads, so PE-BOM-011 compared datasheet pin_count to via numbers. Nested vias join LayoutGraph.vias; soldered THT pads stay pads. --- .../dependency/backend/periscopex/models.py | 1 + .../src/backend/periscopex/bom_pcb_check.py | 25 +- .../backend/periscopex/parsers_kicad_pcb.py | 85 +++++- tests/test_pcb_via_not_pad.py | 271 ++++++++++++++++++ 4 files changed, 370 insertions(+), 12 deletions(-) create mode 100644 tests/test_pcb_via_not_pad.py diff --git a/periscope/dependency/backend/periscopex/models.py b/periscope/dependency/backend/periscopex/models.py index b7b16d8..a90fedd 100644 --- a/periscope/dependency/backend/periscopex/models.py +++ b/periscope/dependency/backend/periscopex/models.py @@ -466,6 +466,7 @@ class LayoutPad(BaseModel): x: float y: float net: str = "" + pinfunction: str = "" class LayoutFootprint(BaseModel): diff --git a/periscope/src/backend/periscopex/bom_pcb_check.py b/periscope/src/backend/periscopex/bom_pcb_check.py index 7d4e18f..21301d4 100644 --- a/periscope/src/backend/periscopex/bom_pcb_check.py +++ b/periscope/src/backend/periscopex/bom_pcb_check.py @@ -35,7 +35,10 @@ _FAMILIES = ( "WLCSP", "BGA", "LGA", "CSP", "SC-70", "SC70", "TO-252", "TO252", "TO-263", "QFP", ) -_EP_PAD = re.compile(r"^(?:EP|PAD|TAB|TH|THERMAL|DIEPAD)$", re.I) +_EP_PAD = re.compile( + r"^(?:EP|EXP|EPAD|PAD|TAB|TH|THERMAL|DIEPAD)(?:[_-]?\d+)?$", + re.I, +) _V_PARAM = re.compile( r"(?:^|[\s_/])(VCC|VDD|VIN|VSUPPLY|V_IN|SUPPLY|VDS|VCEO|VOLTAGE)", re.I, @@ -75,6 +78,22 @@ def _signal_pads(numbers: list[str]) -> set[str]: return out +def _pcb_signal_pads(fp_layout) -> set[str]: + """Component lands only. Vias are not in ``fp.pads``; EP/thermal names skip.""" + out: set[str] = set() + if fp_layout is None: + return out + for pad in fp_layout.pads: + n = str(pad.number).strip() + if not n or _EP_PAD.match(n): + continue + pf = str(getattr(pad, "pinfunction", "") or "").strip() + if pf and _EP_PAD.match(pf): + continue + out.add(n) + return out + + def _package_row(cons: ComponentConstraints | None, specs) -> PackageInfo | None: if cons and cons.package_info: return cons.package_info @@ -238,9 +257,7 @@ def _pinout_findings( ds_pins = _signal_pads( [str(p.number) for p in (cons.pintable if cons else [])] ) - pcb_pins = _signal_pads( - [p.number for p in (fp_layout.pads if fp_layout else [])] - ) + pcb_pins = _pcb_signal_pads(fp_layout) ds_count = pkg.pin_count if pkg else (len(ds_pins) or None) pcb_count = len(pcb_pins) if pcb_pins else None if ds_count and pcb_count and abs(ds_count - pcb_count) > 1: diff --git a/periscope/src/backend/periscopex/parsers_kicad_pcb.py b/periscope/src/backend/periscopex/parsers_kicad_pcb.py index 6af94d5..533b0da 100644 --- a/periscope/src/backend/periscopex/parsers_kicad_pcb.py +++ b/periscope/src/backend/periscopex/parsers_kicad_pcb.py @@ -219,6 +219,65 @@ def _abs(fx: float, fy: float, frot: float, lx: float, ly: float) -> tuple[float return fx + rx, fy + ry +def _pad_kind(pad: object) -> str: + if not isinstance(pad, list) or len(pad) < 3 or isinstance(pad[2], list): + return "" + return str(pad[2]).lower() + + +def _pad_layers(pad: object) -> list[str]: + layers = _kid(pad, "layers") + if not layers: + return [] + return [str(x) for x in layers[1:] if not isinstance(x, list)] + + +def _is_npth_pad(pad: object) -> bool: + return _pad_kind(pad) in {"np_thru_hole", "np_through_hole", "npth"} + + +def _is_footprint_via(pad: object) -> bool: + """Footprint child that is a via, never a component pin. + + Board ``(via)`` and nested footprint ``(via)`` are handled separately. + KiCad has no via pad type in most versions — stitching/thermal vias are + stored as ``(pad … thru_hole …)`` (or type ``via``). Those are vias when + they are not a schematic pin (no pinfunction/pintype) and are copper-only + (no mask/paste land). Net name and XY are ignored: GND/3V3/VCC/signal + and courtyard proximity do not turn a via into a pad, or a pad into a via. + """ + kind = _pad_kind(pad) + if kind == "via": + return True + if kind != "thru_hole": + return False + if _val(pad, "pinfunction") or _val(pad, "pintype"): + return False + layers = [x.lower() for x in _pad_layers(pad)] + if any("mask" in x or "paste" in x for x in layers): + return False + return True + + +def _via_from_node( + node: object, + nets: dict[str, int], + *, + fx: float = 0.0, + fy: float = 0.0, + frot: float = 0.0, + local: bool = False, +) -> LayoutVia: + drill_el = _kid(node, "drill") + drill = _fnum(drill_el[1]) if drill_el and len(drill_el) > 1 else None + vx, vy, _ = _at(node) + if local: + rx, ry = _rotate(vx, vy, frot) + vx, vy = fx + rx, fy + ry + net = _pad_net(node) or _net_name(node, nets) + return LayoutVia(x=vx, y=vy, net=net, drill=drill) + + def _courtyard_pts(node: object, fx: float, fy: float, frot: float) -> list[tuple[float, float]]: """Courtyard vertices from the PCB file. Empty if KiCad has no CrtYd.""" pts: list[tuple[float, float]] = [] @@ -291,17 +350,30 @@ def parse_kicad_pcb(path: str | Path) -> LayoutGraph: if not ref or ref.startswith("#"): continue pads: list[LayoutPad] = [] + for via_node in _kids(node, "via"): + vias.append(_via_from_node( + via_node, nets, fx=fx, fy=fy, frot=frot, local=True, + )) for pad in _kids(node, "pad"): + px, py, _ = _at(pad) + rx, ry = _rotate(px, py, frot) + ax, ay = fx + rx, fy + ry + if _is_footprint_via(pad): + drill_el = _kid(pad, "drill") + drill = _fnum(drill_el[1]) if drill_el and len(drill_el) > 1 else None + vias.append(LayoutVia(x=ax, y=ay, net=_pad_net(pad), drill=drill)) + continue + if _is_npth_pad(pad): + continue num = str(pad[1]) if len(pad) > 1 else "" if not num: continue - px, py, _ = _at(pad) - rx, ry = _rotate(px, py, frot) pads.append(LayoutPad( number=num, - x=fx + rx, - y=fy + ry, + x=ax, + y=ay, net=_pad_net(pad), + pinfunction=_val(pad, "pinfunction"), )) footprints[ref] = LayoutFootprint( reference=ref, @@ -332,10 +404,7 @@ def parse_kicad_pcb(path: str | Path) -> LayoutGraph: )) continue if tag == "via": - drill_el = _kid(node, "drill") - drill = _fnum(drill_el[1]) if drill_el and len(drill_el) > 1 else None - vx, vy, _ = _at(node) - vias.append(LayoutVia(x=vx, y=vy, net=_net_name(node, nets), drill=drill)) + vias.append(_via_from_node(node, nets)) continue if tag == "zone": zones.extend(_parse_zone(node, nets)) diff --git a/tests/test_pcb_via_not_pad.py b/tests/test_pcb_via_not_pad.py new file mode 100644 index 0000000..c8467e0 --- /dev/null +++ b/tests/test_pcb_via_not_pad.py @@ -0,0 +1,271 @@ +"""Via is never a footprint pad. PE-BOM-011 uses parsed component pads only.""" + +from __future__ import annotations + +from pathlib import Path + +from backend.periscopex.bom_pcb_check import check_bom_pcb_datasheet +from backend.periscopex.models import ( + Component, + ComponentConstraints, + ComponentType, + DesignGraph, + LayoutGraph, + PackageInfo, + Pin, +) +from backend.periscopex.parsers_kicad_pcb import parse_kicad_pcb + + +def _smd_pad(num: int, x: float, y: float, net: str = "SIG") -> str: + return ( + f' (pad "{num}" smd rect (at {x} {y}) (size 0.25 0.4) ' + f'(layers "F.Cu" "F.Mask" "F.Paste") (net 2 "{net}") ' + f'(pinfunction "P{num}") (pintype "passive"))\n' + ) + + +def _via_pad(num: int, x: float, y: float, net: str, net_code: int = 1) -> str: + """KiCad stitching via stored as a copper-only thru_hole pad.""" + return ( + f' (pad "{num}" thru_hole circle (at {x} {y}) (size 0.6 0.6) ' + f'(drill 0.3) (layers "*.Cu") (net {net_code} "{net}"))\n' + ) + + +def _board_via(x: float, y: float, net: str, net_code: int = 1) -> str: + return ( + f' (via (at {x} {y}) (size 0.8) (drill 0.4) ' + f'(layers "F.Cu" "B.Cu") (net {net_code}))\n' + ) + + +def _fp_via(x: float, y: float, net: str, net_code: int = 1) -> str: + return ( + f' (via (at {x} {y}) (size 0.8) (drill 0.4) ' + f'(layers "F.Cu" "B.Cu") (net {net_code} "{net}"))\n' + ) + + +def _qfn24_pcb( + tmp_path: Path, + *, + n_pads: int = 24, + stitch_via_pads: list[tuple[int, float, float, str]] | None = None, + nested_vias: list[tuple[float, float, str]] | None = None, + board_vias: list[tuple[float, float, str, int]] | None = None, + extra_smd: list[tuple[int, float, float, str]] | None = None, + ep_pad: bool = False, +) -> Path: + nets = { + 0: "", + 1: "GND", + 2: "SIG", + 3: "3V3", + 4: "VCC", + } + net_code = {v: k for k, v in nets.items() if v} + body = ["(kicad_pcb (version 20240108) (generator pcbnew)\n"] + for code, name in nets.items(): + body.append(f' (net {code} "{name}")\n') + body.append( + ' (footprint "Package_DFN_QFN:QFN-24"\n' + ' (layer "F.Cu")\n' + " (at 50 40 0)\n" + ' (property "Reference" "U1" (at 0 0 0) (effects (font (size 1 1))))\n' + ' (fp_rect (start -2.2 -2.2) (end 2.2 2.2) (layer "F.CrtYd") (stroke (width 0.05)))\n' + ) + for i in range(1, n_pads + 1): + body.append(_smd_pad(i, -1.5 + (i % 6) * 0.5, -1.5 + (i // 6) * 0.5)) + if extra_smd: + for num, x, y, net in extra_smd: + body.append(_smd_pad(num, x, y, net)) + if ep_pad: + body.append( + ' (pad "25" smd custom (at 0 0) (size 0.2 0.2) ' + '(layers "F.Cu") (net 1 "GND") ' + '(pinfunction "EXP_25") (pintype "power_in"))\n' + ) + for num, x, y, net in stitch_via_pads or []: + body.append(_via_pad(num, x, y, net, net_code.get(net, 2))) + for x, y, net in nested_vias or []: + body.append(_fp_via(x, y, net, net_code.get(net, 2))) + body.append(" )\n") + for x, y, net, _code in board_vias or []: + body.append(_board_via(x, y, net, net_code.get(net, 2))) + body.append(")\n") + p = tmp_path / "board.kicad_pcb" + p.write_text("".join(body)) + return p + + +def _ds24() -> tuple[DesignGraph, dict]: + pins = [Pin(number=str(i), name=f"P{i}") for i in range(1, 25)] + cons = ComponentConstraints( + mpn="IC24", + package_info=PackageInfo(base_family="QFN", package="QFN-24", pin_count=24), + pintable=pins, + absolute_maximum_ratings=[], + rules=[], + ) + graph = DesignGraph( + components={ + "U1": Component( + reference="U1", + value="IC", + footprint="Package_DFN_QFN:QFN-24", + component_type=ComponentType.IC, + mpn="IC24", + pins={str(i): "SIG" for i in range(1, 25)}, + ), + }, + ) + return graph, {"IC24": cons} + + +def _bom_ids(tmp_path: Path, **pcb_kw) -> tuple[set[str], LayoutGraph]: + layout = parse_kicad_pcb(_qfn24_pcb(tmp_path, **pcb_kw)) + graph, cmap = _ds24() + ids = {f.rule_id for f in check_bom_pcb_datasheet(graph, cmap, layout)} + return ids, layout + + +def test_24_pads_0_vias_no_mismatch(tmp_path: Path): + ids, layout = _bom_ids(tmp_path, n_pads=24) + u1 = layout.footprints["U1"] + assert len(u1.pads) == 24 + assert layout.vias == [] + assert "PE-BOM-011" not in ids + + +def test_stitching_via_pads_are_vias_not_pads(tmp_path: Path): + stitches = [(25 + i, -1.0 + (i % 3) * 0.7, -1.0 + (i // 3) * 0.7, "GND") for i in range(9)] + ids, layout = _bom_ids(tmp_path, n_pads=24, stitch_via_pads=stitches) + u1 = layout.footprints["U1"] + assert [p.number for p in u1.pads] == [str(i) for i in range(1, 25)] + assert len(u1.pads) == 24 + assert len(layout.vias) == 9 + assert all(v.net == "GND" for v in layout.vias) + assert "PE-BOM-011" not in ids + + +def test_nearby_external_board_vias_not_counted_as_pads(tmp_path: Path): + nearby = [ + (52.0, 40.0, "GND", 1), + (48.0, 42.0, "3V3", 3), + (50.5, 37.5, "SIG", 2), + ] + ids, layout = _bom_ids(tmp_path, n_pads=24, board_vias=nearby) + assert len(layout.footprints["U1"].pads) == 24 + assert len(layout.vias) == 3 + assert "PE-BOM-011" not in ids + + +def test_vias_inside_footprint_courtyard_not_pads(tmp_path: Path): + inside = [ + (50.0, 40.0, "GND", 1), + (50.4, 40.4, "VCC", 4), + (49.6, 39.6, "SIG", 2), + ] + ids, layout = _bom_ids(tmp_path, n_pads=24, board_vias=inside) + assert len(layout.footprints["U1"].pads) == 24 + assert len(layout.vias) == 3 + assert "PE-BOM-011" not in ids + + +def test_nested_footprint_via_token_is_via(tmp_path: Path): + ids, layout = _bom_ids( + tmp_path, + n_pads=24, + nested_vias=[(0.0, 0.0, "GND"), (0.5, 0.5, "3V3")], + ) + assert len(layout.footprints["U1"].pads) == 24 + assert len(layout.vias) == 2 + nets = {v.net for v in layout.vias} + assert nets == {"GND", "3V3"} + assert "PE-BOM-011" not in ids + + +def test_via_pads_on_gnd_3v3_vcc_signal(tmp_path: Path): + stitches = [ + (26, 0.0, 0.0, "GND"), + (27, 0.5, 0.0, "3V3"), + (28, 0.0, 0.5, "VCC"), + (29, 0.5, 0.5, "SIG"), + ] + ids, layout = _bom_ids(tmp_path, n_pads=24, stitch_via_pads=stitches) + assert len(layout.footprints["U1"].pads) == 24 + assert {v.net for v in layout.vias} == {"GND", "3V3", "VCC", "SIG"} + assert "PE-BOM-011" not in ids + + +def test_true_mismatch_24_vs_23_still_fires(tmp_path: Path): + ids, layout = _bom_ids(tmp_path, n_pads=23) + assert len(layout.footprints["U1"].pads) == 23 + assert "PE-BOM-011" in ids + + +def test_true_mismatch_24_vs_25_still_fires(tmp_path: Path): + ids, layout = _bom_ids( + tmp_path, + n_pads=24, + extra_smd=[(25, 1.8, 1.8, "SIG")], + ) + assert len(layout.footprints["U1"].pads) == 25 + assert "PE-BOM-011" in ids + + +def test_vias_do_not_hide_true_mismatch(tmp_path: Path): + stitches = [(40 + i, 0.2 * i, 0.2, "GND") for i in range(6)] + board = [(51.0, 41.0, "3V3", 3), (49.0, 39.0, "SIG", 2)] + ids23, layout23 = _bom_ids( + tmp_path, n_pads=23, stitch_via_pads=stitches, board_vias=board, + ) + assert len(layout23.footprints["U1"].pads) == 23 + assert len(layout23.vias) == 8 + assert "PE-BOM-011" in ids23 + + tmp2 = tmp_path / "b" + tmp2.mkdir() + ids25, layout25 = _bom_ids( + tmp2, + n_pads=24, + extra_smd=[(25, 1.8, 1.8, "SIG")], + stitch_via_pads=stitches, + board_vias=board, + ) + assert len(layout25.footprints["U1"].pads) == 25 + assert "PE-BOM-011" in ids25 + + +def test_exposed_pad_plus_via_array_matches_24_pin_datasheet(tmp_path: Path): + """QFN-24 + EP land + thermal via array: pin_count 24, not 34.""" + stitches = [(26 + i, -0.6 + (i % 3) * 0.6, -0.6 + (i // 3) * 0.6, "GND") for i in range(9)] + ids, layout = _bom_ids(tmp_path, n_pads=24, stitch_via_pads=stitches, ep_pad=True) + u1 = layout.footprints["U1"] + assert len(u1.pads) == 25 + assert any(p.number == "25" and p.pinfunction == "EXP_25" for p in u1.pads) + assert len(layout.vias) == 9 + assert "PE-BOM-011" not in ids + + +def test_tht_solder_pads_stay_component_pads(tmp_path: Path): + p = tmp_path / "tht.kicad_pcb" + p.write_text("""(kicad_pcb (version 20240108) + (net 0 "") + (net 1 "GND") + (net 2 "SIG") + (footprint "Resistor_THT:R_Axial" + (layer "F.Cu") + (at 0 0 0) + (property "Reference" "R1" (at 0 0 0) (effects (font (size 1 1)))) + (pad "1" thru_hole circle (at -3.81 0) (size 1.6 1.6) (drill 0.8) + (layers "*.Cu" "*.Mask") (net 2 "SIG")) + (pad "2" thru_hole circle (at 3.81 0) (size 1.6 1.6) (drill 0.8) + (layers "*.Cu" "*.Mask") (net 1 "GND")) + ) +) +""") + g = parse_kicad_pcb(p) + assert [p.number for p in g.footprints["R1"].pads] == ["1", "2"] + assert g.vias == []