-
Notifications
You must be signed in to change notification settings - Fork 1.3k
ESP32-S2: Add Neopixel support #3232
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
Conversation
rad! |
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.
Code looks good but the CI isn't happy.
I tried to build with this PR but it fails ```-- Build files have been written to: /home/pi/projects/circuitpython/ports/esp32s2/build-espressif_saola_1_wrover/esp-idf
|
I wonder why this didn't come up on my local machine? Weird. |
@tannewt these CI failures are kind of all over the place - right now I see a DE translation failure on the Gemma M0 and Trinket due to flash constraints, and an IDF failure for the ESP32-S2 boards that seems to be related to the virtualenv. Before that it was a server issue with AWS? I don't think any of them are related to my code though. |
I tried building with the updated PR. The build succeeds, but when I put it on my board, it does not boot. Edited to add;
I built it both after syncing the submodes and not -- same results If I build the tip of main ,it works normally |
I was able to build, and no crashes yet, but in circuitpython I can't get the LED to initialize. On the REPL:
|
odd -- not at all clear why it won't even boot for me... |
Try merging from main; it should fix the breaking builds, due to build cache changes in #3230. |
@dhalbert merged. Would having just restarted the jobs had the same effect, since my code doesn't impact the changes on @fede2cr @jerryneedell Thanks for testing! I'm unable to replicate any issues on either the old unmerged version or the merged one - my builds connect the REPL and the status LED changes as appropriate. I'm also able to override it in code.py as |
I always use this bash alias to update submodules after changing branches as per @dhalbert's recommendation: |
@hierophect I am baffled. I do the same for syncing submodules but you PR still fails the same way for me every time -- tried 2 different boards. If i then checkout main and rebuild, it works normally. I am working from the tip of main.
|
on a failed boot, I see this in the debug USB port
|
@jerryneedell I see various reports of "RMT driver already installed for channel" - I didn't use the debug port when developing this so I didn't make any note of these, they don't seem problematic for me. What I'm not getting is your I will try to figure out why it is making these messages in the first place. |
Since mine builds for main, I assume that exonerates any issue with the esp-idf, correct? Your PR does not depend on any additions to it, does it? |
@hierophect and yes, the "RMT driver already installed for channel" errors one only reported with the PR, not with main. |
@jerryneedell I don't think it's something wrong with the IDF, but I'm thinking it might be an IDF setting. It seems suspicious to me that we're both getting the same error flags but it is only halting the CPU for you. I'm wondering if that's an internal flag within the IDF that changes behavior when dealing with exceptions. In any case it would be good if we don't get exceptions over the debug port at all, but I'd like to figure out why we are getting different results. |
@jerryneedell I have a script in |
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 doing this! It worked after I enabled pixelbuf. Please also add the neopixel board support for the wroom board too. Thanks!
ports/esp32s2/mpconfigport.mk
Outdated
@@ -12,6 +12,8 @@ USB_SERIAL_NUMBER_LENGTH = 12 | |||
# Longints can be implemented as mpz, as longlong, or not | |||
LONGINT_IMPL = MPZ | |||
|
|||
CIRCUITPY_NEOPIXEL_WRITE = 1 |
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.
Please add this below in alphabetical order. That way it'll be easier to find.
Please also turn on CIRCUITPY_PIXELBUF. (It worked for me in my testing.)
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.
Was this meant to be alphabetical?
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.
6D40 form>@tannewt It's actually not supposed to need to be there at all - CIRCUITPY_NEOPIXEL_WRITE is supposed to be on by default but it'd bug out if I didn't put it here. You've reminded me I need to look into why that's happening.
deleted previous example -- I realized the the backtrace may not have been from the same build the was used to run the tool. fresh build with newly installed esp-idf.... still crashes but the backtrace is a bit different
this may make a bit more sense |
FYI - From the espressif docs: https://docs.espressif.com/projects/esp-idf/en/latest/esp32/api-guides/fatal-errors.html#loadprohibited-storeprohibited For those who have this working, are you doing anything spec 9E88 ial in your forks so that when I pull the PR ,there is something incompatible? I am getting the PR by using
then build as normal
that all goes as expected as noted, by build of "main" works fine. It is not at all clear what is wrong when I build the pr_3232 branch. |
@jerryneedell @tannewt so if I'm understanding this error correctly, there's an attempt to dereference a null pointer somewhere in the Status LED code? I'm going to dig for it so please correct me if you think I'm wrong. |
@hierophect That seems to be what it is suggesting, but why only when I build it? Are you enabling DEBUG when you build? |
No, I have no special settings. It's definitely unusual. Do you have a sketch in the filesystem that you can access, or does it never even get that far? Is it possible that it could be something in code.py? |
@jerryneedell What is the top commit in your Maybe code.py is throwing a weird exception? https://github.com/adafruit/circuitpython/blob/main/supervisor/shared/rgb_led_status.c#L389 |
there is no code.py I also tried downloading the "artifact" build from CI adafruit-circuitpython-espressif_saola_1_wrover-en_US-20200803-b4c0707.bin -- and loaded it to my board and it fails the same way as my builds :-( |
@jerryneedell try giving my latest build a shot. I added protections to remove the idf error messages, and added in an extra include to see if that null reference problem goes away. Tomorrow I may try getting gdb working with the ESP32-S2 since tracking null reference exceptions is kind of a pain without it. |
And just to confirm, with the latest chances, it is working fine in one of my saolas. Great work! I got 3 from digikey, on the other two I'm getting backtraces. ? 0x40097beb:0x3ffde0e0 0x4009e64b:0x3ffde100 0x4009d938:0x3ffde120 0x4009db93:0x3ffde170 0x4009dd3b:0x3ffde190 0x400b60f9:0x3ffde1b0 0x4002d995:0x3ffde1e0 I have not used board No 2&3 much, but I got the 3 of them on digikey on the same order, in the same plastic case that they ship them in, so there should be no difference with 1 and 2-3, other than I've use 1 in testing previous images of cpy. |
Trying to decode this weirdness. I tested the boards with other circuitpython versions, and after formatting by hand the filesystem, I got them to work. I looks like it is the lack of code.py, and the crash seems to happen "after" code.py or if it is missing. If I reformat, I start getting the errors again. I hope this saves a bit of time debugging it. |
@fede2cr Wow! That works! I'm not imagining things! With an infinite loop, I can break out from the REPL with control-C and everything is happy!! Thanks for finding this @hierophect -- FYI - with the latest PR, there are still a few IDF errors at start -- here is the end of the crash log with a non-infinite-loop code.py
and here is the log from a boot boot with a looping code.py
|
Seems like |
@fede2cr @jerryneedell thanks for your help in testing this! Seems like you've tracked down the way to repro it and it explains why I wasn't getting it since I already had a sketch loaded. @jepler yes, the docs for the error imply this is dereferencing a NULL. I'm not super used to tracking down things like NULL exceptions without GDB, so either I'll figure out how to get GDB running on the ESP32 today based on Scott's video or I'll try just eyeballing it. |
I've got a fix for the LEDC spew in my working code. It's resetting everything even it's not in use. |
@tannewt Yeah I've also fixed that edge case, I had to add a flag for first time reset. I'm working on tracking down the null reference now |
I found the error. At this line in main.c, But what I don't understand is why this does not occur for every other board? I don't see how this is in any way specific to the ESP32-S2. In any case, @tannewt could you take a look and let me know if you'd like me to maybe set a default exception, instead of NULL, or something else along those lines? |
Also, why do we lock the |
Yay!! Works for me! Thank you @hierophect! |
I think this was actually an error on every board, |
I am so relieved that this turned out to be a real issue. The toolchains can be such "black boxes" and I was really worried that something was wrong in my configuration but it was baffling as to where it might have been. |
@jerryneedell Yeah for sure, it's always stressful when you're worrying about whether your meta-level setup is correct. Thanks to @fede2cr for figuring out it only occurred when code.py is empty! |
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 hunting down the code.py issue.
ports/esp32s2/mpconfigport.mk
Outdated
@@ -12,6 +12,8 @@ USB_SERIAL_NUMBER_LENGTH = 12 | |||
# Longints can be implemented as mpz, as longlong, or not | |||
LONGINT_IMPL = MPZ | |||
|
|||
CIRCUITPY_NEOPIXEL_WRITE = 1 |
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.
Was this meant to be alphabetical?
supervisor/shared/rgb_led_status.c
Outdated
status->exception_color = MPY_ERROR; | ||
} else { | ||
status->exception_color = OTHER_ERROR; | ||
if (result->exception_type) { |
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.
How about making the first case the inverse of this and set the exception color to other in that case? Then you get an exception color and no if nesting.
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.
Was found_main true in this case? With code.py it should be false and we can just skip all of the exception flashing.
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 can check out found_main and see what the value is - I'm not that familiar with this portion of Circuitpython so I was kind of blundering around here. exception_type
is the problematic element - it's initialized to NULL in main.c and never changed, so anything based off of it will fail. We could set it to a proper default instead, if you'd like - the problematic stuff starts here. Still not sure why STM and the other ports don't trigger a null reference crash too.
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.
What exactly is found_main
supposed to do? I'm very confused by this whole section. Circuitpython runs with start_mp
, then it tries to find the file afterwards? And you say that it should be false for code.py
, but code.py is in the list of filenames it matches?
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 think this is waiting for the rgb status change we talked about earlier @hierophect
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.
Looks great! Thank you!
This PR adds Neopixel support to the ESP32_S2, and adds the onboard WS2812B as a status LED neopixel to the board profile of the Saola1-wrover.
This module utilizes the RMT (Remote Control) peripheral on the ESP32, which will also be used for PulseIn/PulseOut and possibly other purposes - thus, a simple channel reservation system has been added to
peripherals
, which these other modules will share. Neopixels reserve their channel every time they are written and automatically free it after writes. Up to 4 channels can be active at a time.Tested on the Saola1-wrover dev board.