Pages: [1] 2   Go Down
Author Topic: millis() accuracy  (Read 5793 times)
0 Members and 1 Guest are viewing this topic.
0
Offline Offline
Newbie
*
Karma: 0
Posts: 40
View Profile
WWW
 Bigger Bigger  Smaller Smaller  Reset Reset

Looking at the millis() implementation in wiring.c, I was wondering about the following edge case: suppose timer0_clock_cycles contains 15900 and TIMER0_OVF_vect fires once on a 16 MHz Arduino. So timer0_clock_cycles jumps from 15900 to 32284.

According to the logic in TIMER0_OVF_vect, the timer0_millis value will be incremented twice, returning with timer0_clock_cycles set to 284.

Now suppose delay(2) is called in loop() right after a TIMER0_OVF_vect interrupt occurred. Doesn't that mean that delay will return one millisecond later (just over 1024 microseconds, actually) instead of at least 2, as was probably expected?

I think the problem comes from running the timer interrupt slightly slower than 1 KHz, so once every second a timer "tick" will appear to occur twice in rapid succession, as the logic in TIMER0_OVF_vect adjusts (correctly) for the rate difference.

Wouldn't it be better to run let the timer overflow at 250? Or if that's not possible because of PWM, perhaps run it at twice the current rate? That way the jitter would drop from 100% to 50% max. Then again, that's still a lot of jitter when you need good millisecond timing...

-jcw
Logged

Connecticut, US
Offline Offline
Edison Member
*
Karma: 2
Posts: 1036
Whatduino
View Profile
WWW
 Bigger Bigger  Smaller Smaller  Reset Reset

Just my opinion, but if you need better timing than the default implementation, you're free to disconnect and reimplement the whole TIMER0 affair.

However, most people want a reasonable timer that is very easy to query and takes a minimum of system overhead.  Doubling the timer frequency means doubling the overhead of counting milliseconds.

If you can tweak the existing implementation so it does better without adjusting the timer/interrupt frequency, that would be good to hear.
Logged

0
Offline Offline
Newbie
*
Karma: 0
Posts: 37
Arduino rocks
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

As halley said, if you need better timing, don't use millis().

Letting the timer tick faster would increase the timing overhead (it takes 9us to execute the interrupt routine, but this could be greatly improved), but it would also cause problems with using delayMicroseconds(), for example. This routine turns off interrupts and so if one is waiting more than the timer's interval, it would cause incorrect reading for millis().
Logged

ottawa, canada
Offline Offline
God Member
*****
Karma: 6
Posts: 993
Arduino rocks
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

I have a (maybe) related problem that's driving me nuts.  It seems that each call to delay adds almost exactly 1ms of overhead.  for example, this
Code:
void loop(){
  unsigned long begint=millis();
  for (int i=0;i<100;i++){
    delay(5);
  }
  unsigned long endt=millis();
  Serial.println(endt-begint);
}
consistently returns results like 608.  The 8 ms seems like reasonable overhead for 100 loops but the extra 1 ms/delay() seems unreasonable.
Logged

Bill Rowe
Olduino - An Arduino for the First of Us
www.olduino.wordpress.com

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

@bill2009 - No, that's probably "by design". As of Arduino 0013 (I think), the delay(N) always takes at least N milliseconds. What happens is that delay first waits for a millisecond transition, and then counts N more transitions. Since you just passed a transition when exiting the loop, the next delay will skip the remaining part of that millisecond before counting to 5. Hence 6 per iteration.

I find the "8" more surprising, actually. A 0..100 loop takes only a fraction of one millisecond.

Anyway, it would be nice if we could reduce this sort of indeterminism.

Does your test always return 608 - never 607 or 609?
« Last Edit: March 21, 2009, 05:15:35 pm by jcw » Logged

0
Offline Offline
Newbie
*
Karma: 0
Posts: 37
Arduino rocks
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

As I said above, if you need higher accuracy, don't use millis(). The way millis(N) is written, it waits between N-1 to N+1 milliseconds, as jcw noted. That's usually ok for some dirty timing.
Logged

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

@mekon83 - Yes, good point. Still, we can try to improve what we have (which starts by understanding it better).

One way to avoid the 5 vs. 6 ms issue, would be for delay(N) to note the current TIMER0 count, and wait for it to reappear N times. But that ignores the 1024 usec vs 1000 usec aspect (timer0 overflows at 256, not 250). So here's another thought for delay(N), untested:

  long target = microseconds() + 1000 * N;
  while (microseconds() - target < 0)
     ;

IOW, drop the notion that delay() works with the jittery millisecond value, and use the new microsecond value instead.

The millisecond counter is still very useful for longer timescales, of course. As long as no-one disables interrupts too long, it's still a good way to track time. As for delayMicroseconds() - we could have it re-enable interrupts periodically when called with an argument larger than 900 or so.
Logged

0
Offline Offline
Newbie
*
Karma: 0
Posts: 37
Arduino rocks
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

@jcw: This is a really good idea. delay() could be rewritten this way and that would give us a much more reliable delay with an accuracy of a couple tens of microseconds.
Logged

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

Ok, I've made the following two changes to hardware/cores/arduino/wiring.c:
Code:
void delay(unsigned long ms)
{
      if (ms < 50) {
            delayMicroseconds(1000 * (uint8_t) ms);
            return;
      }
      
      unsigned long start = millis();
      
      while (millis() - start <= ms)
            ;
}
And inserted the following at the start of delayMicroseconds():
Code:
void delayMicroseconds(unsigned int us)
{
      if (us > 500) {
            unsigned long target = micros() + us;
            while ((long) (target - micros()) > 0)
                ;
            return;
      }

(sorry for the re-edits of this post)
« Last Edit: March 21, 2009, 07:54:17 pm by jcw » Logged

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

Ok,  I've stopped the tweaking. The above changes only affect microsecond delays > 500 us and millisecond delays < 50 ms. In that range, a loop is used which should be accurate in the couple-of-microsecond scale and which does not lock out interrupts.
Logged

ottawa, canada
Offline Offline
God Member
*****
Karma: 6
Posts: 993
Arduino rocks
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

Thank you very much for the explanation and the code.  What I'm doing is implementing a pedalling speed booster that tracks a bicycle pedal rotation and issues pulses 50X as fast(sort of like putting a card in the spokes).  It's not that great accuracy is needed but the 1ms was a real problem.  I'm going to take the liberty of using the code to implement a "fdelay(float)" which will simplify my code a lot - the perils of open source.

I am assuming that if I use the 1st version of it without the delayMicroseconds then I don't need to modify delayMicroseconds - is that fair?

Code:
void fdelay(float fms)
{
        unsigned long ms=fms;
      if (ms < 1000) {
            unsigned long target = micros() + 1000 * fms;
            while (target - micros() > 0)
                ;
            return;
      }
      unsigned long start = millis();

      while (millis() - start <= ms)
            ;
}
Logged

Bill Rowe
Olduino - An Arduino for the First of Us
www.olduino.wordpress.com

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

Looks good to me. If you want rounding, you could use "unsigned long ms=fms+0.5;".
Logged

ottawa, canada
Offline Offline
God Member
*****
Karma: 6
Posts: 993
Arduino rocks
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

ah, good, thanks.  
Logged

Bill Rowe
Olduino - An Arduino for the First of Us
www.olduino.wordpress.com

0
Offline Offline
Newbie
*
Karma: 0
Posts: 37
Arduino rocks
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

@bill2009: I'd say it is a really bad idea to do it this way, i.e. using float, since the float => int conversion and multiplication that you have there take 200us already, and if you include addition, another 60 us, that totals to 260us and your accuracy is gone =) Also, the code is 2800 byte larger just because of those 2 operations. Maybe the compiler can perform some optimizations if you use constants...
Logged

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

I really should test my code before posting here...

Had to make another adjustment to delayMicroseconds() to get it to work (now corrected in my earlier post). Sorry about the chatter.
Logged

Pages: [1] 2   Go Up
Jump to: