newbie, need critique/suggestions for clock code

First off, a quick background on myself: I have just started using arduino, but I have a diploma in Electronics Engineering Technology and work with electronics everyday (though more troubleshooting than design).

My first project is a 24 LED clock using the Solarbotics Freeduino SB board, an Atmel ATmega168 and four 74HC595 shift registers. So far I have the circuit wired up and decent code to run the 595's, but I can't get an overly accurate clock.

Currently, I'm using the MsTimer2 library to set a second flag variable every 1 second (using the timer2 interrupt I think?). The main program sits and waits for this flag, then increments the time, clears the flag and starts waiting again. I think this should work, because there is no chance that my main program will ever take more than a second to run. I know this may cause the seconds to not always update at the exact same time, but in the long-run the time should be right.

At first I had all the code that is now in the "loop" section under the timeInc interrupt funtion, but it was very inaccurate. It got alot better when I cut the interupt code down as much as possible, but still after running for only a couple of hours, the time is over a second fast when compared to the time on my PC.

Is there anyway to get more accurate timing without anything drastic?

int secTime = 0;
int minTime = 0;
int hourTime = 0;
boolean secFlag = 0; //second flag

void timeInc() {
  
  secFlag = true; //set second flag to tell main program that 1 second
                        //has occured    
}

void setup() {
  Serial.begin(9600); 
  
  pinMode(13, OUTPUT);
  MsTimer2::set(1000, timeInc);  //setup MsTimer2 to run every 1 second
  MsTimer2::start();
}

void loop() {

  //main program basically waits until a second has passed, then 
  //increments the time accordingly
  
  if(secFlag){
    if(secTime == 59){
        secTime = 0;
    
        if(minTime == 59){
            minTime = 0;
            
            if(hourTime == 11){
                hourTime = 0;
                }
            else{
                hourTime++;
                }
            }
        else{
            minTime ++;
            }
        }
    else{
        secTime ++;
        }
        
    //send time to screen    
    Serial.print("\e[2J");
    Serial.print(hourTime);
    Serial.print(":");
    Serial.print(minTime);
    Serial.print(":");
    Serial.print(secTime);
    
    secFlag = 0;  //reset second flag
  }
        
}

A really useless suggestion.

Your clock logic could be written like this:

if (secFlag){
      secTime == 59  ? secTime = 0  : secTime++;
      minTime == 59  ? minTime = 0  : minTime++;
      hourTime == 11 ? hourTime = 0 : hourTime++; 
      secFlag = false;  //reset second flag
}

It is strange that the arduino is too fast. I would've guessed too slow.

You could try to compensate for this in the MsTimer2::set(1000, timeInc); call.

wow, I had no idea I could clean up the clock logic like that, thanks man!

Also, I have tried changing the MsTimer2::set(1000, timeInc); value, but the problem is that in two hours there are 7200000ms or 7200s, so changing the 1000 to 1001 would give me 7207200ms or 7207s in two hours; a 7s difference. 1 sec/hour error seems as good as I can get without changing my approach.

Why not get a DS1307 IC? They are very cheap and easy to use and will give accurate time. You also get the advantage that with a battery back-up it will maintain the time after a power down.

I was trying to avoid buying an RTC, but I guess that's what I'll have to do?

I've made a clock with an RGB 8x8 matrix and a DS1307 recently. You can have the code and schematic if you want.

Your clock logic could be written like this:

if (secFlag){
      secTime == 59  ? secTime = 0  : secTime++;
      minTime == 59  ? minTime = 0  : minTime++;
      hourTime == 11 ? hourTime = 0 : hourTime++;
      secFlag = false;  //reset second flag
}

You didn't really mean to write it like that, did you?

You didn't really mean to write it like that, did you?

I admit that software is my weak side but I don't get the '?' and ':' use in your above suggested code and could not find their use in the Arduino extended reference section. Is this some C++ magic?

Lefty

It is kind of magic.

http://en.wikipedia.org/wiki/%3F:#C.2B.2B

condition ? true : false;

int i=8;
Serial.println(i==8 ? "i is equal to 8" : "i is not equal to 8");

However. My code is embarrisingly stupid.

It increments all three time variables every time! :-[

/me demonstrating how not to be helpful.

Sorry. :'(

you may want to look at the DatetTime library in the Playground: http://www.arduino.cc/playground/Code/DateTime

It does the timekeeping for you (and avoids you having to use those obscure ternary expressions in your sketch ;)

It is kind of magic.

http://en.wikipedia.org/wiki/%3F:#C.2B.2B

condition ? true : false;

Thanks for the explanation link. That's pretty good example of how to make something terse (dense?) at the expense of clarity. No wonder the Arduino reference left it out ;D

Lefty

No wonder the Arduino reference left it out

Yes, that's exactly why it's left out!

after running for only a couple of hours, the time is over a second fast when compared to the time on my PC.

So the error is about 1/(2*60*60) or 138ppm off? That's not TOO far outside the normal accuracy specifications for low-cost crystals (50-100ppm) The highly though-of accuracy of crystals doesn't stand up too well to time measurement, where we're used to seconds per day accuracy (closer to 10ppm) (it COULD be a software issue; I'm not sure how mstimer2 works internally. There were issues with "delay" where it can be off by close to 1ms; used to be it could be almost 1ms less than the specified time, now it can (more safely) be up to 1ms more than the specified time.)

Could well be a software issue. In my experience, crystal controlled Arduino boards are accurate to within a few seconds per day (at room temperature).

You may want to try a test using the DateTime library and see if the accuracy improves.

That’s pretty good example of how to make something terse (dense?) at the expense of clarity

I disagree; sometimes this ternary operator can improve legibility.

e.g.

x = (a > 3) ? 1 : 12;

vs.

if (a > 3) {
x = 1;
} else {
x = 12;
}

However, like many constructs, it can be abused.
Nested ternaries can be nasty.

The ternary syntax is useful for experienced C programmers but its obscure for Arduino users that are not C programmers (and are more interested in making projects then learning the details of a programming language)

if (a > 3) { x = 1; } else { x = 12; }

is easier on the untrained eye than:

x = (a > 3) ? 1 : 12;

If you're interested in doing more advanced projects with the Arduino, I think it will repay you many times over to learn how to configure a timer and its associated interrupt(s) manually. It's a real pain to figure out the first time, but get ye to the datasheet, I say.

You could use Timer1 in CTC mode with a /256 prescaler. Then the timer will tick 62,500 times per second, so you set your output compare register to 62500, and write the interrupt to increment your seconds variable every time it fires. Voila, you have a subroutine that will run exactly every 1 second, independent of how much you're loading down your processor with your main program routine.