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 Espressif ESP32-S2-DevKitC-1-N8R2 variant #6989

Merged
merged 11 commits into from Oct 6, 2022
Merged

Conversation

kylefmohr
Copy link

@kylefmohr kylefmohr commented Oct 4, 2022

This is a relatively new board, purchased from here, apparently first released August 10th 2022. The official pinout is available here. It is very similar to the N4R2 version that is available for CircuitPython.

I got the majority of my code from #5998, so thanks to anecdata for that!

I would appreciate if somebody could review my changes to ci_check_duplicate_usb_vid_pid.py, as this is my first PR for this project, and I'm a little unclear on how that file works. With a micro USB cable plugged into the "UART" port of the board, this is what shows up in Device Manager in Windows
image

I think I added the PID/VID correctly, just want another set of eyes on them. Thanks!

@kylefmohr kylefmohr marked this pull request as draft October 4, 2022 04:00
@kylefmohr kylefmohr marked this pull request as ready for review October 4, 2022 04:04
Copy link

@Neradoc Neradoc left a comment

Choose a reason for hiding this comment

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

What you are showing is the VID/PID of the CP2104 chip on the UART port. The VID and PID set in Circuitpython is for the native USB port of the board, and must be unique (mostly...) and properly allocated by a VID owner.
In this case, it seems that you should use the common espressif_esp32s2_devkitc_1 PID allocated by Espressif, and therefore add it to the existing list for that VID/PID in the duplicate check.
I made the changes in the suggestions.
I didn't look at the other files.

tools/ci_check_duplicate_usb_vid_pid.py Outdated Show resolved Hide resolved
@kylefmohr kylefmohr marked this pull request as draft October 4, 2022 04:44
@kylefmohr kylefmohr requested a review from Neradoc October 4, 2022 04:47
@kylefmohr kylefmohr marked this pull request as ready for review October 4, 2022 05:04
@dhalbert
Copy link
Collaborator

dhalbert commented Oct 5, 2022

@kylefmohr Thanks for this!

I pushed a minor change to fix the formatting of tools/ci_check_duplicate_usb_vid_pid.py.

It's a good idea to create a separate branch for a PR instead of using your main, because now your main has diverged from upstream. You may want to force it back to adafruit/main later.

@kylefmohr
Copy link
Author

@kylefmohr Thanks for this!

I pushed a minor change to fix the formatting of tools/ci_check_duplicate_usb_vid_pid.py.

It's a good idea to create a separate branch for a PR instead of using your main, because now your main has diverged from upstream. You may want to force it back to adafruit/main later.

Thank you! That makes sense. In the past I've deleted and re-forked the repo, but creating a new branch for the PR seems easier. I appreciate the assistance.

@dhalbert
Copy link
Collaborator

dhalbert commented Oct 5, 2022

We have a Learn Guide explaining about branches: https://learn.adafruit.com/contribute-to-circuitpython-with-git-and-github/always-work-on-a-branch (the whole guide is worth reading if you have not seen it yet).

@kylefmohr
Copy link
Author

We have a Learn Guide explaining about branches: https://learn.adafruit.com/contribute-to-circuitpython-with-git-and-github/always-work-on-a-branch (the whole guide is worth reading if you have not seen it yet).

I have not, I'll definitely give that a read! Another newbie question that I don't see covered in the guide: How did you manage to only build the firmware for the new board, and skip all other boards/ports? That seems very useful, in the past I've just waited for the build to complete entirely.

@dhalbert
Copy link
Collaborator

dhalbert commented Oct 5, 2022

Our scripting for the automated builds checks to see where the changed files are, and if they are only in a particular port or board, it will rebuild only those ports or boards. This was done in PR #5312.

@dhalbert
Copy link
Collaborator

dhalbert commented Oct 5, 2022

@Neradoc Does this look good to you?

@Neradoc
Copy link

Neradoc commented Oct 6, 2022

There's a define for AUTORESET_DELAY_MS in mpconfigboard.h that is unnecessary. The rest looks good to go.
(To quote #6182: it never did anything but was introduced somehow)

Neradoc
Neradoc previously approved these changes Oct 6, 2022
@dhalbert
Copy link
Collaborator

dhalbert commented Oct 6, 2022

I will fix that, and will also shorten board.c, due to the new MP_WEAK default versions of those routines.

Neradoc
Neradoc previously approved these changes Oct 6, 2022
@Neradoc
Copy link

Neradoc commented Oct 6, 2022

Ah we are crossing streams !

@dhalbert dhalbert merged commit 8b6fff2 into adafruit:main Oct 6, 2022
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