button code not working as expected.

Hi,

I'm having an issue where my code is not working as expected, and would love some help on fixing it.

The code compiles and runs except the debounce does not seem to register when i can see no reason for it to.( holding the button flashes the lights for the matching button, and pressing once seems hit and miss to what state the lights will end up on).

i did have it working with my previous code, which i can post if needed. BUT i had a function and a button check copied 7 times and that didn't seem right.

thanks
Andy

#include <Adafruit_NeoPixel.h>
boolean oldState0 = HIGH;
int     mode0     = 0;   

#define PIXEL_PIN    10  // Digital IO pin connected to the NeoPixels.
#define PIXEL_COUNT 49  // Number of NeoPixels
Adafruit_NeoPixel strip(PIXEL_COUNT, PIXEL_PIN, NEO_GRB + NEO_KHZ800);

void setup() {
  Serial.begin(9600);
  strip.begin(); // Initialize NeoPixel strip object (REQUIRED)
  strip.setBrightness(100);
  strip.show();  // Initialize all pixels to 'off'
  for(int i=2; i<8; i++) { // set pins 2>9 as buttons
    pinMode(i, INPUT_PULLUP);
    }
 
}

void loop() {
buttonpress(2,0, 7);
buttonpress(3,7, 15);
buttonpress(4,14, 21);
buttonpress(5,21, 28);
buttonpress(6,28, 35);
buttonpress(7,35, 42);
buttonpress(8,42, 49);
}


void lights(uint32_t color, int wait,int from,int to) {
  for(int i=from; i<to; i++) { // count from start to end pixel for this call
    strip.setPixelColor(i, color);         //  Set pixel's color (in RAM)
    strip.show();                          //  Update strip to match
    delay(wait);                           //  Pause for a moment
  }
}

void buttonpress(int button,int first, int last){
boolean newState0 = digitalRead(button);
  if((newState0 == LOW) && (oldState0 == HIGH)) {
    delay(20);
    newState0 = digitalRead(button);
    if(newState0 == LOW) { 
      if(++mode0 ==2) mode0 = 0;
      switch(mode0) {           
        case 0:
          lights(strip.Color(  0,   0,   0), 10,first,last); 
          Serial.println("OFF"); 
          break;
        case 1:
          lights(strip.Color(255,   255,   0), 10,first,last);
          Serial.println("ON"); 
          break;
          }
  }
  }
  oldState0 = newState0; 
  
}

The most notable issue you have is you are sharing the same variable oldState0 for the previous state on all of your buttons. You need a separate one for each button. Use an array for the previous states or pass a reference or pointer to a previous state variable for the button.

You have a similar issue with the mode variable.

Ahh that makes a lot of sense. i had oldstate and mode 0>7 when i had it as repeated code. i will have a read up on arrays.

thankyou :slight_smile:

Also the following code is only configuring pins 2 thru 7:

  for (int i = 2; i < 8; i++) { // set pins 2>9 as buttons
    pinMode(i, INPUT_PULLUP);
  }

This might work. It compiles but I couldn't test it:

#include <Adafruit_NeoPixel.h>
#define PIXEL_PIN    10  // Digital IO pin connected to the NeoPixels.
#define PIXEL_COUNT 49  // Number of NeoPixels

Adafruit_NeoPixel strip(PIXEL_COUNT, PIXEL_PIN, NEO_GRB + NEO_KHZ800);

struct buttonData_t
{
  int pin;
  boolean oldState;
  int mode;
};

const int numButtons = 7;
buttonData_t buttonData[numButtons];


void setup() {
  Serial.begin(9600);
  strip.begin(); // Initialize NeoPixel strip object (REQUIRED)
  strip.setBrightness(100);
  strip.show();  // Initialize all pixels to 'off'
  
  // Configure button pins and data
  for (int idx = 0; idx < numButtons; idx++) {
    pinMode(idx+2, INPUT_PULLUP);
    buttonData[idx].pin = idx + 2;
    buttonData[idx].oldState = HIGH;
    buttonData[idx].mode = 0;
  }
}

void loop() {
  buttonpress(buttonData[0], 0, 7);
  buttonpress(buttonData[1], 7, 15);
  buttonpress(buttonData[2], 14, 21);
  buttonpress(buttonData[3], 21, 28);
  buttonpress(buttonData[4], 28, 35);
  buttonpress(buttonData[5], 35, 42);
  buttonpress(buttonData[6], 42, 49);
}


void lights(uint32_t color, int wait, int from, int to) {
  for (int i = from; i < to; i++) { // count from start to end pixel for this call
    strip.setPixelColor(i, color);         //  Set pixel's color (in RAM)
    strip.show();                          //  Update strip to match
    delay(wait);                           //  Pause for a moment
  }
}

void buttonpress(buttonData_t& button, int first, int last) {
  boolean newState = digitalRead(button.pin);
  if ((newState == LOW) && (button.oldState == HIGH)) {
    delay(20);
    if (++button.mode == 2) button.mode = 0;
    switch (button.mode) {
      case 0:
        lights(strip.Color(  0,   0,   0), 10, first, last);
        Serial.println("OFF");
        break;
      case 1:
        lights(strip.Color(255,   255,   0), 10, first, last);
        Serial.println("ON");
        break;
    }
  }
  button.oldState = newState;

}

well i did some reading on arrays lastnight and have it working just how it should. Arrays worked a charm.

the code isn't the neatest but each button state and mode is stored in arrays depending on which button is pressed

Thanks again :slight_smile:

#include <Adafruit_NeoPixel.h>
boolean oldState[]{1,1,1,1,1,1,1,1};
boolean newState[]{0,0,0,0,0,0,0,0};
int mode[]{0,0,0,0,0,0,0};

#define PIXEL_PIN    10  // Digital IO pin connected to the NeoPixels.
#define PIXEL_COUNT 60  // Number of NeoPixels
Adafruit_NeoPixel strip(PIXEL_COUNT, PIXEL_PIN, NEO_GRB + NEO_KHZ800);


void setup() {
  Serial.begin(9600);
  strip.begin(); // Initialize NeoPixel strip object (REQUIRED)
  strip.setBrightness(100);
  strip.show();  // Initialize all pixels to 'off'
  for(int i=2; i<9; i++) { // make 8 buttons
    pinMode(i, INPUT_PULLUP);
    }
 
}

void loop() {

buttonpress(2 ,0, 2, 255,255,0);// button, firstled, lastled, red, green, blue
buttonpress(3 ,2  ,4  ,0,255,0);
buttonpress(4 ,4  ,6  ,0,0,255);
buttonpress(5 ,6  ,8  ,255,100,150);
buttonpress(6 ,8  ,10 ,100,255,150);
buttonpress(7 ,10 ,12 ,100,100,255);
buttonpress(8 ,12 ,14 ,255,100,150);
}


void lights(uint32_t color, int wait,int from,int to) {
  for(int i=from; i<to; i++) { // count from start to end pixel for this call
    strip.setPixelColor(i, color);         //  Set pixel's color (in RAM)
    strip.show();                          //  Update strip to match
    delay(wait);                           //  Pause for a moment
  }
}

void buttonpress(int button,int first, int last,int red, int green, int blue){
newState[button-2] = digitalRead(button);
  if((newState[button-2] == 0) && (oldState[button-2] == 1)) {
    delay(20);
    newState[button-2] = digitalRead(button);
    if(newState[button-2] == 0) {
      if(++mode[button-2] ==2) mode[button-2] = 0;
      switch(mode[button-2]) {           
        case 0:
          lights(strip.Color(  0,   0,  0), 10,first,last);
          Serial.print("LIGHT "),Serial.print(button-2),Serial.println(" OFF");
          break;
        case 1:
          lights(strip.Color(red,   green,   blue), 10,first,last);
          Serial.print("LIGHT "),Serial.print(button-2),Serial.println(" ON");
          break;
          }
  }
  }
  oldState[button-2] = newState[button-2];
 
}