Timer2 does not work as it should

Question

I am currently playing with Arduino timers (on UNO currently) and I am building a library that has a function that must be called every millisecond.

There is plenty of code samples on Internet so I went with something that seemed perfect for my needs. I have then updated it to make it a reusable C++ class.

Here is my code (simplified to just isolate the issue):

#define BIT(x) (0x01 << (x))

static volatile uint32_t milliseconds = 0;

class Timer
{
    public:
    Timer()
    {
        // Disable interrupts
        uint8_t savedStatus = SREG;
        cli();
        // Use Timer2 with 1 ms interrupts
        // OC2A & OC2B disconnected, mode 2 (CTC, Clear Timer on Compare match)
        TCCR2A = BIT(WGM21);
        // Don't force output compare (FOCA & FOCB), mode 2, Clock Select clk/128 (CS = 5)
        TCCR2B = BIT(CS22) | BIT(CS20);
        // Set timer counter compare match (when value reached, 1ms has elapsed)
        OCR2A = 125 - 1;
        // Reset Timer2 counter
        TCNT2 = 0;
        // Set Timer2 interrupt mode (Set interrupt on OCR2A compare match)
        TIMSK2 = BIT(OCIE2A);

        // Restore interrupt enabling state
        SREG = savedStatus;
    }
};

static void callEveryMillisecond(uint32_t ms)
{
    // Do something here
}

// Attach interrupt routine to the Timer Compare Interrupt
ISR(TIMER2_COMPA_vect)
{
    milliseconds++;
    callEveryMillisecond(milliseconds);
}

After checking ATmega328 datasheet (section 17) several times, I came to the conclusion that what I do in Timer class constructor above is correct, and for Arduino UNO, the values used must lead to one call of the ISR every ms:

• CPU frequency = 16MHz
• Prescaler = 128
• -> timer frequency = 16MHz/128 = 125KHz
• -> max counter value = 125

I decided to use the CTC (Clear Timer on Compare match) mode, which means that when the timer TCNT2 (which is incremented at 125KHz frequency), reaches OCR2A value, then it will trigger the ISR, and then will be reset on next tick; the additional tick to reset the counter explains why we use 125 - 1 for OCR2A and not just 125.

Now comes the weird stuff. In order to check that my code worked, I decided to blink the LED on pin 13 every 10 seconds, hence I have added the following code to my program:

static void callEveryMillisecond(uint32_t ms)
{
    if (ms % 10000 == 0)
        digitalWrite(13, digitalRead(13) ^ 1);
}

static Timer timer;

void setup()
{
    pinMode(13, OUTPUT);
}

void loop()
{
}

Then I measured the time during which pin 13 LED is lit on (or off, both measures are the same). Measures are not very accurate (I just used my wristwatch) but good enough for my check.

I was surprised to see the measured time to be about 20 seconds instead of 10s.

Initially I thought about a mismatch in CPU frequency, but after performing a lot of small changes, here is what I found which worked perfectly:

//static Timer timer;

void setup()
{
    static Timer timer;
    pinMode(13, OUTPUT);
}

Just moving the Timer instance into setup() did the trick: my measures rightly indicated 10 seconds between 2 states of the pin 13 LED.

Now my questions are:

1. Why the second code sample works perfectly, whereas the first does not?

2. Is there, in standard Arduino included code, some code that would overwrite some of the settings I perform for Timer2 in Timer::Timer()?

3. Is there a way to make the first code sample work? Note that I don't like the second sample because it does not seem natural coding, also, I will need access to Timer timer instance in several locations of my future program, hence it can't be possible if it is declared in the scope of setup().

Answer 1

I have located the source of the problem.

In file wiring.c (arduino-1.5.6-r2\hardware\arduino\avr\cores\arduino) which gets called before setup(), there is the following part that tries to changes the timer 2 prescaller to 64 by setting the CS22 bit.

// set timer 2 prescale factor to 64
#if defined(TCCR2) && defined(CS22)
    sbi(TCCR2, CS22);
#elif defined(TCCR2B) && defined(CS22)
    sbi(TCCR2B, CS22);
#else
    #warning Timer 2 not finished (may not be present on this CPU)
#endif

You already set CS20 and CS22 so this doesn't affect you. What does affect you is the following part which sets bit WGM20

    // configure timer 2 for phase correct pwm (8-bit)
#if defined(TCCR2) && defined(WGM20)
    sbi(TCCR2, WGM20);
#elif defined(TCCR2A) && defined(WGM20)
    sbi(TCCR2A, WGM20);
#else
    #warning Timer 2 not finished (may not be present on this CPU)
#endif

and you have already set WGM21 so WGM21 and WGM20 set changes the timer mode to fast PWM , top=0xFF which gives almost double delay that the mode you were trying to use (CTC top=124).

How to workaround this issue?

Timer2 is used when its associated PWM pins are used with an analogWrite() call, so the Arduino setup code sets all prescalers to well-defined values just in case the timers might get used later.

I would not try to fight that code; instead, I would just make sure that my setup code runs later, e.g. by moving it to a separate setup() method in your Timer class and calling that from the global setup() routine.

Answer 2

As a follow-up up to @alexan_e answer, here is what I finally did to ensure that my Timer2 settings are not overwritten by the init() method in Arduino wiring.c.

#define BIT(x) (0x01 << (x))

static volatile uint32_t milliseconds = 0;

class Timer
{
    public:
    // Don't use the constructor anymore
    void init()
    {
        // Disable interrupts
        uint8_t savedStatus = SREG;
        cli();
        // Use Timer2 with 1 ms interrupts
        // OC2A & OC2B disconnected, mode 2 (CTC, Clear Timer on Compare match)
        TCCR2A = BIT(WGM21);
        // Don't force output compare (FOCA & FOCB), mode 2, Clock Select clk/128 (CS = 5)
        TCCR2B = BIT(CS22) | BIT(CS20);
        // Set timer counter compare match (when value reached, 1ms has elapsed)
        OCR2A = 125 - 1;
        // Reset Timer2 counter
        TCNT2 = 0;
        // Set Timer2 interrupt mode (Set interrupt on OCR2A compare match)
        TIMSK2 = BIT(OCIE2A);

        // Restore interrupt enabling state
        SREG = savedStatus;
    }
};

static void callEveryMillisecond(uint32_t ms)
{
    if (ms % 10000 == 0)
        digitalWrite(13, digitalRead(13) ^ 1);
}

// Attach interrupt routine to the Timer Compare Interrupt
ISR(TIMER2_COMPA_vect)
{
    milliseconds++;
    callEveryMillisecond(milliseconds);
}

static Timer timer;

void setup()
{
    // Explicit call to Timer initialization; this occurs AFTER 
    // Arduino wiring.c init() has been called
    timer.init();
    pinMode(13, OUTPUT);
}

void loop()
{
}

Answer 3

In your setup function, you need to instantiate your timer, like this:

static Timer timer;

void setup()
{
    timer = new Timer();                       <<<--- here
    pinMode(13, OUTPUT);
}

void loop()
{
}

Answer 4

The way most of the libraries do this is to put an instance of (in your case) Timer in the Timer.cpp file, and reference it with an extern definition when needed - typically in your main cpp file. Also you are calling digitalWrite() and digitalRead() during an ISR. This can be bad since those functions are slow, and may not be guaranteed to behave when interrupts are disabled (as they are during an ISR). A safer approach is for your ISR to set a flag in your Timer class that can be read by an Timer.isExpired() or similar method in your loop() code.