Problem with Interrupt function

Other Hardware LEDs and Multiplexing
1

Hi

I have a project where I need to control a strip of ws2812B, 144 LED long, using the FastLed library.
The code should do:

When a momentary Button is pushed, the strip of LED is gradually lit, one LED at a time, each LED with a fade in from black to a green.
If the button has not been pushed again and the full strip has been lit, the program is at an end (lost).

My problem is that I can start the program but I cannot get it to listen to the second push of the button. I thought that using an Interrupt would solve my problem, but I cannot find my error.
Could someone help me?

Thank you thank you


#include "FastLED.h"
const int buttonPin = 2;
int ledPin = 13;
const int Beacon = 5;
#define PIN        4
int flag = 0;
int sflag = 0;
#define NUM_LEDS 143
#define COLOR_ORDER GRB
int fadeAmount = 20;
int brightness = 0;
int reset = 0;
CRGB leds[NUM_LEDS];
enum State {
 IDLE,
 START,
 ONE,
 TWO,
 WON,
 LOST,
};
State state = IDLE;
int i = 0;
int a = 0;
volatile int buttonState = 0;         // variable for reading the pushbutton status

void setup() {
 Serial.begin(9600);
 // initialize the LED pin as an output:
 pinMode(ledPin, OUTPUT);
 // initialize the pushbutton pin as an input:
 pinMode(buttonPin, INPUT);
 // Attach an interrupt to the ISR vector
 attachInterrupt(0, pin_ISR, CHANGE);
 FastLED.addLeds<WS2812B, PIN, COLOR_ORDER>(leds, NUM_LEDS).setCorrection( TypicalLEDStrip );
 FastLED.clear();
 FastLED.show();
}

void loop() {


 delay(40);
 if ( (digitalRead(buttonPin) == LOW) && (flag == 0) && (sflag == 0)) //change button state to 
 {
   flag = 1;
   sflag = 1;
   delay(50);
 }

 switch (state) {
   case IDLE:
     i = 0;     
     Serial.println("Idle");

     delay(40);
     state = START;
     break;

   case START:
     Serial.println("no button pressed"); // endless loop until button pressed for first time
     break;

   case ONE:

     Serial.println(" button pressed");
     delay(2000);
     flag = 1;                   //change button state to pressed once
     sflag = 0;
     delay(50);
     state = TWO;
     break;

   case TWO:

     for (int i = 0; i < NUM_LEDS; i++) // turn on 1 LED at a time using fade in (brightness increase)
     {
       for (int brightness = 0; brightness < 160;) // increase brightness from 0 to 160 
       {
         leds[i].setRGB(40, 160, 80); 
         leds[i].maximizeBrightness(brightness);
         FastLED.show();
         brightness = brightness + fadeAmount;
         delay(50);
       }

       for (int a = 0; a < 12;) { // LED pause 510ms before going on with the next
         a++;
         brightness = reset;   //reset brightness to 0
         delay(5);
       }
       a=0;
     }

     state = LOST;
     break;

   case WON:
     Serial.println("WON");      // button has been pressed twice WON
     FastLED.clear();
     FastLED.show();

     flag = 0;               //reset button
     sflag = 0;
     delay(5000);
     delay(2000);
     state = IDLE;
     break;

   case LOST:
     digitalWrite(Beacon, HIGH);   // button was not pressed again LOST Beacon light turns on

     flag = 0;               //reset button
     sflag = 0;
     Serial.println("LOST!!");
     delay(10000);
     digitalWrite(Beacon, LOW);
     FastLED.clear();
     FastLED.show();
     delay(50);
     state = IDLE;
     break;
 }
}

void pin_ISR() {

 if ( (digitalRead(buttonPin) == HIGH) && (flag == 1) && (sflag == 1)) // if there is nothing in front of the sensor
 {
   state = ONE;
 }
 if ( (digitalRead(buttonPin) == LOW) && (flag == 1) && (sflag == 0))
 {
   state = WON;
 }

}
2

How many interrupts will you get with a single push and release of the button?

3

Use of interrupts usually introduces more problems than it solves. If you can't read a single button in the main loop, you are certainly not going to do any better using an interrupt.

Your code breaks some of the rules, (like forgetting to declare all shared variables as volatile) but fixing that won't solve the problem.

4

It's nice to have a schematic and what processor you're using..

Any variable that is modified within an interrupt, needs to be declared volatile. You are modifying 'state' in the interrupt....

@Paul_KD7HB is correct...

FALLING for when the pin goes from high to low.

Change is probably not what you want, falling is probably what you want. It appears the pin goes low when it's pressed? With 'CHANGE' it will get called at least twice for every push of the button.

If so, you need to pull that pin high with an external or internal resistor. Most of these will allow a write to the input pin of a 1 and that will enable the internal pull-up resistor, one less component you have to use ...

If the above is the case, you have to debounce it. The machine are fast enough to detect the the bouncing that occurs when mechanical switches initially 'make' and bounce. You could detect multiple 'make' connections.

I always use the WDT as a debounce interrupt. You have to work with interrupts to understand how they work and what can happen if you don't hold your end up...

You need to be consistent, this takes time, but if you write it as most read it, it's easier to figure out what the code is supposed to do... This take time and you're doing fine at this point.

That is more confusing than "for (int a = 0; a < 12; ++a) { ..."

When you enter the loop, you bump a, so a, in the code, actually starts at 1 not 0.
Does 'a' run from 0 through 11 or 1 through 12?

It might not seem like much, but if you are indexing arrays you want 0 through 11 for a 12 element array, not 1 though 12 as in your 'loop'.

What is the intent of the last 'a' assignment? I don't see it used elsewhere ?

Is there some reason you wish to do this multiple times? If you take it out of the loop it will be reset after the loop executes...

Make use of the pre-processor, if you aren't going to change a value let the pre-processor put it in... You can define Beacon like you did PIN. No memory use and usually quicker during the code run

You can see how a lot of this works if you get the assembler listing off the compiler.

In situations like the multiple times you assign the same value... compilers are usually smart enough to 'optimize' this out of the loop... sometimes it actually removes a variable from memory and moves it around in registers, which is generally faster...

Good luck...

Have fun...

:smiley_cat:

5

Thanks for your reply,

As far as processor, I am using an Arduino Nano, using Arduino IDE1.8.19

On D2 I have the button, itself with a 10K Ohm resistor to GND
On D4 I have the LED, itself with a 470K Ohm to GND
On D5 I have the beacon light

In a normal “button” code ( like the one in the example), the button is LOW, when pressed HIGH. In my code the first button press ( HIGH), is recognized.
Then I also do a simple code:

if ( (digitalRead(buttonPin) == LOW) && (flag == 0) && (sflag == 0)) //change button state to
{
flag = 1;
sflag = 1;
delay(50);
}
if ( (digitalRead(buttonPin) == HIGH) && (flag == 0) && (sflag == 0)) //change button state to
{
flag = 1;
sflag = 0;
delay(50);
}
if ( (digitalRead(buttonPin) == HIGH) && (flag == 1) && (sflag == 0)) //change button state to
{
flag = 0;
sflag = 0;
delay(50);
}

All button changes are detected.

I think at the root my problem are when I run the i++ and the a++ routines, but these are essential to running the LED the way it needs to. I have tried my code without the interrupt. When I did, it did detect my second button click but would then continue until i = 144.

The “a” routine is just a time waster. It’d rather put a delay(510). It is a short pause before going on with the next LED. The idea here is so that the processor can hear if the button is being pressed in between.

With the brightness, every time a new LED is lit it goes first through the routine with the brightness, going from black to green. If I do not reset, the next LED in line will just turn on, instead of slowly turning on.

I will implement your changes and test and read and learn in between!

Thank you for your time

6

Another point: Variables used in interrupt handler should be volatile . In your case, flag, sflag, state should be volatile

7

If code is written correctly, it is not necessary to use interrupts to detect when a button is pressed, only digitalRead() is needed.

If code is not correctly written, using interrupts to detect button presses will not fix the problems, as you have discovered.

The code you have written (or copied without fully understanding) is written in a style often called "blocking code" because it blocks other tasks from being performed, such as checking for button presses.

Code which uses delay() for timing, especially if delay() is used inside for-loops, is blocking code.

I recommend you rewrite your code in a non-blocking style. This requires removing use of delay() (occasional use of short delay() can be ok) and use of millis() for timing. Any for-loops that contain delay() also need to be removed.

As your first step, study the "blink without delay" example sketch from IDE menu. There are more examples and explanations in the sticky posts at the top of the "Programming" section of the forum.

8

The input to D2 is floating from your software, you have to 'pull' it up or down so it's state is stable when the button is not pushed.

Is the switch wired to 5V so it 'pulls' it up?

I find it much easier to use the internal pull-ups and just wire the button to ground. It is how the chip was designed, less external components, lower cost/complexity.

It would eliminate your 10K resistor with just a change in how you 'look' at the 'state' of the switch...

I generally don't use 'delay()', it ties up parts of the hardware I'd rather use...

I'll have to see if I can dig up one of my routines for reading switches... Like a dummy I lost a drive that didn't have a good backup... :frowning:

Take care...

:smiley_cat:

9

This topic was automatically closed 180 days after the last reply. New replies are no longer allowed.