Pages: [1]   Go Down
Author Topic: delay() documentation include interrupt caveat  (Read 936 times)
0 Members and 1 Guest are viewing this topic.
Offline Offline
Newbie
*
Karma: 0
Posts: 2
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

Just found out the hard way that Arduino delay(xxx) does NOT work reliably in an interrupt service routine.
It does work: sometimes, kinda - which made debugging more difficult. I lost a few hours finding this undocumented "feature".
Easy workaround: for(j=xxx; j; j--) delayMicroseconds(1000);

My suggestion is to update the documentation for delay()
http://arduino.cc/en/Reference/Delay
to included in the "caveat" section
"delay() does NOT work reliably in an interrupt service routine"
to save debug time for other Arduino coders.

Thank you.
Logged

Global Moderator
Offline Offline
Brattain Member
*****
Karma: 452
Posts: 18694
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

http://arduino.cc/en/Reference/AttachInterrupt

Quote
Inside the attached function, delay() won't work and the value returned by millis() will not increment.
Logged

Global Moderator
Offline Offline
Brattain Member
*****
Karma: 452
Posts: 18694
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

Also read this:

http://www.gammon.com.au/interrupts

ISRs should be quick. Attempting to do a delay inside them goes against this fundamental principle. A delay, for example, would mean that the millis() timer would not increment, nor would serial data get processed. And other things.
Logged

Offline Offline
Newbie
*
Karma: 0
Posts: 2
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

Hi Nick,

Thank you for pointing out that the AttachInterrupt() documentation covers that delay() doesn't work in ISR. Also for the link to the extensive interrupt discussions! Very useful.

I admit I did not review the AttachInterrupt documentation, because I am not using that. I just set up a 1 second timer interrupt. The documentation for interrupts() doesn't cover this either. I still recommend that delay() documentation be updated to include a line that it won't work reliably inside interrupt routine.

I know interrupts should be quick. This application monitors slowly changing inputs and creates even slower outputs. One second resolution is sufficient. All the code is in the ISR and executes in less than 100mS, so mostly the processor is waiting. Probably it would be better to use the ISR to just set a flag, and pole that in the main loop.

void setup()
  {
...
  // disable global interrupts
  cli();
  // clear TCCR1A and TCCR1B
  TCCR1A = 0;
  TCCR1B = 0;
  // set compare match register to desired timer count:
  OCR1A = 15624;
  // turn on CTC mode:
  TCCR1B |= (1 << WGM12);
  // set CS10 and CS12 bits for 1024 prescaler:
  TCCR1B |= (1 << CS10);
  TCCR1B |= (1 << CS12);
  // enable timer compare interrupt:
  TIMSK1 |= (1 << OCIE1A);
  // enable global interrupts:
  sei();
  }

void loop()
  {
  // All action done in interrupt routine
  }

ISR(TIMER1_COMPA_vect)
  {
  // This is the Main Routine
  // delay() isn't reliable here
...
}
Logged

0
Offline Offline
Edison Member
*
Karma: 44
Posts: 1477
Arduino rocks
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

You can enable interrupts in an ISR and then delay will work.  Just be sure to return from the ISR before the next interrupt from the timer.

I use this trick in a number of libraries to get a high priority thread.  I did a library, WaveHC, for the Adafruit Wave shield that uses this thread to read data from an SD. Thousands of people use this library http://code.google.com/p/wavehc/.

It is extremely unlikely that the Arduino team will modify the documentation.  The Arduino corporation take very little input from users.
« Last Edit: July 27, 2012, 09:39:16 am by fat16lib » Logged

Global Moderator
Offline Offline
Brattain Member
*****
Karma: 452
Posts: 18694
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

Quote
Probably it would be better to use the ISR to just set a flag, and pole that in the main loop.

Much better.
Logged

0
Offline Offline
Edison Member
*
Karma: 44
Posts: 1477
Arduino rocks
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

If a flag will work, you don't need the timer ISR.  Just do something like this:
Code:
const uint32_t INTERVAL_USEC = 1000000;

// time in usec of last sensor read
uint32_t last = 0;

void setup() {
  Serial.begin(9600);
}

void loop() {
  // this works even when micros() rolls
  // over after about 4295 seconds.
  while ((micros() - last) < INTERVAL_USEC) {}
  // Replace  next println with the stuff in your ISR
  Serial.println(micros());
  last += INTERVAL_USEC;
}
This prints the time in usec with a jitter of a few usec with a one second interval.  Replace the println with whatever you do in the ISR.
Logged

SF Bay Area (USA)
Offline Offline
Tesla Member
***
Karma: 106
Posts: 6374
Strongly opinionated, but not official!
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

Do you also want the delay() documentation to mention that it won't work if you re-program the timer0 interrupts?

The Arduino documentation is aimed at the average Arduino user, who has no idea what it means if you say "delay() will not work when interrupts are disabled, as when running in an ISR."  If you step (pretty far) outside the Arduino environment by reprogramming a timer and using bare C interrupt functions, you're sort-of expected to investigate how that will interact with the Arduino core on your own.

While a more technical "Arduino core API reference" might be a desirable thing, I think the current source code is shorter...
Logged

Pages: [1]   Go Up
Jump to: