Skip to content

fix(pcb): read board info and pads from the board KiCad holds - #207

Open
pauliuszaleckas wants to merge 1 commit into
mixelpixx:mainfrom
pauliuszaleckas:fix/read-board-state-over-ipc
Open

fix(pcb): read board info and pads from the board KiCad holds#207
pauliuszaleckas wants to merge 1 commit into
mixelpixx:mainfrom
pauliuszaleckas:fix/read-board-state-over-ipc

Conversation

@pauliuszaleckas

@pauliuszaleckas pauliuszaleckas commented Aug 15, 2026

Copy link
Copy Markdown
Contributor

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 gave contradictory
answers about the same board:

get_component_list  →  25 components
get_board_info      →  layer_count 0, net_count 0     (the empty stub on disk)

Pad positions were simply unreadable until the user pressed Ctrl+S — place_component
had put the footprint in KiCad, and nothing had written it to the file yet. Since #215
landed, update_pcb_from_schematic stages footprints into the board KiCad holds, so
"did the pads land where I expect?" is now a question the file cannot answer at all
until a save.

Both readers now try IPC first and fall back to the file, reporting which one answered
as source, the way add_board_outline already does. get_pad_position carries that
source through.

Where the numbers come from

copper_layer_count is BoardEnabledLayersResponse's own field, not a tally of layer
names ending in .Cu. The two agree on an ordinary stackup, which is exactly the kind of
agreement that stops holding on one nobody tried. get_layers_in delegates to a new
get_enabled_layers_in returning IpcEnabledLayers { copper_layer_count, layers }; the
existing signatures are unchanged.

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. pad_at in the mock
pins that down.

Three deliberate non-behaviours

  • Paper size is still read from the file on both paths. KiCad's API exposes no page
    settings; the tool description says so.
  • A footprint KiCad does not have is an error, not a file answer. Falling back there
    would report a part the user deleted but has not saved — the exact failure this PR
    exists to remove, pointing the other way.
  • A footprint KiCad has but reports no pads for is refused when the file disagrees.
    A pad-less footprint is legal — a logo, a mounting graphic — so zero is not wrong by
    itself. But zero also reads as a plausible answer rather than a failure, and it is the
    shape an unread response would take, so it stands only when the saved file does not
    give that part pads.

Reading the board that was asked for

Every _in-less client method resolves its document as "the first open one", while
ensure_board_is_active only checks that the requested board is open somewhere. With
two boards open, a read could therefore answer about the other one. The new readers
address the document find_open_board matched. get_board_extents is switched to do the
same — four lines, independent of the rest of this PR, and easy to lift out if you would
rather take it separately.

Testing

cargo fmt --all -- --check, cargo clippy --workspace --locked -- -D warnings,
cargo test --workspace --locked --doc and the workspace test suite all pass. Three
update_symbols_from_library tests fail on my machine both with and without this branch
— pre-existing on main, unrelated.

New tests, each against a mock KiCad holding a board that differs from the file on disk:

  • a live board is reported instead of the last save, with a copper count the old
    derivation would have got wrong (27 layers, ids 3..26 all *.Cu, so a name tally says
    24 where the response says 6), and an offline session still reads the file;
  • pads come from the board KiCad holds, a part deleted in KiCad is not answered from the
    file, an empty live pad list is refused when the file has pads and accepted when it
    does not, pads fall back to the file when KiCad is unreachable, and a pad position
    carries its source;
  • at the client layer: pads come back in board coordinates with their nets, a footprint
    absent from the live board reads as None, and pad reads target the named board among
    several open.

Not live-verified: which shape KiCad returns when it has the footprint but no pads for
us. There is no KiCad here to drive, and the guard above is written so that either shape
is safe — a misread response disagrees with the file and errors, a genuinely pad-less
part agrees with it and reads as zero.

🤖 Generated with Claude Code

pauliuszaleckas added a commit to pauliuszaleckas/Konnect that referenced this pull request Aug 15, 2026
The PCB-side entries are all fixed on this branch:

Board readers parsed the file while board writers used IPC. Fixed for
get_board_info and get_component_pads by the board-reader commit below,
now up as mixelpixx#207. Records the wrong-board reads found on the way, the two
things the live path deliberately does not do, and the readers left on
the file — including why get_layer_list waits for mixelpixx#153.

There was no way to resize or replace a board outline. Fixed by the
delete_graphics commit below. Records the deliberate choices (no filter
means no deletion, top-level graphics only, reference images excluded,
IPC rejection fails closed) and the limitation it inherits:
layer_enum_to_name names 15 layers, so an IPC-path filter on any other
layer matches nothing while the file path matches it.

The IPC socket path was not auto-detected. Fixed by the socket
auto-detection commit below; mixelpixx#18 and mixelpixx#39 are the closed issues the
friction produced. Records why a candidate counts only when something is
listening, why detecting nothing leaves the address empty, and the
limitation the fix keeps: detection runs once at startup, so a KiCad
opened later is missed.

The rest are the schematic side, found redrawing that same board from
an empty file through the MCP tools. Only the junction entry is fixed,
by the commit below. The tracker has been searched and each status line
says what it found: the junction entry was filed as mixelpixx#201 while this file
was being written, its move-and-prune half is mixelpixx#120, the sheet-size entry
is most of the way covered by mixelpixx#175, and the remaining four are
unreported.

Each carries the commit that introduced it, blamed on the line and read
against the surrounding diff where a reformat made git blame misleading
— the ERC library-match entry points at 921fa92 and is really a50c96a.
The junction, power-designator, netlist-summary and orphan-item entries
date from the initial public release; all of them predate this branch.

The junction entry was the cheapest to close: the dedupe it needed
already existed in tree, added by 1866129 and 620da07, and had never
reached the T loop in add_wire and batch_add_wire. Its status now
records the third call site the report missed, connect_to_net, and why
the string-path helper was not reused.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
pauliuszaleckas added a commit to pauliuszaleckas/Konnect that referenced this pull request Aug 15, 2026
The PCB-side entries are all fixed on this branch:

Board readers parsed the file while board writers used IPC. Fixed for
get_board_info and get_component_pads by the board-reader commit below,
now up as mixelpixx#207. Records the wrong-board reads found on the way, the two
things the live path deliberately does not do, and the readers left on
the file — including why get_layer_list waits for mixelpixx#153.

There was no way to resize or replace a board outline. Fixed by the
delete_graphics commit below. Records the deliberate choices (no filter
means no deletion, top-level graphics only, reference images excluded,
IPC rejection fails closed) and the limitation it inherits:
layer_enum_to_name names 15 layers, so an IPC-path filter on any other
layer matches nothing while the file path matches it.

The IPC socket path was not auto-detected. Fixed by the socket
auto-detection commit below; mixelpixx#18 and mixelpixx#39 are the closed issues the
friction produced. Records why a candidate counts only when something is
listening, why detecting nothing leaves the address empty, and the
limitation the fix keeps: detection runs once at startup, so a KiCad
opened later is missed.

The rest are the schematic side, found redrawing that same board from
an empty file through the MCP tools. The junction and power-designator
entries are fixed, by the two commits below. The tracker has been
searched and each status line
says what it found: the junction entry was filed as mixelpixx#201 while this file
was being written, its move-and-prune half is mixelpixx#120, the sheet-size entry
is most of the way covered by mixelpixx#175, and the remaining four are
unreported.

Each carries the commit that introduced it, blamed on the line and read
against the surrounding diff where a reformat made git blame misleading
— the ERC library-match entry points at 921fa92 and is really a50c96a.
The junction, power-designator, netlist-summary and orphan-item entries
date from the initial public release; all of them predate this branch.

The junction entry was the cheapest to close: the dedupe it needed
already existed in tree, added by 1866129 and 620da07, and had never
reached the T loop in add_wire and batch_add_wire. Its status now
records the third call site the report missed, connect_to_net, and why
the string-path helper was not reused.

The power-designator entry went the same way: add_power_symbol numbered
by counting, so a deletion re-issued a live #PWR reference. It now takes
the lowest free number. Its status records why lowest-free rather than
max+1, and that an instances-path reference left stale by mixelpixx#157 stays
invisible to the scan.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@mixelpixx

Copy link
Copy Markdown
Owner

Still held per your own stack order — you asked for this to land after the two
PCB commits ahead of it, and #214 has now merged, so the queue in front of this
is clear whenever you want to take it off draft.

This got more valuable than its draft status suggests. #215 landed
yesterday, giving us update_pcb_from_schematic. That tool stages footprints
into the board KiCad is holding, and the result is unsaved until the user saves.
get_component_pads reads the board file, so right now the natural next
question after a sync — "did the pads land where I expect?" — is answered from a
file that does not yet contain the sync. Reading from the board KiCad holds is
exactly the fix for that, so this PR closes a gap that only opened yesterday.

Three things I would like before it lands:

  1. Take copper_layer_count from the response field rather than deriving
    it
    , and assert it. The derivation happens to agree today; it is the kind
    of agreement that stops holding on a board with a layer set nobody tried.

  2. Live-verify the Ok(None) path. The branch where IPC answers but has
    nothing for us needs to be a hard error, not an empty success — an empty pad
    list reads as "this footprint has no pads", which is a plausible answer and
    therefore a dangerous default. Worth driving through a real KiCad rather
    than a unit test, because what I want to know is which of the two shapes
    KiCad actually returns.

  3. Consider splitting the get_board_extents fix out. It is four lines
    fixing a wrong-board read, and it does not depend on the rest of this PR. I
    would merge that today if it arrived on its own.

For (2), the recipe I have been using: strip footprints from a copy of a demo
board, open it in pcbnew, run the operation, then save_project and read the
file back — so the tool under test is not also the witness. On Windows the IPC
address is a named pipe and no socket file ever appears in %TEMP%\kicad\, so
do not wait for one.

No rush from my side; v0.4.0 is going out without it and that is fine.

@pauliuszaleckas
pauliuszaleckas force-pushed the fix/read-board-state-over-ipc branch from 402710c to b0632bf Compare August 16, 2026 13:40
@pauliuszaleckas

Copy link
Copy Markdown
Contributor Author

Rebased onto main (v0.5.1) and taken off draft — the queue in front of it is clear.

(1) copper_layer_count from the response. BoardEnabledLayersResponse carries the
count as its own field, so the derivation is gone. get_layers_in now delegates to a new
get_enabled_layers_in, which returns IpcEnabledLayers { copper_layer_count, layers };
get_layers/get_layers_in keep their signatures. The live test asserts it, and asserts
it against a case where the old derivation would have been wrong: 27 enabled layers with
ids 3..26 all named *.Cu, so a name tally says 24 while the response says 6.

(2) The Ok(None) path. Two shapes, now separated:

  • No footprint by that reference — already a hard error, unchanged.

  • Footprint present, pad list empty — this is the one worth worrying about, and it was
    passing as pad_count: 0. It is now refused whenever the saved file gives that part
    pads, naming the disagreement. A genuinely pad-less footprint (a logo, a mounting
    graphic) still reads as zero, since nothing contradicts it. Two tests cover both.

    Not live-verified. I could not run your recipe — no KiCad process here to drive,
    and the answer to "which shape does KiCad actually return" needs a real one. The guard
    above is written so that either shape is safe: if KiCad returns the footprint with no
    pads because we failed to read the response, the file disagrees and we error; if it
    genuinely has no pads, we say zero. If you run the recipe and it turns out KiCad returns
    something else again, that is worth knowing and I will follow it.

(3) Splitting the get_board_extents fix. The rebase mostly did this for you — main
already has get_board_extents_in(document) and get_optional_board_extents_in, so what
remains here is the four-line call site switching to the document find_open_board
matched. Happy to lift those four lines into their own PR if you would rather still take
it separately; say the word and it goes up.

Also folded in on rebase: get_nets_in and the extents pair now come from main rather
than this branch, and make_header stayed where it was.

Gate is green — fmt, clippy, doctests, workspace tests. The three update_symbols_from_library
failures are still there with and without this branch.

@pauliuszaleckas
pauliuszaleckas marked this pull request as ready for review August 16, 2026 13:41
@pauliuszaleckas
pauliuszaleckas force-pushed the fix/read-board-state-over-ipc branch from b0632bf to 9f44f35 Compare August 16, 2026 18:18
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>
@pauliuszaleckas
pauliuszaleckas force-pushed the fix/read-board-state-over-ipc branch from 9f44f35 to af5eb5a Compare August 17, 2026 04:53
pauliuszaleckas added a commit to pauliuszaleckas/Konnect that referenced this pull request Aug 17, 2026
The PCB-side entries are all fixed on this branch:

Board readers parsed the file while board writers used IPC. Fixed for
get_board_info and get_component_pads by the board-reader commit below,
now up as mixelpixx#207. Records the wrong-board reads found on the way, the two
things the live path deliberately does not do, and the readers left on
the file — including why get_layer_list waits for mixelpixx#153.

There was no way to resize or replace a board outline. Fixed by the
delete_graphics commit below. Records the deliberate choices (no filter
means no deletion, top-level graphics only, reference images excluded,
IPC rejection fails closed) and the limitation it inherits:
layer_enum_to_name names 15 layers, so an IPC-path filter on any other
layer matches nothing while the file path matches it.

The IPC socket path was not auto-detected. Fixed by the socket
auto-detection commit below; mixelpixx#18 and mixelpixx#39 are the closed issues the
friction produced. Records why a candidate counts only when something is
listening, why detecting nothing leaves the address empty, and the
limitation the fix keeps: detection runs once at startup, so a KiCad
opened later is missed.

The rest are the schematic side, found redrawing that same board from
an empty file through the MCP tools. The junction, wire-delete and
power-designator entries were fixed here and have merged upstream as
mixelpixx#212, mixelpixx#214 and mixelpixx#213, so their entries are gone and the rest renumbered.
Of the four left, the ERC library-match entry is fixed: upstream 94471f8
had already stopped the symbol writer emitting the "~" placeholder a day
after the entry was written, and the create_symbol commit below adds the
datasheet argument that was the other half. The tracker has been
searched and each status line says what it found: the sheet-size entry
is most of the way covered by the merged mixelpixx#175, mixelpixx#226 asks the opposite
question about a different site, and the remaining entries are
unreported.

Each carries the commit that introduced it, blamed on the line and read
against the surrounding diff where a reformat made git blame misleading
— the ERC library-match entry points at 921fa92 and is really a50c96a.
The netlist-summary and orphan-item entries date from the initial public
release; both predate this branch.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
pauliuszaleckas added a commit to pauliuszaleckas/Konnect that referenced this pull request Aug 17, 2026
The PCB-side entries are all fixed on this branch:

Board readers parsed the file while board writers used IPC. Fixed for
get_board_info and get_component_pads by the board-reader commit below,
now up as mixelpixx#207. Records the wrong-board reads found on the way, the two
things the live path deliberately does not do, and the readers left on
the file — including why get_layer_list waits for mixelpixx#153.

There was no way to resize or replace a board outline. Fixed by the
delete_graphics commit below. Records the deliberate choices (no filter
means no deletion, top-level graphics only, reference images excluded,
IPC rejection fails closed) and the limitation it inherits:
layer_enum_to_name names 15 layers, so an IPC-path filter on any other
layer matches nothing while the file path matches it.

The IPC socket path was not auto-detected. Fixed by the socket
auto-detection commit below; mixelpixx#18 and mixelpixx#39 are the closed issues the
friction produced. Records why a candidate counts only when something is
listening, why detecting nothing leaves the address empty, and the
limitation the fix keeps: detection runs once at startup, so a KiCad
opened later is missed.

The rest are the schematic side, found redrawing that same board from
an empty file through the MCP tools. The junction, wire-delete and
power-designator entries were fixed here and have merged upstream as
mixelpixx#212, mixelpixx#214 and mixelpixx#213, so their entries are gone and the rest renumbered.
Of the four left, the ERC library-match entry is fixed: upstream 94471f8
had already stopped the symbol writer emitting the "~" placeholder a day
after the entry was written, and the create_symbol commit below adds the
datasheet argument that was the other half. The tracker has been
searched and each status line says what it found: the sheet-size entry
is most of the way covered by the merged mixelpixx#175, mixelpixx#226 asks the opposite
question about a different site, and the remaining entries are
unreported.

Each carries the commit that introduced it, blamed on the line and read
against the surrounding diff where a reformat made git blame misleading
— the ERC library-match entry points at 921fa92 and is really a50c96a.
The netlist-summary and orphan-item entries date from the initial public
release; both predate this branch.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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.

2 participants