Repository navigation
Conversation
…xelpixx#853) The tool is described as returning a net's pins and labels but built only the labels and a connected_points count. Resolve the net's points once and report every placed pin whose endpoint sits on them as {reference, pin, x, y}, as get_net_components already does.
|
Thanks for picking this up. I checked head 1. Power symbols come back as pins. On the test's own fixture:
2. The test can't fail on a wrong answer. It only checks that R1 pin 2 is somewhere in 3. Labels are still picked by name. Smaller points:
|
…xx#853) Review on mixelpixx#854 found three gaps against `kicad-cli sch export netlist`: - Power symbols and PWR_FLAGs came back in `pins`. They name a net but are not nodes on it, so they are left out, as the netlist does. - `labels` was still picked by name, so a net with two names listed only the labels spelling the one asked for while `pins` and `connected_points` covered the whole net. Labels are now every label on the net's roots, each carrying the `net` it spells. - A symbol with no library entry was skipped, so `pins` came back short without saying so. The tool now refuses through resolved_placed_pins_by_reference. Each pin also carries its `name`, as get_net_components reports it. The tests now compare exact pin sets with KiCad's own netlist of the KiCad-saved two_name_nets fixture, including a net joined only by a pair of labels, a net asked for by an alias, and pins that must not appear.
|
Pushed |
|
Thanks. I re-checked head Oracle. For every net in both fixtures, I compared
None of them includes a Controls. I neutered each guard and ran
Smaller points:
|
…xx#853) Review on mixelpixx#854 found three gaps against `kicad-cli sch export netlist`: - Power symbols and PWR_FLAGs came back in `pins`. They name a net but are not nodes on it, so they are left out, as the netlist does. - `labels` was still picked by name, so a net with two names listed only the labels spelling the one asked for while `pins` and `connected_points` covered the whole net. Labels are now every label on the net's roots, each carrying the `net` it spells. - A symbol with no library entry was skipped, so `pins` came back short without saying so. The tool now refuses through resolved_placed_pins_by_reference. Each pin also carries its `name`, as get_net_components reports it. The tests now compare exact pin sets with KiCad's own netlist of the KiCad-saved two_name_nets fixture, including a net joined only by a pair of labels, a net asked for by an alias, and pins that must not appear.
2 participants
Summary
Fixes #853.
get_net_connectionsis documented as returning a net's pins and labels but only built labels and aconnected_pointscount. It now returns every placed pin on the net as{reference, pin, x, y}, asget_net_componentsdoes.Approach
The handler never collected pins; it now resolves the net's points once and matches pin endpoints against them.
Architectural fit
Extends the existing
sch_analysistool; no workaround.Branch and dependencies
Base: main. No dependencies.
Compatibility and safety
Additive
pinsfield on a read-only tool.Validation
Changed tool behavior
pinsRegression test: 0 passed, 1 failed on base; 2 passed, 0 failed with the fix. Boxes below not ticked were not run on the final head.
cargo fmt --all -- --checkcargo test --workspace --locked --lib --tests(what CI runs)cargo test --workspace --locked --doccargo clippy --workspace --locked --all-targets -- -D warningsReview checklist
upstream/main, has no merge conflicts, and CI passed on this exact head.responsibility for the final current-main refresh.
upstream/main, not a release tag (unless this is an approved backport).has been reviewed on the new exact head. An unchanged unique diff may use a
focused refresh review; changed behavior received substantive re-review.
docs/NAMING_CONVENTIONS.md; public renames include compatibility handling.tool_count,tool-directory.md, DEV.md stats, README count).Maintainer merge state
status:*workflow label.status:ready-to-mergeapplies to this exact head SHA.were cleared under the review-request workflow (manual requests remain open).
mainruleset.@mixelpixx/@neussePR, or explicit authorization names this exact head.gh pr merge N --merge.