Go Down

Topic: millis not accurate? (Read 2 times) previous topic - next topic

dethredic

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

Code: [Select]

#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
}
www.gunnewiek.com

FalconFour

#1
Jun 18, 2011, 11:58 pm Last Edit: Jun 19, 2011, 12:00 am by FalconFour Reason: 1
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: [Select]
 /**********  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: [Select]
 /**********  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...

FalconFour

^ 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.


FalconFour

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?  :%

retrolefty

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.  ;)

Lefty


retrolefty

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


Jack Christensen

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 XD 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.)
MCP79411/12 RTC ... "One Million Ohms" ATtiny kit ... available at http://www.tindie.com/stores/JChristensen/

retrolefty

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.  ;)

CrossRoads

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++;
  }
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.

Coding Badly

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: [Select]
  if ( millis() - PreviousMS >= 10000 )  // Optionally use "while" instead of "if"
  {
    // Do your stuff here.
    PreviousMS = PreviousMS + 10000;
  }

dethredic


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
www.gunnewiek.com

pluggy

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.
http://pluggy.is-a-geek.com/index.html

Nick Gammon


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: [Select]
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: [Select]
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: [Select]
delay(10);

If you happen to hit a button during the day, that means that your next match could be 10 mS out.
Please post technical questions on the forum, not by personal message. Thanks!

More info:
http://www.gammon.com.au/electronics

dethredic

#14
Jun 19, 2011, 03:04 pm Last Edit: Jun 19, 2011, 03:12 pm by dethredic Reason: 1
@ 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()
www.gunnewiek.com

Go Up