Pages: [1] 2 3   Go Down
Author Topic: millis not accurate?  (Read 2002 times)
0 Members and 1 Guest are viewing this topic.
Offline Offline
Newbie
*
Karma: 0
Posts: 11
www.gunnewiek.com
View Profile
WWW
 Bigger Bigger  Smaller Smaller  Reset Reset

After running for 4 hours it is about 3 mins behind. I assume this is a problem with my code:

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();
    delay(10);
  }
  else if(!buttonPressed(hourButtonPin))
  {
    hourButtonIsPressed = false;
    delay(10);
  }
 
  /**********  UPDATE TIME  **********/
  // Move forward one second every 1000 milliseconds
  if(millis() - lastTick >= 1000)
  {
    lastTick = millis();
    serialOutputTime();
    second++;
  } 
  // 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;
  }
  // 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

Everything looks OK to me, like using a variable to hold the value of millis() instead of using delay() at the end of each loop... but... I do see a little problem:

Code:
 /**********  UPDATE TIME  **********/
  // Move forward one second every 1000 milliseconds
  if(millis() - lastTick >= 1000)
  {
    lastTick = millis();
    serialOutputTime();
    second++;
  }
Looks great at first until you realize that (millis() - lastTick) may actually be greater than 1000 (time it takes to run each loop), but is still treated functionally like 1000ms. So if it ends up being 1005, 1002, etc., it gets shaved off and you just lost 2-5 ms.

I'd say a quick fix may be to use a destination stamp instead, also so the CPU can do a quick "compare-to-zero" instead of comparing with a 16-bit integer ("1000"):
Code:
 /**********  UPDATE TIME  **********/
  // Move forward one second every 1000 milliseconds

  if(millis() >= nextTick) {
    nextTick = 1000 - (millis() - nextTick) + millis();
    serialOutputTime();
    second++;
  }
Yeah, it calls millis() twice, but it might be the most accurate way to get the job done. Since millis() will always be greater than nextTick when the "if" is true, we can use millis() - nextTick to find the amount of error. Then, we assign nextTick as 1000 - (error) + millis(), which may end up being, say, 998ms from now (millis()) if there was a 2ms error.

It may seem like excessive accuracy, but micros() may prove to be a better count of time...
« Last Edit: June 18, 2011, 05:00:28 pm by FalconFour » 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

^ I should mention that I have been using a software real-time clock in my car's MPGuino (which uses a 20MHz crystal) and has been 100% accurate to the minute over the course of 3+ weeks running 24/7, so I think it's perfectly suitable for timekeeping, thank you very little.
Logged

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

thank you very little.

lol
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

So you're saying that dedicated thru-hole mount crystals marked "16.000 MHz" (note the triple-digit precision marked on the tin) have the potential to be sloppier than a $1, vending-machine-purchased, Chinese-made piece-of-junk digital watch that also keeps near-perfect time?  smiley-draw
Logged

Left Coast, CA (USA)
Offline Offline
Brattain Member
*****
Karma: 361
Posts: 17301
Measurement changes behavior
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

What does the math say? 3 mins in 4 hours is 1.25% slow. That is a couple of magnitudes higher then the normal crystal tolerance error of 100-150 parts per million, so I would not look first at the crystal to be the problem.

My guess is you are dealing with a math problem in your code that is dropping mill counts. But I can only leave that to the software gurus around here, hardware is my thing.  smiley-wink

Lefty

Logged

Left Coast, CA (USA)
Offline Offline
Brattain Member
*****
Karma: 361
Posts: 17301
Measurement changes behavior
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

Quote
That doesn't change the facts.  Ceramic resonators are even less precise than crystals, and you are seeing more and more microcontrollers (including Arduino boards and clones) using ceramic resonators because there is simply no expectation of long-term timekeeping.

I agee about the "long-term timekeeping" comment not being a good application for the millis() function on a arduino board. However I still say his 3 mins error over 4 hours (-1.25%) is way beyond the frequency tolerence of even ceramic resonators (typically +/- .1% ?) specifications. I still think the sketch has a problem causing the lose of time at that high of a rate.


Lefty

Logged

Grand Blanc, MI, USA
Offline Offline
Faraday Member
**
Karma: 95
Posts: 4089
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

Resonators are more like ±0.5%.  The OP didn't say what the hardware was?  So I might think there could be something else going on.  OTOH, my first reaction was that nothing was wrong, as I don't consider millis() suitable in general for long-term timekeeping.  But lefty is right smiley-lol erm, correct in that the observed error is still over twice what we might expect from a resonator-driven clock.

Now the spooky part is that just before reading this post, just for fun, I'd started a sketch on a breadboarded "Arduino" with a ±10ppm crystal, and am comparing it with a NIST-synchronized RTC.  I'll report back tomorrow.  (So far it's right on, but has only been running 20 minutes, LOL.)
Logged

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

Left Coast, CA (USA)
Offline Offline
Brattain Member
*****
Karma: 361
Posts: 17301
Measurement changes behavior
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

Quote
But lefty is right  erm, correct

 
Lefty is left not right. However my correction tolerance is most likely way worst then even a ceramic resonator on it's worst night.  smiley-wink
Logged

Global Moderator
Boston area, metrowest
Offline Offline
Brattain Member
*****
Karma: 544
Posts: 27352
Author of "Arduino for Teens". Available for Design & Build services. Now with Unlimited Eagle board sizes!
View Profile
WWW
 Bigger Bigger  Smaller Smaller  Reset Reset

Try this:

  // Move forward one second every 1000 milliseconds
duration = millis()-lastTick;
  if(duration >= 1000)
  {
    lastTick = millis() + duration - 1000;  //attempt to capture the time overage
    serialOutputTime();
    second++;
  }
Logged

Designing & building electrical circuits for over 25 years. Check out the ATMega1284P based Bobuino and other '328P & '1284P creations & offerings at  www.crossroadsfencing.com/BobuinoRev17.
Arduino for Teens available at Amazon.com.

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

Quote
I still say his 3 mins error over 4 hours (-1.25%) is way beyond the frequency tolerence of even ceramic resonators (typically +/- .1% ?) specifications

That's actually worse than the internal oscillator.  The one sitting on my desk is currently at 0.181%.

Regarding the code, this is a bit simpler and should solve any minor overrun problem.  If necessary, the if can be changed to while to overcome serious overruns.

Code:
  if ( millis() - PreviousMS >= 10000 )  // Optionally use "while" instead of "if"
  {
    // Do your stuff here.
    PreviousMS = PreviousMS + 10000;
  }
Logged

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

Try this:

  // Move forward one second every 1000 milliseconds
duration = millis()-lastTick;
  if(duration >= 1000)
  {
    lastTick = millis() + duration - 1000;  //attempt to capture the time overage
    serialOutputTime();
    second++;
  }

I just noticed the slight overrun. I have this running and will report back tomorrow morning to see if it is still accurate. Thanks
Logged


Lancashire, UK
Offline Offline
Edison Member
*
Karma: 9
Posts: 1991
View Profile
WWW
 Bigger Bigger  Smaller Smaller  Reset Reset

My old Duemilanoves (Crystal) are good to single  figure seconds over the millis() overrun period ( ~50 days ).  My Uno (Ceramic) is OK to 20 seconds a day.  These are long term figures, measurements to the single millisecond in the short term may be off.
Logged


Global Moderator
Online Online
Brattain Member
*****
Karma: 495
Posts: 19027
Lua rocks!
View Profile
WWW
 Bigger Bigger  Smaller Smaller  Reset Reset

After running for 4 hours it is about 3 mins behind. I assume this is a problem with my code:

I think the code has flaws, it isn't the crystal necessarily.

The test for >= 1000 ms and then taking whenever-that-happened-to-match as the next test point basically guarantees that you will eventually drift.

Then you take a different reading to synchronize the next second to:

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

At the very least, use the same reading, so the time taken to execute the "if" isn't counted, eg.

Code:
unsigned long now = millis ();

if(now - lastTick >= 1000)
  {
    lastTick = now;
    serialOutputTime();
    second++;
  } 

But better still, take some baseline time, and keep adding 1000 to it (or subtracting, whatever) so that you don't drift if an interrupt or something means your loop happens to execute at 1001 rather than 1000 mS.

Plus of course this:

Code:
delay(10);

If you happen to hit a button during the day, that means that your next match could be 10 mS out.
Logged


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

@ CrossRoads after letting your thing run overnight (10 hours, it was 8 mins behind).

Trying Coding Badly's code with the while.

As for my hardware, I have an Arduino Uno that was purchased about 7 months ago.

edit: @ Nick Gammon I thought millis would still count during the delay()
« Last Edit: June 19, 2011, 08:12:25 am by dethredic » Logged


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