Repository navigation
SYN-229 - Put three registry parts on the pads and symbols KiCad ships - #235
Conversation
2434258 to
b44efe8
Compare
|
This PR still has two blockers against issue #229:
|
b44efe8 to
afdc10a
Compare
rodneyosodo
left a comment
rodneyosodo
left a comment
There was a problem hiding this comment.
Re-reviewed delta: rebase only. LGTM.
ed895b4 to
130d4ab
Compare
f5dcb64 to
21e0b35
Compare
|
[P1] TMP117 still cannot qualify against its footprint. The corrected WSON-6-1EP footprint has exposed pad 7, but the stock |
@SammyOina no stock symbol carries pad 7, so added the mechanism: |
|
[P1] Do not let footprint_only bypass a real symbol pin\n\nfootprint_only is currently accepted without checking that the flagged number is absent from the referenced KiCad symbol. That means a future part can mark an ordinary symbol pin as footprint_only and make both the loader and symbol_pin_coverage silently accept a wrong pin map. Please make the invariant explicit: when the symbol is available, a footprint-only pin must not exist in the symbol (and reject it if it does), with a regression test for the invalid footprint_only=true/existing-symbol-pin case. The TMP117 exposed-pad fix should still pass because pad 7 is absent from TMP117xxDRV.\n\nUntil this guard exists, I recommend holding approval because the new escape hatch weakens the qualification guarantee it is meant to preserve. |
4e83de5 to
bc38791
Compare
…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.
Review — changes requestedCI passes and the latest head includes the TMP117 exposed-pad mechanism, but issue #229 is not fully resolved.
The EEPROM, ferrite-bead, and TMP117 pad/footprint corrections appear addressed. I would not approve until the |
|
Addressed review: added loader/qualifier guard so |
SammyOina
left a comment
SammyOina
left a comment
There was a problem hiding this comment.
Approved conditionally. The registry and footprint_only changes are addressed, and the test/routability checks pass. Please merge only after the required Lint and Build check passes on the latest head; the current failure must be resolved or rerun successfully first.
…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.
…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.
All four declared pins named the wrong pad. `Memory_EEPROM:24LC256` extends `24LC16` and numbers its pins as the datasheet does — A0 A1 A2 GND SDA SCL WP VCC — so the part had `vdd` on A0, `gnd` on A1, `sda` on A2 and `scl` on GND, and pads 5 through 8, the real SDA, SCL, WP and VCC, had no pin at all. A board using it put the rail on an address input, ground on another, and the I2C bus on a third and VSS. `fixtures/designs/feather_m4_express.synth` has been doing exactly that. The three address inputs and WP are declared without `required`, because a design is free to strap them to either rail; a design that means to use the part straps all four, and this card does. `synth registry qualify --part 24lc256_eeprom` now reports nine checks passing where it reported `E-SYNTH-QUAL-002`, four undeclared pads. Nothing reported the four that were declared wrongly, which is its own defect.
The part declared `Device:Ferrite_Bead`. KiCad 10.0.6 ships `Device:FerriteBead` and `Device:FerriteBead_Small`, with no underscore, so the symbol could not be read: `qualify` reported `symbol could not be read` even with KICAD_SYMBOL_DIR set, and an export of any board using the part reached E-SYNTH-EXPORT-001 and synthesized a rectangle instead.
The part names symbol `Sensor_Temperature:TMP117xxDRV` and footprint `Package_TO_SOT_SMD:SOT-563`. Those are two different bodies: DRV is TI's code for the 2.00 x 2.00 mm six-pin WSON, and SOT-563 is a 1.6 x 1.2 mm leaded package. Every exported land, pad position and courtyard came from the wrong outline. The pin numbers were already right -- they came from the symbol, which is the WSON part -- so only the body changes: SCL 1, GND 2, ALERT 3, ADD0 4, V+ 5, SDA 6 all stay where they are. The exposed pad is pad 7 of the new footprint and is **not** declared, so `E-SYNTH-QUAL-002` still reports it. That is not an oversight and not something this commit can fix. `TMP117xxDRV` extends `TMP102xxDRL`, which declares pins 1 to 6, and the loader requires a declared pin number to exist in the symbol as well as the footprint -- a pin for pad 7 makes the entire registry fail to load, which is how CI rejected the first attempt at this. So a part whose exposed pad is absent from the stock symbol has no way to cover that pad today. `ina3221_power_monitor` can declare `ep` on pad 17 only because the `INA3221` symbol happens to carry pin 17. Worth knowing how that got past a local run: the loader skips the symbol check entirely when no symbol library resolves, and tests here run without one. Source: TI TMP117 datasheet, package DRV0006B.
…urtyard U9's courtyard grew 1.6 x 1.6 mm to 2.0 x 2.0 mm when the part took the WSON body its symbol names, and every component on the board moved. The size of that is worth writing down rather than rediscovering. The outline went 90.57 x 76.16 mm to 65.07 x 72.96 mm -- 25.5 mm narrower, 31% less area -- from 0.4 mm on one part of forty. Isolated here: reverting only the two dimension lines and keeping the new pad makes this snapshot pass unchanged, so the swing is the courtyard and nothing else. That is absmach#158's non-monotonicity, measured a second time and in the opposite direction to the one seen when the oscillator body was corrected. The re-record is accepted because the direction is the safe one and `feather_m4_express_outline_area_does_not_grow_past_baseline` still passes on its own terms. It is not evidence that the placer is sound.
A footprint's exposed pad is copper, and copper no pin accounts for is what `E-SYNTH-QUAL-002` exists to report. But the loader also requires every declared pin number to exist on the KiCad symbol, and a thermal pad is precisely the pad a symbol does not draw. Those two rules met on `tmp117_temperature` and left no legal answer: name the real WSON-6 footprint and fail to account for pad 7, or declare pad 7 and make the whole registry refuse to load. `footprint_only = true` on a pin says "this pad exists in copper and has no symbol pin". The loader skips the symbol cross-check for those pins, and so does `symbol_pin_coverage`; the pad checks do not, so the pad still has to be real and still has to be covered. Deliberately narrow. The flag suppresses one check for one pin, and the control test proves it: the same part with the same pad and no flag is still rejected with `KicadPinMismatch`. It is a way to describe a pad, not a way to silence the check. `ina3221_power_monitor` needs no flag, because the `INA3221` symbol happens to carry pin 17. That it works there and not on the TMP117 was an accident of which symbol an author drew, not a property of the package. The field is not serialised when false, so no IR snapshot moved. `registry qualify` now reports no `E-SYNTH-QUAL-002` for the TMP117. What remains is `E-SYNTH-QUAL-008`, which every part in this registry reports because none records a reviewer -- that is absmach#51.
The outline is fitted with a perimeter channel round every connector and IC, but the review-driven refinement sets coordinates directly. On the Feather with a 20-pin header soft-hinted to the bottom, the TMP117's real WSON courtyard changed the packing enough that `move_connector_to_edge` put J4 4 mm inside the edge and `separate_courtyards` then pushed it a further 2.6 mm down, away from U8. J4 finished 1.4 mm from the edge, which is flush in all but name for a hint that only asked for the bottom. A recommendation that takes a connector or IC closer to the outline than the fitted margin is now dropped, the same way a move into a keepout already is. Passives, edge-flush connectors and board features keep their own rims and are not judged; a part already inside the margin may still move as long as it does not get closer. The placement snapshots do not change.
6271b47 to
d39216b
Compare
|
Rebased onto main and force-pushed. |
What type of PR is this?
This is a bug fix.
What does this do?
Moves
24lc256_eeprom's four pins onto the pads24LC256uses, renames the bead symbol, declares TMP117 pads 3 and 4.Which issue(s) does this PR fix/relate to?
Resolves #229
Have you included tests for your changes?
registry qualifynow passes nine checks on all three parts; the feather placement snapshot is re-recorded.Did you document any new/modified features?
No feature changed. Each commit message records the wrong pad and what a board using it got.
Notes
tps22918_loadswitchtakes its output from pad 5, the discharge pin. Left out: that needs TI's datasheet.