Aircoookie / WLED

Control WS2812B and many more types of digital RGB LEDs with an ESP8266 or ESP32 over WiFi!
https://kno.wled.ge
MIT License
14.57k stars 3.12k forks source link

first led turns on after a while from firmware version 0.11.x #2172

Closed sommo closed 2 years ago

sommo commented 3 years ago

Describe the bug first led turns on after a while, when the wled is off

To Reproduce i upgrade my wemos d1 mini + ws2812 + pin (pushbutton) -no levelshifter, from 0.10.x to the last stable one. with the upgrade start the problem. so i downgrade till the 0.10.2 and the problem disappear.

WLED version

Additional context

seems something wrong from 0.11.x that intruduce that bug, also with the beta version

blazoncek commented 3 years ago

Since 100's of other installations do not exhibit the problem it is most likely that your set-up may be the culprit. Make sure you check all connections and add level shifter. White leds may be the result of wire noise which indeed may be caused by higher current demand by ESP due to higher number of functions it has to perform since 0.10 was released.

If you still think it is a WLED problem reproduce it on a new ESP system with different LED strip.

sommo commented 3 years ago

the same wemos d1, run with 0.10.x without the bug but once upgrade on 0.11.x the bug appears. I know I have no level shifter, but basically all (hw) remains the same the only thing that changes is the firmware (sw). I personal don't use any new features once upgraded, for ex. sync hue or others, just on/off from a phisical switch.

blazoncek commented 3 years ago

You did not read my post entirely or understood it.

What I want to say is that due to increase of complexity of WLED code so did increase resource requirements (CPU power, etc). This may induce noise in data line which affects your particular LEDs. As I said, reproduce the error on a different set using level shifters and it will be acknowledged as a WLED error.

BTW if you do not require any new feature then do not upgrade. If you do require a new feature then you may need to upgrade your setup too.

I am telling you this since I have 30+ WLED installs ranging from ESP01 to ESP32 using as low as 10 LEDs to as many as 600+ without any such issue.

sommo commented 3 years ago

ok, now i understand what you mean. thank you.

cmtmc commented 3 years ago

Please check https://github.com/Aircoookie/WLED/issues/1674 - 5 leds lit due to refrigerator.

xmas79 commented 3 years ago

@blazoncek I'm being hit by this as well. In my setup I have two pins involved in the issue. Now... I understand the setup is the main issue, but software can come right to the rescue saving some level shifters around the world: a simple fix would be to tie the pin line LOW instead of HIGH when WLED is off. I understand the HIGH state is required to turn off the onboard led, but in my case the pin 2 is not involved, so there's really no reason to keep the line HIGH.

If it's really as I described above, can I propose to add a configuration parameter to let the user choose if they want the line HIGH or LOW during off state, knowing that this could impact the onboard led? The user would do an informed choice then.

Do you think this is feasible to implement? Thanks.

blazoncek commented 3 years ago

As WS28xx protocol is timing critical protocol (hence 1 wire) it really doesn't matter if line is LOW or HIGH. If you get a noise on your line random LEDs may be lit by interpreting this noise as data (this is true only for the first LED since other leds, get 5V data from the first one).

So, no, this would not fix your issue. At least not in general terms.

xmas79 commented 3 years ago

Thanks for your response.

However I think this is not the case. The problem described here is that when the WLED is off the line is kept HIGH when no communication occurs. Without a level shifter, it means the line is kept at 3.3V, and that's into the uncertainty zone of the Din pin of the first led of the stripled. A relatively high noise around 3.3V will indeed cause the Din to interpret this as data bits and will turn on the first led of the strip. Instead, if the data line were at 0V noise there's absolutely no way for a relatively high noise to reach the "on" zone of the Din pin (0.7*Vdd typical, eg 3.5V) and interpret it as data bits.

So, yes, IMHO having the data pin tied to a LOW state instead of HIGH will probably resolve this issue definitely.

Anyway, I'd like to solve the issue, if you could you please point me to the right code section to edit I could try it by myself and come back with some results later. Unfortunately I'm not very much familiar with the WLED source code (yet).

Thanks!

blazoncek commented 3 years ago

Look into button.cpp handleIO() since this is a requirement for turning onboard LED off. You can just undefine LED_BUILTIN for trying it out.

If this does not resolve your issue then talk to @makuna the author of NeoPixelBus who is responsible for driving LEDs.

xmas79 commented 3 years ago

Thanks for your suggestions. I've implemented the "force to low state" and I now have my "problematic" nodemcu with two WS2812 strips in pin 1 and 3 updated with this under test. I will observe the behavior and will report back.

I may have made something wrong and incomplete (eg no GUI, I don't know the impact with other stripled types), so it would be great if someone else could give a look at it. This is not a PR since I don't know if it will ever be merged (well at least not in this stage), so I will just post the code modification I made.

I edited bus_manager.h in three parts:

added this function to Bus class:

  inline void setOutputPinState(uint8_t state) {
    uint8_t pinArray[5];
    uint8_t numPins = getPins(pinArray);
    for (uint8_t i = 0; i < numPins; i++) {
      pinMode(pinArray[i], OUTPUT);
      digitalWrite(pinArray[i], state);
    }
  };

Changed the implementation of setBrightness in the BusDigital class so that it basically reinitialize the busses:

  void setBrightness(uint8_t b) {
    if (_bri == 0 && b > 0) {
      PolyBus::begin(_busPtr, _iType, _pins); 
    }
    _bri = b;
    PolyBus::setBrightness(_busPtr, _iType, b);
  }

Implemented a "wrapper" method in the BusManager class:

  inline void setOutputPinState(uint8_t state) {
    for (uint8_t i = 0; i < numBusses; i++) {
      busses[i]->setOutputPinState(state);
    }
  }; 

I then just call this method in the handleIO function in the button.cpp file:

void handleIO()
{
  handleButton();

  //set relay when LEDs turn on
  if (strip.getBrightness())
  {
    lastOnTime = millis();
    if (offMode)
    {
      if (rlyPin>=0) {
        pinMode(rlyPin, OUTPUT);
        digitalWrite(rlyPin, rlyMde);
      }
      offMode = false;
    }
  } else if (millis() - lastOnTime > 600)
  {
    if (!offMode) {
      #ifdef ESP8266
      // turn off built-in LED if strip is turned off
      // this will break digital bus so will need to be reinitialised on On
      pinMode(LED_BUILTIN, OUTPUT);
      digitalWrite(LED_BUILTIN, HIGH);
      busses.setOutputPinState(LOW);   // <---- added this
      #endif
      if (rlyPin>=0) {
        pinMode(rlyPin, OUTPUT);
        digitalWrite(rlyPin, !rlyMde);
      }
    }
    offMode = true;
  }
}
blazoncek commented 3 years ago

Nice approach. Nevertheless if you happen to assign the same GPIO for LEDs as is used for LED_BUILTIN you will not turn it off since it requires output to be high for it to turn off. So you may as well remove the code for LED_BUILTIN.

xmas79 commented 3 years ago

The patch has been running for 4 days on my device, and it addressed the issue quite good. Although it was not perfect: Typically, I saw some LEDs on after waking up, setting the stripleds off overnight (eg 8 hours idle). Sometimes I saw them on also during the day. With this patch I've never had spotted one led on after waking up. In 96 continuous off hours I found the first led on only once (and in one of the two stripleds only). That same LED went off by itself after a short amout of time (cannot quantify). Considering that with the official 12.x version just after a couple of hours both I had at least 3/4 LEDs turned on (on both stripleds, pins 1 and 3) I can say that the patch makes at least some difference. The effectiveness depends on the noise level on the lines (in my case was very effective on one pin, and less effective on the other pin).

Now I'm wondering if the best way to address the issue is to avoid letting the lines go idle, eg continue servicing the LEDs by pushing the "black" color (eg equivalent to setting the brightness to zero). This way, even if we have noise on the lines, at the next cycle everything will be forced off again.

However, we already know that doing so means having the onboard led always on. For fixing that, I would keep servicing the LEDs just once in a while (eg once per second). The net effect would be that the onboard LED is "always" off and would "blink" for a few milliseconds, just the time to force off the stripleds.

What do you think?

Thanks.

blazoncek commented 3 years ago

That is done by solid effect using a black color. No patching needed.

xmas79 commented 3 years ago

Yes but that's not part of the main power button, nor it "almost" fixes the onboard led on issue. Isn't it?

blazoncek commented 3 years ago

Yes but that's not part of the main power button, nor it "almost" fixes the onboard led on issue. Isn't it?

The on-board LED requires GPIO2 being held HIGH to turn off. This defeats your purpose of holding GPIO LOW to reduce stray LEDs.

The notion of Power in WLED only relates if you are using a relay. Otherwise you can keep Power always on and use black color.

mikey0000 commented 2 years ago

I ran into this on 12 b2, and yes I'm not using a level shifter, I'll add one and see if it fixes my problem.

blazoncek commented 2 years ago

Closing this as being hardware (wiring) issue and not software issue. NOTE: Use level shifted output and a good power supply. Level shifter will change 3.3V to 5V reducing noise influence on random LED flicker. You may as well add decoupling capacitors to power lines.