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

Support for GPIO16/D0 #222

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Support for GPIO16/D0 #222

wants to merge 1 commit into from

Conversation

sascha432
Copy link

Adds support for GPIO16/D0 for ESP8266 by using the RTC register address instead the GPIO register address.

The changes can be tested with any example code.

I tested it with an 8 pixel strip (WS2812 800kHz, CPU 80MHz) and measured timings with an oscilloscope. The timings match the timings without modifications and are within the datasheets specifications.
Ports tested 12/D7 and 16/D0.

ESP32 code isn't affected.

Copy link
Contributor

@PaintYourDragon PaintYourDragon left a comment

Choose a reason for hiding this comment

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

Howdy. I’d be happy to merge this functionality, but would request changes. I’ll summarize below if you’d like to do yourself, otherwise I’ll merge as-is and then follow up with my own edits. In either case, the commit history would acknowledge you as the author. If I don’t hear back by next time I’m making a pass through PRs, will do the latter.

Rather than compensating for the branches by subtracting cycles, I’d suggest simply having a distinct loop for the (pin == 16) case — it could all go in the test starting around line 66. Yes this would increase code size by a couple dozen bytes, but I feel it’s worth it to avoid anything cycle-counting-related, as cycles can be thrown off by optimizer settings, clock speed, etc.

(The irony of that last part isn’t lost on me, since like 90% of the code is cycle-counting nastiness, but any opportunity to make that better instead of worse is welcome, like when the SAMD51 code got cleaned up.)

Thanks!

@sascha432
Copy link
Author

Hi,

feel free to modify or change anything, no need to mention my name or anything.

Maybe this is interesting for you too, kinda clean rewrite of my library while I was working on FastLED on the issues with framework-arduinoespressif8266 3.0.0

https://github.com/sascha432/NeoPixelEspEx
https://github.com/sascha432/NeoPixelEspEx/blob/master/include/NeoPixelEspEx.h

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.

2 participants