Ep roundradius handling #422
Conversation
|
Code Climate has analyzed commit 06d7d27 and detected 12 issues on this pull request. Here's the issue category breakdown:
View more on Code Climate. |
poeschlr
force-pushed
the
ep_roundradius_handling
branch
from
0a032a1
to
242ff86
Compare
Using a fixed round radius ratio of 0.25 with maximum 0.25mm radius does not make sense for the exposed pad as it is created without positive fillet and therefore such a large radius would interfere with the "lead". Our research showed that using the same radius as the normal pads is a much better option. There also has been a parameter added to directly control the round radius from the size definition.
poeschlr
force-pushed
the
ep_roundradius_handling
branch
from
242ff86
to
9241a3d
Compare
|
I only managed to read ~50% of the code so far. Its a rather big patch and not that simple. My goal is to get this done this weekend. |
There was a problem hiding this comment.
So I read the through the complete patch set and could not find anything that is obviously wrong.
I don't really like the fact that the documentation for the parameters is duplicated for each class. But I don't have a good solution for that either.
The one change that I don't understand is why the one parameter in "qfn-24.yams" is part of this patch.
I also did compare some footprints generated with the old versions. I did not spot an error or corner-case. But obviously not every affected FP was checked :)
|
The change of this one parameter is part of this set because i could not generate the footprints without that fix. And this fix is connected to a single footprint pull request. |
poeschlr
force-pushed
the
ep_roundradius_handling
branch
from
defc8d8
to
bb831c4
Compare
scripts/Packages/Package_Gullwing__QFP_SOIC_SO/ipc_gullwing_generator.py
Show resolved
Hide resolved
scripts/Packages/Package_NoLead__DFN_QFN_LGA_SON/ipc_noLead_generator.py
Show resolved
Hide resolved
|
The resulting footprints seem OK to me and at a high-level this appears fine. I admit to not following everything you've written but am OK merging this if nobody else objects. My comments above are reduced mostly to semantics without any logical effect. |
|
@poeschlr |
|
@pointhi yes it was a difference regarding the default round radius handling of chamfered pads, chamfered pad grids and exposed pads. All of them used 0 as the default in the past but i had unified this over all nodes with the introduction of the centralized round radius handler class. |
|
Is this seems to be (almost) finished, I collected affected PRs above. |
|
@pointhi |
|
@evanshultz I did a rough look, and the concept looks good. If you or @poeschlr are ok you can merge anytime. From my side: It would be nice to add a few tests of this feature, to validate nothing breaks. |
|
The unit tests i had already showed weaknesses which i solved in my last commit. So yes there are unit tests already there. Could there be more? Of course. I invite anyone to write some, i simply do not have the time fir this. |
|
Alright then. Thank you! |
|
Looks like these slipped through: |
* Added WSON-10-1EP_2.5x2.5mm_P0.5mm_EP1.2x2mm. Datasheet: http://www.ti.com/lit/gpn/tps63030 Generated by script, using PR pointhi/kicad-footprint-generator#453 * Regenerated footprint after script change, using PR: pointhi/kicad-footprint-generator#422
* Added footprint for DFN-8-1EP_2x2mm_P0.5mm_EP0.7x1.3mm. Datasheet: https://www.onsemi.com/pub/Collateral/NUF4401MN-D.PDF * Regenerated footprint after script change: pointhi/kicad-footprint-generator#422 * Regenerated footprint after data change: pointhi/kicad-footprint-generator@99f5bef
* Added QFN-32-1EP_5x5mm_P0.5mm_EP3.3x3.3mm. Generated by script, using PR: pointhi/kicad-footprint-generator#458 Datasheet: http://ww1.microchip.com/downloads/en/DeviceDoc/00002164B.pdf * Regenerated footprint after script change: pointhi/kicad-footprint-generator#422
Using large a rather large round radius for exposed pads was a bad idea. See KiCad/kicad-footprints#1798
TlDr: use the same radius as the normal pads instead. Also added a parameter to overwrite this.
This required adding a more powerful round radius handler to avoid code duplication.