Button Counting issues

I am attempting to use button counting to cycle through different options in my program, specifically how fast a strip of 7 NeoPixels cycle through a rainbow patern.

The issue I am running into is that the only time the change in state of the push button is being read is at the end of the function that cycles the rainbow. Meaning only a perfectly timed button press gets the counter to increment.

Here is the code that I am running:

#include <Adafruit_NeoPixel.h>

#define PIN 6
#define Pix_Num 7

#define switchP 2

int state =0;
int lastState =0;

int counter =0;

Adafruit_NeoPixel strip = Adafruit_NeoPixel(Pix_Num, PIN, NEO_GRB + NEO_KHZ800);


void setup() {
  pinMode(switchP, INPUT);
  strip.begin();
  strip.show(); // Initialize all pixels to 'off'
}

void loop(){
  state = digitalRead(switchP);
  if (state != lastState){
    if (state == HIGH){
      counter++;
    }
    lastState = state;
  }
  
  if (counter == 0){
    rainbowCycle(2);
  }
  else if (counter == 1){
    rainbowCycle(10);
  }
  else if (counter == 2){
    rainbowCycle(20);
  }
  
  if (counter>2){
    counter = 0;
  }
}

void rainbowCycle(uint8_t wait) {
  uint16_t i, j;

  for(j=0; j<256; j++) { // 5 cycles of all colors on wheel
    for(i=0; i< strip.numPixels(); i++) {
      strip.setPixelColor(i, Wheel(((i * 256 / strip.numPixels()) + j) & 255));
    }
    strip.show();
    delay(wait);
  }
}

uint32_t Wheel(byte WheelPos) {
  if(WheelPos < 85) {
   return strip.Color(WheelPos * 3, 255 - WheelPos * 3, 0);
  } else if(WheelPos < 170) {
   WheelPos -= 85;
   return strip.Color(255 - WheelPos * 3, 0, WheelPos * 3);
  } else {
   WheelPos -= 170;
   return strip.Color(0, WheelPos * 3, 255 - WheelPos * 3);
  }
}

What am I doing wrong here?

any help is appreciated.

You could use interrupts to increase "counter" and Case statements instead of if(counter == 0), if(counter == 1). . .
Give it a try. And please format your code press CTRL + T or EDIT -> auto-format.

Interrupts paired with a switch statement worked like a charm thank you very much.

Interrupts paired with a switch statement worked like a charm thank you very much.

Now, try doing it the right way. Get rid of the for loops and the delay()s. Implement a state machine and the blink without delay philosophy.

Quite right; putting in an interrupt means you won't miss a button press, but does nothing to improve latency.

I know it's not a proper solution, but it is a quick and very dirty solution to a small piece of code. If it was any longer then yes, I would have told him to use IF statements instead of the FOR loops, and to take out the delay.

Well I am really just learning this stuff, when I come up with an idea I try to splice together all the little things I have learned from reading book after book and pages and pages of arduino sketches (however un-commented).

If there is a better way still I would like to hear it.

Here is the code as I have it now.

#include <Adafruit_NeoPixel.h>

#define PIN 6
#define Pix_Num 7

#define switchP 2

const int button = 0;

volatile int b = 0;

Adafruit_NeoPixel strip = Adafruit_NeoPixel(Pix_Num, PIN, NEO_GRB + NEO_KHZ800);

void setup() {
attachInterrupt(button, selectSpeed, RISING);
strip.begin();
strip.show(); // Initialize all pixels to 'off'
}

void loop(){
switch(b){
case 0:
rainbowCycle(2);
break;
case 1:
rainbowCycle(10);
break;
case 2:
rainbowCycle(20);
break;
}

}

void selectSpeed(){
++b %= 3;
}

void rainbowCycle(uint8_t wait) {
uint16_t i, j;

for(j=0; j<256; j++) { // 5 cycles of all colors on wheel
for(i=0; i< strip.numPixels(); i++) {
strip.setPixelColor(i, Wheel(((i * 256 / strip.numPixels()) + j) & 255));
}
strip.show();
delay(wait);
}
}

uint32_t Wheel(byte WheelPos) {
if(WheelPos < 85) {
return strip.Color(WheelPos * 3, 255 - WheelPos * 3, 0);
} else if(WheelPos < 170) {
WheelPos -= 85;
return strip.Color(255 - WheelPos * 3, 0, WheelPos * 3);
} else {
WheelPos -= 170;
return strip.Color(0, WheelPos * 3, 255 - WheelPos * 3);
}
}

it is working pretty much how I want it to minus the fact that even after a button press it must finish what ever loop it is on before changing the speed.

again I am just trying to learn all of this, if there is a better way to do it I would love for some one to tell me about it.

It should look like this. Note I did not test this. I most likely have something in the wrong spot.

You need to add a new unsigned long variable, oldTime = millis(); at the top of your code.

void rainbowCycle(uint8_t wait) 
{
  uint16_t i, j;
  
  if(j < 256)
  {
    if(i < strip.numPixels() ) 
    {
      strip.setPixelColor(i, Wheel(((i * 256 / strip.numPixels()) + j) & 255));
      i++;
    }
    else {
      strip.show();
      if( (millis() - oldTime) > wait)
      {
        oldTime = millis();
        i = 0;
        j++;
      }
    }
  }
  else j = 0;
}

uint32_t Wheel(byte WheelPos) {
  if(WheelPos < 85) {
    return strip.Color(WheelPos * 3, 255 - WheelPos * 3, 0);
  } 
  else if(WheelPos < 170) {
    WheelPos -= 85;
    return strip.Color(255 - WheelPos * 3, 0, WheelPos * 3);
  } 
  else {
    WheelPos -= 170;
    return strip.Color(0, WheelPos * 3, 255 - WheelPos * 3);
  }
}

One more thing, you'll want to reset variables i and j back to zero when the button is pressed, So add i = 0; j=0; in your selectSpeed function.