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 esp32-c6 symbol #72

Merged
merged 5 commits into from Jan 24, 2023
Merged

Conversation

dreamcmi
Copy link
Contributor

  1. I created the symbols of QFN40 and QFN32 through the esp32-c6 datasheet on the espressif official website, and created the current pull request. Maybe the current symbol naming is not very reasonable., if you have better suggestions, please reply to me, I will fix it immediately.
  2. The relevant design information of ESP32-C6-MINI-1 and ESP32-C6-WROOM-1 has not been published on the official website, so I did not submit this part of the content.
  3. Added the description of ESP8684 symbol, because I forgot to add it in the last pull request.

@dreamcmi
Copy link
Contributor Author

#69

@pedrominatel pedrominatel added the new symbol New symbol request label Jan 23, 2023
@pedrominatel pedrominatel self-requested a review January 23, 2023 10:12
@pedrominatel
Copy link
Member

Hi, @dreamcmi

We are working on making all symbols and footprints compliant with the KLC.
Have you tried to run the symbols script checker?

https://klc.kicad.org/
https://gitlab.com/kicad/libraries/kicad-library-utils

Thank you so much for your contribution!

@dreamcmi
Copy link
Contributor Author

Hi, @dreamcmi

We are working on making all symbols and footprints compliant with the KLC. Have you tried to run the symbols script checker?

https://klc.kicad.org/ https://gitlab.com/kicad/libraries/kicad-library-utils

Thank you so much for your contribution!

I am correcting the symbol according to klc, but when running klc_check, the version of the symbol seems to be wrong. The version of this repo is 20211014, but the check script requires a version of 20220914. I temporarily modified this version number manually (it will not be pushed to the repo ), other work is in progress.

Traceback (most recent call last):
  File "./check_symbol.py", line 173, in check_library
    library = self._load_library(filename)
  File "./check_symbol.py", line 156, in _load_library
    return KicadLibrary.from_file(filename)
  File "/home/darren/kicad-check/kicad-library-utils/common/kicad_sym.py", line 1093, in from_file
    raise KicadFileFormatError(f'Version of symbol file is "{version}", not "20220914"')
kicad_sym.KicadFileFormatError: Version of symbol file is "20211014", not "20220914"
Could not parse library: ../../espressif-kicad-libraries/libraries/Espressif.kicad_sym. (Version of symbol file is "20211014", not "20220914")
Error: Version of symbol file is "20211014", not "20220914"

@dreamcmi
Copy link
Contributor Author

dreamcmi commented Jan 23, 2023

I have corrected the symbols according to KLC, please review.
Although klc-check still says Violating S5.1, I don't know how to solve it now, and most of the existing symbols also have this problem.

Copy link
Member

@pedrominatel pedrominatel left a comment

Choose a reason for hiding this comment

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

Nitpicking on the symbol pin grouping.
I think we should try to meet the same pin grouping strategy as the other symbols.

My grouping suggestion:
image

libraries/Espressif.kicad_sym Show resolved Hide resolved
@dreamcmi
Copy link
Contributor Author

regrouped, please review

Copy link
Member

@pedrominatel pedrominatel left a comment

Choose a reason for hiding this comment

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

LGTM

@pedrominatel pedrominatel merged commit a8a536c into espressif:main Jan 24, 2023
@pedrominatel pedrominatel mentioned this pull request Jan 24, 2023
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
new symbol New symbol request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants