-
Notifications
You must be signed in to change notification settings - Fork 176
Fix decimal pincounts in custom names #649
base: master
Are you sure you want to change the base?
Fix decimal pincounts in custom names #649
Conversation
Code Climate has analyzed commit a49d89b and detected 0 issues on this pull request. View more on Code Climate. |
I gave it a quick look and tbh, I don't like it. It seems way to complicated. I would rather keep pincount a string and use some |
OK. I'm fine with another approach that works better for everyone. I can take a swing at that method in a bit. No problem. Is this holding anything up? I know we're almost out of time so the more discussion and time it takes to merge a fix, the less time we have to merge in the rest of the library assets that rely on this fix. Is this a problem or we're OK? |
One PR (#645) I know of is related on this. But I could merge the footprints without this fix - the author did change the format strings. Once this is sorted out, we can merge #645 One might argue it would be best to fix the problem ASAP. But I would rather discuss the |
@cpresser @evanshultz Another option, if it's preferred, is that I leave the name formats alone in my PR, since I don't actually use the custom name format in any of the three footprints I added. This would mean soic.yaml would still be broken, but I would still use the workaround (replace In terms of fixing the issue, I think it would make sense to just convert all the custom format strings to handle I suppose another alternative would be to make |
So #645 is the only issue we know of. @cpresser Your proposal to get the footprints made works for me. But I'll leave it up to you and @cpresser since you two are the ones commenting on that PR. That was what I attempted to do, using a string suffix after the digit number of pins. Or I misunderstood you. But either way, if what I wrote above is correct I'm more happy to go that way. |
As mentioned at #604 (comment), I broke custom names that used the pincount. This fixes that. At least, the
test_hidden_deleted_pins.yaml
footprints still generate with the correct names andsoic.yaml
runs cleanly and gives, I believe, correct outputs. The footprints all look fine to me.I check for deleted or hidden pins and, if so, use a custom name format which retains a digit for the pincount. A second pincount can be used, but is an empty string if not needed. And it goes nowhere for a custom name format that doesn't know about the second pincount. This way, I don't need any fixed format and the second pincount is more like a free-form text field I can populate if needed.
As I wrote the above, I realized I didn't check for all instances of
pincount
in the script so I pushed another couple of commits. The first corrects the EP name and also uses the number of pins present in the package in the description. The last corrects the name of footprints with hidden or deleted pins to match the IPC 7351 convention as shown on page 2 of http://ohm.bu.edu/~pbohn/__Engineering_Reference/pcb_layout/pcbmatrix/IPC-7x51%20&%20PCBM%20Land%20Pattern%20Naming%20Convention.pdf. I proposed adopting their terminology and then I didn't even get it right in the script! If you don't like it or don't want to change since only a few footprints have been merged I put it in a separate commit. But it seems right and we can now easily re-generate any footprints with hidden or deleted pins.