diff --git a/fill_resistance/board_io.py b/fill_resistance/board_io.py index a530d8c..b86ba4b 100644 --- a/fill_resistance/board_io.py +++ b/fill_resistance/board_io.py @@ -182,11 +182,18 @@ def _pad_default_contact(pad: Pad) -> str: return "all" -def _pad_polygons(board: Board, pad: Pad, contact: str) -> list[Polygon] | None: +def _pad_polygons(board: Board, pad: Pad, contact: str, + prefer: str | None = None) -> list[Polygon] | None: + """Exact pad copper. The first probed layer that has a shape wins, so + `prefer` (the solder side of a THT joint) must be tried before the + F.Cu/B.Cu fallback: KiCad allows a different pad size per copper + layer, and the solder coat is sized from this shape.""" layer_ids = [] - if contact != "all": + for name in (contact if contact != "all" else None, prefer): + if not name: + continue try: - layer_ids.append(layer_from_canonical_name(contact)) + layer_ids.append(layer_from_canonical_name(name)) except Exception: pass for name in ("F.Cu", "B.Cu"): @@ -274,8 +281,10 @@ def _to_electrode(board: Board, item, stackup: StackupInfo | None = None, raise SelectionError(f"Could not get the bounding box of {label}.") rect = _box2_to_rect(box, "pad") drill, slot_dx, slot_dy = _drill_info(pad) + prot = _tht_protrusion_side(pad, pad_map or {}) if drill > 0 else None return Electrode(rect=rect, contact=contact, - polygons=_pad_polygons(board, pad, contact), label=label, + polygons=_pad_polygons(board, pad, contact, prefer=prot), + label=label, # through-hole pad: current enters at the soldered # barrel; the joint is solder-filled + pad-coated, # with a solder cone around the protruding lead @@ -283,9 +292,7 @@ def _to_electrode(board: Board, item, stackup: StackupInfo | None = None, pad_min_nm=_padstack_pad_min_nm(pad), slot_dx_nm=slot_dx, slot_dy_nm=slot_dy, center=(pad.position.x, pad.position.y), - solder=drill > 0, - protrusion_side=(_tht_protrusion_side(pad, pad_map or {}) - if drill > 0 else None)) + solder=drill > 0, protrusion_side=prot) def _net_hint_of(items: list) -> str | None: @@ -605,6 +612,10 @@ def gather_smd_pad_copper(board: Board, net_name: str continue layer = _pad_default_contact(pad) # SMD: its own copper layer if layer == "all": + # zero or >1 copper layers (custom padstack): no single layer + # to stamp it on. Say so - a silent skip loses a real junction + print(f"note: pad {pad.number}@{net_name} sits on no single " + f"copper layer - its pad copper is not modelled") continue polys = _pad_polygons(board, pad, layer) if polys: @@ -657,20 +668,48 @@ def _create_reference_image(board: Board, ref) -> None: def remove_overlays(board: Board, layer) -> int: - """Remove every reference image on the given layer; returns count.""" + """Remove every reference image on the given layer; returns count. + + remove_items with the per-item status surfaced: kipy discards the + DeleteItemsResponse, and its own proto warns the overall status "may + return IRS_OK even if no items were deleted" - a locked image comes + back IDS_IMMUTABLE. Unchecked, the stale image survives and the new + one is stacked on top of it instead of replacing it.""" + from kipy.proto.common.commands.editor_commands_pb2 import ( + DeleteItems, DeleteItemsResponse, ItemDeletionStatus) + ours = [r for r in board.get_reference_images() if r.layer == layer] - if ours: - board.remove_items(ours) - return len(ours) + if not ours: + return 0 + + cmd = DeleteItems() + cmd.header.document.CopyFrom(board._doc) + cmd.item_ids.extend([r.id for r in ours]) + results = board._kicad.send(cmd, DeleteItemsResponse).deleted_items + + stuck = [r for r in results + if r.status not in (ItemDeletionStatus.IDS_OK, + ItemDeletionStatus.IDS_NONEXISTENT)] + if stuck: + locked = sum(1 for r in stuck + if r.status == ItemDeletionStatus.IDS_IMMUTABLE) + raise RuntimeError( + f"{len(stuck)} existing overlay image(s) could not be removed" + + (f" ({locked} locked)" if locked else "") + + " - unlock them in KiCad, or delete them by hand, then run " + "again (a new image would otherwise stack on top).") + return len(results) def push_result_overlays(board: Board, stack, result, lock: bool = False) -> None: """EXPERIMENTAL: the solved |J| of every included copper layer as an unlocked reference image on config.OVERLAY_LAYERS (stackup order, - top first; existing images there are replaced). Editor-only - - reference images never plot. Per-layer failures are reported and - skipped, never fatal to the run.""" + top first; existing images there are replaced, and slots this run + does not write are cleared so no stale heatmap is left behind). + The whole push is one commit, so a single undo reverts it. Editor- + only - reference images never plot. Per-layer failures are reported + and skipped, never fatal to the run.""" from kipy.board_types import ReferenceImage from kipy.geometry import Vector2 @@ -683,23 +722,46 @@ def push_result_overlays(board: Board, stack, result, f"{', '.join(names[len(config.OVERLAY_LAYERS):])} skipped") ny, nx = stack.shape2d w_nm, h_nm = nx * stack.h_nm, ny * stack.h_nm - for src, dest_name in pairs: - try: - dest = layer_from_canonical_name(dest_name) - png = heatmap_png(result.Jmag * 1e-6, names.index(src)) - remove_overlays(board, dest) - ref = ReferenceImage() - ref.layer = dest - ref.position = Vector2.from_xy(round(stack.x0_nm + w_nm / 2), - round(stack.y0_nm + h_nm / 2)) - ref.image_scale = w_nm / (nx * OVERLAY_PIX_NM) - ref.image_data = png - ref.locked = lock - _create_reference_image(board, ref) - print(f"overlay: |J| of {src} -> {dest_name} " - f"({len(png) / 1024:.0f} kB)") - except Exception as e: - print(f"overlay: {src} -> {dest_name} failed: {e}") + + commit = board.begin_commit() if hasattr(board, "begin_commit") else None + done = False + try: + # a narrower run than last time writes fewer slots; whatever the + # zip above left out still holds the previous solve's heatmap and + # would read as current, so clear it + for dest_name in config.OVERLAY_LAYERS[len(pairs):]: + try: + if remove_overlays(board, layer_from_canonical_name(dest_name)): + print(f"overlay: cleared stale {dest_name}") + except Exception as e: + print(f"overlay: clearing stale {dest_name} failed: {e}") + + for src, dest_name in pairs: + try: + dest = layer_from_canonical_name(dest_name) + png = heatmap_png(result.Jmag * 1e-6, names.index(src)) + remove_overlays(board, dest) + ref = ReferenceImage() + ref.layer = dest + ref.position = Vector2.from_xy(round(stack.x0_nm + w_nm / 2), + round(stack.y0_nm + h_nm / 2)) + ref.image_scale = w_nm / (nx * OVERLAY_PIX_NM) + ref.image_data = png + ref.locked = lock + _create_reference_image(board, ref) + print(f"overlay: |J| of {src} -> {dest_name} " + f"({len(png) / 1024:.0f} kB)") + except Exception as e: + print(f"overlay: {src} -> {dest_name} failed: {e}") + if commit is not None: + board.push_commit(commit, "Fill Resistance |J| overlays") + done = True + finally: + if commit is not None and not done: + try: + board.drop_commit(commit) + except Exception: + pass # --- top level ---------------------------------------------------------------- diff --git a/fill_resistance/dialog.py b/fill_resistance/dialog.py index 8d2beba..f37063f 100644 --- a/fill_resistance/dialog.py +++ b/fill_resistance/dialog.py @@ -214,7 +214,11 @@ class _Dialog(QDialog): raise ValueError("Cell size must be > 0 µm.") try: freq = skin.parse_frequency(self.freq_edit.text()) - except ValueError: + except ValueError as exc: + # as in number(): keep parse_frequency's own explanation for + # the inputs it rejects deliberately, not just "unparseable" + if any(k in str(exc) for k in ("separator", "negative")): + raise ValueError(f"Frequency: {exc}") raise ValueError( f"Frequency: cannot parse '{self.freq_edit.text()}' " f"(examples: 0, 142k, 1.5M).") diff --git a/fill_resistance/main.py b/fill_resistance/main.py index 8403086..7df40db 100644 --- a/fill_resistance/main.py +++ b/fill_resistance/main.py @@ -18,9 +18,21 @@ from .errors import CandidateError, UserFacingError def _fail(message: str, outdir) -> None: print(f"ERROR: {message}") - from . import plots - fig = plots.fig_error(message) - plots.save_and_show([(fig, "error")], outdir) + try: + if outdir is None: + # A failure before the run has an output directory (a broken + # plugin environment throws on import) would otherwise save + # no PNG - and with no GUI toolkit, plots falls back to + # opening the saved PNGs, so the figure would never be shown + # either. Exactly the case the docstring promises to cover. + import tempfile + from pathlib import Path + outdir = Path(tempfile.gettempdir()) / "fill-resistance-error" + from . import plots + fig = plots.fig_error(message) + plots.save_and_show([(fig, "error")], outdir) + except Exception: # reporting must not mask the fault + traceback.print_exc() sys.exit(1) diff --git a/tests/test_board_io_api.py b/tests/test_board_io_api.py new file mode 100644 index 0000000..e2c8f06 --- /dev/null +++ b/tests/test_board_io_api.py @@ -0,0 +1,201 @@ +"""board_io's kipy-facing paths, against a fake board. + +Real protobuf messages, a fake transport. These cover what a live KiCad +would otherwise be needed for: the overlay push (kipy's +Board.remove_items discards the DeleteItemsResponse, so board_io talks +to the proto layer directly and these pin the status handling that +depends on) and per-layer pad copper selection. +""" +from types import SimpleNamespace as NS + +import numpy as np +import pytest +from kipy.proto.common.commands.editor_commands_pb2 import ( + CreateItemsResponse, DeleteItemsResponse, ItemDeletionStatus) +from kipy.proto.common.types.base_types_pb2 import KIID +from kipy.util.board_layer import layer_from_canonical_name + +from fill_resistance import board_io, config + + +class _Ref: + """Stand-in for a reference image already on the board (kipy board + items carry a KIID message, not a bare id).""" + def __init__(self, layer_name, ident): + self.layer = layer_from_canonical_name(layer_name) + self.id = KIID(value=f"00000000-0000-0000-0000-{ident:012d}") + + +class _FakeKiCad: + def __init__(self, delete_status=ItemDeletionStatus.IDS_OK): + self.delete_status = delete_status + self.deleted = [] # layers we were asked to clear + self.created = [] # ReferenceImages we were asked to add + + def send(self, cmd, response_type): + if response_type is DeleteItemsResponse: + resp = DeleteItemsResponse() + for _ in cmd.item_ids: + resp.deleted_items.add().status = self.delete_status + self.deleted.append(len(cmd.item_ids)) + return resp + if response_type is CreateItemsResponse: + resp = CreateItemsResponse() + resp.created_items.add().status.code = 1 # ISC_OK + self.created.append(cmd) + return resp + raise AssertionError(f"unexpected command {type(cmd).__name__}") + + +class _FakeBoard: + def __init__(self, existing=(), delete_status=ItemDeletionStatus.IDS_OK): + self._kicad = _FakeKiCad(delete_status) + self._refs = list(existing) + self.commits = [] + self.pushed = [] + self.dropped = [] + + # kipy Board surface board_io actually uses + @property + def _doc(self): + from kipy.proto.common.types.base_types_pb2 import DocumentSpecifier + return DocumentSpecifier() + + def get_reference_images(self): + return list(self._refs) + + def begin_commit(self): + self.commits.append("open") + return object() + + def push_commit(self, commit, message=""): + self.pushed.append(message) + + def drop_commit(self, commit): + self.dropped.append(commit) + + +class _Stack: + layer_names = ["F.Cu", "B.Cu"] + shape2d = (12, 16) + h_nm = 100_000 + x0_nm = 0 + y0_nm = 0 + + +class _Result: + def __init__(self, nlayers=2, ny=12, nx=16): + self.Jmag = np.full((nlayers, ny, nx), 1e6) + + +def test_remove_overlays_counts_deleted(): + layer = layer_from_canonical_name("User.9") + board = _FakeBoard(existing=[_Ref("User.9", 1), _Ref("User.9", 2), + _Ref("User.10", 3)]) + assert board_io.remove_overlays(board, layer) == 2 # not the User.10 one + + +def test_remove_overlays_no_images_is_a_noop(): + board = _FakeBoard() + assert board_io.remove_overlays( + board, layer_from_canonical_name("User.9")) == 0 + assert board._kicad.deleted == [] # no DeleteItems sent at all + + +def test_locked_overlay_raises_instead_of_stacking(): + """A locked image comes back IDS_IMMUTABLE while the overall request + still reports OK. Unchecked, the caller would add a second image on + top of the one it believed it had replaced.""" + board = _FakeBoard(existing=[_Ref("User.9", 1)], + delete_status=ItemDeletionStatus.IDS_IMMUTABLE) + with pytest.raises(RuntimeError, match="could not be removed"): + board_io.remove_overlays(board, layer_from_canonical_name("User.9")) + + +def test_already_gone_overlay_is_not_an_error(): + board = _FakeBoard(existing=[_Ref("User.9", 1)], + delete_status=ItemDeletionStatus.IDS_NONEXISTENT) + assert board_io.remove_overlays( + board, layer_from_canonical_name("User.9")) == 1 + + +def test_push_clears_slots_this_run_does_not_write(monkeypatch): + """A 2-layer run after a 4-layer run must not leave the previous + solve's heatmap sitting on User.11/User.12.""" + stale = [_Ref(n, i) for i, n in enumerate(config.OVERLAY_LAYERS)] + board = _FakeBoard(existing=stale) + board_io.push_result_overlays(board, _Stack(), _Result()) + + written = {c.items[0].type_url for c in board._kicad.created} + assert len(board._kicad.created) == 2 # F.Cu, B.Cu -> 2 slots + assert written # images really created + # 2 written slots cleared + 2 unwritten slots cleared = 4 delete calls + assert len(board._kicad.deleted) == 4 + + +def test_push_is_one_undo_step(): + board = _FakeBoard() + board_io.push_result_overlays(board, _Stack(), _Result()) + assert board.commits and board.pushed and not board.dropped + + +def _square(side): + """Minimal duck-typed PolygonWithHoles: an origin square.""" + pts = [(0, 0), (side, 0), (side, side), (0, side)] + return NS(outline=NS(nodes=[NS(has_point=True, has_arc=False, + point=NS(x=x, y=y)) for x, y in pts]), + holes=[]) + + +class _PadBoard: + """F.Cu carries a small pad, B.Cu a deliberately larger one - KiCad + allows a different pad size per copper layer.""" + def __init__(self): + self.f = layer_from_canonical_name("F.Cu") + self.b = layer_from_canonical_name("B.Cu") + self.asked = [] + + def get_pad_shapes_as_polygons(self, pad, layer): + self.asked.append(layer) + return {self.f: _square(1000), self.b: _square(5000)}.get(layer) + + +def _width(polys): + xs = [p[0] for p in polys[0].outline] + return max(xs) - min(xs) + + +def test_tht_pad_copper_comes_from_the_solder_side(): + """The solder coat is sized from this shape, so a B.Cu-protruding + joint must not be measured with F.Cu's (here smaller) pad.""" + board = _PadBoard() + polys = board_io._pad_polygons(board, pad=None, contact="all", + prefer="B.Cu") + assert _width(polys) == 5000 + assert board.asked[0] == board.b # probed before the F.Cu default + + +def test_pad_copper_falls_back_when_no_side_is_known(): + board = _PadBoard() + polys = board_io._pad_polygons(board, pad=None, contact="all") + assert _width(polys) == 1000 # F.Cu, the documented fallback + + +def test_explicit_contact_layer_still_wins(): + board = _PadBoard() + polys = board_io._pad_polygons(board, pad=None, contact="B.Cu", + prefer="F.Cu") + assert _width(polys) == 5000 + + +def test_push_drops_the_commit_if_it_cannot_finish(monkeypatch): + board = _FakeBoard() + monkeypatch.setattr(board_io.config, "OVERLAY_LAYERS", ("User.9",)) + + def boom(*a, **k): + raise RuntimeError("transport died") + monkeypatch.setattr(board, "push_commit", boom) + + with pytest.raises(RuntimeError): + board_io.push_result_overlays(board, _Stack(), _Result()) + assert board.dropped