sonyhome / FAB_LED

Fast Arduino Bitbang LED library supports programmable LEDs (WS2812B, APA102...), color palettes for Arduino AVR
GNU General Public License v2.0
125 stars 17 forks source link

Palette based not working #22

Closed rlogiacco closed 2 years ago

rlogiacco commented 6 years ago

First of all thanks a million for this library! I'm having difficulties in making your library work on an Arduino Nano with a 288 WS2812B led strip. Due to memory limitations, I'm using the palette based implementation, but my led strip is flickering at the beginning, then behaving as expected mid way the loop() function and finally behaving erratically at the end.

Here is my sketch, I tried to keep it simple for demonstration purposes:

#include <FAB_LED.h>

ws2812b<D,6> myLeds;
const uint16_t numPixels = 288;
const uint8_t bitsPerPixel = 4;
const uint8_t numColors = 1<<bitsPerPixel;

grb  palette[2*numColors] = {};
uint8_t pixels[ARRAY_SIZE(numPixels, bitsPerPixel)];

void setup() {
    myLeds.clear(numPixels);
    for (uint8_t i = 0; i < numColors; i++)  {
        palette[i] = {255/i,0,0}; // gradient of green
    }
}

void loop() {
    for (uint8_t c = 0; c <= numColors; c++) {
        for (uint16_t i = 0; i < numPixels; ++i) {
            SET_PIXEL(pixels, i, bitsPerPixel, c);
            myLeds.sendPixels<bitsPerPixel,grb>(numPixels, pixels, palette);
        }
        delay(1000);
    }
}

I was expecting to see the strip fill up of green and then fill up again of a darker shade of green and so forth, down to black and then back again. I started having a compilation issue in line 1523 sendBytes(bytesPerPixel, &palette[bytesPerPixel*colorIndex]); Which I solved with a cast sendBytes(bytesPerPixel, (uint8_t*)(&palette[bytesPerPixel*colorIndex]));

Either my fix is wrong and I don't understand your library principles or the problem lies somewhere else...

BTW, your library deserves some proper documentation: it is really promising for low memory consumption.

sonyhome commented 4 years ago

Thank you for trying the palettes solution.

Note that that mode adds indirections while updating the LED strip, hence extra cycles: The color index is looked up in the palette, and the 3 byte color is sent to the LED strip from the palette.

Looking at your code I think you should have a loop that updates your pixels array first, and then outside of the loop, update the LED strip, hence the following change:

void loop() {
    for (uint8_t c = 0; c <= numColors; c++) {
        for (uint16_t i = 0; i < numPixels; ++i) {
            SET_PIXEL(pixels, i, bitsPerPixel, c);
        }
        myLeds.sendPixels<bitsPerPixel,grb>(numPixels, pixels, palette);
        delay(1000);
    }
}

The flickering you're seeing might be caused by the pixels array being displayed before all the pixels are set, because your original code calls sendPixels() in the inner for loop.

As I read it, your code sets all pixels to a single color, displays it for a second, and moves to the next color.

If that's all the code intends to do, with FAB_LED you don't need to use a palette. You just need to use 3 bytes to hold the color of 1 pixel, and loop over numPixels to send the same 3 bytes over and over to each LED of the strip.

The palette trick is needed if you want your LED strip to have a complex pattern, but it has so many pixels that you could not allocate an array representing each pixel.

rlogiacco commented 4 years ago

Hi and thanks for answering. What I am actually trying to do is to light up one LED at a time in a relatively fast progression. If I move the sendPixels outside of the loop I end up with the whole strip coming up at once, which is not what I'm looking for. With regards to the palette additional indirection, that's very clear, but thanks for pointing it out.

sonyhome commented 4 years ago

I'm not 100% sure of your intent, but there's many ways to do that that might work better.

You could use only 2 colors of the palette and define an array with 2x numPixels. You init 1/2 of it with green, and 1/2 of it with black. Then you create a loop using sendPixels(), and at every iteration you only change the array position of the first pixel you draw and the palette value.

Something like this (untested):

// Changes array pointer so that count pixels are lit with the rest off
inline uint8_t * getLitPixels(uint8_t count) {
        return pixels + (numPixels-count) * bitsPerPixel/8;
}

void setup() {
        for (uint16_t i = 0; i < numPixels; ++i) {
            SET_PIXEL(pixels, i, bitsPerPixel, 1);
        }
        for (uint16_t i = numPixels; i < numPixels*2; ++i) {
            SET_PIXEL(pixels, i, bitsPerPixel, 0);
        }
        palette[0] = {0,64,0}; //red
        palette[1] = {0,0,0}; //black

        // init demo: all red
        myLeds.sendPixels<bitsPerPixel,grb>(numPixels, pixels, palette);
        delay(1000);
        // init demo: all black
        myLeds.sendPixels<bitsPerPixel,grb>(numPixels, getLitPixels(0), palette);
        delay(1000);
        // init demo: 8 LEDs red
        myLeds.sendPixels<bitsPerPixel,grb>(numPixels, getLitPixels(8), palette);
        delay(1000);
}

// Set green gradient intensity
void setGreen() {
    const ub8 g = palette[0].g;
    if (g == 255 || g <= 32) {
        palette[0] = {32, 0, 0};
    } else {
        palette[0] = {g+16, 0, 0};
    }
}

void loop() {
    for (uint16_t i = 0; i < numPixels; ++i) {
        setGreen();
        myLeds.sendPixels<bitsPerPixel,grb>(numPixels, getLitPixels(i), palette);
        delay(1000);
    }
}

You could also use one green pixel and one black. Then instead of sending one array of pixels, you send the same green (or black) pixel numPixels times. Then you delay(1000).

I just realized my answer is exactly 2 years late! :P