Skip to content

feat(schematic): place add_power_symbol on a named pin - #782

Open
pauliuszaleckas wants to merge 1 commit into
mixelpixx:mainfrom
pauliuszaleckas:fix/727-power-symbol-pin
Open

pauliuszaleckas wants to merge 1 commit into
mixelpixx:mainfrom
pauliuszaleckas:fix/727-power-symbol-pin

Conversation

@pauliuszaleckas

Copy link
Copy Markdown
Contributor

Summary

add_power_symbol only took x/y. To put a rail on a pin, a caller had to copy the pin's endpoint and choose the rotation by hand. It now also takes reference + pin_number, the way connect_to_net does. The symbol lands on the pin's endpoint and, when rotation is omitted, faces away from the component body.

The scope is the narrowed contract from the maintainer's comment. stub_length and a batch form are left for follow-ups.

Closes #727

Approach

  • Selector. The call gives exactly one of reference + pin_number or x + y. Mixing them, or giving half of one, is refused with invalid_argument naming the field, before the file is read.

  • Pin lookup (resolve_power_pin, built on the shared placed_pins_by_reference). It is stricter than resolve_placed_pin, which takes the first match. The following are refused, and nothing is written:

    • Missing reference or pin: stale_target, with the reason saying which of the two is missing.
    • Pin carried by more than one placed symbol: ambiguous_target, with candidates named by unit, UUID and position.
    • Placed copy with no embedded definition: refused, because that copy might be the one carrying the pin.

    Stacked drawings of one pin within one placed unit count as a single point. KiCad's De Morgan symbols draw every pin twice.

  • Position. The symbol is placed so that its own pin lands on the target pin's endpoint. That point is not snapped, so an off-grid pin is still reached. Coordinate calls still snap.

  • Rotation. The new konnect_sexp::schematic::rotation_facing turns the power symbol so its body side, which is its pin's own angle, faces the target pin's outward direction. An explicit rotation still wins.

  • Single read. The pin lookup and the edit read the file once, so the pin cannot come from a different revision than the one the write is checked against.

Departures from the issue's suggested direction: none in scope. I used stale_target for a reference or pin that isn't on the sheet. That follows component_target_from_source ("component X is not present") and delete_trace, which report a missing target the same way.

Architectural fit

The change reuses placed_pins_by_reference, pin_endpoint, pin_outward_direction, the placement readback and the existing error kinds. The rotation rule lives in konnect-sexp next to pin_outward_direction, and cse::library::embedded_lib_symbol is now pub instead of being copied.

resolve_power_pin sits beside the first-match resolve_placed_pin that connect_to_net, connect_pins and their batch forms still use. I filed that as #781, with a reproduction on this PR's fixture: connect_to_net on a duplicated R4 wires the first copy and reports success. Switching those tools to the strict resolver changes their results, so it belongs in its own PR.

Branch and dependencies

  • Base: main @ bf7bfd6. No dependencies; one commit.

Compatibility and safety

  • Schema:
    • x and y are no longer listed as required.
    • reference and pin_number are added.
    • The advertised rotation default of 0 is removed, because it now applies only to coordinates.
    • Coordinate calls behave exactly as before; the existing tests pass unchanged.
  • docs/API_MIGRATIONS.md has a new entry at the top. Nothing else in the file moved.
  • Writes go through the same revision-checked overwrite() as before. Every refusal above happens before the write.
  • Known, unchanged behaviour: add_power_symbol doesn't check what net a pin is already on. The coordinate form never did either. A GND named on a pin that is already +3V3 merges the two, just as a GND placed at that point always has. Refusing that would be new behaviour, outside this scope.

Validation

Fixtures and oracles

crates/konnect-core/tests/fixtures/power_on_pin_kicad10.README.md has the full tables.

  • power_on_pin_kicad10.kicad_sch
    • Built through Konnect against stock Device, Amplifier_Operational, 74xx and power, then force-resaved by KiCad 10.0.6.
    • Contents: pins facing each way, a mirrored R3, an LM358 with its power unit placed apart, a De Morgan 74LS00, and two symbols both annotated R4.
    • Expected points are KiCad's own ERC pin positions.
    • After placing every case through the debug build, KiCad's netlist reads +3V3 = R1.1, U1.1, U1.3, U1.8 and GND = R1.2, R2.1, R2.2, R3.1, U1.4, U2.3. R3.2 stays unconnected, which a mirror-blind implementation would get wrong. kicad-cli sch export svg shows every rail facing away from its symbol.
  • power_on_off_grid_pin.kicad_sch
    • KiCad 10.0.6's hv_converter demo, byte for byte.
    • C4 pin 2 sits at (149.352, 84.328), off-grid on both axes.
    • Placed on that pin, GND merges /d4 into GND in KiCad's netlist. Placed by coordinates at the same point, it snaps to (149.86, 83.82) and /d4 is unchanged.
  • Rotation evidence:
    • All 101 stock power symbols have their pin at the origin, angled into the body: GND 270 with its body below, +3V3 90 with its body above.
    • Across the 10.0.6 demos, 268 stock power symbols sit directly on a pin. All 204 on vertical pins face away. On horizontal pins it's 44 turned and 24 pointing down, which is why an explicit rotation still wins.

End to end, same call over stdio

add_power_symbol {power_net: GND, reference: R2, pin_number: 2} on the fixture:

Build Result
Installed invalid_argument on x ("missing"), file unchanged
This branch (debug) #PWR05 at (134.62, 80.01), rotation 90, R2.2 on GND in KiCad's netlist

Tests

All new tests go through the served tools/call dispatch, except the one that has to get past the schema gate. Every refusal also asserts the file is byte-identical afterwards.

Test Proves
a_named_pin_takes_the_symbol_to_kicads_endpoint_facing_away 10 pins: KiCad's position and the table's rotation, in the response and in the committed file
an_explicit_rotation_wins_over_the_pin rotation: 180 on a right-pointing pin lands at 180
coordinates_still_snap_and_default_to_no_rotation x/y unchanged: snapped, rotation 0
an_off_grid_pin_is_reached_where_it_is C4.2 at (149.352, 84.328), unsnapped
bad_selectors_are_refused_without_writing mixed (×2), half-given (×3), none: invalid_argument and field. R99 and R1 pin 9: stale_target, target and reason
a_pin_two_symbols_share_is_ambiguous_and_nothing_is_written ambiguous_target, R4 pin 1, both candidates at KiCad's positions
a_copy_without_a_definition_is_not_skipped one R4 copy's lib_id renamed in memory: refused, not placed on the other copy
a_malformed_selector_keeps_its_own_reason direct call with pin_number: 1: the helper's own reason, not "give either…"
rotation_facing_turns_the_body_side_onto_the_direction (konnect-sexp) all 16 pin-angle/direction pairs round-trip through transform_direction

Negative controls

Each guard was neutered on 3882e3e, and cargo test -p konnect-core -p konnect-sexp --lib --no-fail-fast was run. The file was then restored and checked with cmp.

Control Failed Tests
C1 mixed selector not refused 1 bad_selectors_are_refused_without_writing
C2 ambiguity resolved to the first match 1 a_pin_two_symbols_share_is_ambiguous_and_nothing_is_written
C3 omitted rotation left at 0 2 a_named_pin_takes…, an_off_grid_pin…
C4 explicit rotation ignored 1 an_explicit_rotation_wins_over_the_pin
C5 rotation_facing sign flipped 3 rotation_facing_turns…, a_named_pin_takes…, an_off_grid_pin…
C6 pin endpoint snapped 1 an_off_grid_pin_is_reached_where_it_is
C7 copy without a definition skipped 1 a_copy_without_a_definition_is_not_skipped
C8 stacked pins not deduplicated 1 a_named_pin_takes… (U2.3, De Morgan)
C9 missing component falls through to "has no pin" 1 bad_selectors_are_refused_without_writing (reason)
C10 malformed half reported as missing 1 a_malformed_selector_keeps_its_own_reason
C11 power pin offset not subtracted 0 see below

C11 fails nothing, and on purpose. All 101 stock power symbols have their pin at the origin, so no KiCad-authored fixture can tell the difference. The subtraction stays because power: resolves through the user's library table, and an overriding library can put the pin elsewhere. Without it, that symbol would miss the pin.

Changed tool behavior

Behavior Contract and evidence for this change
Accepted inputs and declared defaults reference + pin_number (strings) or x + y. rotation is optional: when omitted it is derived from the pin for the pin form and is 0 for coordinates. Tests: a_named_pin_takes…, an_explicit_rotation_wins…, coordinates_still_snap…
Invalid/unsupported inputs and structured errors Mixed, half-given or absent selectors: invalid_argument naming the field. A malformed present value keeps the helper's reason. A power symbol without exactly one pin: invalid_argument on power_net. Tests: bad_selectors…, a_malformed_selector…
Target, data source and prerequisite state The saved .kicad_sch, read once, and the pin resolved from that same read. Missing reference or pin: stale_target. A pin on more than one placed symbol: ambiguous_target with candidates. A copy with no definition: stale_target. Existing sheet-instance preflight unchanged
Observed changes and preserved unrelated objects One new power symbol, plus mid-wire junctions as before. The response is read back from the committed file (placed_component_readback), and tests compare the committed symbol's point and rotation
Failure before/after mutation, including applied work Every new refusal happens before the write and leaves the file byte-identical (asserted). Post-write behaviour is unchanged: placed_component_readback checks the committed file and returns its structured error when the placement does not match
Recovery from partial/uncertain results without repeating applied work A refusal proves nothing was written, so the caller corrects the selector or gives x + y for the candidate they mean. A post-write readback error is handled as before: reload and inspect the file before retrying

Gate (local, on 3882e3e, Linux)

  • cargo fmt --all -- --check: clean
  • cargo clippy --workspace --locked --all-targets -- -D warnings: clean
  • cargo test -p konnect-core --locked reliability_contract: 2 passed
  • cargo test --workspace --locked --lib --tests: 2306 passed, 0 failed
  • cargo test --workspace --locked --doc: 9 passed
  • cargo xtask fix-doc-counts --check: current, with no tool added or removed
  • Not run: the schematic viewer (untouched), and Windows/macOS locally, which are left to CI.

Risk and rollback

The risk is confined to the new pin form: coordinate calls go through the same snap, readback and write as before. To roll back, revert the one commit. Documents written with the pin form are ordinary KiCad files.

🤖 Generated with Claude Code

@neusse neusse added status:waiting-on-review Next actor: maintainer enhancement New feature or request P2 Wanted, not urgent area:schematic Schematic capture and analysis labels Oct 3, 2026
Copying a pin's endpoint into x/y and picking a rotation by hand is where
mistakes slip in (mixelpixx#727). add_power_symbol now also takes reference +
pin_number: the symbol lands on the pin's endpoint, unsnapped, and faces
away from the body unless rotation is given. Mixed or half-given selectors,
missing pins and pins two symbols share are refused before writing.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@pauliuszaleckas
pauliuszaleckas force-pushed the fix/727-power-symbol-pin branch from 3882e3e to f6cbf63 Compare October 4, 2026 17:01

@neusse neusse left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Substantive exact-head review: f6cbf63. Spec: no findings against the narrowed #727 acceptance: alternative pin selector, no-write mixed/incomplete/missing/ambiguous refusal, exact pin endpoint, derived omitted rotation, explicit rotation and legacy coordinate behavior preserved; stubs/batches deferred. Standards: no blocking findings; shared pin geometry, revision-checked single-source edit, observed readback, migration/tool/guidance updates and KiCad-authored served-call fixtures are coherent. The PR records real KiCad netlist/ERC/SVG observations and guard negative controls; I reviewed those records, not a fresh live run. One unique commit, Closes #727 registered, no unresolved threads, all ten checks green on this head, but it does not include current main 9488e5f. Next actor: maintainer integration/current-main readiness verification when its schematic lane reaches the front, followed by exact-head user authorization because this is a contributor PR. No deep-queue branch refresh was performed. Keep status:waiting-on-review specifically for that integration/readiness step; do not ask the author for a routine base refresh.

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

Labels

area:schematic Schematic capture and analysis enhancement New feature or request P2 Wanted, not urgent status:waiting-on-review Next actor: maintainer

Projects

None yet

Development

Successfully merging this pull request may close these issues.

add_power_symbol takes only coordinates; accept reference + pin_number like connect_to_net

2 participants