Switch interrupt problem

I am making some disco lights in the hope that one day when this current situation is all over I will be able to use them. It is just a fun project more than anything else using addressable LEDs. The problem is I am stuck on getting a function (Christmas lights style) switch to work. I want to simply change the sequence of LED animations when the user presses the button. I have it sort of working but i only works once. E.g when the user presses the button the sequence changes but the button doesn’t respond when I press again. E.g it will start with Sequence 0, then on the press of a button it runs Sequence 1, but if I press the button again it is still stuck at sequence 1.

I haven’t done much with the Arduino for a long time so I am very rusty so may be breaking all sorts of rules with my code. Can anybody spot the problem? Also serial print is not printing the value of the buttonflag variable but just an inverted question mark.

Here is the code.

#include <FastLED.h>

// How many leds in your strip?
#define NUM_LEDS 15

// For led chips like Neopixels, which have a data line, ground, and power, you just
// need to define DATA_PIN.  For led chipsets that are SPI based (four wires - data, clock,
// ground, and power), like the LPD8806 define both DATA_PIN and CLOCK_PIN
#define DATA_PIN 6
#define CLOCK_PIN 13

// Define the array of leds
CRGB leds[NUM_LEDS];
int speed = 70;
int speed2 = 95;
int speed3 = 100;
int colourcnt = 0;
int R = 0;
int G = 0;
int B = 0;
int buttonflag = 0;

const int buttonPin = 2; 

volatile int buttonState = 0;
void setup() { 
      Serial.begin(9600);
        FastLED.addLeds<WS2812B, DATA_PIN, GRB>(leds, NUM_LEDS); // for GRB LEDs
     
        pinMode(buttonPin, INPUT);
        attachInterrupt(0, pin_ISR, CHANGE);
  
}

void loop() { 
  Serial.print(buttonflag);
  if (buttonflag ==0){
sq0();

}

 
  
}

void pin_ISR() {

static unsigned long last_interrupt_time = 0;
 unsigned long interrupt_time = millis();

 if (interrupt_time - last_interrupt_time > 200)
    {
     //clear the LEDs
    for (int i = 0; i <= NUM_LEDS; i++) {
    leds[i] = CRGB::Black;
    FastLED.show();
    }
    //button has been pressed to increase variable value
buttonflag++;
Serial.print(buttonflag);
    if (buttonflag ==0){
    sq0();
    }
    if (buttonflag ==1) {
    sq1();
    }

    if (buttonflag ==2) {
    sq2();
    }
    if (buttonflag ==3) {
    sq3();
    }
    if (buttonflag > 3){
    buttonflag=0;
    sq0();
    }
    else {
    sqerror();
    }
 }
 last_interrupt_time = interrupt_time;
      
}
  
 void sq0(){
    leds[1] = CRGB::Red;
    FastLED.show();
    delay(1000);
   }


void sq1(){
    leds[2] = CRGB::Blue;
    FastLED.show();
    delay(1000);
 }
        
void sq2(){
    leds[3] = CRGB::Green;
    FastLED.show();
    delay(1000);
 }


void sq3(){
    leds[4] = CRGB::Red;
    FastLED.show();
    delay(1000);
 }
void sqerror(){
    leds[10] = CRGB::Red;
     leds[11] = CRGB::Red;
    FastLED.show();

 delay(1000);
}


void blackclear(){
  
for (int i = NUM_LEDS; i >= 1; i--) {
     leds[i] = CRGB::Black;
    FastLED.show();
    delay(speed); 
    }
 
}

I think I an implementing the interrupt all wrong and this may be the problem. Thanks for any help.

I have sorted the Serial print problem, I have now taken it out of the interrupt and put in the functions. I am confirm that the variable won't increase above one, so the interrupt function is only being called once.

You can't use FastLED methods in an ISR, either. You can NOT use delay() in an ISR or in any function called by an ISR.

You do NOT need an interrupt to read a switch pressed by a v e r y slow human.

What you MUST do is get rid of any blocking code so you can read the switch often enough.

You REALLY need to dump that code and start over.

Thanks So you mean I need just poll the switch in the loop, then write the LED flashing sequences while avoiding the use of delay full at all?

neworder82:
Thanks So you mean I need just poll the switch in the loop, then write the LED flashing sequences while avoiding the use of delay full at all?

Yes. There are plenty of examples around to help you. One even ships with the IDE, called Blink Without Delay.

The demo Several Things at a Time is an extended example of BWoD and illustrates the use of millis() to manage timing without blocking. It may help with understanding the technique.

Have a look at Using millis() for timing. A beginners guide if you need more explanation.

...R

Learn how to get rid of the delay() calls, and use millis)).
Also, you may swap the chain of multiple if() commands for a single switch() structure.

Thanks, I have completely written the code and I am now polling the switch with a denouncing routine which works perfectly. I just need to get my head around trying to animate 16 LEDs purely using mills now :). I have always struggled with using millis as a timer which is why I have often got lazy and just use delay which is fine if the program isn't expecting any sort of input.

You don’t really need those delays given your problem statement. You only want to change the animation when the user pressed the button. The issue is that you need to detect the button change, not the button state and only react when it goes from LOW->HIGH (I hope you have an external pull-down resistor on that button)

#include <FastLED.h>

// How many leds in your strip?
#define NUM_LEDS 15

// For led chips like Neopixels, which have a data line, ground, and power, you just
// need to define DATA_PIN.  For led chipsets that are SPI based (four wires - data, clock,
// ground, and power), like the LPD8806 define both DATA_PIN and CLOCK_PIN
#define DATA_PIN 6
#define CLOCK_PIN 13

// Define the array of leds
CRGB leds[NUM_LEDS];
int speed = 70;
int speed2 = 95;
int speed3 = 100;
int colourcnt = 0;
int R = 0;
int G = 0;
int B = 0;
int ledState = 0;

const int buttonPin = 2;

void setup() {
  Serial.begin(9600);
  FastLED.addLeds<WS2812B, DATA_PIN, GRB>(leds, NUM_LEDS); // for GRB LEDs

  pinMode(buttonPin, INPUT);
}

void loop() {

  checkButton();
  switch (ledState) {
    case 0:
      sq0();
      break;

    case 1:
      sq1();
      break;

    case 2:
      sq2();
      break;

    case 3:
      sq3();
      break;

    default:
      ledState = 0;
      break;
  }
}

void checkButton() {

  static int lastButtonState = digitalRead(buttonPin);
  int buttonState = digitalRead(buttonPin);

  if ( buttonState != lastButtonState ) {
    if (buttonState == HIGH) {
      ledState++;
    }
    delay(50);
  }
  lastButtonState = buttonState;
}

void sq0() {
  leds[1] = CRGB::Red;
  FastLED.show();
}


void sq1() {
  leds[2] = CRGB::Blue;
  FastLED.show();
}

void sq2() {
  leds[3] = CRGB::Green;
  FastLED.show();
}


void sq3() {
  leds[4] = CRGB::Red;
  FastLED.show();
}
void sqerror() {
  leds[10] = CRGB::Red;
  leds[11] = CRGB::Red;
  FastLED.show();
}


void blackclear() {

  for (int i = NUM_LEDS-1; i >= 0; i--) {
    leds[i] = CRGB::Black;
    FastLED.show();
    delay(speed);
  }
}

The trick to using millis() is to stop thinking “I need a delay ‘now’”... and work. at a higher level, with the millis timer calling the shots.

That is, “Is it time to do something?”... then do it now, otherwise continue whatever was happening.