ISR Doesn't stop a FOR loop

Hi,

Essentially, my program has an ISR as follows...

void pin_ISR() {

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

  buttonState = digitalRead(button);

  
if (interrupt_time - last_interrupt_time > 1000) 
 {
//if(PIND & (1<<PIND6)){
Serial.print("interrupt \n");
  Serial.print("mode button pressed! \n");
      currentMode = currentMode +1;
        modeSelect();


        if(currentMode > 4){
          
       currentMode = 0;
       
  
        }
 }

My button interrupt increments through 4 modes, mode 3 contains a for loops than continuously loops up and down to change the brightness of an LED via PWM, so the loop is continuous. However, when I am in mode 3, my interrupt pin is not working, it never moves on to mode 4 because of the FOR loop in mode 3, why cannot my interrupt stop the FOR loop and move onto the next function?

We can't see your code.

Don't do serial I/O in interrupt context.

this is my function than never gets interrupted

void mode3(){

r = (stage * log10(2))/(log10(255));



  for (stage = 0; stage<255; stage ++){
  Serial.print("up-loop");
  

   brightness = pow(2,(stage/r)) - 1; //exponential change


  OCR0B = brightness + 1;
   //delay(50);
   Serial.print(OCR0B);
    Serial.print('\n');

    if (stage >= 254){
    
        for (stage = 255; stage>0; stage --){
          Serial.print("down-loop");
          
   brightness = pow(2,(stage/r)) + 1;

  OCR0B = brightness - 1;
   //  delay(50);
   Serial.print(OCR0B);
    Serial.print('\n');

}

}

}

}

Jozef1990:
this is my function than never gets interrupted

How did you arrive at that conclusion?

because the loop within the function just runs and runs and does not respond to my interrupt button.

The problem is in the code you forgot to post.

#include <avr/io.h>
#include <avr/interrupt.h>

int t1;
int t2;
int t3;
int t4;
int t5;
int t6;
int t7;
int t8;
int t9;
int t10;
int button = 2;
volatile bool interrupted = 0;
int currentMode;
int stage = 255;  //stages of PWM level
float r; //calculation for the PWM brightness exponential curve

volatile int buttonState = 0;     // variable for reading the pushbutton status

int brightness = 0;

//forward declaration of functions
void mode0(void);
void mode1(void);
void mode2(void);
void mode3(void);
void mode4(void);
void modeSelect(void);

volatile uint8_t duty=64;


int period = 1000;

unsigned long time_now = 0;

void setup() {
  
  PINC|=(1<<PINC5);
Serial.begin(9600);

TCCR0A|= (1<<COM0B1)|(1<<WGM01)|(1<<WGM00); //setting timer0 up for fast pwm with 0xFF as top and toggling OC0A/B on match with OCR0A/B respectively
TCCR0B|= (1<<CS00)|(1<<WGM02); //setting timer0 pre-scaler to 1, this allows a pwm frequency of OCR0A
OCR0A=128;        //setting period
OCR0B=0;        //setting initial duty




pinMode(t1,   OUTPUT);
pinMode(t2,   OUTPUT);
pinMode(t3,   OUTPUT);
pinMode(t4,   OUTPUT);
pinMode(t5,   OUTPUT);
pinMode(t6,   OUTPUT);
pinMode(t7,   OUTPUT);
pinMode(t8,   OUTPUT);
pinMode(t9,   OUTPUT);
pinMode(t10,  OUTPUT);

pinMode(button,  INPUT_PULLUP);

DDRD|= (1<<DDD5);  //setting PWM port as output


 attachInterrupt(0, pin_ISR, FALLING); //attach interrupt to mode button

}

void loop() {

}

void pin_ISR() {

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

  buttonState = digitalRead(button);

  
if (interrupt_time - last_interrupt_time > 1000) 
 {
//if(PIND & (1<<PIND6)){
Serial.print("interrupt \n");
  Serial.print("mode button pressed! \n");
      currentMode = currentMode +1;
        modeSelect();


        if(currentMode > 4){
          
       currentMode = 0;
       
  
        }
 }
      
     last_interrupt_time = interrupt_time;
}


//All lights on steady
void mode0(){
  TCCR0A|= (1<<COM0B1)|(1<<WGM01)|(1<<WGM00); //setting timer0 up for fast pwm with 0xFF as top and toggling OC0A/B on match with OCR0A/B respectively
TCCR0B|= (1<<CS00)|(1<<WGM02); //setting timer0 pre-scaler to 1, this allows a pwm frequency of OCR0A
OCR0A=128;        //setting period
OCR0B=0;        //setting initial duty



  digitalWrite(t1,  HIGH);
  digitalWrite(t2,  HIGH);
  digitalWrite(t3,  HIGH);
  digitalWrite(t4,  HIGH);
  digitalWrite(t5,  HIGH);
  digitalWrite(t6,  HIGH);
  digitalWrite(t7,  HIGH);
  digitalWrite(t8,  HIGH);
  digitalWrite(t9,  HIGH);
  digitalWrite(t10, HIGH);

}

//chase (slow)
void mode1(){
TCCR0A|= (1<<COM0B1)|(1<<WGM01)|(1<<WGM00); //setting timer0 up for fast pwm with 0xFF as top and toggling OC0A/B on match with OCR0A/B respectively
TCCR0B|= (1<<CS00)|(1<<WGM02); //setting timer0 pre-scaler to 1, this allows a pwm frequency of OCR0A
OCR0A=128;        //setting period
OCR0B=0;        //setting initial duty

 
//for(;;); //run forever


OCR0B=0x68; //full brightness




}
//chase (fast)
void mode2(){

OCR0B=0x68; //full brightness





}

//fade(all)
void mode3(){

r = (stage * log10(2))/(log10(255));



  for (stage = 0; stage<255; stage ++){
  Serial.print("up-loop");
  

   brightness = pow(2,(stage/r)) - 1; //exponential change


  OCR0B = brightness + 1;
   //delay(50);
   Serial.print(OCR0B);
    Serial.print('\n');

    if (stage >= 254){
    
        for (stage = 255; stage>0; stage --){
          Serial.print("down-loop");
          
   brightness = pow(2,(stage/r)) + 1;

  OCR0B = brightness - 1;
   //  delay(50);
   Serial.print(OCR0B);
    Serial.print('\n');

}


}


}


}
//mix - (cycle through 30 seconds of each mode)
void mode4(){


  Serial.print("mode4");
}

void modeSelect(){


        switch(currentMode){

      case 0:
      Serial.print("mode0");
      mode0();
      break;
      

      case 1:
      Serial.print("mode1");
      mode1();
        break;
      


      case 2:
      Serial.print("mode2");
      mode2();
        break;
      
      

      case 3:
      Serial.print("mode3");
      mode3();
        break;
      
   

      case 4:
      Serial.print("mode4");
      mode4();
        break;

      default:
      break;
      
      
      
      }
}

Your mode3 function has no way of leaving. When your interrupt service routine runs, it will change the variable currentMode and then return to the mode3 function. But this function does not care about the currentMode variable and therefore will run forever.

If you add a check in your loop e.g.

if( currentMode != 3){return;}

the function will return to the caller, where you can then call mode4 or whatever you need.

Thanks, I can see exactly what you are saying but for some reason,

currentMode does not change whilst the loop is running, all that runs is that loop and the ISR doesn't pick-up the fact that a button is being pressed.

All variables that are changed in an ISR, such as currentMode, should be declared as volatile

currentMode should be qualified volatile.

Edit: pipped.

That's a good point but it still doesn't solve the problem

Have you taken the prints out of the interrupt service routine?

Hi, yes I have removed the Serial.print(s)

Please post your complete program as it is now

#include <avr/io.h>
#include <avr/interrupt.h>

int t1;
int t2;
int t3;
int t4;
int t5;
int t6;
int t7;
int t8;
int t9;
int t10;
volatile int button = 2;
volatile bool interrupted = 0;
volatile int currentMode = 0;
volatile int stage = 255;  //stages of PWM level
float r; //calculation for the PWM brightness exponential curve

volatile int buttonState = 0;     // variable for reading the pushbutton status

volatile int brightness = 0;

//forward declaration of functions
void mode0(void);
void mode1(void);
void mode2(void);
void mode3(void);
void mode4(void);
void modeSelect(void);

volatile uint8_t duty=64;


int period = 1000;

unsigned long time_now = 0;

void setup() {
  
  PINC|=(1<<PINC5);
Serial.begin(9600);

TCCR0A|= (1<<COM0B1)|(1<<WGM01)|(1<<WGM00); //setting timer0 up for fast pwm with 0xFF as top and toggling OC0A/B on match with OCR0A/B respectively
TCCR0B|= (1<<CS00)|(1<<WGM02); //setting timer0 pre-scaler to 1, this allows a pwm frequency of OCR0A
OCR0A=128;        //setting period
OCR0B=0;        //setting initial duty




pinMode(t1,   OUTPUT);
pinMode(t2,   OUTPUT);
pinMode(t3,   OUTPUT);
pinMode(t4,   OUTPUT);
pinMode(t5,   OUTPUT);
pinMode(t6,   OUTPUT);
pinMode(t7,   OUTPUT);
pinMode(t8,   OUTPUT);
pinMode(t9,   OUTPUT);
pinMode(t10,  OUTPUT);

pinMode(button,  INPUT_PULLUP);

DDRD|= (1<<DDD5);  //setting PWM port as output


 attachInterrupt(0, pin_ISR, FALLING); //attach interrupt to mode button

}

void loop() {

}

void pin_ISR() {

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

  buttonState = digitalRead(button);

  
if (interrupt_time - last_interrupt_time > 1000) 
 {
//if(PIND & (1<<PIND6)){
//Serial.print("interrupt \n");
//Serial.print("mode button pressed! \n");
      currentMode = currentMode +1;

//Serial.print("currentMode = ");
//Serial.print(currentMode);
      
        modeSelect();


        if(currentMode > 4){
          
       currentMode = 0;
       
  
        }
 }
      
     last_interrupt_time = interrupt_time;
}


//All lights on steady
void mode0(){
  TCCR0A|= (1<<COM0B1)|(1<<WGM01)|(1<<WGM00); //setting timer0 up for fast pwm with 0xFF as top and toggling OC0A/B on match with OCR0A/B respectively
TCCR0B|= (1<<CS00)|(1<<WGM02); //setting timer0 pre-scaler to 1, this allows a pwm frequency of OCR0A
OCR0A=128;        //setting period
OCR0B=0;        //setting initial duty



  digitalWrite(t1,  HIGH);
  digitalWrite(t2,  HIGH);
  digitalWrite(t3,  HIGH);
  digitalWrite(t4,  HIGH);
  digitalWrite(t5,  HIGH);
  digitalWrite(t6,  HIGH);
  digitalWrite(t7,  HIGH);
  digitalWrite(t8,  HIGH);
  digitalWrite(t9,  HIGH);
  digitalWrite(t10, HIGH);

}

//chase (slow)
void mode1(){
TCCR0A|= (1<<COM0B1)|(1<<WGM01)|(1<<WGM00); //setting timer0 up for fast pwm with 0xFF as top and toggling OC0A/B on match with OCR0A/B respectively
TCCR0B|= (1<<CS00)|(1<<WGM02); //setting timer0 pre-scaler to 1, this allows a pwm frequency of OCR0A
OCR0A=128;        //setting period
OCR0B=0;        //setting initial duty

 
//for(;;); //run forever


OCR0B=0x68; //full brightness




}
//chase (fast)
void mode2(){

OCR0B=0x68; //full brightness





}

//fade(all)
void mode3(){

r = (stage * log10(2))/(log10(255));



  for (stage = 0; stage<255; stage ++){
  Serial.print("up-loop");
  

   brightness = pow(2,(stage/r)) - 1; //exponential change


  OCR0B = brightness + 1;
   //delay(50);
   Serial.print(OCR0B);
    Serial.print('\n');

    if (stage >= 254){
    
        for (stage = 255; stage>0; stage --){
          Serial.print("down-loop");
          
   brightness = pow(2,(stage/r)) + 1;

  OCR0B = brightness - 1;
   //  delay(50);
   Serial.print(OCR0B);
    Serial.print('\n');
if( currentMode !=3){
  return;
}
 }
  }
   }





}
//mix - (cycle through 30 seconds of each mode)
void mode4(){


  Serial.print("mode4");
}

void modeSelect(){


        switch(currentMode){

      case 0:
      Serial.print("mode0");
      mode0();
      break;
      

      case 1:
      Serial.print("mode1");
      mode1();
        break;
      


      case 2:
      Serial.print("mode2");
      mode2();
        break;
      
      

      case 3:
      Serial.print("mode3");
      mode3();
        break;
      
   

      case 4:
      Serial.print("mode4");
      mode4();
        break;

      default:
      break;
      
      
      
      }
}

Why have you qualified a pin number as volatile?

It would be polite to auto-format your code before posting.

And don't do serial I/O in interrupt context.
This has already been mentioned.

Code posted properly, after formatting and removing about 100 lines of junk.

#include <avr/io.h>
#include <avr/interrupt.h>

int t1;
int t2;
int t3;
int t4;
int t5;
int t6;
int t7;
int t8;
int t9;
int t10;
volatile int button = 2;
volatile bool interrupted = 0;
volatile int currentMode = 0;
volatile int stage = 255;  //stages of PWM level
float r; //calculation for the PWM brightness exponential curve

volatile int buttonState = 0;     // variable for reading the pushbutton status

volatile int brightness = 0;

//forward declaration of functions
void mode0(void);
void mode1(void);
void mode2(void);
void mode3(void);
void mode4(void);
void modeSelect(void);

volatile uint8_t duty = 64;
int period = 1000;
unsigned long time_now = 0;

void setup() {
  PINC |= (1 << PINC5);
  Serial.begin(9600);
  TCCR0A |= (1 << COM0B1) | (1 << WGM01) | (1 << WGM00); //setting timer0 up for fast pwm with 0xFF as top and toggling OC0A/B on match with OCR0A/B respectively
  TCCR0B |= (1 << CS00) | (1 << WGM02); //setting timer0 pre-scaler to 1, this allows a pwm frequency of OCR0A
  OCR0A = 128;      //setting period
  OCR0B = 0;      //setting initial duty
  pinMode(t1,   OUTPUT);
  pinMode(t2,   OUTPUT);
  pinMode(t3,   OUTPUT);
  pinMode(t4,   OUTPUT);
  pinMode(t5,   OUTPUT);
  pinMode(t6,   OUTPUT);
  pinMode(t7,   OUTPUT);
  pinMode(t8,   OUTPUT);
  pinMode(t9,   OUTPUT);
  pinMode(t10,  OUTPUT);
  pinMode(button,  INPUT_PULLUP);

  DDRD |= (1 << DDD5); //setting PWM port as output
  attachInterrupt(0, pin_ISR, FALLING); //attach interrupt to mode button
}

void loop() {
}

void pin_ISR() {
  static unsigned long last_interrupt_time = 0;
  unsigned long interrupt_time = millis();

  buttonState = digitalRead(button);
  if (interrupt_time - last_interrupt_time > 1000)
  {
    currentMode = currentMode + 1;
    modeSelect();
    if (currentMode > 4) {
      currentMode = 0;
    }
  }
  last_interrupt_time = interrupt_time;
}

//All lights on steady
void mode0() {
  TCCR0A |= (1 << COM0B1) | (1 << WGM01) | (1 << WGM00); //setting timer0 up for fast pwm with 0xFF as top and toggling OC0A/B on match with OCR0A/B respectively
  TCCR0B |= (1 << CS00) | (1 << WGM02); //setting timer0 pre-scaler to 1, this allows a pwm frequency of OCR0A
  OCR0A = 128;      //setting period
  OCR0B = 0;      //setting initial duty
  digitalWrite(t1,  HIGH);
  digitalWrite(t2,  HIGH);
  digitalWrite(t3,  HIGH);
  digitalWrite(t4,  HIGH);
  digitalWrite(t5,  HIGH);
  digitalWrite(t6,  HIGH);
  digitalWrite(t7,  HIGH);
  digitalWrite(t8,  HIGH);
  digitalWrite(t9,  HIGH);
  digitalWrite(t10, HIGH);
}

//chase (slow)
void mode1() {
  TCCR0A |= (1 << COM0B1) | (1 << WGM01) | (1 << WGM00); //setting timer0 up for fast pwm with 0xFF as top and toggling OC0A/B on match with OCR0A/B respectively
  TCCR0B |= (1 << CS00) | (1 << WGM02); //setting timer0 pre-scaler to 1, this allows a pwm frequency of OCR0A
  OCR0A = 128;      //setting period
  OCR0B = 0;      //setting initial duty
  OCR0B = 0x68; //full brightness
}
//chase (fast)
void mode2() {
  OCR0B = 0x68; //full brightness
}

//fade(all)
void mode3() {
  r = (stage * log10(2)) / (log10(255));
  for (stage = 0; stage < 255; stage ++) {
    Serial.print("up-loop");
    brightness = pow(2, (stage / r)) - 1; //exponential change
    OCR0B = brightness + 1;
    Serial.print(OCR0B);
    Serial.print('\n');
    if (stage >= 254) {
      for (stage = 255; stage > 0; stage --) {
        Serial.print("down-loop");
        brightness = pow(2, (stage / r)) + 1;
        OCR0B = brightness - 1;
        //  delay(50);
        Serial.print(OCR0B);
        Serial.print('\n');
        if ( currentMode != 3) {
          return;
        }
      }
    }
  }
}
//mix - (cycle through 30 seconds of each mode)
void mode4() {
  Serial.print("mode4");
}

void modeSelect() {
  switch (currentMode) {
    case 0:
      Serial.print("mode0");
      mode0();
      break;
    case 1:
      Serial.print("mode1");
      mode1();
      break;
    case 2:
      Serial.print("mode2");
      mode2();
      break;
    case 3:
      Serial.print("mode3");
      mode3();
      break;
    case 4:
      Serial.print("mode4");
      mode4();
      break;
    default:
      break;
  }
}

Since the loop function does nothing, this is one large ISR that is activated only by a falling interrupt on a button pin. It has many problems, including lots of serial prints within the ISR, failure to protect multibyte shared variables from corruption, etc. Very poor programming practice.

Most importantly, the loop structure in mode3() can't work, because the loop variable "stage" is modified within the loop. The mode3 function needs to be completely rewritten.

Please drop the idea of using an ISR to read a button.

?? there is no serial.print in the interrupt they have been commented out...

jremington - it may be "junk" to you but I was told to paste my entire code, so that is what i did