Clock driver- Prize to you for a fix!

I have built a slave clock driver- a device to send a 24 pulse to a slave clock (one of those old institutional clocks that we all saw in school run by a maste clock in the office). I cannot get it to keep accurate time!! (off 1 min. 22 sec. in 12 hours) I have read about the counter timeout at 9 hours and 32 minutes. I tried using the millis rollover function to compensate for this, but I'm afraid I just am not that good at this- I'm just a beginner. Here's what I have. The clock should be n for 2s and off for 58s. This seems to me to be a very simple request of the Arduino. I am really frustrated that I/it cannot accomplish it. To whoever sends me a working fix, I'll send you a free old skool clock!

/* Millis Rollover
*

  • Example of counting how many times the millis() function has rolled over
  • by Rob Faludi http://www.faludi.com
  • for the ATMEGA168 with Arduino 0010 the max value is 34359737 or about 9 hours and 32 mintues
  • for more information see: millis() - Arduino Reference

*/

#define ledPin 13 // light for status output

void setup() {
Serial.begin(9600); // start serial output
digitalWrite(ledPin, HIGH); // startup blink
delay(250); // waits
digitalWrite(ledPin, LOW); // sets the LED off
delay(250); // startup blink
digitalWrite(ledPin, HIGH); // startup blink
delay(250); // waits
digitalWrite(ledPin, LOW); // sets the LED off
delay(1000); // wait
}

void loop() {
int rollovers = millisRollover(); // get the number of rollovers so far

Serial.print("Rollovers: "); // show the number of rollovers so far
Serial.println(rollovers,DEC); //
digitalWrite(ledPin, HIGH); // sets the LED on
delay(2000); // waits
digitalWrite(ledPin, LOW); // sets the LED off
delay(58000); // waits

}

int millisRollover() {
// get the current millis() value for how long the microcontroller has been running
//
// To avoid any possiblity of missing the rollover, we use a boolean toggle that gets flipped
// off any time during the first half of the total millis period and
// then on during the second half of the total millis period.
// This would work even if the function were only run once every 4.5 hours, though typically,
// the function should be called as frequently as possible to capture the actual moment of rollover.
// The rollover counter is good for over 35 years of runtime. --Rob Faludi http://rob.faludi.com
//
static int numRollovers=0; // variable that permanently holds the number of rollovers since startup
static boolean readyToRoll = false; // tracks whether we've made it halfway to rollover
unsigned long now = millis(); // the time right now
unsigned long halfwayMillis = 17179868; // this is halfway to the max millis value (17179868)

if (now > halfwayMillis) { // as long as the value is greater than halfway to the max
readyToRoll = true; // you are ready to roll over
}

if (readyToRoll == true && now < halfwayMillis) {
// if we've previously made it to halfway
// and the current millis() value is now less than the halfway mark
// then we have rolled over
numRollovers = numRollovers++; // add one to the count the number of rollovers
readyToRoll = false; // we're no longer past halfway
}
return numRollovers;
}

There are a few ways around the millis overflow problem. My preference is to use the code that David Mellis has implemented to fix this in the next Arduino relase. If you don't want to wait for this release, I have patched the file below with the fix.

You need to replace wiring.c with this file. Wiring.c is in the arduino install directory, in a path similar to: arduino-0011\hardware\cores\arduino\wiring.c

Make a backup copy of wiring.c and then replace all the code in wiring.c with the code in the code block in post.

/*
  wiring.c  
  Part of Arduino - http://www.arduino.cc/

  This is a version with patched millis code
  Copyright (c) 2005-2006 David A. Mellis

 See original wiring.c for full copyright

*/
#include "wiring_private.h"
 
volatile unsigned long timer0_clock_cycles = 0;
volatile unsigned long timer0_millis = 0;

SIGNAL(SIG_OVERFLOW0)
{
        // timer 0 prescale factor is 64 and the timer overflows at 256
        timer0_clock_cycles += 64UL * 256UL;
        while (timer0_clock_cycles > clockCyclesPerMicrosecond() * 1000UL) {
                timer0_clock_cycles -= clockCyclesPerMicrosecond() * 1000UL;
                timer0_millis++;
        }
}

unsigned long millis()
{
        unsigned long m;
        uint8_t oldSREG = SREG;
        
        cli();
        m = timer0_millis;
        SREG = oldSREG;
        
        return m;
} 

void delay(unsigned long ms)
{
      unsigned long start = millis();
      
      while (millis() - start < ms)
            ;
}

/* Delay for the given number of microseconds.  Assumes a 16 MHz clock. 
 * Disables interrupts, which will disrupt the millis() function if used
 * too frequently. */
void delayMicroseconds(unsigned int us)
{
      uint8_t oldSREG;

      // calling avrlib's delay_us() function with low values (e.g. 1 or
      // 2 microseconds) gives delays longer than desired.
      //delay_us(us);

#if F_CPU >= 16000000L
      // for the 16 MHz clock on most Arduino boards

      // for a one-microsecond delay, simply return.  the overhead
      // of the function call yields a delay of approximately 1 1/8 us.
      if (--us == 0)
            return;

      // the following loop takes a quarter of a microsecond (4 cycles)
      // per iteration, so execute it four times for each microsecond of
      // delay requested.
      us <<= 2;

      // account for the time taken in the preceeding commands.
      us -= 2;
#else
      // for the 8 MHz internal clock on the ATmega168

      // for a one- or two-microsecond delay, simply return.  the overhead of
      // the function calls takes more than two microseconds.  can't just
      // subtract two, since us is unsigned; we'd overflow.
      if (--us == 0)
            return;
      if (--us == 0)
            return;

      // the following loop takes half of a microsecond (4 cycles)
      // per iteration, so execute it twice for each microsecond of
      // delay requested.
      us <<= 1;
    
      // partially compensate for the time taken by the preceeding commands.
      // we can't subtract any more than this or we'd overflow w/ small delays.
      us--;
#endif

      // disable interrupts, otherwise the timer 0 overflow interrupt that
      // tracks milliseconds will make us delay longer than we want.
      oldSREG = SREG;
      cli();

      // busy wait
      __asm__ __volatile__ (
            "1: sbiw %0,1" "\n\t" // 2 cycles
            "brne 1b" : "=w" (us) : "0" (us) // 2 cycles
      );

      // reenable interrupts.
      SREG = oldSREG;
}

void init()
{
      // this needs to be called before setup() or some functions won't
      // work there
      sei();
      
      // following used for millis() and delay()
      timer0_clock_cycles = 0;
    timer0_millis = 0;

      // on the ATmega168, timer 0 is also used for fast hardware pwm
      // (using phase-correct PWM would mean that timer 0 overflowed half as often
      // resulting in different millis() behavior on the ATmega8 and ATmega168)
#if defined(__AVR_ATmega168__)
      sbi(TCCR0A, WGM01);
      sbi(TCCR0A, WGM00);
#endif  
      // set timer 0 prescale factor to 64
#if defined(__AVR_ATmega168__)
      sbi(TCCR0B, CS01);
      sbi(TCCR0B, CS00);
#else
      sbi(TCCR0, CS01);
      sbi(TCCR0, CS00);
#endif
      // enable timer 0 overflow interrupt
#if defined(__AVR_ATmega168__)
      sbi(TIMSK0, TOIE0);
#else
      sbi(TIMSK, TOIE0);
#endif

      // timers 1 and 2 are used for phase-correct hardware pwm
      // this is better for motors as it ensures an even waveform
      // note, however, that fast pwm mode can achieve a frequency of up
      // 8 MHz (with a 16 MHz clock) at 50% duty cycle

      // set timer 1 prescale factor to 64
      sbi(TCCR1B, CS11);
      sbi(TCCR1B, CS10);
      // put timer 1 in 8-bit phase correct pwm mode
      sbi(TCCR1A, WGM10);

      // set timer 2 prescale factor to 64
#if defined(__AVR_ATmega168__)
      sbi(TCCR2B, CS22);
#else
      sbi(TCCR2, CS22);
#endif
      // configure timer 2 for phase correct pwm (8-bit)
#if defined(__AVR_ATmega168__)
      sbi(TCCR2A, WGM20);
#else
      sbi(TCCR2, WGM20);
#endif

      // set a2d prescale factor to 128
      // 16 MHz / 128 = 125 KHz, inside the desired 50-200 KHz range.
      // XXX: this will not work properly for other clock speeds, and
      // this code should use F_CPU to determine the prescale factor.
      sbi(ADCSRA, ADPS2);
      sbi(ADCSRA, ADPS1);
      sbi(ADCSRA, ADPS0);

      // enable a2d conversions
      sbi(ADCSRA, ADEN);

      // the bootloader connects pins 0 and 1 to the USART; disconnect them
      // here so they can be used as normal digital i/o; they will be
      // reconnected in Serial.begin()
#if defined(__AVR_ATmega168__)
      UCSR0B = 0;
#else
      UCSRB = 0;
#endif
}

Re-compile your sketch & your overflows should no longer be a problem :wink:

Thanks! I actually sent you a PM with additional info. and a pic. I'll give it a try.

I burn my own bootloaders and selected to use the external 16MHz clock when setting the chip up. Does this wiring.c override those settings? Do I need an external crystal with the 168 chip? (Yes, the external crystal is probably more accurate)

The code will work for both the ATmega168 and ATmega8

p.s. replied to your pm