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

Add XP_POWER ISU02 series DC to DC converter symbol. #1116

Merged
merged 5 commits into from
Nov 14, 2018

Conversation

stambaughw
Copy link
Contributor

@stambaughw stambaughw commented Nov 2, 2018

I've added the XP Power ISU02 series DC to DC converter symbol to the DC to DC convert library. The footprint pull request is coming shortly. Unfortunately, the 3D model code for DC to DC converters doesn't really have nice gull-wing leads (at least that I was willing to spend the time on) so I didn't make a 3D model.

Here is the footprint pull request KiCad/kicad-footprints#1065.


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
  • 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

xp_power-isu0205d12

@antoniovazquezblanco antoniovazquezblanco added Pending reviewer A pull request waiting for a reviewer Addition Adds new symbols to library labels Nov 2, 2018
@stambaughw
Copy link
Contributor Author

What do I need to do to get this pull request to pass the checks? Does the footprint library change need to be merged first? The reason I ask is that I have a few more changes pending and I would like to keep my repos tidy.

@poeschlr
Copy link
Collaborator

poeschlr commented Nov 3, 2018

The error about missing footprints will only go away if the footprint is merged. However we still prefer the symbol and footprint pull requests to be opened at the same time. (This allows us to check them in one go) We then merge the footprint first and restart the testscript before finally merging the symbol.


There is however a second error reported. You need to fill out the footprint filter in addition to the footprint field. This is for both future proofing the symbol and also to allow users to add alternative footprints (example a handsolder version) and find it easily in cvpcb.

@stambaughw
Copy link
Contributor Author

Thanks Rene. I missed the footprint filter issue. I just pushed the fix for this. Thanks!

@stambaughw
Copy link
Contributor Author

Here is the datasheet link that I forgot to attach.

@stambaughw
Copy link
Contributor Author

What's left to get this merged? It looks like everything is good to go or am I missing something.

@antoniovazquezblanco
Copy link
Collaborator

Someone should manually go over it and perform a quality review in depth. No one is assigned yet to your PR.

@evanshultz evanshultz self-assigned this Nov 13, 2018
@evanshultz
Copy link
Collaborator

  • The pin names for pins 1, 6, 7, and 14 should have the "+" or "-" at the front of the pin name.
  • From KLC: Document how galvanic isolation should be indicated kicad-website#284, can you change the double dashed line to a single one?
  • Please remove "regulated" from the description. This is implied and I don't see it used elsewhere (I so see "fixed" but it could be removed too).
  • Lastly, note for the future that this symbol is only for the Dual output version. I'd obviously rather have you writing or reviewing code for KiCad, so I won't ask you to add the Single output symbol. But if you already have it that would flesh out this series.

Remove one of the dashed isolation lines per request.

Remove "regulated" from description field.

Change pin names with +/- as a suffix to prefix.

Add single channel symbol and aliases.
@stambaughw
Copy link
Contributor Author

stambaughw commented Nov 13, 2018 via email

@evanshultz
Copy link
Collaborator

It's easier on the librarian team if any new or notably changed symbols are added as screenshots.
image

  • ISU0205S05, ISU0224S15, ISU0224S24, and all ISU0248Sxx are missing the word "Module" in the description.
  • ISU0205S15 says 12V in the description but it should be 15V.
  • I'd like to see the single-channel put -VOUT where COM is on the dual-channel since that would make changing from single to dual easier. I'm not sure if that's common, but it would require the least amount of changes if going between them.
  • The symbol's datasheet field (F3) should be empty.
  • Please push the end of the NC pins to the edge of the body for the single output version so the NC pins are unlikely to connect to anything.

Change negative output pin to be in same location as common pin for dual
converters.

Make no connection pins zero length and move them to the body edge to
prevent inadvertent connections.

Minor description error fixes.
@stambaughw
Copy link
Contributor Author

stambaughw commented Nov 13, 2018 via email

@evanshultz
Copy link
Collaborator

Thanks!

I just have one more comment: http://kicad-pcb.org/libraries/klc/S4.1/ states pins should be 100mil for this symbol, and there's no clause about NC pin length. Their length is 0mil here and we've always used 100mil even for NC pins. For consistency, at least until we can figure out if NC pins can be handled differently, can you length them? Keep their end where it is but lengthen the pin inside the symbol body.

@stambaughw
Copy link
Contributor Author

stambaughw commented Nov 14, 2018 via email

@evanshultz
Copy link
Collaborator

Don't capitulate yet! Thank you. This looks good and I hope you're willing to submit more PRs. :)

@evanshultz evanshultz merged commit 91c1bed into KiCad:master Nov 14, 2018
@DanSGiesbrecht DanSGiesbrecht removed the Pending reviewer A pull request waiting for a reviewer label Dec 2, 2018
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.

5 participants