Simple LED Bar-Graph Question

Hey,

I’m trying to drive a NeoPixel 8 LED module using the Adafruit library using the following sketch:

#include <Adafruit_NeoPixel.h>
#define PIN 6
#define NUMPIXELS 8
Adafruit_NeoPixel pixels = Adafruit_NeoPixel(NUMPIXELS, PIN, NEO_GRB + NEO_KHZ800);

byte Nred = 0;


void setup() {
  pixels.begin(); 
  
}

void loop() {

  // number of red pixels 

  Nred = analogRead(xpin);
  Nred = map(x, 100, 600, 0, 7);
  
  for(int i=0;i<Nred+1;i++){

    pixels.setPixelColor(i, pixels.Color(255,0,0)); 
    pixels.show(); 
    delay(100); 

  }  
 }

I’m attempting to implement a bar-graph style display and with the current sketch, it works to an extent. At the moment, the LED’s light up to the maximum level reached (as determined by Nred) but do not move back down dynamically as the analog input varies.

I realize this is due to the ‘for’ loop which does not allow LED’s to turn off after being switched on. How can I incorporate a mechanism to ‘reset’ the position of the LED’s and thus have a dynamic display rather than one which essentially freezes once all 8 LED’s have been lit?

Thanks

Somewhere, probably just ahead of the existing for loop, you need a for loop that turns off all LEDs. There is nothing wrong with turning off an LED that is already off except a little wasted time, and your delay(100) statement is already doing that.

By the way, each for loop should be in its own function, say clearAllLights and setLights.

These work because (i <= Nred) is either 1 (true) or 0 (false).

  for(int i=0;i<NUMPIXELS;i++){
    pixels.setPixelColor(i, pixels.Color((i<=Nred)*255,0,0)); 
    pixels.show(); 
    delay(100); 
  }

More responsive would be:

  for(int i=0;i<NUMPIXELS;i++){
    pixels.setPixelColor(i, pixels.Color((i<=Nred))*255,0,0)); 
  }
  pixels.show();   // Only show after they are all set
  delay(100);

Somewhere, probably just ahead of the existing for loop, you need a for loop that turns off all LEDs.

Thanks, just made another one:

 for(int i=7;i>Nred-1;i--){


    pixels.setPixelColor(i, pixels.Color(0,0,0)); 

    pixels.show(); 

    delay(0); 

  }

Works great except it flickers quite a bit (even though I set both delays to 0). Is this due to the code structure itself?

Also, why should each for loop be in its own function?

More responsive would be:

Thanks! This fixes the flicker issue. Could you explain a little further about why/how this works:

 pixels.setPixelColor(i, pixels.Color((i<=Nred)*255,0,0));

fghfghdfghtrdhdtrf wrote:

Also, why should each for loop be in its own function?

They do not have to be in separate functions. It is just a matter of personal preference. The suggestions by johnwasser work also.

pixels.setPixelColor(i, pixels.Color((i<=Nred)*255,0,0));

works because i is less than or equal to Nred, or it is not. Thus i<=Nred is either 1 (true) or 0 (false). Thus 1255 is 255, and 0255 is 0. Thus the color of a pixel is either set to red or to black. It works, and probably blinks less.

Also eliminates one for loop, but may not be as clear to a newbie. I knew that my solution might blink but I wanted to work your way up to a better solution. This gets there in one step.

    delay(0);

Why? If you don’t want a delay, do not call the stupid function.

fghfghdfghtrdhdtrf:
Thanks! This fixes the flicker issue. Could you explain a little further about why/how this works:

 pixels.setPixelColor(i, pixels.Color((i<=Nred)*255,0,0));

As I said at the time: “These work because (i <= Nred) is either 1 (true) or 0 (false).” Surely you can do the math from there.
To make it a little more explicit you could use the ‘?:’ ternary operator:

 pixels.setPixelColor(i, pixels.Color((i<=Nred)?255:0,0,0));

That means “if (i<=Nred) is true, use the value 255 else use the value 0”. It’s like an if/else statement but inside an expression.