Some bugs in the color & brightness related code

[Salamandar]

Hi there ! Currently using the library for a Led Ring. The code used to manage the pixels color and brightness is very buggy : we can't setBrightness lots of times in a row without re-calling setPixelColor each time.

For example here I create a rainbow. Then, I create a "breath" with lots of calls to setBrightness(). You can see that after a few setBrightness cycles, most of the leds are off. That's probably related to the re-scale code used here :

    if(oldBrightness == 0) scale = 0; // Avoid /0
    else if(b == 255) scale = 65535 / oldBrightness;
    else scale = (((uint16_t)newBrightness << 8) - 1) / oldBrightness;
    for(uint16_t i=0; i<numBytes; i++) {
      c      = *ptr;
      *ptr++ = (c * scale) >> 8;
    }

Why don't you store the brightness somewhere without touching the original color values (that is buggy), and do some multiplication in show() ?

Salamandar

[PaintYourDragon]

That's correct. setBrightness() is a lossy, non-reversible operation -- it modifies the pixel data in RAM because the show() function needs to maintain exceedingly strict timing for the NeoPixels.

It was never intended as an animation effect. Typically a sketch should call setBrightness() once in setup() if it needs it, in order to limit the current/brightness of the LEDs throughout the sketch, and never again. But it's sort of gotten out of hand. In hindsight, I'd call the entire 'feature' a mistake...perhaps setting brightness in the constructor would've made its intended use obvious.

The DotStar library handles brightness scaling on-the-fly in show(), as those LEDs don't require any specific data rate.