Navigation Menu

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

Remove analog flicker #678

Merged
merged 64 commits into from Feb 22, 2020
Merged

Conversation

Def3nder
Copy link
Contributor

Hi @Aircoookie,

when using dumb (analog) LED strips there was a kind of flicker - especialy when using solid colors.

I found out that this is the case when setPixelColor isn't called frequently (and thus the analog PIN don't get updated).

I change the code in that way that setRgbwPwm will be called from the main loop.
This has two advantages:

  1. it will be regularly updated
  2. it can "display" changes coming from fade_out() or in general: all functions that change the strip appearance without calling setPixelColor.

Def3nder and others added 30 commits November 5, 2019 15:55
When adding an RGBW device to Alexa and then selecting a white color tone, Alexa will send CT values to the device. Having a warm white or cold white strip should use 100% of that strip and then add the RGB colors to get either a warmer color or a colder one.
Added IR remote with 40 keys to use with RGBW stripes.
...not implemented for ESP32 yet
ESP32 Solid RGBW defines

moved from env:esp32dev to env:esp32
ESP32 Solid RGBW defines
Update readme.md
To turn off IR remote use "0" in the settings, use 1 to 4 for different IR remote types.
adapt the logic to use CW and WW for different CT-values
Changed code from #define WLED_DISABLE_ANALOG_LEDS to #define WLED_USE_ANALOG_LEDS
NightLight used to brighten the light combined with fading from the actual (primary) color to the secondary color
RLYPIN definition dependant on WLED_USE_ANALOG_LEDS

Corrected list-item indents (readme.md)

updated to match upstream master (briLast in wled00.ino)
RLYPIN definition dependant on WLED_USE_ANALOG_LEDS

Fixed headline and corrected list-item indents (readme.md)

updated to match upstream master (briLast in wled00.ino)
run SetRgbwPwm from main loop and with GetPixelColor(0) to get all effects using fade_out() working.
@Aircoookie
Copy link
Owner

Interesting. I was assuming the driving is happening via an interrupt handler on ESP8266 (and dedicated LEDC pwm driver on ESP32). Which would mean that after setting the duty cycle it should stay on that duty cycle until the setRgbwPwm is called again. Strange that calling it more often seems to fix the issue.
It seems like there are a few bitwise ands & where I would have expected &&. Is that intentional?

Thank you though, I will merge this fix soon!

@Def3nder
Copy link
Contributor Author

Yes, you are right - funny - this had been in the code before - seems that nobody really used the 5CH code until now.

I remember some comments in an issue and now it makes sense as this way 5CH output would never get used because none of the if statements became true.

I will fix the source...

@Def3nder
Copy link
Contributor Author

Def3nder commented Feb 14, 2020

for some reason Travis CI does not get a single compile okay.

For all ESP8266 builds:

AsyncTCP.cpp:24:0:
.pio/libdeps/nodemcuv2/AsyncTCP_ID1826/src/AsyncTCP.h:28:33: fatal error: freertos/semphr.h: No such file or directory
     #include "freertos/semphr.h"

and for ESP32 builds:

ESPAsyncTCP.cpp.o
.pio/libdeps/esp32dev/ESPAsyncTCP_ID305/src/ESPAsyncTCP.cpp: In member function 'bool AsyncClient::connect(IPAddress, uint16_t)':
.pio/libdeps/esp32dev/ESPAsyncTCP_ID305/src/ESPAsyncTCP.cpp:115:8: error: 'ip_addr_t {aka struct ip_addr}' has no member named 'addr'
   addr.addr = ip;

I do not get any of this locally with PlatformIO (for both ESP32 and ESP8266)

@Aircoookie
Copy link
Owner

Yeah, I do not understand the origin of the Travis fail either. It definitely has nothing to do with the contents of your PR.

Sorry to be so nitpicky this time, but _analogLastShow doesn't appear to be used, because it isn't set to nowUp after the check if enough time has elapsed. That way, SetRgbwPwm is called every main loop iteration. I've added that line, could you quickly check whether it still removes the flicker when called every MIN_SHOW_DELAY (15) milliseconds?

@Def3nder
Copy link
Contributor Author

Hi @Aircoookie, the change is okay - however: the flicker isn't gone 😢

I think that the change is good anyway, because this way we get the changes done through the NeoPixelBus library sent to the dumb LEDs, too.

However, the flicker must have another reason:
https://youtu.be/ZrXai4OYj20

In my bedroom I#ve flashed Tasmota on exactly the same device / strip.
And there is no flicker at all.

I dig into this and look how tasmota did drive the LEDs.

@mike2nl
Copy link
Contributor

mike2nl commented Feb 16, 2020

@Aircoookie @Def3nder
About the flicker effect with single colors.
Hint:

  • We had the same on tasmota but that is fixed now. Ask @arendst (Theo) via mail or get into the discord channel a talk to s-hadinger, Jason2866 or Adrian. I was not involved in that fix because i have other work for Tasmota. s-Hadinger was the one.

@Def3nder
When had you flashed your led strips with Tasmota (no flicker)?
Or better which version you have used?
Which core? 2.6.1 or 2.6.3?
ANd a binary from thehackbox or have you compiled for your own?

@Def3nder
Copy link
Contributor Author

Hi @mike2nl,
I did not compile any Tasmota myself, so definitely from thehackbox.

Here the details from the Information tab:

Program Version		6.5.0.9(889b017-sonoff)
Build Date & Time	2019-05-08T13:11:30
Core/SDK Version	2_5_0/3.0.0-dev(c0f7b44)
Uptime			95T16:30:32

...it's quite old (like one year or so) and I did not touch it after that.

Thx for the hint towards tasmota - I will get in touch with the named persons 😄

@Def3nder
Copy link
Contributor Author

Def3nder commented Feb 19, 2020

I got in contact with the Tasmota team as they had the same flicker on analog RGB LEDs.
They found a solution that has been merged to the master already.

The phases of the different RGB channels become out-of-sync in certain situations and this leads to color-variation that is visable as flicker.

I implemented the same approach here and yes: the flicker has gone away 😄

As the solution is done in one of the Arduino Core files, there is a PR (7057) open on the ESP8266/Arduino Library.
There this approach is discussed as there is another - more complex - solution (PR 7022).
I will test this solution, too - let's see which of them is better for us.

@Def3nder
Copy link
Contributor Author

So, I've tested the second solution, too: it has visable the same effect (flicker is more or less gone - it's exactl the same compared with the eyes only). However: looking at the osciloskope, hte later (PR7022) seems to be "better".

I've posted my results here

@Aircoookie: which solution do you like most ?

@debsahu
Copy link
Contributor

debsahu commented Feb 19, 2020

for some reason Travis CI does not get a single compile okay.

For all ESP8266 builds:

Add this to platformio.ini lib_ignore = AsyncTCP for ESP8266 part

AsyncTCP.cpp:24:0:
.pio/libdeps/nodemcuv2/AsyncTCP_ID1826/src/AsyncTCP.h:28:33: fatal error: freertos/semphr.h: No such file or directory
     #include "freertos/semphr.h"

and for ESP32 builds:

Add this to platformio.ini: lib_ignore = ESPAsyncTCP in ESP32 part

ESPAsyncTCP.cpp.o
.pio/libdeps/esp32dev/ESPAsyncTCP_ID305/src/ESPAsyncTCP.cpp: In member function 'bool AsyncClient::connect(IPAddress, uint16_t)':
.pio/libdeps/esp32dev/ESPAsyncTCP_ID305/src/ESPAsyncTCP.cpp:115:8: error: 'ip_addr_t {aka struct ip_addr}' has no member named 'addr'
   addr.addr = ip;

I do not get any of this locally with PlatformIO (for both ESP32 and ESP8266)

@Aircoookie
Copy link
Owner

@Def3nder thank you for all that testing! Both these PRs seem to get the job done well at least visually, so I would tend towards using the fastest implementation. Out of interest, is the solution you implemented here more like 7022 or 7057?

@mike2nl
Copy link
Contributor

mike2nl commented Feb 20, 2020

@Def3nder @Aircoookie
Lucky that i could help you guys out here. Jep, sharing know-how that's the community.
You talked with s-hadinger i saw, he told it in the admin or contrib channel 4 hours ago. When i can help out more please ask. I'm not always visible because i get to many sensor questions so a PM get me always there.

@Def3nder
Copy link
Contributor Author

Hi @Aircoookie, I implemented PR #7057 because PR #7022 caused reboots after 6 to 12 minutes when any form of light was on (no matter which color and no matter what brightness).

I think this was an issue raised from calling analogWrite every 15ms, but this must not be a problem for a Arduino Core library ! So, I stick with PR#7057 which basically kept the phases total constant 😄

This had been the phases for red & green channel before: YouTube
and this is the result with PR#7057 implemented: YouTube

The author of PR#7057 did suggest to not call analogWrite if the values did not change, so I catched that idea and introduced two more variables _analogLastColor and _analogLastBri that are checked at the start of setRgbwPwm() .

I thought increasing / refreshing the PWM signals more often would help, but the problem did not have anything to do with that - so this "color-changed-check" will reduce loop-time.

I think we can go with this solution - the Tasmota team did exactly the same.

@Aircoookie Aircoookie merged commit e621fde into Aircoookie:master Feb 22, 2020
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.

None yet

4 participants