Pages: 1 [2] 3   Go Down
Author Topic: millis not accurate?  (Read 1829 times)
0 Members and 1 Guest are viewing this topic.
Grand Blanc, MI, USA
Offline Offline
Faraday Member
**
Karma: 93
Posts: 3972
CODE is a mass noun and should not be used in the plural or with an indefinite article.
View Profile
WWW
 Bigger Bigger  Smaller Smaller  Reset Reset

After almost 14 hours, my little test is within a second, maybe better, but I can't really measure closer than that.  So it seems that millis() has the potential to be fairly accurate, but certainly care must be taken as either hardware or software can compromise it.  I'd like to leave this running several days, but have other plans, so maybe another time.  Wouldn't want to draw too many conclusions from a single test, but gut feel is that this one example is keeping time at least as well as the average free-running DS1307, in my experience.
Logged

MCP79411/12 RTC ... "One Million Ohms" ATtiny kit ... available at http://www.tindie.com/stores/JChristensen/

Global Moderator
Offline Offline
Brattain Member
*****
Karma: 474
Posts: 18696
Lua rocks!
View Profile
WWW
 Bigger Bigger  Smaller Smaller  Reset Reset

edit: @ Nick Gammon I thought millis would still count during the delay()

Yes it will, but it depends where the delay ends up. eg.

Code:
-- about to tick (at 999)
delay (10);

-- notice tick now (it is 1009)
if(millis() - lastTick >= 1000)

You have the potential to lose 10 mS of accuracy (admittedly you would be unlucky) if you press the button on a 1-second boundary.
Logged

Global Moderator
Offline Offline
Brattain Member
*****
Karma: 474
Posts: 18696
Lua rocks!
View Profile
WWW
 Bigger Bigger  Smaller Smaller  Reset Reset

Also this:

Code:
void serialOutputTime()
{
  Serial.print(hour, DEC); // the hour, sent to the screen in decimal format
  Serial.print(":"); // a colon between the hour and the minute
  Serial.print(minute, DEC); // the minute, sent to the screen in decimal format
  Serial.print(":"); // a colon between the minute and the second
  Serial.println(second, DEC); // the second, sent to the screen in decimal format
}

Now, as I read it, Serial.print is blocking (I may be wrong, but I think it is). Now you are printing something like 12:45:22 which is 8 bytes, which at 9600 baud will take about 8.33 mS. Now if you "tick over" during that 8.33 mS (and remember that you are doing this every second) then you are potentially going to miss the next "tick" by up to 8 mS. And because your tick handling just starts the next 1000 mS when the previous one ends (rather than when it *should* end) you are likely to gradually miss time.

Quote
After running for 4 hours it is about 3 mins behind.

By my reckoning you are losing 12.5 mS every tick, that's quite a lot.

Code:
print ((3 * 60) / (60 * 60 * 4)) --> (3 minutes) / (4 hours) = 0.0125
Logged

Offline Offline
Newbie
*
Karma: 0
Posts: 11
www.gunnewiek.com
View Profile
WWW
 Bigger Bigger  Smaller Smaller  Reset Reset

Using the suggestions in the thread I think I fixed the overflow issue. I also change it to only output the time every minute, so that if printing to the serial causes time loss then it will disappear. Let me know if there are any obvious problems with this, but otherwise I will let it run for a couple of hours.
Code:
#define minButtonPin 2
#define hourButtonPin 3

int second=0, minute=0, hour=1;
boolean minButtonIsPressed = false;
boolean hourButtonIsPressed = false;

void setup(){
  Serial.begin(9600);
 
  pinMode(minButtonPin, INPUT);
  pinMode(hourButtonPin, INPUT);
}

void loop(){
  static unsigned long lastTick = 0; // holds the last time we moved forward one second
 
  /**********  CHECK BUTTONS  **********/
  // Check Minute Button
  if(buttonPressed(minButtonPin) && !minButtonIsPressed)
  {
    //Serial.println("Minute was pressed");
    minute++;
    minButtonIsPressed = true;
    serialOutputTime();
    delay(10);
  }
  else if(!buttonPressed(minButtonPin))
  {
    minButtonIsPressed = false;
    delay(10);
  }
  // Check Hour Button
  if(buttonPressed(hourButtonPin) && !hourButtonIsPressed)
  {
    //Serial.println("Hour was pressed");
    hour++;
    hourButtonIsPressed = true;
    serialOutputTime();
  }
  else if(!buttonPressed(hourButtonPin))
  {
    hourButtonIsPressed = false;
  }
 
  /**********  UPDATE TIME  **********/
  // Move forward one second every 1000 milliseconds
  unsigned long now = millis ();
  if(now - lastTick >= 1000)
  {
    lastTick = now;
    second++;
  }
  // Move forward one minute every 60 seconds
  if(second > 59)
  {
    minute++;
    second = 0;
    serialOutputTime();
  }
  // Move forward one hour every 60 minutes
  if(minute > 59)
  {
    hour++;
    minute = 0;
  }
  // Reset the hour
  if(hour > 12)
  {
    hour = 1;
  }
}

boolean buttonPressed(int aPin){
  if(digitalRead(aPin) == HIGH)
  {
    return true;
  }
 
  return false;
}

void serialOutputTime()
{
  Serial.print(hour, DEC); // the hour, sent to the screen in decimal format
  Serial.print(":"); // a colon between the hour and the minute
  Serial.print(minute, DEC); // the minute, sent to the screen in decimal format
  Serial.print(":"); // a colon between the minute and the second
  Serial.println(second, DEC); // the second, sent to the screen in decimal format
}
Logged


Fresno, CA, USA
Offline Offline
Full Member
***
Karma: 1
Posts: 153
Arduino rocks (literally, with a WaveShield!)
View Profile
WWW
 Bigger Bigger  Smaller Smaller  Reset Reset

Hm, couple of things to recap...

- Serial.print() is blocking, and VERY, VERY much so. During debugging of a program with AVR Studio, I actually had to jump over the Serial.print() commands, because they were taking so long in the debugger. That's why I figured the loops may be taking longer than 1ms and "overflowing" over the 1000ms conditional. It actually takes a significantly alarming amount of time to run a Serial.print() statement.
- I have the sudden urge to write an accurately coded clock, and upload it to 10 Arduino boards and just prove to these g*dd*amn knuckleheads that Arduino clock crystals and timing are just as accurate, if not more so, than a cheap chinese watch. There's a reason they have that number on the tin, and that's that THEY are calibrated for that frequency at THEIR factory, not the device they run on... it's not the responsibility of the device manufacturer to calibrate a device for crystal error; that's the crystal's job: to BE 100% accurate to within 3 decimal places. That's why you buy a crystal. I was asking a rhetorical question back there.
- I also posted code that should've fixed the overflow issue, but did it work? I dunno, I never even read any acknowledgement that I even posted any code. Also a rhetorical question here; I don't expect anyone to actually test it now that others have posted the same exact concept under their own nickname and it also worked  smiley-roll-blue

Eh, minor disappointments, but whatever works...
Logged

Offline Offline
Newbie
*
Karma: 0
Posts: 11
www.gunnewiek.com
View Profile
WWW
 Bigger Bigger  Smaller Smaller  Reset Reset

@FalconFour I did try your code but it work about the same as the others. I think that the serial.Print was the culprit, as when I changed it to display every minute things worked out much better. I have now changed it to every 15 mins and things are working out better still.
Logged


Global Moderator
Offline Offline
Brattain Member
*****
Karma: 474
Posts: 18696
Lua rocks!
View Profile
WWW
 Bigger Bigger  Smaller Smaller  Reset Reset

Sorry, FalconFour - I saw your code but wasn't totally certain. So I just expressed the idea (which you had) which was to count up from when it should have matched rather than when it did match, but without giving code.

But now I look at your code I'm not convinced:

Code:
if(millis() >= nextTick) {
    nextTick = 1000 - (millis() - nextTick) + millis();
    serialOutputTime();
    second++;
  }

Basically this will fail when millis() wraps around. Imagine once it becomes zero again (after 49.7 days) then it will stall because you are waiting for millis() (which is small) to become >= nextTick (which is large).

I think you need to subtract, something like this:

Code:
unsigned long now = millis()
unsigned long diff = now - lastTick;  // handles wrap-around
if (diff >= 1000)
  {
  unsigned long overshoot = diff - 1000;  // how many mS over are we?
  second++;  // one second later
  serialOutputTime();   // show the time
  lastTick = now - overshoot;   // last tick was actually potentially a bit earlier  
  }
Untested.
Logged

Fresno, CA, USA
Offline Offline
Full Member
***
Karma: 1
Posts: 153
Arduino rocks (literally, with a WaveShield!)
View Profile
WWW
 Bigger Bigger  Smaller Smaller  Reset Reset

True... I wasn't 100% on it myself but I was just a little ruffled to be kinda glazed-over being practically the first person to acknowledge that accurate timing even *could* be done, given accurately-timed code smiley-wink

Just mostly ruffled that the topic transitioned from "it can't be done" to "here's how to do it" without even recognizing the "oh, well, I guess it can be done!" aspect smiley-sweat

No biggie, I'm just glad it works as I wrote from the start... give a crystal-based Arduino good code, and it'll give you good time! Only thing to worry about is the millis() overflow every 49 days smiley

BTW, here's why millis() is so accurate... millis() runs in TIMER0, which overflows every 1.024 milliseconds on a 16MHz clock (every 16,384 clock cycles, with 256 values and a clock/64 prescaler):
Code:
SIGNAL(TIMER0_OVF_vect)
{
// copy these to local variables so they can be stored in registers
// (volatile variables must be read from memory on every access)
unsigned long m = timer0_millis;
unsigned char f = timer0_fract;

m += 1; // add 1 to timer0_millis
f += 3; // add 3 to timer0_fract (24 >> 3 to save bits)
if (f >= FRACT_MAX) { // if timer0_fract >= 125, add another 1ms since all the 0.024's added up
f -= FRACT_MAX; // but don't lose precision.
m += 1;
}

timer0_fract = f; // apply all we've counted so far
timer0_millis = m;
timer0_overflow_count++; //used to count cycles since this is incremented every 16,384 cycles
}
That's from wiring.c, modified with the values calculated in the macros, and with my comments describing what it does smiley

And millis() just pauses interrupts, reads timer0_millis, resumes interrupts, and returns timer0_millis. smiley
« Last Edit: June 19, 2011, 05:49:19 pm by FalconFour » Logged

Global Moderator
Offline Offline
Brattain Member
*****
Karma: 474
Posts: 18696
Lua rocks!
View Profile
WWW
 Bigger Bigger  Smaller Smaller  Reset Reset

FWIW I agree with you about the crystal. A crystal marked 16.000 is implying 62 ppm error (1 / 16000), whereas the observed accuracy was more like 12500 ppm.

Don't worry too much about being "blanked" - often people are happy when something is fixed, and don't comment, or unhappy when it isn't fixed, and don't acknowledge the intermediate steps that might have worked (ones they may not have even tried).
Logged

Global Moderator
Offline Offline
Brattain Member
*****
Karma: 474
Posts: 18696
Lua rocks!
View Profile
WWW
 Bigger Bigger  Smaller Smaller  Reset Reset

@dethredic - the other thing you can do is compensate, if it turns out the crystal is a bit fast or a bit slow (assuming it is consistent). Say you regularly are 1% slow, just shave 1% off the number of milliseconds you are checking for.
Logged

Australia
Offline Offline
Jr. Member
**
Karma: 0
Posts: 99
Arduino rocks
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

Coding_Badly's solution (page 1) is the correct one.

Only read millis() once per loop. If you're reading millis() more than once in your calculation, chances are you are accumulating overrun.

The essential idea is to only keep track of when you last *should* have triggered, not when you last actually did, that way any overrun is ignored in the calculation of the time at which you next should trigger, and therefore doesn't accumulate. You can't avoid a certain amount of overrun on each trigger, the best you can do is avoid its accumulation.

Set your first trigger time in setup(), then in your loop check to see if the current time exceeds this by your delay amount (1000). If so, add your delay (1000) to the previous trigger time and do your required work. That's all:

Code:
unsigned long trigger;
unsigned long delay = 1000;

setup()
{
  trigger = millis();
}

loop()
{
  if ( (millis() - trigger) > delay )  // Optionally use "while" instead of "if"
  {
    // Do your stuff here.

    trigger += delay;
  }
}

By using subtraction in the comparison you can ensure that rollover is handled correctly (see http://arduino.cc/forum/index.php/topic,42997.msg311529.html#msg311529)

This method also has the benefit that it will "catch up" if the processing is delayed for some reason, because you are only adding 1 second to the trigger each time through the loop, eg. if there is a 10 second delay for some reason (perhaps a very slow print every 10 mins or so), millis() will be 10000 counts ahead of the value of "trigger" so the loop body will execute 10 times in a row quickly until "trigger" is eventually ahead of millis().
« Last Edit: June 19, 2011, 09:26:37 pm by crimony » Logged

Global Moderator
Offline Offline
Brattain Member
*****
Karma: 474
Posts: 18696
Lua rocks!
View Profile
WWW
 Bigger Bigger  Smaller Smaller  Reset Reset

I agree, that looks neat and simple.
Logged

Fresno, CA, USA
Offline Offline
Full Member
***
Karma: 1
Posts: 153
Arduino rocks (literally, with a WaveShield!)
View Profile
WWW
 Bigger Bigger  Smaller Smaller  Reset Reset

Talk about thinking outside the box... I really really like that solution! Here, stuck on calculating per loop, I never even thought of looking at millis() on the outside. Very good idea. It also accounts for the overflow, while the "trigger" will also overflow just the same!
Logged

Offline Offline
Newbie
*
Karma: 0
Posts: 11
www.gunnewiek.com
View Profile
WWW
 Bigger Bigger  Smaller Smaller  Reset Reset

Okay, after using the following code (more of a timer), my clock has fallen behind 40 seconds after 10 hours.

Code:
#define displayPin 2
#define goPin 3

int second = 0, minute = 0, hour = 0;
boolean displayIsPressed = false;
boolean goPinIsPressed = false;
boolean startClock = false;
unsigned long aTime = 0;

unsigned long trigger;
unsigned long secondDelay = 1000;

void setup(){
  Serial.begin(9600);
 
  pinMode(displayPin, INPUT);
  pinMode(goPin, INPUT);
 
  trigger = millis();
}

void loop(){ 
  /**********  CHECK BUTTONS  **********/
  // Display time Button
  if(buttonPressed(displayPin) && !displayIsPressed)
  {
    displayIsPressed = true;
    serialOutputTime();
  }
  else if(!buttonPressed(displayPin))
  {
    displayIsPressed = false;
  }
  // Go button
  if(buttonPressed(goPin) && !goPinIsPressed)
  {
    goPinIsPressed = true;
    startClock = true;
    aTime = millis();
  }
  else if(!buttonPressed(goPin))
  {
    goPinIsPressed = false;
  }
 
 
  if(startClock == true)
  {
    updateTime(aTime);
  }
}

void updateTime(unsigned long adjustment){
  /**********  UPDATE TIME  **********/
  // Move forward one second every 1000 milliseconds
  while(millis() - adjustment - trigger >= secondDelay)
  {
    second++;
    trigger += secondDelay;
  }
  // Move forward one minute every 60 seconds
  if(second > 59)
  {
    minute++;
    second = 0;
  }
  // Move forward one hour every 60 minutes
  if(minute > 59)
  {
    hour++;
    minute = 0;
  }
}

boolean buttonPressed(int aPin){
  if(digitalRead(aPin) == HIGH)
  {
    return true;
  }
 
  return false;
}

void serialOutputTime()
{
  Serial.print(hour, DEC); // the hour, sent to the screen in decimal format
  Serial.print(":"); // a colon between the hour and the minute
  Serial.print(minute, DEC); // the minute, sent to the screen in decimal format
  Serial.print(":"); // a colon between the minute and the second
  Serial.println(second, DEC); // the second, sent to the screen in decimal format
}

This is a much better error, but I would still like to make it a little closer, so I will try adding an adjustment.
By my calculations It seems my crystal is off by 40 seconds every 10 hours or 36000 seconds, so to compensate for this:

40 / (10 * 60 * 60) = 0.00111111111

Now:

0.00111111111 * 1 000 = 1.11111111

So I will adjust my "secondDelay" from 1000 to 998.99999.

Let me know if there are any errors in my math.
Logged


Global Moderator
Dallas
Offline Offline
Shannon Member
*****
Karma: 200
Posts: 12782
View Profile
WWW
 Bigger Bigger  Smaller Smaller  Reset Reset

Quote
Let me know if there are any errors in my math.

It doesn't matter.  You need to fix the bug in your program first and then rerun the test.  I believe removing this line of code solves the problem...

Code:
  trigger = millis();
Logged

Pages: 1 [2] 3   Go Up
Jump to: