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

Add sym for two optical rotary encoders #67

Merged
merged 2 commits into from
Sep 19, 2024

Conversation

ouabache
Copy link
Contributor

SUMMARY

added two optical encoders

OPEN QUESTIONS / UNRESOLVED ISSUES
CHECKLIST
  • [x ] I have read and followed the library conventions.
  • For packages, I followed IPC7351C (see details in library conventions).
  • [ x] I'm the copyright owner of the added content (i.e. the changes are made by myself, not copied/imported from somewhere else).
  • [ x] I agree to publish all my changes under the CC0 Public Domain License, allowing everyone to use and modify the content without any restrictions.

@ubruhin ubruhin added the addition New library element. label May 1, 2020
@ubruhin
Copy link
Contributor

ubruhin commented May 1, 2020

Generally, looks not bad to me 👍 But I still have some thoughts:

  • Maybe remove the "CH_" prefix from pin names ("A", "B", "I" should be clear enough IMHO)?
  • The symbol should represent an optical rotary encoder, right? Then I would say the term "Rotary" should also be contained in the name, e.g. "Encoder Rotary Optical".
  • The symbol is unnecessary large and irregular. See the "Encoder Rotary With Switch" symbol how it should look like to be compact. Of course you can make it slightly wider (but not higher) because of the longer pin names.

@ubruhin ubruhin added the needs corrections Pull request needs corrections before next review. label May 1, 2020
@ouabache
Copy link
Contributor Author

ouabache commented May 1, 2020 via email

@ubruhin ubruhin changed the title added two optical encoders Add sym for two optical rotary encoders Sep 19, 2024
@ubruhin ubruhin self-assigned this Sep 19, 2024
@ubruhin ubruhin added ready for review Waiting for review by maintainers. and removed needs corrections Pull request needs corrections before next review. labels Sep 19, 2024
@ubruhin
Copy link
Contributor

ubruhin commented Sep 19, 2024

I applied the changes by myself now. Thanks for the contribution 🎉

@ubruhin ubruhin merged commit dd9c1bc into LibrePCB-Libraries:master Sep 19, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
addition New library element. ready for review Waiting for review by maintainers.
Development

Successfully merging this pull request may close these issues.

2 participants