Is it possible to use a for loop inside ISR?

To manage a set of array of buttons/LEDs states in a for loop, I need to read multiple button states to toggle multiple corresponding LEDs inside the ISR.

The ISR works pefectly on a single button/LED set, but the code below is an array of buttons/LEDs set and it behaves unexpectedly mixing up the **i**ndex vaulues.

const int MAX = 2;
const int buttonPins[MAX] = {2, 3};    // the number of the pushbutton pin
const int ledPins[MAX] =  {13, 12};     // the number of the LED pin

// variables will change:
volatile int buttonStates[MAX] = {0, 0}, ledStates[MAX] = {0, 0};       // variable for reading the pushbutton status

void setup() {
  // initialize the LEDs pins as outputs:
  for (int i = 0; i < MAX; i++)
  {
    pinMode(ledPins[i], OUTPUT);
    digitalWrite(ledPins[i], ledStates[i]);

    // initialize the pushbuttons pins as inputs:
    pinMode(buttonPins[i], INPUT);
  }

  // Attach an interrupt to the ISR vector
  attachInterrupt(0, pins_ISR, CHANGE);
}

void loop() {
  // Nothing here!

}

void pins_ISR() {
  for (int i = 0; i < MAX; i++)
  {
    buttonStates[i] = digitalRead(buttonPins[i]);
    ledStates[i] = digitalRead(ledPins[i]);

    if (buttonStates[i] == LOW &&  ledStates[i] == 1) {
      digitalWrite(ledPins[i], LOW);
    }

    if (buttonStates[i] == LOW && ledStates[i] == 0) {
      digitalWrite(ledPins[i], HIGH);
    }
  }
}

What do you mean exactly, "behaves unexpectedly mixing up the index vaulues"? Can you please describe it in terms of button/LED behaviour instead?

What do you mean exactly, "behaves unexpectedly mixing up the index vaulues"? Can you please describe it in terms of button/LED behaviour instead?

Button 2 toggles LED 13 and sometimes both LEDs 12 & 13 together.

Button 3 does not toggle led 12 unless button 2 is kept pressed and then sometimes it does not do any toggling at all r it swap toggles the LEDs.

need to read multiple button states to toggle multiple corresponding LEDs inside the ISR.

No, you don't. Use of interrupts very often causes more problems than it solves, and yours is a good example.

None of what you are doing in the ISR should be there. Just set a flag indicating that the button was pushed, and do all that work outside of the interrupt. Then you can forget the "volatile" declarations, etc. and your life will become simpler.

And best of all, you don't need to worry about multibyte variables being corrupted by an interrupt, which, courtesy of switch bounce, could cause of the erratic behavior you observe.

I didn't unravel what it is you're trying to do, but I'll wager it doesn't need an interrupt in the first place and that polling the inputs will do the trick.

This says xy-problem to me, where you might be asking for help on an inappropriate solution rather than stating the underlying need.

How have you wired the buttons? This is for pin 2 only:

attachInterrupt(0, pins_ISR, CHANGE) ;

For pin 3, it is:

attachInterrupt(1, pins_ISR, CHANGE) ;

Or look at digitalPinToInterrupt(). For multiple buttons, pin change interrupts can be use instead of external interrupts, then you are not restricted to pins 2 and 3. However, as has been said, polling buttons is usually a better solution than using interrupts.

blomcrestlight:
I didn't unravel what it is you're trying to do, but I'll wager it doesn't need an interrupt in the first place and that polling the inputs will do the trick.

:o To manage a set of array of buttons/LEDs states in a for loop, I need to read multiple button states to toggle multiple corresponding LEDs :fearful:

This thing has already been done by using the button polling & state change method. I Just thought if the ISR was possible, and now know that it is not!

6v6gt:
How have you wired the buttons? This is for pin 2 only:

attachInterrupt(0, pins_ISR, CHANGE) ;

For pin 3, it is:

attachInterrupt(1, pins_ISR, CHANGE) ;

Or look at digitalPinToInterrupt(). For multiple buttons, pin change interrupts can be use instead of external interrupts, then you are not restricted to pins 2 and 3.

Good catch & points.

However, as has been said, polling buttons is usually a better solution than using interrupts.

Done this already and all good. Just thought to try and prove of the concept so I put an example to have the experts feedback.

Trying it again to feedback later. Hope it solves and proves possible!

6v6gt:
For pin 3, it is:

attachInterrupt(1, pins_ISR, CHANGE) ;

Hope this makes sense to anyone, as with above added, it maps each single button to toggle both LEDs at the same time, but after pressing the buttons a second time, it reads the proper button/LED mapping and toggles the correct LED per the button pressed!

So, I'm somewhat positive that the concept could work, otherwise it still should'nt work after both buttons were pressed and LEDs toggles once either! Right?

Is this an Arduino bug?

What bit of hardware do you have sitting on that ISR? If its something that just5 sends a periodic signal to "wake up" the button reader, the loop() function does exactly that job.

PaulMurrayCbr:
What bit of hardware do you have sitting on that ISR? If its something that just5 sends a periodic signal to "wake up" the button reader, the loop() function does exactly that job.

This is basically an example to test the if possible of using ISR for many buttons controlling many corresponding LEDs, instead of the usual polling/debouncing approach.

Just so anyone could spare few seconds to test and feedback with code fixes/examples. Simple as that.

Yes, it is possible. It is a bad idea.

To do it successfully, you will need to learn more about how to program with interrupts. Fortunately, there is much in the way of tutorials and reference material on line, as well as examples in this forum, so do your reading.

You can also search the forum, because about twice a week someone tries it and fails, needs a whole lot of help to get it going.

aarg:
You can also search the forum, because about twice a week someone tries it and fails, needs a whole lot of help to get it going.

Found this and thanks to Nick for the great article. Why has it not yet become part of Arduino refernce manual?

Just a link added to the interrups page!

There are circumstances where it is correct, even necessary to trigger an interrupt on a key press, for example if the Arduino is sleeping and the key press also has to force it to wake.

Here is an example based on bits from a working sketch I've put together to read a keyboard matrix. It uses pin change interrupts so you are not restricted to pins 2 and 3. The difference with external interrupts is that, having triggered the interrupt, you must then discover which pin triggered it. You may find something useful in it:

const uint8_t keypadRowPins[ 3 ] = { A2, A1, A0 } ;
bool keyPressIsrTrigger = false ;


void pciSetup(byte pin)
{
  // https://playground.arduino.cc/Main/PinChangeInterrupt/
  *digitalPinToPCMSK(pin) |= bit (digitalPinToPCMSKbit(pin));  // enable pin
  PCIFR  |= bit (digitalPinToPCICRbit(pin)); // clear any outstanding interrupt
  PCICR  |= bit (digitalPinToPCICRbit(pin)); // enable interrupt for the group
}


ISR( PCINT1_vect ) {
  // pin change interrupts Port C  ( A0 .. A3 )
  keyPressIsrTrigger = true ;
}


void setup() {

  for ( uint8_t i = 0; i < ROWS ; i++ )  {
    pciSetup( keypadRowPins[ i ]  )  ;
  }

}


void loop() {
  . . . 
  if ( keyPressIsrTrigger ) {

    // scan all rows
    for ( uint8_t i = 0 ; i < ROWS; i++ ) {
      pinMode( keypadRowPins[ i ] , INPUT_PULLUP ) ;
      if ( digitalRead( keypadRowPins[ i ] ) == LOW  ) {
        rowFound = i ;
        break ;
      }
    }
    keyPressIsrTrigger = false ;
  }
  . . . 
}

6v6gt:
There are circumstances where it is correct, even necessary to trigger an interrupt on a key press, for example if the Arduino is sleeping and the key press also has to force it to wake.

Yes. One such scenario would be a washing machine with multiple programs (Half Wash/Full Wash/Tumble Dry/Quick Rinse/...) that the program could be changed using interrupts while another program is running or need a pause/emergency stop or a timer increase/decrease.

6v6gt:
Here is an example based on bits from a working sketch I've put together to read a keyboard matrix. It uses pin change interrupts so you are not restricted to pins 2 and 3. The difference with external interrupts is that, having triggered the interrupt, you must then discover which pin triggered it. You may find something useful in it:

I coded a simple test using your example with debug outputs but it does not fire the interrups on key presses.

Is it the code missing something, or maybe the buttons wiring?

Each buttons' one end separately to the pins A0,A1,A3 and all ends to ground.

const byte ROWS = 3; //Three rows

const uint8_t keypadRowPins[ ROWS ] = { A2, A1, A0 } ;
bool keyPressIsrTrigger = false ;

uint8_t rowFound;

void pciSetup(byte pin)
{
  // https://playground.arduino.cc/Main/PinChangeInterrupt/
  *digitalPinToPCMSK(pin) |= bit (digitalPinToPCMSKbit(pin));  // enable pin
  PCIFR  |= bit (digitalPinToPCICRbit(pin)); // clear any outstanding interrupt
  PCICR  |= bit (digitalPinToPCICRbit(pin)); // enable interrupt for the group
}


ISR( PCINT1_vect ) {
  // pin change interrupts Port C  ( A0 .. A3 )
  keyPressIsrTrigger = true ;
}


void setup() {

  Serial.begin(9600);
  Serial.println("started");
  
  Serial.print("keyPressIsrTrigger: ");Serial.println(keyPressIsrTrigger);
  
  for ( uint8_t i = 0; i < ROWS ; i++ )  {
    pciSetup( keypadRowPins[ i ]  )  ;
    pinMode( keypadRowPins[ i ] , INPUT_PULLUP ) ;
  }

}


void loop() {

  if ( keyPressIsrTrigger ) {

    // scan all rows
    for ( uint8_t i = 0 ; i < ROWS; i++ ) {
      if ( digitalRead( keypadRowPins[ i ] ) == LOW  ) {
        rowFound = i ;
        Serial.print("keypadRowPins[");Serial.println(keypadRowPins[ rowFound ]);Serial.print("]");
        break ;
      }
    }
    keyPressIsrTrigger = false ;
    Serial.println(keyPressIsrTrigger);
  }

}

I've just tested it with this slightly varied version of your sketch on a Uno clone and it produces the following output.

const byte ROWS = 3; //Three rows

const uint8_t keypadRowPins[ ROWS ] = { A2, A1, A0 } ;
bool keyPressIsrTrigger = false ;

uint8_t rowFound;

void pciSetup(byte pin)
{
  // https://playground.arduino.cc/Main/PinChangeInterrupt/
  *digitalPinToPCMSK(pin) |= bit (digitalPinToPCMSKbit(pin));  // enable pin
  PCIFR  |= bit (digitalPinToPCICRbit(pin)); // clear any outstanding interrupt
  PCICR  |= bit (digitalPinToPCICRbit(pin)); // enable interrupt for the group
}


ISR( PCINT1_vect ) {
  // pin change interrupts Port C  ( A0 .. A3 )
  keyPressIsrTrigger = true ;
}


void setup() {

  Serial.begin(9600);
  Serial.println("started");
 
  Serial.print("keyPressIsrTrigger: ");Serial.println(keyPressIsrTrigger);
 
  for ( uint8_t i = 0; i < ROWS ; i++ )  {
    pciSetup( keypadRowPins[ i ]  )  ;
    pinMode( keypadRowPins[ i ] , INPUT_PULLUP ) ;
  }

}


void loop() {

  if ( keyPressIsrTrigger ) {

    // scan all rows
    for ( uint8_t i = 0 ; i < ROWS; i++ ) {
      if ( digitalRead( keypadRowPins[ i ] ) == LOW  ) {
        rowFound = i ;
        Serial.print("keypadRowPins[");Serial.print(keypadRowPins[ rowFound ]);Serial.println("]");
        break ;
      }
    }
    keyPressIsrTrigger = false ;
    Serial.println(keyPressIsrTrigger);
  }

}
21:38:14.444 -> keypadRowPins[15]
21:38:14.478 -> 0
21:38:14.478 -> keypadRowPins[15]
21:38:14.478 -> 0
21:38:14.478 -> keypadRowPins[15]
21:38:14.512 -> 0
21:38:14.817 -> keypadRowPins[15]
21:38:14.851 -> 0
21:38:14.851 -> keypadRowPins[15]
21:38:14.885 -> 0
21:38:14.885 -> keypadRowPins[15]
21:38:14.885 -> 0
21:38:14.885 -> keypadRowPins[15]
21:38:14.919 -> 0
21:38:15.668 -> keypadRowPins[15]
21:38:15.701 -> 0
21:38:15.870 -> keypadRowPins[15]
21:38:15.905 -> 0
21:38:15.905 -> keypadRowPins[15]
21:38:15.905 -> 0
21:38:15.940 -> keypadRowPins[15]
21:38:15.940 -> 0
21:38:15.940 -> keypadRowPins[15]
21:38:15.974 -> 0
21:38:16.671 -> keypadRowPins[15]
21:38:16.704 -> 0
21:38:16.704 -> keypadRowPins[15]
21:38:16.704 -> 0
21:38:16.872 -> keypadRowPins[15]
21:38:16.905 -> 0
21:38:16.905 -> keypadRowPins[15]
21:38:16.939 -> 0
21:38:23.961 -> keypadRowPins[14]
21:38:23.995 -> 0
21:38:24.302 -> keypadRowPins[14]
21:38:24.335 -> 0
21:38:24.781 -> keypadRowPins[14]
21:38:24.781 -> 0
21:38:24.918 -> keypadRowPins[14]
21:38:24.918 -> 0
21:38:25.198 -> keypadRowPins[14]
21:38:25.198 -> 0
21:38:25.232 -> keypadRowPins[14]
21:38:25.232 -> 0
21:38:25.232 -> keypadRowPins[14]
21:38:25.267 -> 0
21:38:25.369 -> 0
21:38:25.369 -> 0
21:39:59.609 -> keypadRowPins[16]
21:39:59.642 -> 0
21:39:59.850 -> 0
21:40:00.887 -> keypadRowPins[16]
21:40:00.922 -> 0
21:40:01.092 -> 0
21:40:01.301 -> keypadRowPins[16]
21:40:01.301 -> 0
21:40:01.301 -> keypadRowPins[16]
21:40:01.334 -> 0
21:40:01.470 -> keypadRowPins[16]
21:40:01.504 -> 0
21:40:01.504 -> keypadRowPins[16]
21:40:01.504 -> 0

I used pins A0, A1 and A2 and switched these to ground to trigger the interrupt.

6v6gt:
I've just tested it with this slightly varied version of your sketch on a Uno clone and it produces the following output.

I used pins A0, A1 and A2 and switched these to ground to trigger the interrupt.

All good and getting same results as you mentioned. Much appreciate your help.

Quote from: 6v6gt on Today at 11:15 am
There are circumstances where it is correct, even necessary to trigger an interrupt on a key press, for example if the Arduino is sleeping and the key press also has to force it to wake.

Yes. One such scenario would be a washing machine with multiple programs (Half Wash/Full Wash/Tumble Dry/Quick Rinse/...) that the program could be changed using interrupts while another program is running or need a pause/emergency stop or a timer increase/decrease.

I don't see why that would be a correct or even necessary circumstance. That could all be done without interrupts with non-blocking code.

evanmars:
Yes. One such scenario would be a washing machine with multiple programs (Half Wash/Full Wash/Tumble Dry/Quick Rinse/...) that the program could be changed using interrupts while another program is running or need a pause/emergency stop or a timer increase/decrease.

I don't see why that would be a correct or even necessary circumstance. That could all be done without interrupts with non-blocking code.

By non-blocking your hinting at the use of millis?

Any possibility for a simple code example running a 15 seconds countdown interruptible with multiple buttons to convey your method please?

Appreciated.