Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adding second footprint for 3V3 voltage regulator #98

Merged
merged 5 commits into from
Oct 11, 2022

Conversation

jamesSaporito
Copy link
Contributor

@jamesSaporito jamesSaporito commented Oct 10, 2022

The voltage regulator that was originally used isn't available at the moment. To provide more options, a second footprint was added to the PCB for an alternative regulator that has a different pinout. Important to note that only one regulator should be used.

Also, adjusted .gitignore for Mac files and KiCad auto-generated backups.

@CrosseyeJack
Copy link
Contributor

Fairly neat addition, Only thing that jumps out to me is the 5v trace width on U9 isn't the same width as it is for the rest of the 5V rail.
image

And MAYBE move the label for U9 to above the tab just so the labels visability is consistant with the rest of the labels. (So its not hidden if the part is soldered down) But thats just a visibity issue not really a game breaker.

No complaints on the schematic, was the same method I did last night in discord. I don't know what your personal reasoning for using a global label for GND in the power section but my reasoning when I did the same last night was because I feel it was more "readable" signalling ath either of the two regs could be used then simply tying the GND.

The .gitignore changes might be best in a sep pull request - don't see an issue adding ignoring .DS_Store but you are also adding an ignore for *backups

@jamesSaporito
Copy link
Contributor Author

Fairly neat addition, Only thing that jumps out to me is the 5v trace width on U9 isn't the same width as it is for the rest of the 5V rail. image

And MAYBE move the label for U9 to above the tab just so the labels visability is consistant with the rest of the labels. (So its not hidden if the part is soldered down) But thats just a visibity issue not really a game breaker.

No complaints on the schematic, was the same method I did last night in discord. I don't know what your personal reasoning for using a global label for GND in the power section but my reasoning when I did the same last night was because I feel it was more "readable" signalling ath either of the two regs could be used then simply tying the GND.

The .gitignore changes might be best in a sep pull request - don't see an issue adding ignoring .DS_Store but you are also adding an ignore for *backups

I can adjust the trace to be the same width as the others. If the U9 label is bothering you then I can adjust the location for that as well. Probably best to match all the other labels visibility. The addition of *backups was to get rid of all the auto-generated backup files/directories that KiCad creates. Perhaps it is too vague. I'll split out the .gitignore changes as well if you think it's best in a separate PR.

@CrosseyeJack
Copy link
Contributor

CrosseyeJack commented Oct 10, 2022

If the U9 label is bothering you then I can adjust the location for that as well. Probably best to match all the other labels visibility.

Its no big deal to me, was something I first noticed when looking at the board in the 3D Viewer, Its not a game breaker, but imo having the label visable might help with helping others debug things later down the road (However it also won't be hard to ask "is it the left or right footprint you haave populated?"). Just thinking forward thats all.

The addition of *backups was to get rid of all the auto-generated backup files/directories that KiCad creates. Perhaps it is too vague. I'll split out the .gitignore changes as well if you think it's best in a separate PR.

Doh, ofcause it is, Brain was obv not firing on all cylinders.

EDIT: on further thinking, might be better to change it to something like electronics\*\*backups (not actually checked this example) so its more targeted to just the kicad backup files.

Copy link
Owner

@scottbez1 scottbez1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool, thanks for the PR! A couple thoughts:

  • What's the reason for using global labels for power nets rather than the standard symbols (which are already global)?
  • I'm wondering about the net renumbering (see inline comment) - maybe related to the added global labels?
  • trace width - would be good to keep this consistent as mentioned

electronics/view_base/view_base.kicad_pcb Outdated Show resolved Hide resolved
@jamesSaporito
Copy link
Contributor Author

Cool, thanks for the PR! A couple thoughts:

  • What's the reason for using global labels for power nets rather than the standard symbols (which are already global)?
  • I'm wondering about the net renumbering (see inline comment) - maybe related to the added global labels?
  • trace width - would be good to keep this consistent as mentioned

Thanks for reviewing!

No good reason for using global labels, I was attempting to make it easier to read in my mind, but I think it just made it messier. The global labels do seem to be a direct cause of the net re-numbering as well.

Went ahead and fixed the trace widths and adjusted the schematic to just use the standard power symbols instead. Let me know if you'd like me to redo this to preserve the original net numberings.

@scottbez1 scottbez1 merged commit 935a2d2 into scottbez1:master Oct 11, 2022
@scottbez1
Copy link
Owner

Thanks again for the PR! FYI I did a little cleanup in 04475a8 - I moved the LDOs so they mirror each other on the board, and updated the footprints to label the IN/GND/OUT pins on each to hopefully make the pinout distinction more obvious. Also added part numbers for the new LDO U9 and notes on both mentioning to only populate one or the other. Also a little bit of tidying the schematic.
Screenshot from 2022-10-10 21-15-37

@jamesSaporito
Copy link
Contributor Author

@scottbez1 Thanks for approving! Love the cleanup and clarifications you made.

lgc00 pushed a commit to lgc00/smartknob that referenced this pull request Jul 15, 2023
The voltage regulator that was originally used isn't available at the moment. To provide more options, a second footprint was added to the PCB for an alternative regulator that has a different pinout. Important to note that only one regulator should be used.

Also, adjusted .gitignore for Mac files and KiCad auto-generated backups.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants