-
Notifications
You must be signed in to change notification settings - Fork 1.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 Ai Thinker ESP32-CAM boards #9231
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.
It probably varies from board to board but you might consider adding aliases for the pins that are silk screened.i.e.:
{ MP_ROM_QSTR(MP_QSTR_SDIO_CLK), MP_ROM_PTR(&pin_GPIO14)},
{ MP_ROM_QSTR(MP_QSTR_IO14), MP_ROM_PTR(&pin_GPIO14)},
This may already be clear but since you said this was your first board, I just wanted to be sure... The silkscreen does show the actual GPIO names IO14, etc so a programmer can access them by those names using microcontroller.pin, although most CircuitPython users will be looking for the silk screen name in board.xxxx (board.IO14). In order to support that you would need to add a second line for each of the silk screened pins with both the logical name and the silk screened name. { MP_ROM_QSTR(MP_QSTR_SDIO_CLK), MP_ROM_PTR(&pin_GPIO14)},
{ MP_ROM_QSTR(MP_QSTR_IO14), MP_ROM_PTR(&pin_GPIO14)}, I'm pretty sure not all boards add this type of alias though. |
I'm reluctant to introduce a whole pile of non-standard non-portable aliases simply to drop the leading "G" off everything (and waste memory and add confusion)... while I can see where circuitpython is going with that, I would very much prefer NOT to encourage that kind of thing. I'm working currently on the ESP32-WROVER-DEV board which (minus the SD card) is almost identical. Code written for either will run fine, unchanged, on the other... so long as people don't use silly pin names. The latter drops the "IO" prefix on its silkscreen, which will add a whole extra level of extreme confusion if I go down this route ...
will resolve to an int instead of a string in those cases... That said - I'm happy to keep you happy. This is my first time doing this stuff. Update: You can't use numeric silkscreens:-
|
Turns out a very large number of other ESP32 ports introduce IOnn aliases... so in the interests of letting those folks run their code unchanged on this port, I added the aliases. |
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 loaded up the artifact from this PR and tested WiFi, SD Card, LED and Camera operations. I didn't find any issues other than the annoyingly bright "flashlight" led defaulting to on.
FYI... More handy info about these boards is here: https://randomnerdtutorials.com/esp32-cam-ai-thinker-pinout/ |
I loaded up the artifact with the flashlight fix and everything seems good, no more bright light 😁 |
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 looks good to me, I've done some basic testing and everything seems to work fine. I'm actually using the latest artifact in a project that allows me to snap a photo of the charge state of my solar batteries using adafruit.io. That allows me to turn on/off the inverter when I'm away from home depending on the charge state. I first tried using the Memento but ran into an issue so it was fortunate that this PR was in place 😁
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 the testing @RetiredWizard and the PR @gitcnd
I think the first step is adding a test that will fail if the board name isn't an id. Otherwise we'll continue to have this problem. After we have that, we can fix all the cases that need to be fixed. |
Others:- raytac_mdbt50q-db-40 |
Why would it need to be a valid python identifier? If someone wants this to be a rule and actually has a sensible reason why - all the board porting instructions need to be changed so that everyone knows this... it's actually quite confusing already - there's at least 3 names involved (a human name with spaces, some kind of identifier constructed from CIRCUITPY_CREATOR_ID and CIRCUITPY_CREATION_ID, and the folder name where the files live) - with none of this stuff being explained in the porting instructions... Oh yeah - and there appear to be aliases for variants as well, also not explained I think. |
Some folks use the board id in a module name.
Yes, agreed.
|
LOL - I was all fired up to argue against a "no hyphen" requirement because I couldn't think of any valid reason why... then you give a valid reason :-) I'd estimate it's probably a days work to add the rule, fix the doc, and update all the boards currently using hyphens. |
The board_id's are also used for https://circuitpython.org, so if you rename a board, you need to make that still work with both the 9.0.x and the 9.1.0 versions. There is a |
This adds support for the cheap-and-popular ESP32-CAM boards.
Here is my working sample script which tests the SD Card:
Here is my working sample script which takes a photo (and saves it to "out.jpg" in flash)
This is my first PR - if I've done anything wrong, please let me know, along with more-than-average details about what I'm supposed to do to fix it, thanks!