kitesurfer1404 / WS2812FX

WS2812 FX Library for Arduino and ESP8266
MIT License
1.6k stars 347 forks source link

`FX_MODE_TWINKLE_FADE` never blinks the last led in a segment #299

Closed DanielWeigl closed 3 years ago

DanielWeigl commented 3 years ago

I have two adjacent segments and if an effect acts on the first segment with a fadeout (FX_MODE_TWINKLE_FADE in my case), the first led of the next segment also gets fade out and flickers depending on which effect is on that segment.

As far as I can see, this should be i < _seg->stop:

https://github.com/kitesurfer1404/WS2812FX/blob/d3047cc416cf77f06f85005585ecf98041dc6b3e/src/WS2812FX.cpp#L1078

I tried to change it locally and then it works as expected, but I am not sure if it has other side effects on other setups

moose4lord commented 3 years ago

Hmmm...I don't know. It seems to be working ok for me. This is what I tried:

#include <WS2812FX.h>

#define LED_PIN     4  // digital pin used to drive the LED strip
#define LED_COUNT 144  // number of LEDs on the strip

WS2812FX ws2812fx = WS2812FX(LED_COUNT, LED_PIN, NEO_GRB + NEO_KHZ800);

void setup() {
  Serial.begin(115200);

  ws2812fx.init();
  ws2812fx.setBrightness(255);

  // parameters: index, start, stop, mode, color, speed, reverse
  //ws2812fx.setSegment(0,  0,  9, FX_MODE_TWINKLE_FADE,   RED,  100);
  ws2812fx.setSegment(0,  0,  9, FX_MODE_FADE,   RED,  4000);
  ws2812fx.setSegment(1, 10, 19, FX_MODE_STATIC, BLUE, 1000);

  ws2812fx.start();
}

void loop() {
  ws2812fx.service();
}

Can you post your code?

DanielWeigl commented 3 years ago

Ah... maybe thats a different bug, or a misunderstanding:

This is my setup (with the fix from above) and is now working:

#define PIN_LED 27
#define NUM_LED (9+4)   // 13 leds in two segments, first 9 second 4

WS2812FX leds = WS2812FX(NUM_LED, PIN_LED, NEO_GRB + NEO_KHZ800);

...

                leds.setSegment(0, 0, 9, FX_MODE_TWINKLE_FADE, 0xFF0000, 100, false);
                leds.setSegment(1, 9, 12, FX_MODE_BREATH, 0x00FF00, 500, false);

But I just realized, that the end of the TWINKLE_FADE segment should have end=8 and not 9 (i thought that is count) -> but the problem which let to this misunderstanding is, that if I set it up as

                leds.setSegment(0, 0, 8, FX_MODE_TWINKLE_FADE, 0xFF0000, 100, false);
                leds.setSegment(1, 9, 12, FX_MODE_BREATH, 0x00FF00, 500, false);

the last led of the first segment never TWINKLE's.

https://github.com/kitesurfer1404/WS2812FX/blob/d3047cc416cf77f06f85005585ecf98041dc6b3e/src/WS2812FX.cpp#L1138 -> If size==1 led, this means that random16(_seg_len - 1) will never return _seg_end - or?

moose4lord commented 3 years ago

Ah, I see. You're right. random16(_seg_len - size) will always return a number < (_seg_len - 1), so index will never be >= _seg->stop and the last LED in the segment will never flash.

Maybe it should be uint16_t index = _seg->start + random16(_seg_len - size + 1);.

DanielWeigl commented 3 years ago

yes _seg_len - size + 1 sounds better

moose4lord commented 3 years ago

There are a few more effects that use that random16(_seg_len - size) calculation. I'll look through the code and try to fix them. Thanks for catching this bug.

moose4lord commented 3 years ago

I just released WS2812FX v1.3.5, which contains this fix as well as some other bug fixes and enhancements.

DanielWeigl commented 3 years ago

Wow - that was fast, thanks - looks like https://github.com/kitesurfer1404/WS2812FX/pull/301/commits/d3fffa4e3ce6200ae3c5183663fc7f67bd3f3487 fixes this issue. UT sofar, but will report back if somethings wrong.

Thanks.