Repository navigation
Conversation
570f13c to
eab3c07
Compare
SammyOina
left a comment
There was a problem hiding this comment.
[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.
eab3c07 to
8480c2a
Compare
|
Fixed in 8480c2a. |
|
[P1] Do not gate power-budget checking on a closed list of part kinds
|
`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.
c8f484c to
da5de3d
Compare
…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.
da5de3d to
0ec12b2
Compare
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 thepassiveanddo_not_connectexemptions.Did you document any new/modified features?
Yes,
check_kicad_pin_rolesrecords why a number that exists is not evidence the pad is right.Notes
Three files on top of #195. Only
ch340gpad 4 still tripped the rule, so that type is corrected here.