Skip to content

fix(pcb): harden live footprint pad readback - #265

Draft
nordic-style wants to merge 2 commits into
mixelpixx:mainfrom
nordic-style:codex/fix/live-pad-readback
Draft

fix(pcb): harden live footprint pad readback#265
nordic-style wants to merge 2 commits into
mixelpixx:mainfrom
nordic-style:codex/fix/live-pad-readback

Conversation

@nordic-style

@nordic-style nordic-style commented Aug 17, 2026

Copy link
Copy Markdown
Contributor

Summary

This is now a stacked follow-up to #207, not a competing implementation.

The branch contains #207's commit unchanged, followed by edcc9b4. Review only the second commit. Once #207 lands, the shared base commit will disappear from this PR's diff.

Depends on #207.

Approach

Keep #207's broader live-board reader and its explicit document targeting, then add only the remaining hardening:

  • require exact protobuf Any types before decoding footprints and pads;
  • report malformed pads and missing positions instead of silently dropping them or fabricating 0,0;
  • return live pad-stack layer names, with the same layers field in the saved-file fallback;
  • distinguish transport-unreachable from KiCad-rejected IPC failures, so a reachable KiCad can never trigger a stale-file fallback;
  • verify that move → pad readback observes the updated live board.

Compatibility and safety

The existing tools remain source-compatible. Responses gain the additive layers field; source remains unchanged.

This is read-only. It keeps #207's named-document selection, absent-footprint handling, and pad-less-footprint consistency guard.

Validation

  • cargo fmt --all -- --check
  • cargo test --workspace --locked --doc
  • cargo clippy --workspace --locked --all-targets -- -D warnings
  • cargo test -p konnect-ipc --locked --test mock_server_test --test footprint_transform_test — 29 passed, 1 intentionally ignored timeout test
  • cargo test -p konnect-core --locked tools::pcb_components::tests — 62 passed
  • cargo test --workspace --locked --lib --tests — 551 core tests passed; the same three unchanged update_symbols_from_library_* / Device:R failures reproduced on the local baseline remain
  • The retained layer/filtering behavior was previously checked against real KiCad 10 on a 100-contact connector: 100 numbered pads plus 4 real unnamed mechanical pads, no footprint artwork, and the expected B.Cu/B.Mask/B.Paste layers

Review checklist

The writers in these toolsets act on the live board over IPC while
get_board_info and get_component_pads parsed the file, so a session with
unsaved edits got contradictory answers: get_component_list reported 25
components while get_board_info reported layer_count 0 and net_count 0
from the empty stub on disk, and pad positions were unreadable until the
user pressed Ctrl+S.

Both now try IPC first and fall back to the file, reporting which they
used as "source" the way add_board_outline already does.
get_pad_position carries the source through.

Two deliberate non-behaviours. Paper size is still read from the file on
both paths — KiCad's API exposes no page settings, and the tool
description says so. A footprint KiCad does not have is an error rather
than a file answer, because falling back would report a part the user
deleted but has not saved.

Every _in-less client method resolves its document as "the first open
one", so with two boards open a read answered about the wrong one --
ensure_board_is_active only checks the requested board is open
somewhere. The new readers address the document find_open_board matched,
and get_board_extents was switched to do the same.

Pads come out of the API in absolute board coordinates (see transform),
so the live path applies none of the anchor/rotation math the file path
needs.

The copper layer count comes from BoardEnabledLayersResponse's own field
rather than a tally of layer names ending in .Cu. The two agree on an
ordinary stackup, which is the kind of agreement that stops holding on
one nobody tried.

KiCad answering with the footprint but no pads is refused whenever the
saved file gives that part pads. A pad-less footprint is legal, so zero
is not wrong by itself, but zero also reads as a plausible answer rather
than a failure — and that is the shape an unread response would take.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@nordic-style
nordic-style force-pushed the codex/fix/live-pad-readback branch from afa28a7 to 934dbf3 Compare August 17, 2026 23:02
@pauliuszaleckas

Copy link
Copy Markdown
Contributor

Looks like this duplicates some of my PR #207

@nordic-style
nordic-style force-pushed the codex/fix/live-pad-readback branch from 934dbf3 to edcc9b4 Compare August 18, 2026 09:23
@nordic-style nordic-style changed the title fix(pcb): read live footprint pad positions fix(pcb): harden live footprint pad readback Aug 18, 2026
@nordic-style
nordic-style marked this pull request as draft August 18, 2026 09:26
@nordic-style

Copy link
Copy Markdown
Contributor Author

You're right — thanks for pointing me to #207. I missed it when I opened the original version of this PR.

I've now rewritten #265 as a stacked follow-up:

  • your af5eb5a commit is the unchanged base;
  • the independent duplicate implementation is gone;
  • the only follow-up commit is edcc9b4, which adds exact Any type checks and strict decode errors, pad-stack layer names, transport-vs-rejection fallback classification, and a move → live-readback regression test;
  • fix(pcb): read board info and pads from the board KiCad holds #207's named-document/multi-board handling and pad-less-footprint guard remain intact.

I've also converted #265 to draft. Once #207 lands, its shared commit will disappear from this diff and #265 will contain only the hardening delta. I'll keep it rebased if #207 changes. Thanks again.

@mixelpixx

Copy link
Copy Markdown
Owner

Holding review on this until #207 (its base) is decided — reviewing it standalone would double-count #207's diff. It stays in the queue; nothing needed from you now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants