Inaccurate micros() with interrupts

I observed that the micros() function is not accurate when called inside an interrupt (Arduino pro 5V 16 mMHz)

The structure of the code is similar to the following:

volatile unsigned long trig_time = 0;
volatile byte trig_flag = 0;

void setup() {
  attachInterrupt(0, trig_detected, RISING);
}

void loop() {
  if (trig_flag == 1){
    // Do some timimg calculations with trig_time
    trig_flag = 0;
  }
  else {
    // Do other stuff
  }  
}

void trig_detected (){
  trig_time = micros();
  trig_flag = 1;
}

When I use a function generator to send trigger signal at well defined intervals of 100 000 us (100 ms), the timing calculated using the Arduino oscillate randomly between either 99 080 or 98 056us. The Arduino is thus making error of either 1 or 2 ms.

This issue is very similar to a problem recently observed for the Due . This problem was also reported a very long time ago and was reported as fixed since December 2009.

Apparently, the problem resides in the way that the function micros() handles interrupts. The code for micros() in wiring.c appears to be OK as the command cli() should deactivate the interrupts:

unsigned long micros() {
	unsigned long m;
	uint8_t oldSREG = SREG, t;
	
	cli();
	m = timer0_overflow_count;
#if defined(TCNT0)
	t = TCNT0;
#elif defined(TCNT0L)
	t = TCNT0L;
#else
	#error TIMER 0 not defined
#endif

  
#ifdef TIFR0
	if ((TIFR0 & _BV(TOV0)) && (t < 255))
		m++;
#else
	if ((TIFR & _BV(TOV0)) && (t < 255))
		m++;
#endif

	SREG = oldSREG;
	
	return ((m << 8) + t) * (64 / clockCyclesPerMicrosecond());
}

I made several tests, and I found that I can solve the problem for my application by simply adding sei() just before the return of the micros() command in wiring.c. This is somewhat surprising for me. My understanding is that adding sei() is not required as the interrupts would be naturally re-enabled and the end of the function.

I am missing something? Is this a bug? Has the way the complier interprets cli() and sei() changed since this function was written?

Thanks.

I'll probably will not notice this variation issue in my paintball chronograph post since I am not using interrupts I suppose?

DBgit:
The Arduino is thus making error of either 1 or 2 ms.

The other possibility, the more likely possibility, is that the error is in your code.

This problem was also reported a very long time ago and was reported as fixed since December 2009.

That's because the problem from that link was fixed a very long time ago...
https://github.com/arduino/Arduino/blob/master/hardware/arduino/cores/arduino/wiring.c#L80

Apparently, the problem resides in the way that the function micros() handles interrupts.

Or in the way your code is handling the value returned from micros. Or in the amount of time spent in the interrupt service routine.

I made several tests, and I found that I can solve the problem for my application by simply adding sei() just before the return of the micros() command in wiring.c.

Which is a very bad idea. And, it indicates the problem is an overrun caused by your code.

My understanding is that adding sei() is not required as the interrupts would be naturally re-enabled and the end of the function.

They are not. The interrupt flag is restored.

I am missing something?

Can't say. But we are. Your code.

Is this a bug?

Doubt it.

Has the way the complier interprets cli() and sei() changed since this function was written?

Nope.

DBgit:
The structure of the code is similar to the following:

Post the actual code, not something similar. That's like taking your daughter to the doctor and saying your son has similar symptoms.

(Arduino pro 5V 16 mHz)

I'm not surprised micros is inaccurate :wink: sp. "16MHz".

OK. Thanks for your help. I understand that my previous post did not contain enough information to find the source of the problem.

I took some more time today to look at this problem. I was able to make a very simple sketch that shows this inaccuracy I was describing in my previous post.

volatile unsigned long trig_time = 0;
volatile unsigned long last_trig_time = 0;
volatile byte trig_flag = 0;

void setup() {
  attachInterrupt(0, trig_detected, RISING);
  Serial.begin(115200);
}

void loop() {
  if (trig_flag == 1){
    noInterrupts(); // Remove interrupt because what follows would normally be time sensitive code.
      Serial.println(trig_time - last_trig_time); 
      delayMicroseconds(1000); // To simulate the time taken for the calculation of the time sensitive code.
      trig_flag = 0;
    interrupts();
  }

}

void trig_detected (){
  last_trig_time = trig_time;
  trig_time = micros();
  trig_flag = 1;
}

When I run this code with a function generator sending pulses at intervals of 100 000 us to pin 2, I get the following output in Serial Monitor:

99924         // This value is good enough. 
98900         // About 1024 us is missing here
99924
99928
99924
99924
98900         // About 1024 us is missing here
99924
98904         // About 1024 us is missing here
99924
99924
99924
99928
98900         // About 1024 us is missing here
99924

In other words, it looks like the we are missing 1024 us here and there while running this code. Note that the problem disappears if the noInterrupts() and interrupts() commands are removed from the main loop (i.e., all values are then about 99924 us).

I made some tests and it appears that the amount of inaccuracy depends on the time spent between the commands noInterrupts() and interrupts(). For example, by playing with the amount of delay in delayMicroseconds(1000), I found that some strange things occur. As the amount of delay is increased up to 16000us, the values reported by the program decrease down to either 84564 or 85588. On the other hand, when this delay is increased to 17000us, the values reported by the program jump back to 99924! :astonished: If this delay is increased to 18000us and above, the values reported by the program start decreasing again and show similar effects than when a delay of 1000 us is used. I should also mention that the code outputs values of about 100 000us for any delay if the noInterrupts() and interrupts() commands are removed from the code.

Note that I could also generate this effect when the function delayMicroseconds(1000) is replaced by some lengthy operations (e.g. analogRead(), pow() functions, etc.).

Is this something that should be expected when working within noInterrupts(), interrupts() and micros()?

Thanks.

AWOL:

(Arduino pro 5V 16 mHz)

I'm not surprised micros is inaccurate :wink: sp. "16MHz".

Corrected. Thanks for pointing this "small" error (I'm sure that a factor of 1 billion is considered "small" in some circumstance...)

DBgit:

  if (trig_flag == 1){

noInterrupts(); // Remove interrupt because what follows would normally be time sensitive code.
     Serial.println(trig_time - last_trig_time);
     delayMicroseconds(1000); // To simulate the time taken for the calculation of the time sensitive code.
     trig_flag = 0;
   interrupts();

In this section you have interrupts disabled for a shade over 1,000 microseconds.

micros() is, in part, powered by the Timer0 overflow interrupt which fires every ( 256 * 64 ) / 16E6 seconds, or once every 1,024 microseconds.

You're holding interrupts off for long enough to, once in a while, miss one, and therefore miss the 1,024 microseconds that would have been accumulated by the missed overflow.

micros() is, in part, powered by the Timer0 overflow interrupt which fires every ( 256 * 64 ) / 16E6 seconds, or once every 1,024 microseconds.

You're holding interrupts off for long enough to, once in a while, miss one, and therefore miss the 1,024 microseconds that would have been accumulated by the missed overflow.

Ah! Thanks of your fast answer. That explains it I guess...

My understanding was that noInterrupt() would prevent only the interrupts declared in the sketch... So, it means that we cannot use noInterrupt() to protect complex sections of code from external interrupt if we ever need to measure time... In fact, as most sketch would involve calls to millis() or micros(), I hardly understand how we are suppose to use noInterrupt() if it also affects the interrupts used for Timer0...

Would it be better to use the function detachInterrupt() instead to protect sections of code from external interrupts?

In this section you have interrupts disabled for a shade over 1,000 microseconds.

Agreed. You forgot to mention in your original post that you turned off interrupts for a long time.

Would it be better to use the function detachInterrupt() instead to protect sections of code?

That would be one approach. You could set up timer 1 to act as a timer, that will overflow less often, as it is a 16-bit timer.

However I don't understand why you need to turn off interrupts for so long.

You forgot to mention in your original post that you turned off interrupts for a long time.

Sorry for that. I was really not thinking that turning off the interrupts would create problems with timing. The description of noInterrupts() does mention that "Interrupts can slightly disrupt the timing of code". However, it does not mention that disabling the interrupts can disrupt the timing of code. I thus wrongly assumed that the problem came directly from the interrupt routine.

However I don't understand why you need to turn off interrupts for so long.

Well, it turns out that I don't really need to turn off the interrupts for very long. I wanted to turn them off mostly to avoid having multiple interrupts if the signal is bouncing after initial detection. For this application, I need to activate a digital pin very quickly after a trigger signal is detected (say less than 40 us). After, I know that the next detection event will not occur before 10 ms to 100 ms. I therefore take some time to make few analogRead and some calculations. I also had some lengthy Serial.print commands used for debugging (they are deactivated normally) when I found this problem. Not knowing that noInterrupts() would create problems with timing, I just put all of this code under this condition... :roll_eyes:

Thanks for you help.