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

add HCPL-0201 opto-coupler (and variants) #2087

Merged
merged 5 commits into from Apr 8, 2020

Conversation

hvraven
Copy link
Contributor

@hvraven hvraven commented Aug 17, 2019

This adds the HCPL-0201 and some variants as aliases for different packages.

Screenshot_20190817_212240


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.

@myfreescalewebpage myfreescalewebpage added Addition Adds new symbols to library Pending reviewer A pull request waiting for a reviewer labels Aug 24, 2019
@matthijskooijman
Copy link
Contributor

IIUC, when multiple footprint options are available for a part, each footprint option should get its own part (rather than aliases), even if the pin numbering is the same for all footprints. This makes the parts fully specified, so you can just put them in your schematic and then the default footprint will be correct (with your current PR, you still need to pick the correct footprint even when you have already chosen it through the schematic symbol).

See also KiCad/kicad-website#407 and KiCad/kicad-website#386

@matthijskooijman
Copy link
Contributor

Looking more closely at the datasheet, I also see you:

  • Also included the HCPL2202, which has a different pinout (output on pin 6 instead of 7), so that should definitely be a separate symbol.
  • Missed a number of aliases from the datasheet. It specifies HCPL-2201, HCPL-2202, HCPL-2211, HCPL-2212, HCPL-2231, HCPL-2232, HCPL-0201, HCPL-0211, HCNW2201, HCNW2211, but you only included a number. Since these are mostly just non-functional variations, it would be easy to include them as aliases (except for the dual versions, which would need another symbol).

@hvraven
Copy link
Contributor Author

hvraven commented Sep 9, 2019

I expanded it into different parts for each possible footprint. My original plan was just to add the ones with identical symbol to the HCPL-0201, but I missed the pin different between the HCPL-xxx1 and HCPL-xxx2 versions.

Also rebased against master to fix merge conflict

@matthijskooijman
Copy link
Contributor

Thanks for updating, I have had a closer look at your symbols.

AFAICS, you now included all single parts from the datasheet. There are two variants of the pinout, one of which is available in three packages and the other only in one package, for a total of four symbols (and a bunch more aliases for differnt isolation ratings).

I had a look at the descriptions, but there seems to be an inconsistency.

In the datasheet, some parts are listed as 5kV/300V, with a footnote "c" stating they can be 10kV/1000V under some conditions:

image

However, in your symbol descriptions, it seems you have used 10kV/1000V for the HCNW2211 and 5kV/300V for the others. I'm not sure which of these would be best (perhaps to lower ones to be on the safe side?), but they should at least be consistent.

Furthermore:

  • The datasheet and footprint fields are on top of the symbol now. They are invisible, but for readability inside the symbol editor I would suggest moving them outside of the symbol.
  • The output pin is a bit funny. It seems you used a 150mil pin rather than 100mil, extending the pin inside of the body rectangle, so the inverting circle on the pin is actually inside the body. It also means that the pin number for the output pin is closer to the body than the other pins. Did you copy this from another symbol? I'm note sure what the conventions are exactly, but I think it would better to just use a normal 100-mil pin, with either a manually drawn circle inside the body (e.g. like the HCPL-063A), or with the inverting circle outside of the body.
  • I doublechecked the footprint associations, those look correct to me.
  • Electrical types look good to me.

@hvraven
Copy link
Contributor Author

hvraven commented Sep 24, 2019

I used the values from the second table on page 13 "Switching Specifications". There they use the higher current of 5mA. I am not quite sure how to understand the comment in the first table.

The symbol drawing is copied from the H11L. I will change it to a hand-drawn circle instead. I can also move the invisible fields to the outside. So far I never bothered about those.

@matthijskooijman
Copy link
Contributor

I used the values from the second table on page 13 "Switching Specifications". There they use the higher current of 5mA. I am not quite sure how to understand the comment in the first table.

Ah, I see. Maybe the extra isolation really only applies to the HCNW2211 and HCPL-2232 but the first table fails to reflect that? Since your current descriptions match the (more detailed) table on page 13, I think they are good as they are right now.

The symbol drawing is copied from the H11L. I will change it to a hand-drawn circle instead. I can also move the invisible fields to the outside. So far I never bothered about those.

Thanks. If the librarians agree that this is indeed better, maybe the H11L should also be fixed?

@hvraven
Copy link
Contributor Author

hvraven commented Sep 25, 2019

Checking the truth table again on page two the inversion itself is an error for these symbols. I fixed the pin length and added a line to fill the gap. Currently I am thinking it would be a good idea to draw the stylised Schmitt-Trigger as in the symbols on page 2 of the datasheet. Do you have any opinions on this?
Screenshot_20190925_145724

@matthijskooijman
Copy link
Contributor

Checking the truth table again on page two the inversion itself is an error for these symbols. I fixed the pin length and added a line to fill the gap.

Good call, should have seen that. Thanks for updating.

Currently I am thinking it would be a good idea to draw the stylised Schmitt-Trigger as in the symbols on page 2 of the datasheet. Do you have any opinions on this?

I think some optos have this, but not consistently. I'm not sure what the best style is here. Maybe following the lead of the datasheet? See also #2115 for a slightly related discussion.

@hvraven
Copy link
Contributor Author

hvraven commented Sep 27, 2019

I now added a drawing of the Schmitt-Trigger, similar to the one in the datasheet
Screenshot_20190927_181432

@evanshultz
Copy link
Collaborator

I recall some conversation discussing not showing the triangles but just the lines that make up the Schmitt trigger symbol.

Ah! #2115 This doesn't mean it's consensus, just that the people in that issue (and I) prefer no triangles. They're not always shown in part documentation and can result in clutter, while just the two 'L' lines are always shown and can convey the concept clearly.

@hvraven
Copy link
Contributor Author

hvraven commented Oct 2, 2019

I removed the arrows, now it looks like this:
Screenshot_20191002_134735

@myfreescalewebpage myfreescalewebpage self-assigned this Apr 8, 2020
@myfreescalewebpage myfreescalewebpage removed the Pending reviewer A pull request waiting for a reviewer label Apr 8, 2020
@myfreescalewebpage
Copy link
Collaborator

Hi @lorem-ipsum I have performed a new review here, no comment to add. Merging. Joel

@myfreescalewebpage myfreescalewebpage merged commit 7cf4c3d into KiCad:master Apr 8, 2020
@antoniovazquezblanco antoniovazquezblanco added this to the 5.1.6 milestone Apr 9, 2020
@hvraven hvraven deleted the hcpl0201 branch April 15, 2020 12:53
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.

None yet

5 participants