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

Playlist output glitchfix update: found it also happens on S3 #4462

Merged
merged 3 commits into from
Jan 6, 2025

Conversation

DedeHai
Copy link
Collaborator

@DedeHai DedeHai commented Jan 6, 2025

No description provided.

@DedeHai DedeHai added this to the 0.15.1 candidate milestone Jan 6, 2025
@DedeHai DedeHai merged commit 3adcbb7 into Aircoookie:main Jan 6, 2025
19 checks passed
@softhack007
Copy link
Collaborator

@DedeHai thanks 👍
I think that the RMT output driver of NPB is most sensitive to timing, so it might be best to enable the fix on any esp32 variant ( #ifdef ARDUINO_ARCH_ESP32 ). Maybe you did not observe the flickering on esp32 yet, because its a bit tricky to not get the I2S driver:

  • more than 2 outputs needed (output 0 will use I2S#1)
  • more than 300 leds needed on one or more outputs (so the parallel I2S#1 driver cannot be used)

Did you see any hickups or bad side-effects with the glitch fix?

@DedeHai
Copy link
Collaborator Author

DedeHai commented Jan 7, 2025

I initially wanted to remove the ifdef, but could not get ESP32 to glitch... will try again to force it to RMT and see if that glitches.
will also test if it has a side-effect on I2S outputs when the fix is applied
edit:
I will also check if the ESPnow glitch (which is related) also happens on S3 / ESP32 -> edit2: it does, the fix #4386 stays as it is (no #ifdefs)

@DedeHai
Copy link
Collaborator Author

DedeHai commented Jan 7, 2025

ran some more tests:
I can not get any glitches on the ESP32, not in I2S nor RMT output mode (tested with 4 outputs, one of them set to 400 LEDs). I found no difference with or without adding the "wait for bus". So it could be also added for ESP32 but I am a bit torn on doing that without any reason behind it.

DedeHai added a commit that referenced this pull request Jan 12, 2025
* Fix output glitches when playlist changes preset update: glitches also happen on S3
@netmindz netmindz added the bug label Jan 16, 2025
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.

3 participants