Skip to content

fix(pcb): write the Default netclass complete, and repair one that is not - #333

Merged
mixelpixx merged 2 commits into
mixelpixx:mainfrom
omelia-iliffe:fix/326-default-netclass-completeness
Aug 29, 2026
Merged

fix(pcb): write the Default netclass complete, and repair one that is not#333
mixelpixx merged 2 commits into
mixelpixx:mainfrom
omelia-iliffe:fix/326-default-netclass-completeness

Conversation

@omelia-iliffe

@omelia-iliffe omelia-iliffe commented Aug 26, 2026

Copy link
Copy Markdown
Contributor

Fixes #326.

Fixing this turned up adjacent problems in the same code — netclass priorities, the Default's PCB values, how get_netclasses reports an omitted key. Rather than fold them in, they're split by what they touch: the first commit changes only the class named Default, the second changes everything else.

The first commit does stands alone, if you'd prefer a small pr.

fix(pcb) — the fix

The triage suggested extending the FIELDS table, but that table drives creation for every class, and only the Default must be complete:

  • addMissingDefaults fills other classes from the Default, so a named class omitting a key is inheriting, not broken. Completing them would sever that. They're untouched here.
  • Nothing fills the Default itself, and a parsed Default replaces KiCad's seeded one (net_settings.cpp:216) rather than merging. An update now backfills absent keys only, leaving present ones alone, so fix(netclass): stop create_netclass resetting values it was not givenFix/create netclass partial update #220 stays fixed.
  • It's the name, not the position: SetName sets m_isDefault on an exact match (netclass.h:96) and the loader has no index branch. KiCad's writer emits the Default first, so it's always classes[0] in KiCad-written files — an output convention, not a read rule. Completing whichever class comes first would leave the bug live.

refactor(pcb) — beyond the fix

Priorities now match KiCad (int max for Default, -1 otherwise). The Default takes KiCad's PCB values rather than this tool's schema defaults, since a written Default replaces the seeded one and was silently re-speccing boards — worth your explicit call, happy to split it out. And get_netclasses reports settings resolved with inherits/missing_fields instead of null, which breaks callers reading null as "unset".

Testing

Nine unit tests plus an #[ignore] e2e test that drives the MCP tools and plots through eeschema. Against KiCad 10.0.5 it passes with the fix and fails without it. The junction dot's radius is the sharpest signal — 0.4572 mm healthy, 0.0001 mm when the Default is incomplete — so the test compares against a baseline plot rather than a literal. Note a bare stroke:none grep false-positives: a healthy junction dot is filled, not stroked.

… not

create_netclass wrote every class with four PCB fields and nothing else.
For a class named "Default" that is not a partial description, it is a
replacement: net_settings.cpp:216 swaps KiCad's seeded default out for
whatever the file holds, and addMissingDefaults fills other classes *from*
the Default while nothing fills the Default itself. Every key omitted there
is left resolved from nothing.

wire_width is the one that bites. Without it eeschema silently refuses to
place a junction anywhere in the project, strips existing dots on the next
save, and drops pins joined by a T to unconnected-*. There is no ERC
violation and no error message. Plotted, the wire group comes out as
"fill:none; stroke:none;" and the junction dot collapses from 0.4572 mm to
0.0001 mm.

Which class is the default is decided by name: SetName sets m_isDefault on
an exact match (netclass.h:96) and the loader branches on that alone. KiCad
writing the Default first is an output convention, not a read rule, so this
keys off the name and not the position. Confirmed against KiCad 10.0.5 —
a sparse class at classes[0] with a complete Default behind it plots fine.

The completeness requirement belongs to the Default alone. Named classes
inherit every key they omit, so writing a full set for them would sever
that inheritance and freeze them against later edits to the Default. They
are untouched here.

An existing incomplete Default cannot recover on its own, so an update
backfills the keys that are absent and leaves every key that is present
alone. Applying the creation defaults on update is mixelpixx#220, which is why they
still do not apply here.

Fixes mixelpixx#326.
Beyond mixelpixx#326, which only needed the Default written complete. Each of these
stands alone and can be dropped without affecting that fix.

Netclass priorities were hardcoded to 0. KiCad gives the Default
std::numeric_limits<int>::max() and every other class -1 (net_settings.cpp:69,
netclass.cpp:57), so a class this tool created sorted differently from one
KiCad wrote.

A written Default replaces KiCad's seeded one rather than merging with it,
so the PCB values it carries become the project's. This tool's schema
defaults are not KiCad's, which made create_netclass(name="Default") re-spec
a board's routing rules as a side effect of a call meant to do something
else. The Default now takes KiCad's values (netclass.cpp:36-50); the schema
defaults still apply to named classes.

get_netclasses reported a key absent from a class as null, which reads as
"unset" when it means "inherits" — addMissingDefaults fills it from the
Default at load. Settings are now reported resolved, with `inherits` naming
what came from the Default. `missing_fields` on the Default names what
nothing can resolve, which is the mixelpixx#326 state a caller otherwise has no way
to see. Callers reading null as "unset" will need updating.

create_netclass also reports `is_default`, and says what it repaired when it
backfills.
@mixelpixx
mixelpixx merged commit ebdd07b into mixelpixx:main Aug 29, 2026
9 checks passed
mixelpixx added a commit that referenced this pull request Aug 29, 2026
Minor: three new tools and two response-shape changes.

New tools:
- set_predefined_sizes / get_predefined_sizes (verification, #346) —
  the PCB Editor's track/via palette, which is a third .kicad_pro key
  distinct from DRC floors and netclass optima.
- delete_graphics (pcb_board, #335) — the missing generic delete verb,
  so a board outline can be replaced instead of only appended to.

Response-shape changes, both toward reporting what is true:
- get_netclasses reports settings resolved, with `inherits` naming what a
  class takes from the Default and `missing_fields` naming what nothing
  resolves (#333). Callers reading null as "unset" need updating: null
  claimed a class had no clearance when it inherited the Default's.
- auto_place_from_schematic and refine_placement_force_directed report
  `held`, naming every footprint they refused to move and why (#350).

Correctness:
- Neither placement planner relocates a footprint KiCad has locked. The
  lock was never parsed at all, so both planners moved locked parts (#350).
- update_footprints_from_library accepts the fp_text user that 15,238 of
  15,447 official footprints carry, losslessly (#331).
- design_review, sch_analysis, sch_export and sch_batch resolve pins
  through the placed unit; #182 is closed.
- The Default netclass is written complete, and a incomplete one repaired
  (#326) — junction dots were suppressed project-wide.
- Per-user Windows KiCad installs are discovered (#254); MCP startup no
  longer reinstalls guidance after an explicit uninstall (#242); placed
  symbols carry Datasheet and Description from the library (#226).

Project:
- GOVERNANCE.md states how the project is run; CODEOWNERS registers areas.
- `cargo xtask fix-doc-counts` derives every documented count from the
  registry, so tool-adding PRs no longer conflict by construction.

20 toolsets, 217 registered tools, 223 total.
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.

create_netclass omits wire_width, which silently disables junction dots across the whole project

2 participants