Skip to content

NOISSUE - Reject a registry pin whose role contradicts its KiCad symbol - #253

Open
JeffMboya wants to merge 13 commits into
absmach:mainfrom
JeffMboya:SYN-pin-maps-match-symbols
Open

JeffMboya wants to merge 13 commits into
absmach:mainfrom
JeffMboya:SYN-pin-maps-match-symbols

Conversation

@JeffMboya

@JeffMboya JeffMboya commented Oct 21, 2026 •

Copy link
Copy Markdown
Contributor

What type of PR is this?

This is a bug fix

What does this do?

Refuses a load where a pin's declared role contradicts the symbol's: a rail on a signal pad, or the reverse.

Which issue(s) does this PR fix/relate to?

Stacked on #195, which fixes the data this rule would reject. Found auditing parts for #184.

Have you included tests for your changes?

Yes, four in kicad_pin_check: type extraction, both contradictions, and the passive and do_not_connect exemptions.

Did you document any new/modified features?

Yes, check_kicad_pin_roles records why a number that exists is not evidence the pad is right.

Notes

Three files on top of #195. Only ch340g pad 4 still tripped the rule, so that type is corrected here.

@JeffMboya JeffMboya changed the title NOISSUE - Registry pin maps must match the symbols they name NOISSUE - Reject a registry pin whose role contradicts its KiCad symbol Oct 21, 2026
@JeffMboya
JeffMboya force-pushed the SYN-pin-maps-match-symbols branch from 570f13c to eab3c07 Compare October 21, 2026 19:10

@SammyOina SammyOina 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.

[P1] Do not classify CH340G V3 as a regulator source without updating the power-budget model

Changing v3 to power_output is electrically more accurate for the KiCad symbol, but it currently regresses the existing pass fixture: pass__power_004_sufficient_decoupling now emits two E-SYNTH-POWER-010 errors. PowerBudgetRule treats every part with a power_output pin and max_current_ma as a regulator, so CH340G's auxiliary 3.3 V output is incorrectly treated as a regulator rail.

Please either distinguish regulator outputs from auxiliary/generated supply outputs in the registry/model, or update the power-budget rule so this pin cannot become a regulator source. Add a regression test proving the CH340G fixture remains clean. The current head also has failing CI shards and a merge conflict, so this needs to be resolved before merge.

@JeffMboya
JeffMboya force-pushed the SYN-pin-maps-match-symbols branch from eab3c07 to 8480c2a Compare October 22, 2026 18:11
@JeffMboya

Copy link
Copy Markdown
Contributor Author

Fixed in 8480c2a. PowerBudgetRule now reads max_current_ma as a budget only for a regulator or charger; elsewhere it is the part's own draw. Rebased, conflict resolved.

@SammyOina

Copy link
Copy Markdown
Collaborator

[P1] Do not gate power-budget checking on a closed list of part kinds

part_supplies_a_rail() now returns true only for regulator and charger, which silently disables E-SYNTH-POWER-010 for user-defined or future supply parts with a different kind. Before this change, any part with a power_output pin and max_current_ma was checked. Please model “provides a current budget” explicitly, or preserve the generic power-output behavior while excluding only auxiliary outputs such as CH340G v3. Add regression coverage for both a non-regulator supply that must still be checked and the CH340G auxiliary output that must not be treated as a rail source.

JeffMboya and others added 7 commits October 30, 2026 18:07
`jst_ph_3pin`, `jst_ph_4pin`, `jst_xh_2pin` and `jst_xh_4pin` all declared
`footprint_dimensions` of 1.0 x 1.6 mm — the size of one contact, not the
connector. `jst_ph_3pin`'s real courtyard is 8.9 x 8.6 mm.

`synth registry qualify` reports this as `E-SYNTH-QUAL-006`, a declared
package size that disagrees with the footprint's own pad extent, on all four.
The exporter consults these values when it has to synthesize a footprint, and
a 1 mm courtyard where an 8.9 mm connector goes is a part that places into its
neighbour.

Measured from the stock footprints rather than estimated.
`xtal_8mhz` and `xtal_generic` both name
`Crystal:Crystal_SMD_3225-4Pin_3.2x2.5mm`, a four-pad package, and both
declared two pins: `p1` on pad 1 and `p2` on pad **2**.

On an SMD3225 the two crystal terminals are pads 1 and 3, diagonally opposite
each other. Pads 2 and 4 are the grounded case. So `p2` named the case tab,
and every board that drove `p2` drove the can rather than the crystal.

`p2` now carries pad 3 and keeps its name, so no board source changes. The two
case tabs are declared as `case_1` and `case_2`, which also clears the
`E-SYNTH-QUAL-002` both parts reported: two copper pads no pin declared.

The symbol follows. `Device:Crystal` is the two-pin symbol; the four-pad part
is `Device:Crystal_GND24`, which is the one that says the case pads exist.
`synth registry qualify` reports `E-SYNTH-QUAL-002` — "footprint has copper
pads no pin declares" — on twenty of the 124 shipped parts. It is on the
structural allow-list, so `E-SYNTH-QUAL-000` refuses a fab export over it with
no override: those parts cannot be submitted for manufacture at all.

The pads are not cosmetic. `w5500_ethernet` declared 8 pins of a 48-pad
LQFP-48; `esp32_wroom_32` left 39 pads undeclared; `mpu6050_imu` declared 3 of
25. A pad nothing declares is copper the router does not know about and the
netlist cannot reach, which is what the finding's own text calls a short
waiting to happen.

Sixteen parts here, with their pin maps completed against the stock KiCad
symbol and footprint each one names:

  w5500_ethernet esp32_wroom_32 nrf52840 stm32g030 txs0108e
  mcp23017_gpio pca9685_pwm micro_usb_b 24lc256_eeprom at24c512_eeprom
  m24m02_eeprom bq24074 tps22918_loadswitch ant_helical_433mhz
  mpu6050_imu tmp117_temperature

Carried from the SYN-39 branch, but re-derived rather than copied: a
three-way merge against the common ancestor, because `main` has since added
`max_power_mw` and `thermal_resistance_c_per_w` to `bq24074`,
`esp32_wroom_32`, `nrf52840` and `tps22918_loadswitch`, and taking the
branch's files wholesale would have reverted all four.

After this, `registry qualify` reports a structural or geometry finding on
**3** parts rather than 24. The three that remain are out of scope here and
each has a reason: `tvs_array_5v` is corrected in absmach#157's branch; `bg95` needs
94 pads declared and the branch never did it; and `mpu6050_imu` stops at one
undeclared pad, the QFN's exposed pad, which cannot be declared while
`Sensor_Motion:MPU-6050` has 24 pins and its footprint has 25 pads.
… renamed

Completing this part's pin map renamed `vcc` to `vdd`, but the
`[[required_decoupling]]` block still keyed on `vcc`.

E-SYNTH-POWER-001 resolves that key by pin name and silently `continue`s
when no pin matches, so the W5500's two 100 nF caps stopped being checked
rather than being reported as missing. A requirement that names a pin the
part does not have is indistinguishable from a satisfied one.
…using it

Completing the W5500's pin map renamed its digital supply from `vcc` to `vdd`,
pin 28, and three fixtures kept connecting `vcc`. The pin no longer exists, so
each of them reported `E-SYNTH-COMP-002` for an undefined pin and
`E-SYNTH-CONNECT-002` for the supply that never arrived.

CI stayed green throughout, which is the part worth noting. The ERC fixture
harness asserts that a `<CODE>__name.synth` file emits the code it is named
for and allows any others, so `E-SYNTH-RESET-001` and `E-SYNTH-SPI-002` kept
firing and nothing failed -- while the boards they fire on had become
different boards, with an unpowered Ethernet controller and two extra errors
each. A fixture that no longer isolates its own case stops being evidence for
it.

Both codes are gone now and the named ones remain. The two `U1.vcc` left in
the SPI fixture are a `ch340g`, which does declare `vcc`.

The harness weakness is the same one the qualification matrix had on absmach#75,
where `expect_source_codes` was a subset check: a declared list that is a
floor rather than a set lets a case decay quietly. Worth fixing there too,
and out of scope here.
…obsolete one contradicting itself

`at24c512_eeprom` and `m24m02_eeprom` were given the 24LC256's comment
verbatim: "Pin names and numbers come from the stock symbol the part
names, which extends `24LC16`". Neither symbol exists -- KiCad 10 ships no
`Memory_EEPROM:AT24C512` and no `Memory_EEPROM:M24M02-DR` -- so the comment
claimed a cross-check that could not have happened. The pin numbers are the
SOIC-8 pinout these parts share with the 24LC256, and the comment now says
that, and that the symbol name resolves to nothing.

Main marked `24lc256_eeprom` obsolete with a description saying it numbers
VDD, GND, SDA and SCL as pins 1 to 4. This branch moves those pins, so the
description now says what was wrong and that it is corrected; the text is
the same as absmach#235's change to the same line, so the two branches merge
cleanly.
The re-records this branch carried were taken against an older main, and
the commits that made them could not be replayed: at the first one the
registry did not load yet (the W5500 decoupling key is fixed two commits
later). Recorded once at the tip instead, with this machine's libraries,
which reproduce main's own snapshot unchanged.
@rodneyosodo
rodneyosodo force-pushed the SYN-pin-maps-match-symbols branch from c8f484c to da5de3d Compare October 30, 2026 15:19
rodneyosodo and others added 6 commits October 30, 2026 18:27
…the rebase

ref_battery_charger and secure_tracker are the two references this
branch's part data moves, as before the rebase; main has since refreshed
every fingerprint, so the values carried from the old base were stale.
Both values are CI's: this machine renders the stock symbols differently
(main's own goldens fail locally), so a local regeneration would record
the wrong fingerprint.
The existing load-time check asks whether a declared pin number exists
in the KiCad symbol. That is necessary and not sufficient: a part which
numbers its pins 1, 2, 3 in the order the author listed the signals hits
numbers every package has, so the check passes and every pin still
points at the wrong pad. Five shipped parts were in that state.

This compares the declared role against the symbol's for the same
number, and refuses the load on either contradiction that matters:

  a supply or ground pin on a symbol pin that drives or reads a signal
  a signal pin on a symbol supply pin

`passive`, `unspecified` and `free` agree with anything, because KiCad
uses them for stacked grounds, exposed pads and crystal terminals alike;
a pin the part declares `do_not_connect` agrees with everything, since
it asserts the pad stays floating and no symbol type contradicts that.
Those exemptions are why the rule reports nothing across the other 120
parts: it is narrow enough to be a fact rather than a guess.

The diagnostic names the signal the pad actually carries, because
"pad 1 is wrong" is not actionable and "the symbol carries output there
(TXN)" is:

  kicad_symbol `Interface_Ethernet:W5500` disagrees about pad `1`: the
  part declares `vcc` as PowerInput, the symbol carries output there
  (TXN). A rail and a signal would meet on that pad.

Like the number check it is skipped when no symbol library resolves, so
it is CI that enforces it. Verified by putting the w5500 defect back and
watching the load refuse it, then taking it out again.
Pad 4 was already the right pad; its type was not. V3 is the bridge's
internal 3.3 V regulator output -- tied to VCC on a 3.3 V design and
decoupled to ground on a 5 V one -- and KiCad types it `power_out`.
Declared `passive`, no power rule could reason about it: the pin that
sets the logic level of thirty designs was invisible to the rail checks.

It is the one pin left in the registry that the role invariant added in
the previous commit rejects, so the two belong in one change.
`PowerBudgetRule` treated any part with `max_current_ma` and a `power_output`
pin as a regulator feeding whatever that pin touched. The field means
opposite things depending on the part: on a regulator or a charger it is what
the device can deliver, and on everything else it is what the device itself
consumes.

Correcting the CH340G's `v3` to `power_output` to match the stock symbol
therefore turned a 20 mA figure that describes the chip's own draw into a
20 mA rail budget. `v3` is the internal LDO's decoupling tap -- there to be
decoupled, not loaded -- so `pass__power_004_sufficient_decoupling` reported
two E-SYNTH-POWER-010 overruns on a board that is fine, each charging a
CH340G 100 mA for the header its own decoupling pin reaches.

The rule now asks whether the part supplies a rail before reading the number
as a budget. Regression test is the fixture's own topology: against the
previous code it fails with those two findings, naming 100 mA drawn on
`net_2` and `net_3` against a 20 mA regulator.
Main added `ch340c` after this branch was cut, with the same `v3` pad 4 the
CH340G had, declared `passive`. `Interface_USB:CH340C` types it `power_out`,
exactly as the CH340G's symbol does, so it gets the same correction for the
same reason. With the role check in place the seed registry refused to load
on it.

It stays out of the power budget for the reason the CH340G does: its kind
is `ic`, and only a regulator or a charger has a current budget to spend.
`roles_agree` blocks two contradictions: a rail on a signal pad and a
signal on a supply pad. A registry pin declared `passive` is neither a rail
nor a driver, but `is_power()` is false for it, so the check counted it as a
signal and rejected it under any `power_in` or `power_out` symbol pin.

That refused `lmr33640`, which main added after this branch was cut and
which declares its internal-LDO `VCC` passive deliberately, so that nothing
reads a bias pin as a rail to load. Typed `power_output` instead it would
also become the part's first power output, which is the pin
`PowerBudgetRule` charges as the converter's rail.

The symbol side already lets `passive` agree with anything; the declared
side now does the same. A test pins it against every symbol type.
@rodneyosodo
rodneyosodo force-pushed the SYN-pin-maps-match-symbols branch from da5de3d to 0ec12b2 Compare October 30, 2026 15:27
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