kicad-footprint-generator
kicad-footprint-generator copied to clipboard
Generate separate C, L, and R chip footprints
Fixes https://github.com/pointhi/kicad-footprint-generator/issues/420
Because IPC tells us there are differences in the body sizes, not all chip parts should result in the same footprints. This generates unique footprints for each part type.
- Add unique body size info for C, L, and R part types in accordance with IPC-SM-782 (with a slight deviation for 0805 capacitors)
- File renames to remove 'chip' if file contained more than chip info
- Aligned size definition file names
- Print part type and definition during execution
- Corrected descriptions and keywords in SMD_chip_devices.yaml
- Fixed a few typos in comments
See a comparison of resulting footprints at https://docs.google.com/spreadsheets/d/1OAOwxWGSfmM8BzPS2x2mySmMnPzjYmWQ7ZMfdHtCbws/edit?usp=sharing.
Footprint PR: https://github.com/KiCad/kicad-footprints/pull/2280
Related: https://github.com/KiCad/kicad-footprints/issues/711
Code Climate has analyzed commit f83b9878 and detected 0 issues on this pull request.
View more on Code Climate.
@poeschlr Would you be willing and able to take a look at this?
@poeschlr I made a separate commit I found for an error in one YAML file, but I should have made all the files changes in one commit and then done a separate commit for all the file renames (or put that in a separate branch). That probably would have been easier to review. My apologies for not thinking about how to do this better earlier. Is there anything I can do now to help?
And one question: because there's now a YAML file just for resistors, should the wide-body YAML file be merged into the regular resistor file? I kept them separate for continuity reasons but I really don't see why they can't be together in one file.
Should I merge in master the same way I did at https://github.com/pointhi/kicad-footprint-generator/pull/439? Or is that not the right way?
@poeschlr Could you take a look at this? Do you see problems with using unique body sizes for chip footprints as defined by IPC?
Also, merge in master as I did above to fix the conflict?
I do not see a problem with using unique body sizes for different devices. However, be aware that we did not use the outdated IPC document everywhere. And we did avoid it for 0805 completely as we did our own research.
The reality is that such devices are much less standardized than we would like. Which means the footprints of the official lib can only ever be a compromise. Unless we really want to add one footprint per component series (not really feasible).
I hope that 5.1.6 finally fixes a deal breaker bug for the footprint wizard allowing me to continue working on a IPC footprint wizard for these sort of devices that will give users the option to generate fully compliant footprints for the device they really use.
Regarding fixing the merge conflicts: You can either merge master into this one or rebase this onto the current master. The latter might be better in the sense of a cleaner git history but it also requires more git skills.
Edit: also remember that rebasing becomes impossible once one has used the merge workflow for a given branch.
Yes. As this was a while ago I may have forgotten some details, but from memory and looking at the Google Sheets I linked the IPC-SM-782 0805 cap size and the body size you came up with from reviewing datasheets is quite similar. So I think the initial issues were related from us not realizing that C and L body sizes were given in the IPC doc, and instead using the R body size for all part types.
I will try a rebase, then. First time for everything...
@poeschlr Now I see what you meant. I had already merged in master above to rebase wouldn't work.
I merged in master again with a little manual work. Branch conflicts resolved but the size definitions added at https://github.com/pointhi/kicad-footprint-generator/pull/440 have a SMD_
prefix instead of C_
. When I've encountered this before I didn't find a way to resolve it in the same PR without GH seeing more branch conflicts so either made a second PR or a new replacement PR. Is there a way to fix that in this PR?
Also, I noticed a typo of 'tantalum' (there is a trailing 'n') so can that be done here too?
One option to fix up the branch is to locally rename it, make a new branch with the current online name and then cherry-pick the commits from your original (now renamed branch) to your new branch correcting merge conflicts as necessary. At the end (after checking the state locally) force push the new branch to the online branch connected to this pull request.
This might help a bit: https://learngitbranching.js.org/
Edit: If necessary then i could look into fixing your branch but it might take a while till i find the time for it (don't tell that to normal contributors, this would be a service reserved to fellow maintainers).
@poeschlr The branch is now fixed without conflicts. In addition, I fixed a typo and also an error when running the script. Not sure how that remained.
New footprints are submitted at https://github.com/KiCad/kicad-footprints/pull/2280.
@poeschlr I created a footprint PR and requested your review, but most of the discussion was here so this is where I'm commenting. Will you be able to take a look at this? I have documented more info in the footprint PR.
@poeschlr Ping!
@diegoherranz If you're feeling up to it and want to review this that's fine with me. It seems @poeschlr is busy and @pointhi generally will merge whatever is approved into /scripts. But of course, take your time to review and make sure everything makes sense and you don't see anything I messed up. I don't want to break anything when I'm trying to fix things!
@poeschlr Ping again!
@cpresser You wanna take a look at this one?
@cpresser Are you able to work on this now? I would quite like to have this in 5.1.7. I know we have a lot of footprints to sort out now as well.
Since Rene wrote the original generator script he knew the history. Let me try to capture what I recall here. There are also a few links above with more background info.
When Rene wrote the script we found that the cap footprints, specifically 0805, didn't see correct. Rene then surveyed info from cap datasheets and faked some body size for caps based on real datasheets. IPC-SM-782 has body size info and it turns out that the resistor dimensions were being used for all part types. When inputting the correct cap dimensions, the resulting footprints are almost exactly the same as the ones from Rene's research. While caps were noted by Wayne and precipitated making some manual adjustments earlier, other part types also have specific body sizes in that IPC doc. I captured all those body sizes here and then made updates so the correct body sizes are being used for each specific part type.
You may be able to search online and find IPC-SM-782, but if not I guess you just have to trust me on the dimensions??
In essence, it was just an oversight when developing the script to miss that IPC-SM-782 shows resistor body sizes first but there are other body sizes in later pages.
The reason why I kept pushing on this and marked it as a bug is that:
- We claim to have IPC 7351 footprints, but the body sizes are wrong for some parts. So bad input gives bad output. Some of our chip footprints are known to not meet the sizes they should according to IPC documentation.
- For some part types, the body size is quite wrong. There could be issues because the pad sizes don't match the part size. If there are soldering issues, I'd be very sad since this is a known problem and should have been corrected so users of the library weren't affected. Nobody wants to pay money and get back a bad board that is due to our library. Yes, this is only one of many known issues but it's one I found and care about.
Sorry for the delay, I do have this open in a browser-tab for quite a while now.
It is of medium size, so I guess It will be possible to get this done by september 23.
However, I will invest my time the next few days into finishing some footprint-reviews I already started. My brain has a limited stacksize.
@cpresser Thank you!
Yes, let's wrap everything else up and then go back to the tantalum YAML file.
Edit: I'll evaluate the footprint changes I've mentioned above and report back so we can look at the potential impact to users. Edit2: Done. See comments above for current vs proposed and my thoughts on it.
While we're talking about this, I really dislike some of the naming here. I think it's because this was done first and Rene did things a little better with the gullwing and SON generator scripts.
- Each of the YAML files in /size_definitions starts with 'size_' which seems unnecessary. Remove it?
- The
series_config
parameter is the same file name as theglobal_config
parameter. I would like to rename topackage_config_KLCv3.yaml
like the SON script. OK? - The folder and script name of
SMD_chip_package_rlc-etc
is just ugly to type. This script does chip and tantalum capacitors body types, and other part types besides Rs and Ls and Cs, but there has to be a better name. Any ideas?
1. Each of the YAML files in /size_definitions starts with 'size_' which seems unnecessary. Remove it?
Agree.
2. The `series_config` parameter is the same file name as the `global_config` parameter. I would like to rename to `package_config_KLCv3.yaml` like the SON script. OK?
I don't quite understand that.
TBH the difference between series_config
and global_config
is not clear from the command-line help. It does not really describe what it does.
3. The folder and script name of `SMD_chip_package_rlc-etc` is just ugly to type. This script does chip and tantalum capacitors body types, and other part types besides Rs and Ls and Cs, but there has to be a better name. Any ideas?
Agree. Here are some of my ideas. None of them sound really appealing to me. Perhaps they give you the right inspiration.
- Package_Chip_SMD
- SMD_2Terminal_Packages
- Generic_Chip_SMD
- Generic_2Terminal_SMD
One more point, should we move the discussion about the optimal pad geometry of the high-impact footprints somewhere else? This PR is getting quite cluttered. Also for documentation issues it would be nice if the discussion can easily be found.
global_config is for the entire library, which all scripts can use. series_config is only for a particular generator (part/package type). It's simple and I just did it so let me know if you have an issue. We can now make further changes for all generator scripts since they will be aligned.
global_config is for the entire library, which all scripts can use. series_config is only for a particular generator (part/package type). It's simple and I just did it so let me know if you have an issue. We can now make further changes for all generator scripts since they will be aligned.
Sorry, my comment also was not clear. I was referencing to the help text of the generator. Which currently has those two lines:
the config file defining how the footprint will look like. (KLC)
the config file defining series parameters.
From what I understand they have the same effect, but series overrides global. That should be reflected in the help text.
For the global_config vs series_config, this text matches the SON generator. I agree better wording could be used. Would it be OK to shelve this and update all the generators together in another PR?
Here are the three IPC 7351B tables that are specific to this generator:
It handles more than just chip packages. And SMD crystals and aluminum electrolytic caps also have 2 pins and a different table (I previously implemented the electrolytic cap IPC generator). Maybe ipc_chip_molded.py
? The others are ipc_ plus the package types they handle.
OK. I think that wraps up everything for chip parts. Yay!
While tantalum caps are out-of-scope for what I intended to do here, what are you thinking?
Proposed process to continue with this PR and the footprints. We have a little more than 1 week left to get footprints merged for 5.1.7:
- Merge this generator+yaml changes even when we are not 100% sure about the high-impact (0402 0603 0805) RLC footprints.
- Compare and discuss the resulting footprints in https://github.com/KiCad/kicad-footprints/pull/2280
- When changes are needed, open a new PR for footprint-generators, iterate the process
For the global_config vs series_config, this text matches the SON generator. I agree better wording could be used. Would it be OK to shelve this and update all the generators together in another PR?
Yes. We are building quite a backlog. I am going to open an issue for that right away so it does not get lost. edit: this one https://github.com/pointhi/kicad-footprint-generator/issues/631
I find terminal spacing in the following YAML files:
- resistor_chip_smaller_0603.yaml (only 0402)
- default_chip_smaller_0603.yaml
- default_chip.yaml
- capacitor_tantal.yaml
The first one (R_04020) is simple enough so I'll share the difference when using terminal length here:
- Pad centers: +/-0.485mm to +/-0.51mm
- Pad width: 0.59mm to 0.54mm
- Pad inside: 0.38mm to 0.48mm
- Pad outside: 1.56mm to 1.56mm
Current is above and new is below. In an absolute sense it a relatively large change but it because in comparison it's not as much. Pad spacing is much wider than the Kemet datasheet you shared at https://github.com/pointhi/kicad-footprint-generator/pull/438#discussion_r488921448, though. And compared with https://www.yageo.com/upload/media/product/productsearch/datasheet/mlcc/UPY-GPHC_X7R_6.3V-to-50V_18.pdf the min lead spacing could be 0.25mm (0.95mm body length - 2 * 0.35mm terminals) so even our existing footprint fails there. I'm not totally sure about this one. Here IPC pads seem to far apart compared to this datasheet while with 0805 they were too close together...
IPC-SM-782 has 3216, 3528, 6032, and 7343 tantalum cap body sizes.
For the others, however, there is no 'default' part in IPC-SM-782 so we don't really have any reference to use that I see. The link http://www.tortai-tech.com/upload/download/2011102023233369053.pdf is referenced for many of the non-tantalum parts but the link doesn't work so I'm not sure where to go for those. Thoughts? Just leave them if we don't have a reason to change?
And also, rename the generator. And then any tantalum stuff if you want to look into it.
And also, rename the generator. And then any tantalum stuff if you want to look into it.
Tantalum: not as part of this PR. Rename: yes please.
The R_0402
change looks good to me at first glance. I like that there is more space between pads. That allows to route a signal between them.
I would like to merge this ASAP. And if necessary open another PR for fixes. See my answer here https://github.com/pointhi/kicad-footprint-generator/pull/438#issuecomment-693662901 Do you agree, or do you want to discuss all the high impact footprints as part of this PR?
0402 resistor dimensions updated. One extraneous bit of info found and deleted. And file/folder renamed. I'm open to comments on the name as I'm not really in love with it.
Now that I also discovered that IPC-SM-782 has some tantalum cap body sizes I think perhaps it is worth taking a look at. Perhaps I should start by updating the format and then comparing the existing footprints to the ones that we would get if we used IPC dimensions?
@poeschlr http://www.tortai-tech.com/upload/download/2011102023233369053.pdf is used as a reference many times here, but that link is dead. Do you have the document or, if it's some kind of standard, can you share what it is? I have no idea and it may help.