Code Climate has analyzed commit f83b987 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 #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 #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 KiCad/kicad-footprints#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!

Thanks! Just a hair of change on a few footprints. Did you see anything concerning?

Nope, the R & C footprints seem fine.

• The changes are hardly visible
• I can get a trace through each of them
• They haven't gotten bigger (0805 did, but only 0.04mm in both directions)

Are you ready for me to drop these footprints into the footprint PR so we can compare with master over there?

Yes, lets do that!

tantalum

• You missed to remove one n in ipc7351_smd_two_terminal.yaml
• I did a little bit of chasing. Its the heel parameter in line 88. Change it to anything not positive and the pads don't crash together. I tried to follow where that parameter, but got lost in scripts/tools/ipc_pad_size_calculators.py. I have no clue what the functions there are supposed to do. They are in dire need of documentation and test-cases.
• It seems to me the generator is buggy here.

From https://content.kemet.com/datasheets/KEM_T2005_T491.pdf page 15

The (current) KiCad footprint is quite different:

We are missing about 0.5mm on the L parameter, which corresponds to the 0.5mm heel in the IPC spec file.

Here is the equivalent info from AVX. I will add it to the table in the above post.
http://www.avx.com/docs/techinfo/TechSumAppGuide.pdf Page 16

I just pushed the footprints into that PR. Seems to me everything looks good, but take a look through the diff and see what you see.

Reviewing those footprints also made two things pop out at me which wouldn't take long to incorporate here but I suspect it's best to do as a later step:

1. There are wide body capacitor footprints available but they aren't here.
2. The inductor footprints are, at least in some cases, used for ferrite beads. I view ferrite beads and indutors as different parts, and they have unique schematic symbols in our library. We have a Ferrite_THT footprint library. Should the chip 'inductor' footprints go into Ferrite_SMD or somewhere else instead of being with inductors?

I'll look at the tantalum stuff a bit later today. We can always decide to punt on it if the current footprints are OK. Waiting also gives more time to get into #491 and (possibly... hopefully??) #439.

Edit: I should have clearly started that I didn't touch any tantalum footprints. I only pushed other footprints.

ad1: I vote for a different PR
ad2: There are both

• chip ferrites for example murata BL
• and chip inductors (https://www.digikey.com/product-detail/en/tdk-corporation/MLZ2012N6R8LT000/445-6761-1-ND/2523582)
• The chip-inductors are usually higher, comparable to chip-capacitors (they are also manufactured with screen printing).
• I think we need more research into this topic.
• Thus, I vote for postponing that. Should be another PR.

ipc7351_smd_two_terminal.yaml isn't being used (the B version should be used) but I fixed the typo. Thanks.

IPC 7351 give equations for determining pad size given package dimensions and fillet goals. The heel fillet is the inside fillet, which would be under the part body. I verified all the fillet goals so unless IPC7351B has a typo in it that is correct. And I've done hand calculations to verify the equations in the calculator script so I think that is correct as well. A mistake in the fillet goals seems most likely, or perhaps we're using the wrong table. Here is the table and part construction paragraph:

It sure seems like the right stuff. But I notice to phrases in parenthesis to find G dim and to find Z dim. That's strange. And backwards compared to the 'typical' equation usage (see lines 253-256 in ipc_pad_size_calculators.py). Swapping the numbers on lines 87 and 88 of ipc7351B_smd_two_terminal.yaml didn't quite do it, but it's much closer (pad centers at +/-0.7125mm with pad size 1.375x0.95mm). I gotta run right now but I'll keep on this a bit later. The equations aren't quite the same so I may need to read in toe and heel fillets and then swap the numbers in the ipc_data variable just for tantalum caps.

2. I'd rather not duplicate footprints. I thought KLC said this but I don't see it now. Anyway, it feels funny to me but if it works for inductor footprints to also be for ferrite beads then I'm OK leaving it alone.

Lastly, I happened to find this in IPC 7351B:

I haven't looked at that, but it may be another possible set of body dimension data. Just posting it her for posterity since I think we should finish this PR as it is.

Looks like I mixed up heel and toe in my head. Then the current behavior of the generator makes sense (pads grow inward). Swapping heel and toe might be what happened in that table. But that is a whole different story to investigate.

ad 2) I also don't see any reason to duplicate the footprints. In my personal use I selected resistor-footprints for ferrites so far (because I did not know better). They worked fine (a few thousand boards without problems).

Regarding that last block from IPC-7351B. Those are good points. We should discuss having different libraries for v6.
Perhaps like so

• Resistor_SMD_IPC7351_Nominial
• Resistor_SMD_IPC7351_MaxDensity
• ...
  I am not sure if that is a good idea. Lets discuss that outside of this PR as well.

I think this one is ready to merge. Do you want anything else done/checked @evanshultz
It is quite late in my Timezone already, I will come back to this and the footprint PR in ~16 hours so we can merge.

Right. Because the end of the lead is wrapped under the part on tantalum caps (and other molded body parts), the toe is under the part body instead of sticking out on the ends of the part. Confusing. But it finally clicked for me, too.

I checked the footprints at the end of https://www.vishay.com/docs/40182/tmch.pdf, and it's not too different. Pad centers just a bit off and the pads are a bit bigger. If I use the LMC (smallest pads) they are smaller than the datasheet. So with the tantalum fillet goals with heel and toe flipped I think we might be OK.

The only caveat are the 1608-08 and 1608-10 footprints where the pads are just too close with nominal fillet goals. I checked body dimensions and they appear correct. With LMC pads the pad inner gap is 0.2mm apart which should be fine. My thought with this would be to update the script to set a minimum pad spacing that automatically makes adjustments if the pads are too close. That distance would be in the series config YAML file.

However, that would take too much time. So I see a few options now:

1. Merge it just like it is. No tantalum cap changes.
2. Push updated tantalum caps except for the 1608 ones above.
3. Push all new tantalum caps, but make some simple manual adjustments to give more clearance on the 1608 ones. I suggest 0.2mm inner pad gap which is what we've done many other places (though not codified in KLC or elsewhere AFAIK).

Any of those are OK with me. Just let me know. But regardless, I will push another commit to this PR. Fix the extra 'n' on tantalum and a few other things that I figured out so far (even if he tantalum caps aren't re-generated). So please don't merge this first.

FYI, there already is an issue about having chip footprints with varying IPC densities: KiCad/kicad-footprints#1836. And a PR to add that functionality to the script: #439. We can pick this up later. :)

I pushed the tantalum updates, but I expect we will merge them as they are. And later take a closer look at the footprints and add some feature to adjust things if pad-pad spacing is too tight. But we'll see.

As noted, I didn't copy over tantalum footprints to the footprint PR. So if you want to hold off on other tantalum changes there are only two minor questions:

1. Generator script name. Is there something better?
2. Remove _PadXX.XxYY.Y in hand solder footprint names?

As noted, I didn't copy over tantalum footprints to the footprint PR. So if you want to hold off on other tantalum changes there are only two minor questions:

1. Generator script name. Is there something better?

2. Remove `_PadXX.XxYY.Y` in hand solder footprint names?

0. Regarding the tanatal caps: Lets keep them as they are. I am afraid we (mostly me) will fuck it up when trying to finish that in a rush.
1. How about Package_2Terminal_SMD ? Its not perfect, but the best I could come up with. If you want to stay with chip_molded I suggest to capitalize the first c as almost all other folders are captialized.
   Also, I would set the +x bit on the generator to make it executable.
2. I vote for not now. Lets not change their filenames for a minor release. I was somehow confused because I assumed that 5.x did not include the _PadXX.XxYY.Y information. But I was wrong there.
3. You did duplicate the scripts/chip_molded/ipc_chip_molded.py file in the last commit. Please remove the one that does not have generator in its name.

I figured out what happened with the missing footprints. Since the YAML files for body size info has been split, new entries were needed in the YAML file that contains the info for each part type. I would like to simplify the part_definitions.yaml file which I think is possible right now. My fix for this issue added a lot of entries. If this PR can wait to be merged for a few hours I'll do that tonight when I have a chance to work on it.

1. I went with SMD_two_terminal_chip_molded. There are crystals and SMD electrolytic caps and other 2-terminal SMD footprints, but chip and molded are two terms specific to this generator. So we could add a SMD_2terminal_crystal later, for example. And I realized this was like the existing IPC 7351 fillet goal files. It's awfully long, though.

We should also update the datasheets and dimensions for generic parts since some (all?) use R or C datasheets which don't apply to the diodes, fuses, LEDs, etc. I think we end up with a single YAML file for each part type and get rid of the generic YAML files entirely. But I don't think it's a showstopper to get this merged and it will affect footprints so let's not do it now.

Good catch on the L_1806 footprint at KiCad/kicad-footprints#2280 (comment) and fixed at KiCad/kicad-footprints#2487. Pads end up 1.275x1.9mm at +/-2.1125mm. A little more narrow but a bit taller than the pads now. And wider than the actual part's body width now!

I was able to remove about half the entries in part_definitions.yaml by allowing an ipc_reference for one size definition in the YAML file to overwrite the general one for the entire part group. That was easy. Supporting a paste pad overwrite requires changing parameters in IPC calculator functions that I plan to pull out and share across all generators so I'm leaving it for now. I checked each footprint compared to the current master, and excluding L_1806 only the timestamp changed (except for some castellated footprints; see below). So I think this change is good.

I also noticed there are 2816 diode, fuse, and LED footprints which should have been removed from the footprint repo. Oh well.

All the castellated footprints are wrong. The castellated table applies only the LCC footprints, not chips. So these footprints should be removed IMO. We are saying they are something when they clearly are not. What do you want to do about that? Here is the table and generic package drawing, from which you can compare the fillet goals with the YAML file to see for yourself:

If you have no further comments, I believe this can be merged. Or I can whack out the castellated stuff first. Let me know.

Edit: Actually, I'd rather remove the castellated footprints, the 2816 ones, and update L_1806. And remove the castellated stuff from this PR before merging. Are you OK with that?

@cpresser
How do you want to proceed?

Edit: Actually, I'd rather remove the castellated footprints, the 2816 ones, and update L_1806. And remove the castellated stuff from this PR before merging. Are you OK with that?

Yep, that sounds good.
Once that is done, I will give the whole thing another full review. I am losing track of individual changes/features. During all the incremental reviews the stack in my brain got corrupted.

One option would be so squash all the commits and rework the commit message so it properly reflects all the changes. What do you think?

I submitted KiCad/kicad-footprints#2495 and made the corresponding cleanup changes to the script.

I'm fine with a squash merge. That's what I usually do in this repo. Maybe that's a mistake?

I submitted KiCad/kicad-footprints#2495 and made the corresponding cleanup changes to the script.

I'm fine with a squash merge. That's what I usually do in this repo. Maybe that's a mistake?

Looks like my comment was not clear at all. Sorry about that. What I meant to say was:
"Please (as in optional, if you have the time and motivation for it) cleanup the git-history of this PR".

• Squash fixups, squash related commits
• Rework the commit messages
• Then force-push the results to this PRs branch
• That might be an insane amount of work to do properly, so one option would be to squash it into one commit
  • That would still require to write one nice commit message

Reasons:

• There are currently 35 commits in this branch
• Most of them overlap each other or undo each other
• The scope of this PR changed as we were working on it
• I feel like I lost track of individual changes

Oh. I see. I did misunderstand.

I usually squash merge in the web interface and can change the commit message then if I want. As a reviewer and merger (is that a word?), desktop/desktop#3580 captures what I usually do.

I've not actually done all the stuff above. Doesn't look too tough, just new to me. I can figure it out. But let me work on the gullwing footprint name format thing first. Hang on...

The difference is squashing everything into one commit vs. explicitly choosing which commits to squash.

I have no clue how to clean up the history in a gui. In this case, something like git rebase -i HEAD~36 is how I would to that on the commandline.

A quick google-search did return this video: https://mattstauffer.com/blog/squashing-git-commits-with-interactive-rebase/ that explains this workflow.