sei() and cli() inside interupt service routines

I'm studying the code which drives the colorduino. It sets up an interupt on overflow of timer 2, but uses sei() and cli() within the interrupt routine. Are they neccessary? Are they dangerous? I thought that when serviciing an interrupt sei and cli are handled "automatically".

Here is the code:

ISR(TIMER2_OVF_vect)          //Timer2  Service 
{ 
  cli();  
  TCNT2 = 0x64;      //flash a led matrix frequency is 100.3Hz,period is 9.97ms
  //TCNT2 = 0x63;      //flash a led matrix frequency is 99.66Hz,period is 10.034ms   
    if(line > 7) line = 0;    
    close_all_line; // switch off all 8 bits 
    run(line); 
    open_line(line); // switch on a give line
    line++;
    sei();
}

I'm studying the code which drives the colorduino. It sets up an interupt on overflow of timer 2, but uses sei() and cli() within the interrupt routine.

Are they neccessary?

No. On entry to a ISR, All interrupts are automatically disabled. So only if you need to re-enable them inside the ISR to allow for 'nested' interrupts to be serviced while inside your ISR.

Are they dangerous?

Probably more complex then dangerous, but certainly not for the weak of heart or mind. ;)

I thought that when serviciing an interrupt sei and cli are handled "automatically".

Yes, upon servicing the orginal interrupt all other interrupts are globably disabled and upon completion of the ISR, interrupts are re-enabled, all automatically. Only if you have a requirement for enabling interrupts while inside your ISR do you need to deal with sei() and cli().

Lefty

retrolefty: On entry to a ISR, All interrupts are automatically disabled. So only if you need to re-enable them inside the ISR to allow for 'nested' interrupts to be serviced while inside your ISR.

The code seems to include no other interrupt handlers so I imagine I can take out sei and cli, I'll try.

Thanks for the feedback!

ofransen:

retrolefty: On entry to a ISR, All interrupts are automatically disabled. So only if you need to re-enable them inside the ISR to allow for 'nested' interrupts to be serviced while inside your ISR.

The code seems to include no other interrupt handlers so I imagine I can take out sei and cli, I'll try.

Thanks for the feedback!

Well keep in mind that in an arduino the timer0 which is used to service the millis() and micros() are updated via timer0 interrupts and when inside your ISR they no longer update, but rather stay at their last value. Perhaps your program requires the millis() or micros() to remain active so it re-enables interrupts while inside the ISR?

Whatever the reason for the ISR having those commands are, it should be researched to understand the consequences of removing them, otherwise your sure to get unexpected behaviour.

Lefty

retrolefty: Well keep in mind that in an arduino the timer0 which is used to service the millis() and micros() are updated via timer0 interrupts and when inside your ISR they no longer update, but rather stay at their last value. Perhaps your program requires the millis() or micros() to remain active so it re-enables interrupts while inside the ISR?

Whatever the reason for the ISR having those commands are, it should be researched to understand the consequences of removing them, otherwise your sure to get unexpected behaviour.

Lefty

Some responses to your reply:

1) Thanks! I did not know about timer1 ! 2) But the code looks as if it disables then re-enables rather than the other way round. 3) It occurred to me that the Colorduino can be daisy chained for communications using I2C, would that complicate matters?

3) It occurred to me that the Colorduino can be daisy chained for communications using I2C, would that complicate matters?

Well the 328p avr chip uses it's I2C internal hardware and the required support software does utilize interrupts when active.

Lefty

retrolefty: Well the 328p avr chip uses it's I2C internal hardware and the required support software does utilize interrupts when active.

It works without those sei and cli statements, but when I get to the I2C part of the project I expect I'll have to put them back in.

As far as I can tell the cli() does absolutely nothing as interrupts are disabled anyway in the ISR.

The sei() may have an affect in that it allows any pending interrupts to happen before this one has quite finished. However why that would be a good idea escapes me.


Rob

Graynomad: As far as I can tell the cli() does absolutely nothing as interrupts are disabled anyway in the ISR.

The sei() may have an affect in that it allows any pending interrupts to happen before this one has quite finished. However why that would be a good idea escapes me.

Yes, if anything the order should be reversed if you wanted to respond to nested interrupts:

ISR(TIMER2_OVF_vect)          //Timer2  Service 
{ 
    sei () ; // respond to interrupts even inside this ISR!
    ... do stuff
    cli () ; // er... not needed?
}

I wonder if I need the sei() to catch any I2C data coming along? Tons of stuff to learn!

The sei() may have an affect in that it allows any pending interrupts to happen before this one has quite finished. However why that would be a good idea escapes me.

It is not a good idea. It solves no problems (the interrupt is enabled when the ISR returns; within a few machine instructions of the sei instruction) and it increases the chances of running out of SRAM.