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

Use a translator callback to convert GRB data to RMT pulses. #3509

Open
wants to merge 1 commit into
base: dev-esp32
Choose a base branch
from

Conversation

docbacardi
Copy link

@docbacardi docbacardi commented Feb 23, 2022

Fixes #3508.

  • This PR is for the dev-esp32 branch rather than for the release branch.
  • This PR is compliant with the other contributing guidelines as well (if not, please describe why).
  • I have thoroughly tested my contribution.
  • The code changes are reflected in the documentation at docs/*.

This patch replaces the custom ISR in ws2812.write with a translator callback.
Furthermore it introduces 2 small improvements:

  1. If the hardware supports it, all channels in the write request are placed in a group. This moves the start of all RMT sequences much closer together. On my hardware I observed >=50ns .
  2. New optional parameters can set the duration of the reset signal and the duration for the high and low phase of the 0 and 1 symbols. This allows to adapt the timings to many other RGB LEDs.

@pjsg
Copy link
Member

pjsg commented Feb 25, 2022

This looks plausible to me -- I need to get some ws2812s attached to an esp32 to test.....

@docbacardi
Copy link
Author

The module name might have become a little bit misleading. It is not ws2812 specific, but works with a lot of other RGB LEDs. I did my tests with a single SK6812 on the ESP32-C3 devkit. Another unknown LED strip from some home improvement store is on the way.

@HHHartmann
Copy link
Member

But there are other led strips which will not work, so that would be as misleading.

@jmattsson
Copy link
Member

@docbacardi @pjsg any chance of seeing what needs tweaking to make this compatible with the current IDF5 branch? I'm out of my depth with the RMT stuff.

@docbacardi
Copy link
Author

I will try and port this. Also a good chance to move to the new RMT driver.

@docbacardi
Copy link
Author

Please find the patch here: #3629

The migration to the new RMT code in IDF5 would affect more modules. One step after another...

@jmattsson
Copy link
Member

Is this PR still relevant with #3629 having been merged?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants