Timer1 Interrupt Won't Run ISR

I have just written some code that sends a set of data over the serial port every 100ms using timer1 interrupts. I have set pin 13 to toggle every time it runs to get feedback on what it is doing. The problem is that this appears to toggle pin 13 once (ie turn it on) then stop, Pin 13 ceases to toggle and no data appears in the serial monitor. The problem appears to be with the for loop because without it the ISR runs fine. Any ideas on how to fix this?

byte data[64];
void setup(){
  noInterrupts();//Deactivate interrupts
  TCCR1A = 0;
  TCCR1B = 0;
  TCNT1  = 0;
  OCR1A  = 6250;
  TCCR1B |= (1<<WGM12)|(1<<CS12); // Set CTC and prescaler to clk/256
  TIMSK1 |= (1<<OCIE1A);//Enable timer compare interrupt
  interrupts();//Activate interrupts
  pinMode(13,OUTPUT);
  Serial.begin(115200); 
}
ISR(TIMER1_COMPA_vect){
  digitalWrite(13,!digitalRead(13));
  for(byte i=0;i<64;i++){
    Serial.print(data[i]);
  }
}

mgiigm:
I have just written some code that sends a set of data over the serial port every 100ms using timer1 interrupts. I have set pin 13 to toggle every time it runs to get feedback on what it is doing. The problem is that this appears to toggle pin 13 once (ie turn it on) then stop, Pin 13 ceases to toggle and no data appears in the serial monitor. The problem appears to be with the for loop because without it the ISR runs fine. Any ideas on how to fix this?

byte data[64];

void setup(){
 noInterrupts();//Deactivate interrupts
 TCCR1A = 0;
 TCCR1B = 0;
 TCNT1  = 0;
 OCR1A  = 6250;
 TCCR1B |= (1<<WGM12)|(1<<CS12); // Set CTC and prescaler to clk/256
 TIMSK1 |= (1<<OCIE1A);//Enable timer compare interrupt
 interrupts();//Activate interrupts
 pinMode(13,OUTPUT);
 Serial.begin(115200);
}
ISR(TIMER1_COMPA_vect){
 digitalWrite(13,!digitalRead(13));
 for(byte i=0;i<64;i++){
   Serial.print(data[i]);
 }
}

Typically, you want a I/O setup function that invokes sei() in order to enable interrupts.

See: avr-libc: A simple project

Would I be right in thinking that sei() is user to enable interrupts?
If so this should be the same as the interrupts() function. I tried adding sei() in it's place and, unfortunately, it made no difference :frowning:

ISRs need to be fast. Serial printing is not fast, and is generally not something you should do in an ISR.

The ISR sets a flag; loop() sees that the flag is set, and sends the data.

So instead i should put something in the loop? Something like:

if(bitRead(TIFR,OCF2A)){
digitalWrite(13,!digitalRead(13));
 for(byte i=0;i<64;i++){
   Serial.print(data[i]);
 }
}

PaulS:
ISRs need to be fast. Serial printing is not fast, and is generally not something you should do in an ISR.

The ISR sets a flag; loop() sees that the flag is set, and sends the data.

that's like a chicken and egg problem. If you ISR is just going to set a flag and then you check the flag in the loop, then you might as well just get rid of the whole ISR and just poll.

I think if what you need to do takes too long in an ISR or does something that will interfere with interrupts, then that means you should not be using ISR.

the code above inside loop() would seem more appropriate.

I think if what you need to do takes too long in an ISR or does something that will interfere with interrupts, then that means you should not be using ISR.

In general, I agree. The exception, as in this case, is when something needs to be done on a relatively regular basis. Having a timer generate a "It's time to do your thing" flag is easy. Then, loop() doesn't need to figure out if it's time to do something. It knows, because the flag is set.

Thanks for the help, I'll just use the flag in the loop :slight_smile:

mgiigm:
Would I be right in thinking that sei() is user to enable interrupts?
If so this should be the same as the interrupts() function. I tried adding sei() in it's place and, unfortunately, it made no difference :frowning:

Huh. I suppose so. I guess I was confused because the code didn't look like a sketch. Rather, it looked like AVRlib style C code.

I don't know if I'll be much help, but I have an alternate solution that you can consider, as long as you don't use delay() in your loop. It uses millis() to repeat an action.

Put this before the setup:

void timer(unsigned long interval, void (*g)()){
  static unsigned long prev = 0;
  if (millis() - prev >= interval){
    g();
    prev = millis();
  }
}
void printData(){
  digitalWrite(13,!digitalRead(13));
  for(byte i=0;i<64;i++){
    Serial.print(data[i]);
  }
}

And this in your loop:

timer(100, printData);

And I'm not sure that the digitalRead(13); will work either. If it doesn't do something else (clue: it requires a global or static boolean variable, and toggling it afterward.)

That's very similar to what I had before but i was trying to get it out of the loop. Also i didn't know you could feed a function as an argument into another function. Handy thing to know :slight_smile:

How many solutions worked, and what was the best one? Perhaps post your full code?

The last code worked great and I ended up using something very similar in the final sketch, I won't post it because all of it except a few lines have nothing to do with this post and they are already covered in this post.

dkl65:
I don't know if I'll be much help, but I have an alternate solution that you can consider, as long as you don't use delay() in your loop. It uses millis() to repeat an action.

Put this before the setup:

void timer(unsigned long interval, void (*g)()){

static unsigned long prev = 0;
 if (millis() - prev >= interval){
   g();
   prev = millis();
 }
}
void printData(){
 digitalWrite(13,!digitalRead(13));
 for(byte i=0;i<64;i++){
   Serial.print(data[i]);
 }
}




And this in your loop:


timer(100, printData);

Thanks! :slight_smile: So now your sketch works completely?
P.S. I'm almost a full member! :slight_smile: