Go Down

Topic: Interrupts (Read 57834 times) previous topic - next topic

Coding Badly

Dec 18, 2009, 09:27 am Last Edit: Dec 19, 2009, 09:15 am by bcook Reason: 1
Some notes about interrupt service routines.  Included are some questions and answers to hopefully add clarity.




When an interrupt service routine (ISR) is called, interrupts are automatically disabled.  Interrupts are automatically enabled when returning from an ISR.  It is not necessary to explicitly enable nor disable interrupts inside of an ISR.  The processor does these two things for you.




Data that is shared with an ISR must be declared volatile...
  volatile unsigned long PulseCounts;

Data that is accessed by a single ISR and is not accessed outside the ISR does not need to be declared volatile.  

When in doubt, declare data volatile.  It's better to have correct reliable code than fast code.




Data that cannot be accessed atomically and is shared with an ISR needs to be protected by disabling interrupts during the access.  Access means reading or writing.  Only byte-sized data can be accessed atomically.  Anything larger than a byte needs to be protected.

Accessing shared data involves saving the interrupt flag, disabling interrupts, performing the access, then restoring the interrupt flag...

  unsigned long CalledFromLoopToGetPulseCounts( void )
 {
   unsigned long c;
   {
     uint8_t SaveSREG = SREG;   // save interrupt flag
     cli();   // disable interrupts
     c = PulseCounts;  // access the shared data
     SREG = SaveSREG;   // restore the interrupt flag
   }
   return( c );
 }


Q: Isn't retrieving the value of PulseCounts an indivisible call?

A: No.  Anything larger than a byte cannot be accessed atomically.  (That's not entirely true.  There are a few 16 operations that are atomic.  But, determining what C-code maps to atomic 16 machine instructions can be tricky and time-consuming.)

Q: It would matter if he were retrieving the value of PulseCounts, modifying it, and then writing it back, but in this case he is simply retrieving the value.

A: It is not possible for an 8 bit processor to perform any operation atomically on a 32 bit value.  Just reading the value requires four machine instructions; an interrupt can occur between any of them.




The vast majority of interrupt service routines written for the Arduino are not reentrant.  The process goes something like this: An enternal event occurs (like a pin changes state) that triggers the interrupt.  The processor stops what it's doing, globally disables interrupts, and calls the ISR.  The ISR handles the interrupt and returns.  The processor enables interrupts and resumes what it was doing before the interrupt occurred.

A reentrant ISR runs with interrupts enabled.  The process goes something like this: An enternal event occurs (like a pin changes state) that triggers the interrupt.  The processor stops what it's doing, globally disables interrupts, and calls the ISR.  The ISR enables interrupts, handles the interrupt, and returns.  The processor resumes what it was doing before the interrupt occurred.

Because the ISR enabled interrupts, any interrupt that occurs while the ISR is running interrupts the current ISR.  It is possible for a reentrant ISR to be interrupted and called multiple times.  As you might suspect, this kind of ISR is typically very complicated.  If not carefully written, there is a long list of problems that can occur.

An examle routine used to access data shared with an ISR...
  volatile unsigned long SomethingShared;

 void ProcessSomethingShared( void )
 {
   {
     cli();   // disable interrupts
     // do something interesting with SomethingShared
     sei();   // enable interrupts
   }
 }

Interrupts are disabled to safely access SomethingShared.  Instead of saving and restoring the interrupt flag, interrupts are explicitly enabled with sei.  ProcessSomethingShared is only called from loop so everything works as we expect it to work.  

Months pass.  Some new features need to be added.  We decide ProcessSomethingShared should be called during the interrupt and at a certain point in loop.  After some testing it's determined that the Sketch works correctly.  A few weeks pass and the Arduino locks-up.  A few more weeks pass and the Arduino locks-up again.  We have a problem and it's going to be extremely difficult to debug.

Fortunately, a keen eye notices the sei call.  Because interrupts are explicitly enabled in ProcessSomethingShared, the interrupt service routine has accidently become reentrant.  On the rare occassion when a pair of interrupts occurs in quick succession, the ISR is reentered and goes haywire.

So long as sei can never be executed in the context of an interrupt it is safe to use.  Saving the interrupt flag, disabling interrupts, then restoring the interrupt flag is always safe to use; the context is irrelevant.  When in doubt, use the save-disable-restore method.  It's better to have correct reliable code than a very nasty bug.




retrolefty

Nice writeup CB.

At some IDE version upgrade they added interrupts() http://arduino.cc/en/Reference/Interrupts

And noInterrupts() :http://arduino.cc/en/Reference/NoInterrupts to the core library.

Do these perform the same atomic protections that your method does? If so they might be simpler for beginners to understand.

Speaking of newcomers to programming with interrupts, they should probably be told (or shown) that a good ISR is written lean and mean, as short and fast as possible and if at all possible no function calls. Calling some functions like delay() inside a ISR will lock up the program.

Lefty

Coding Badly

Quote
Nice writeup CB

Thanks.  Unfortunately, I suspect my ISR experiences are going to leave me chained to this post!

Quote
Do interrupts() / noInterrupts() perform the same atomic protections that your method does?

They are aliases for sei() and cli().  Tomorrow I'll add a post regarding sei() (aka interrupts()) and why it should be avoided.

Quote
If so they might be simpler for beginners to understand.

That brings an idea to mind... ISR safe data-types (e.g. isr_uint_32).  If anyone is interested, please send me a Personal Message.

Quote
Speaking of newcomers to programming with interrupts, they should probably be told (or shown) that a good ISR is written lean and mean, as short and fast as possible and if at all possible no function calls.

Excellent idea.

Capio

Thanks, Coding Badly, for starting this thread.

I am just at the starting point where I've read enough to 'recognize' that I should use interrupts... and that I need to be rational about what I task the ISR to do (and consume). So I'll be watching this thread with interest.

FYI, my project is remote camera control. Will use two 4-ch keyfobs plus a separate x-y joystick. I recognize that I don't want have continuous monitoring of the 10 Arduino input pins (4+4+2), and that interrupts are a way to regulate input from these 3 devices. (I'll likely be posting my progress in a few weeks. Still at breadboarding and thinking.)

erisan500

So, if I understand all of the above, the following code should be ok?

Code: [Select]
// Count pulses on digital pins 2 and 3 using interrupts
// Print the counter values to serial every 30 secs and reset the counters to 0

#include <MsTimer2.h>

volatile unsigned int Counter1 = 0;
volatile unsigned int Counter2 = 0;

void setup()
{
 Serial.begin(9600);
 MsTimer2::set(30000, printCounters); // 30s period
 MsTimer2::start();

 attachInterrupt(0, increaseCounter1, FALLING);
 attachInterrupt(1, increaseCounter2, RISING);
}

void loop()
{

}

void increaseCounter1()
{
 Counter1++;
}

void increaseCounter2()
{
 Counter2++;
}

void returnCounter1()
{
 unsigned int c;
 {
   uint8_t SaveSREG = SREG;   // save interrupt flag
   noInterrupts();   // disable interrupts
   c = Counter1;  // access the shared data
   Counter1 = 0; // reset Counter1 to 0
   SREG = SaveSREG;   // restore the interrupt flag
 }
 return( c );
}

void returnCounter2()
{
 unsigned int c;
 {
   uint8_t SaveSREG = SREG;   // save interrupt flag
   noInterrupts();   // disable interrupts
   c = Counter2;  // access the shared data
   Counter2 = 0; // reset Counter2 to 0
   SREG = SaveSREG;   // restore the interrupt flag
 }
 return( c );
}

void printCounters()
{
 Serial.print("Counter1: ");
 Serial.println(returnCounter1);
 Serial.print("Counter2: ");
 Serial.println(returnCounter2);  
}


Feedback is welcome.

Greetings
EriSan500

GrooveFlotilla

Quote
Feedback is welcome.


"return" is not a function; it doesn't require parentheses.
Some people are like Slinkies.

Not really good for anything, but they bring a smile to your face when pushed down the stairs.

mem

#6
Dec 18, 2009, 11:52 am Last Edit: Dec 18, 2009, 11:59 am by mem Reason: 1
The parens are valid.
The original K&R used parens in return statements and and many an old programmer still uses the original form out of habit.

The parens were removed in the ANSI update to K&R but both styles are valid C. The first ANSI version of the C Programming language says:
"Parenthesis are often used around return expressions, but they are optional"

GrooveFlotilla

I didn't say they weren't valid, I said they weren't needed.

(And I *am* an old programmer   8-)
Some people are like Slinkies.

Not really good for anything, but they bring a smile to your face when pushed down the stairs.

mem

#8
Dec 18, 2009, 12:07 pm Last Edit: Dec 18, 2009, 12:08 pm by mem Reason: 1
what's wrong with using a coding style that may include some things not strictly needed.

For example, including parens around a composite expression, even if the associative rules don't require them, is also not needed. But its a stylistic decision that is often much appreciated even though its not required.

;)

erisan500

OK, i read the initial post over and over, but the more i read it, the more i'm getting confused.

Would it be possible to demonstrate the following 2 cases with some simple code examples (increasing and reading a counter) ?

Quote
Data that is shared with an ISR needs to be declared volatile...

 volatile unsigned long PulseCounts;


Quote
Data that is accessed by a single ISR does not need to be declared volatile.

mfm9

Quote
Data that is accessed by a single ISR does not need to be declared volatile.
CB,

Thanks for taking the time to do the ISR writeup.  It's very helpful.

I don't understand why 'volatile' is not necessary when only one ISR accesses the data.  I thought 'volatile' was to tell the compiler to generate a memory fetch every time the variable was accessed, as opposed to using a value from a register, in case the value changed in the middle of the code that is using it.

This change can occur even when only one ISR is changing the variable, so I'm confused by your statement.  Could you please expound?

I've been programming in 'C' since 1974, and to me, a return looks naked without its parens.  Just a matter of style.

Regards,

-Mike

GrooveFlotilla

#11
Dec 18, 2009, 02:24 pm Last Edit: Dec 18, 2009, 02:30 pm by GrooveFlotilla Reason: 1
Quote
I don't understand why 'volatile' is not necessary when only one ISR accesses the data


If only the ISR accesses the variable, then there's no other process that could be affected by the interrupt. The ISR can't be interrupted, and even if it were, the variable would not be accessed elsewhere.

Part of the reason for a "volatile" is if the non-interrupt code is, say, polling a variable. e.g.
Code: [Select]
while (variable == 0) {;}

There is nothing to stop the compiler here simply optimising-out accesses to "variable" beyond the first one, since "variable" (obviously) cannot take a new value within the body of the loop.
"volatile" ensures that "variable" is reloaded from memory each time it is compared.

Quote
I've been programming in 'C' since 1974

That's interesting - K&R was first published in 1978. Were you at Bell?
Some people are like Slinkies.

Not really good for anything, but they bring a smile to your face when pushed down the stairs.

mfm9

Groove,

I think I understand now.  When CB said, "when only one ISR accesses the data" I was thinking "as opposed to multiple ISRs accessing the data" all the while it was being accessed outside of the ISR.  What CB must have meant is "when the data is not accessed outside an ISR".  Thanks for clearing that up.

K & R?  We didn't need no stinkin' K & R!  I was at Georgia Tech starting in 1974.  Their Information and Computer Science Dept. had a PDP11/45 running Unix V6.  I may have exaggerated a bit -- I probably didn't start using C until 1975 or 1976.  The ICS Dept. was kind enough to let Mechanical Engineering majors like me use their UNIX machine.

I started out writing assembly language for the 11/45, but eventually switched to C.  At the time, the only formal CS training I had was an 101-level Fortran course.  I took the processor reference manual home over Christmas break and had to read it twice before I started to understand how a CPU works.  The rest is history :)

Regards,

-Mike

Coding Badly

@EriSan500...

Quote
So, if I understand all of the above, the following code should be ok?

Exactly!

Quote
OK, i read the initial post over and over, but the more i read it, the more i'm getting confused.

Did the explanations help?

I made some changes to the original post.  Does it make more sense now?

Quote
Would it be possible to demonstrate the following 2 cases with some simple code examples (increasing and reading a counter) ?

The code you posted is a very good example and is, in my opinion, well written.  The code very nicely illustrates the most common case (data shared with an ISR) and includes a nice twist (counter is reset).

Do I have permission to use your code as one of the examples?  In either case, I will try to add examples within a few days.

Thank you everyone for the kind words, good ideas, and critiques.  If you have any more to add, please do.

Coding Badly

@Mike Murdock...

Quote
When CB said, "when only one ISR accesses the data" I was thinking "as opposed to multiple ISRs accessing the data" all the while it was being accessed outside of the ISR.  What CB must have meant is "when the data is not accessed outside an ISR".

Exactly.  I modified the original post (I hope you don't mind that I used your words).  Does it make more sense?


Go Up