Skip to content
This repository has been archived by the owner on Oct 2, 2020. It is now read-only.

Add D2PAK diodes with NC pin 1 #2000

Merged
merged 4 commits into from
Oct 5, 2019
Merged

Conversation

evanshultz
Copy link
Collaborator

The existing D2PAK diode symbol we have in this lib has pin 1 connected to the cathode. Some diodes in this package have pin 1 set as NC, and in this case connecting them to the anode is usually beneficial for layout since the copper can pour all across both leadformed pins.

So I created silicon and Schottky symbol to support this pinout. Pin 1 is floating above the symbol so it's easily connectable to the anode net. If you have other suggestions please share.

https://www.onsemi.com/pub/Collateral/ISL9R1560S3S-D.PDF
image

https://www.vishay.com/docs/89060/v20150sg.pdf
image

For convenience, here is the footprint:
image


All contributions to the kicad library must follow the KiCad library convention

Thanks for creating a pull request to contribute to the KiCad libraries! To speed up integration of your PR, please check the following items:

  • Provide a URL to a datasheet for the symbol(s) you are contributing
  • An example screenshot image is very helpful
  • Ensure that the associated footprints match the official footprint library
    • A new fitting footprint must be submitted if the library does not yet contain one.
  • If there are matching footprint PRs, provide link(s) as appropriate
  • Check the output of the Travis automated check scripts - fix any errors as required
  • Give a reason behind any intentional library convention rule violation.

@antoniovazquezblanco antoniovazquezblanco added Addition Adds new symbols to library Pending reviewer A pull request waiting for a reviewer labels Jul 23, 2019
@cpresser
Copy link
Contributor

tbh, this looks pretty weird. But I recognize that it might be useful.

  • If the pin is NC in the Datasheet, I assume its not connected to the die?
  • Why would I want to connect it to the anode, perhaps there is a use-case where cathode is better
  • I think the description/keywords should include some hint that this symbol allows to connect the 3rd, usually NC pin
  • Appending something to the symbol-name seems like a good option to me. "ISL9R1560S3S_3Pin". I have no good ideas that the suffix should be though :/
  • IMHO the symbol needs more visual hints what this 3rd pin is. How about showing the pin-numbers for all 3 pins?

@cpresser cpresser removed the Pending reviewer A pull request waiting for a reviewer label Aug 14, 2019
@cpresser cpresser self-assigned this Sep 12, 2019
@evanshultz
Copy link
Collaborator Author

evanshultz commented Sep 20, 2019

@cpresser
Thanks for looking and commenting!

  1. That is what I believe. I checked a few that I had on hand and pin 1 is not electrically connected to either of the other pins.
  2. That may be nice. It depends on the layout. In my layouts, the anode was better. Would you like me to submit other symbols with pin 1 near the cathode? Do you have a good symbol name?
  3. Fair enough. After hearing from you on the above item I'll do that so I have more direction.
  4. Would just generic symbols be better than specific ones? In that case something like D_NAK (for NC, anode, cathode) and then some suffix to indicate pin 1's position. Maybe _Pin1A and Pin1K? Have you thought of anything?
  5. I'm OK with that, but note that also requires the user to know the footprint's pad number and locations for it all to make sense.

@cpresser
Copy link
Contributor

  1. Generic symbol does sound good. How about D_AAKand D_KKA. Putting a visible N/C pin on a symbol does violate KLC. Also just the letter N might not clear to the average user.
    Besides, I do like your _Pin1A suggestion for a non-generic symbol.

  2. How about this?
    image

  3. Instead of solving this by a new symbol it could also be solved by footprint.

- Removed the specific symbols created earlier in favor of these generic ones
- On the existing Schottky AAK and KAK symbols changed the one Input pin type to Passive
@evanshultz
Copy link
Collaborator Author

I'm fine with the concept above and the symbol names, but how about stacking the pins which are duplicated? So there are only two actual pins, but one of them has a third pin stacked under it. I think that conforms to the implementation elsewhere in the library.

Regarding the footprint, if we do that we may have multiple footprints which are otherwise identical. Here, we have clearly-unique symbols which can be used with multiple footprints just as they are. So it seems to me this is a good way to go??

And then after I wrote the above, guess what I found in Device.lib:
image

I've removed the specific symbols and added generic silicon diodes symbols based on the above ones. Let me know if this is good.
image
image

Also, the upper pin on the Schottky symbols was Input type. That baffles me. I changed it to Passive like all the other pins on our diode symbols.

@evanshultz
Copy link
Collaborator Author

@cpresser
The symbols above complement the Schottky ones already in the library. So I'm OK with that.

But it doesn't necessarily capture a diode with pin 1 as NC since it appears that pin 1 is tied to one of the other pins. Do you have a suggestion how to differentiate symbols that would schematically show that pin 1 is NC? Or do we just go with this and accept it's a slight abuse of convention?

By the way, we do have visible NC pins in some symbols. If the symbol has NC pins but they're connected to the lead frame and thus useful for heatsink, for some example. Or if the datasheet calls pins 'NC' but instructs them to be grounded. Just a couple examples I can recall.

@evanshultz
Copy link
Collaborator Author

@cpresser
I really don't have a good solution at this time, but the current branch includes symbols that are otherwise missing and can be abused for the use-case I'm interested in. Are you willing to merge this?

@cpresser
Copy link
Contributor

cpresser commented Oct 1, 2019

Sorry, I was quite busy with other stuff the last few days. I agree that the symbols are useful and should be merged. I will review them asap.

@cpresser
Copy link
Contributor

cpresser commented Oct 1, 2019

  • *_Diode_*and *SingleDiode* don't match anything in the current library. I assume you did copy them from the D symbol which has exactly the same filters. Is there any reason to keep them?
  • D_KAK does not have any FP filters
  • The polyline could be simpler:
    image
  • The two symbols differ in the overall length. Here they are next to each other:
    image
  • All the D_??? and D_Schottky_??? parts do violate KLC S4.3. Not sure what do do here. My feeling is to keep the current graphic style to keep it consistent. Once we can do breaking changes we can rework the library (which is already on your agenda?).

@evanshultz
Copy link
Collaborator Author

@cpresser
Thanks for looking!

  1. Just in the case that some people are using our symbols with their own libraries. If this flexibility is helpful to them, and used elsewhere, it should work for these diodes. That was my reason to keep FP filters that have to value with the official footprint library (for now).
  2. Oops. Added to this one and the Schottky version that was also missing them.
  3. Simplified.
  4. My feeling is to leave this silicon diode as it is, because the Schottky version is already in the lib and making a substitution between these two is definitely possible. For v6, this would be something nice to align.
  5. I see what you mean. This is probably the case where KLC was written with good intentions but there were disobedient symbols lurking in the library that were unknown. I reckon this doesn't hurt things to merge it, and for v6 these symbols could be changed to stack the pins.

@cpresser
Copy link
Contributor

cpresser commented Oct 2, 2019

2. Oops. Added to this one and the Schottky version that was also missing them.

While you are at it, can you also fix D_Schottky_KKA and D_Schottky_KAA. They also don't have any filters.

All my points above are addressed, so I am ready to merge this PR.

- Copies of existing Schottky symbols
- Also added footprint filters to those existing symbols
@evanshultz
Copy link
Collaborator Author

Alright. Done. I also added silicon diode symbols for those Schottky variants.

@evanshultz
Copy link
Collaborator Author

@cpresser
Is there anything to update to this PR?

@cpresser cpresser merged commit cc3cb47 into KiCad:master Oct 5, 2019
@cpresser
Copy link
Contributor

cpresser commented Oct 5, 2019

@cpresser
Is there anything to update to this PR?

No. I am just slow the last few days. Lots of non interwebz stuff to take care of.

@antoniovazquezblanco antoniovazquezblanco added this to the 5.1.5 milestone Oct 7, 2019
@evanshultz evanshultz deleted the d2pak-nc-pin1 branch October 7, 2019 14:59
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Addition Adds new symbols to library
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants