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

Added ESP32 support. #125

Merged
merged 1 commit into from
Mar 18, 2017
Merged

Added ESP32 support. #125

merged 1 commit into from
Mar 18, 2017

Conversation

marcmerlin
Copy link
Contributor

Modified library so that it refuses to build on an unknown architecture
instead of building and then doing nothing.

Added ESP32 support, which might not be optimal, but works for me.

Modified library so that it refuses to build on an unknown architecture
instead of building and then doing nothing.

Added ESP32 support, which might not be optimal, but seems to work for
me.
@marcmerlin
Copy link
Contributor Author

marcmerlin commented Jan 29, 2017

travis failure seems totally unrelated to my change

@MartyMacGyver
Copy link

MartyMacGyver commented Feb 2, 2017

Lots of builds are failing it seems. I hope this gets merged soon - yours appears to be the first working Arduino-ESP32 implementation for WS2812/NeoPixels so far!

Edit: A suggestion and a question:

Make esp32.c it's own file (based on esp8266.c), rather than overloading esp8266.c.

Are you seeing glitchy pixels when running this? I am, and as it's only with this library I think something about the pixel timings is off, but I'm not sure what yet.

I've forked with both your changes and the suggestion above as a basis to test timings, etc:

https://github.com/MartyMacGyver/Adafruit_NeoPixel

@marcmerlin
Copy link
Contributor Author

@MartyMacGyver , so I'm not an ESP32 expert, I just did this as bit banging and it worked.
Making a separate exp32.c file, I thought about it, but almost all the code is identical, so I didn't really want to fork to a new file and then duplicate all the code.
I am not getting glitchy pixels, however the only test board I have right now has APA106 which is mostly compatible but not entirely. I have some real adafruit neopixels at home, but I won't be home for a while long to test them out.
If you want to take my code and try to adjust it to remove the glitchiness you are seeing, feel free.

@MartyMacGyver
Copy link

@marcmerlin Good to know. How many LEDs does your APA106 board have? I see small problems with longer runs (above 64 LEDs).

I already forked your code as noted above and made that change - having the two platforms separated is a little easier to manage in the long run as other platforms come into play (or if other ESP32 optimizations are added over time).

@marcmerlin
Copy link
Contributor Author

@MartyMacGyver I only have 2 LEDs, so that would explain why I don't have timing issues. Given that you have better hardware than me, I recommend you take my preliminary code, and make it yours :)
As for splitting into 2 files, again my preference is that as long as the code is the same, sharing the same file makes more sense, but if you end up having more code that is different to make the timings be proper for ESP32, feel free to split into a 2nd file.

@MartyMacGyver
Copy link

tl;dr - Still glad to have found your branch, and it'll be handy in the short term, but I really hope FastLED gets updated for ESP32 because TBH this kind of bit-banging appears to be a dead-end on the ESP32:

http://esp32.com/viewtopic.php?f=13&t=293

I re-pulled the original code the bit-banging loop in espShow() was based on, refactored it down to something very similar to what was in the code base. I came up with a version that is less glitchy but... it's extremely fragile.

For a simple test pattern cycled within loop(), the first pass is still glitchy, but subsequent passes are not. As the memory is totally clear to start, I suspect internal caching on the ESP32 itself is to blame (possibly the memory issues mentioned in the ESP32 errata as well?)

If I tweaked it even a little, moving a line up or down in a way that might cost a cycle or two or by adding some cycles before the pixel loop even started, it all got very glitchy very quick, in specific patterns that suggest distinct timing issues. I think what's going on is a lot deeper than the code - the hardware itself isn't appropriate for bit-banging to this precision, thus their admonition.

Some related info:
esp8266/Arduino#1536
I did try ICACHE_RAM_ATTR (which is __attribute__((section(".iram0.text")))) as was done for the ESP8266, to no effect.

@BsAtHome
Copy link

BsAtHome commented Feb 12, 2017

The travis failures are related to the build-environment used by travis. See PR #114 and issue #118 for comment and fixes.

However, it seems that the maintainer(s) are very hard to come by these days. There has been no response for a while from the maintainer(s) and the queue of PRs and issues are growing. Even though the library is great, it needs to be maintained. So, hello maintainers, care to join in on the fun?

@ladyada ladyada merged commit 8efb431 into adafruit:master Mar 18, 2017
@ladyada
Copy link
Member

ladyada commented Mar 18, 2017

@marcmerlin tested both with '32 and '8266 - merged!

@BsAtHome hiya, it takes a ton of effort to manage hardware library changes - we need to not only check the hardware support but back verify all the other platforms and compilers since 50% of pull reqs break example code or other platforms - 25% don't even compile! we focus on fixing bugs and making sure things work for the vast majority of users - being jerky/snarky will not make us handle PR or issues faster, it only makes us not want to work on and maintain our ~800 github repositories with free and open source code and designs :) you can help by testing PR with multiple hardware platforms, that is the #1 thing that will get PR merged faster!

@BsAtHome
Copy link

@ladyada I certainly do not want to be "jerky", but it is rather disappointing when no reaction is received at all. A simple "thanks, but we are busy" would have helped instead of a three months silence. A slight indication whether a request will be considered would be nice too. Wrt bugfixes, I did supply a bugfix for CI and examples. Thinking it might speed up testing cross-platform if Travis already indicates the preliminary status.

I do think that you can pull in a lot of resources from a vibrant community. Being a "single" entity with limited resources is very demanding. I guess that there are quite a few people out there who are willing to help.

@ladyada
Copy link
Member

ladyada commented Mar 18, 2017

@BsAtHome hiya, yes thank you for the bug fix. when a bugfix is wrapped up with a feature, it makes it harder to just fix the bug!

there are many people who want to help - however, it is very hard to find people who want to help who are able to do the proper testing that gets things merged. its easy to find people to add new functions for a desired special case, its not easy to find people who will test a PR against 6 to 10 different hardware/compiler platforms!

@adafruit adafruit locked and limited conversation to collaborators Mar 18, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants