-
Notifications
You must be signed in to change notification settings - Fork 2k
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 support for Seeed Studio ESP32C3 Xiao #21267
base: master
Are you sure you want to change the base?
Add support for Seeed Studio ESP32C3 Xiao #21267
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for your contribution and welcome to the community! Looks good at a first glance, some nits below. I'm not familiar with the ESP32 integration in RIOT though.
Following RDM0003, please adapt the board name to be seeedstudio-xiao-esp32c3
.
And according to #21257, please change your board documentation from doc.txt
to doc.md
(getting rid of the surrounding C-comments).
Just yesterday I thought about buying a Xiao-ESP32C3 on Berrybase, but they didn't have the Xiao-ESP32S3 in stock (for which another PR is open) but then decided against it because there is no PR open for it. What a coincidence :D I'll order one and give this a test soon. But so far this looks good, I'll have to take a deeper look when the hardware is here. You have some files where the copyright notice is still saying "Gunar Schorcht", but the author in the doxygen block is already updated. Also please update the commit message with the typical format: https://github.com/RIOT-OS/RIOT/blob/master/CONTRIBUTING.md#commit-conventions |
8b4e7b8
to
7ebb6e9
Compare
The problem is that a bookmark named "doc" at the 1st level of the documentation tree (RIOT/doc/doxygen/html/md_boards_2seeedstudio-xiao-esp32c3_2doc.html). You can see that for other boards as well in the online documentation, e.g. https://doc.riot-os.org/md_boards_2nucleo-f413zh_2doc.html How do I avoid this ? |
7ebb6e9
to
2fc34da
Compare
This is a doxygen bug: #21220 (comment) I'm glad that you noticed it though, that means you test the documentation :D 👍 |
2fc34da
to
419aa55
Compare
Port the Seeed Studio Xiao ESP32C3 to RIOT.
419aa55
to
043a861
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My Xiao ESP32C3 should arrive this week, then I can get this tested with hardware.
For the time being I have some small remarks about the documentation, but nothing major.
Most (all?) of that comes from the original definitions anyway I think.
BOARD=seeedstudio-xiao-esp32c3 make ... | ||
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ | ||
|
||
Almost all GPIOs are broken out and can be used for different peripherals: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think this is true for the Xiao, is it?
* Since the GPIO of the button is pulled up with an external resistor, the | ||
* mode for the GPIO pin has to be GPIO_IN. | ||
*/ | ||
#define BTN0_MODE GPIO_IN_PU |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The documentation contradicts the actual definition.
* For generic boards, two PWM devices are configured. These devices | ||
* contain all GPIOs that are not defined as I2C, SPI or UART for this board. | ||
* Generally, all outputs pins could be used as PWM channels. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should probably be adapted for the Xiao specifically as well.
/** | ||
* @name I2C configuration | ||
* | ||
* For generic boards, only one I2C interface I2C_DEV(0) is defined. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should probably be adapted for the Xiao specifically as well.
* For generic boards, all ADC pins that have broken out are declared as ADC | ||
* channels. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should probably be adapted for the Xiao specifically or removed. For the Xiao, only three pins can be used as ADC channels.
#endif | ||
|
||
/** | ||
* @name ADC and DAC channel configuration |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is no channel definition for the DAC. I'm not familiar with the ESP32s, is a dedicated DAC configuration required or is that done implicitly through the ADC configuration?
Contribution description
Add support for Seeed Studio ESP32C3 Xiao. See issue #21266.
Testing procedure
Tested with the following examples:
Issues/PRs references