Fun with interrupts and timing

Hi all,

Having some ‘fun’ trying to understand interrupts and have come unstuck. The code below I largely copied from an example that I found on the web, however it was originally written to run on an ATMega8 with a 10MHz clock. I have modified it slightly, mainly changing variable names (e.g. TIFR became TIFR1) and changing the prescaler value. However, when I run the code, it is looping far quicker than I expected. I’m hoping someone can explain where I have gone wrong. FWIW, here’s my thinking behind this code and how I think it should be working…

1- Clock is running at 16MHz i.e. 16,000,000 cycles per second, or one tick = 0.0000000625 seconds
2- I am using a 1024 prescaler which means each tick = 0.000064 seconds
3- So, every 0.000064 seconds, the value in TCNT1 is incremented
4- Since this counter will overflow when the count gets to 65536, this means that the overflow interrupt should occur every 4.194304 seconds
5- The variable named ‘tick’ should be set to zero every 4.19 seconds, which means that my Serial.println() statement should print about every 4 seconds

In practice though, what happens is that the code loops far quicker, with the println statements immediately cluttering the Serial Monitor, and obviously NOT occurring once per 4 seconds.

Any ideas on what I’m doing wrong here?

Also, when I enable the Serial.println statement inside the interrupt code (yes, I know this is not a good practice, but just wanted to see what the TCNT1 value was) I see that TCNT1 is ALWAYS 66. I assume this is because 66 ticks have been added to the counter AFTER it overflowed? In other words, the TCNT1 timer counter continues to be incremented even though the interupt code has been called. If I was to artificially increase the path length in the interrupt handler, PRIOR to the println statement (by adding some more code), then I would expect the TCNT1 value to be greater than 66. Can anyone confirm if my understanding is correct here?

Cheers,
Mike

#include <avr/io.h>
#include <avr/interrupt.h>
#include <avr/signal.h>

volatile uint8_t tick;    // 4 seconds  'ish
volatile uint8_t subtick; // 400mS 'ish

#define TICKS 10;

SIGNAL(TIMER1_OVF_vect) {
  if (--subtick==0) {
    subtick=TICKS;
    if (tick) {
      --tick;
//      if (!tick) {
//        Serial.print("TCNT1=");Serial.println(TCNT1, DEC);
//      }
    }
  }
  TCNT1=0;
}

// Start timer running
// ensures you don't get part of the old count
void startsubtimer(void) {
  TCCR1B=5;  // prescaler to divide clock by 1024
  TCNT1=0;
  TIFR1 &= ~(1<<TOV1);  // Set overflow flag off or on? not what this does!!!  
  TIMSK1 |= (1<<TOIE1);  // start timer
}

void starttimer(void)
{
  subtick=TICKS;
  startsubtimer();
}

// Stops the timer
void stoptimer(void) {
  TIMSK1 &= ~(1<<TOIE1);  // stop the timer
  TCCR1B=0;  // halt timer
}

void setup()
{
  sei();   // enable interrupts (req'd for timer)
  Serial.begin(9600);
  Serial.println("About to do clock stuff");  
}

void loop()
{
  tick=1; // wait approx 4 seconds
  starttimer();
  while (tick); // wait for it!
  Serial.println("Got the tick");
}

What if you move starttimer() from loop() to setup()?

Also, you don't need the sei() in setup(); interrupts are enabled by default (e.g. for millis()).

There seems to be a lot going on in your code, which makes it difficult to figure out what might be wrong. If I were in your shoes I would simplify things greatly. Specifically, I would set up the timer1 interrupt to blink an LED. That’s it. Then if the LED turns on for 4 seconds and off for 4 seconds, you know you have your timing right. If it blinks much more rapidly, you have a problem that will hopefully be easier to troubleshoot.

Note: After writing a bunch of the stuff that follows, I figured out exactly what’s wrong with your program, so see the bottom of this post for the ultimate solution.

Also, I have a few suggestions for your code:

  1. I believe <avr/signal.h> has been depricated and replaced by <avr/interrupt.h>, which you are already including in your file. I would just ditch the <avr/signal.h>. Replace your SIGNAL(TIMER1_OVF_vect) with ISR(TIMER1_OVF_vect) and you’re good to go.

  2. This line is wrong:

TIFR1 &= ~(1<<TOV1); // Set overflow flag off or on? not what this does!!!

It appears you are trying to clear the Timer1 overflow interrupt flag before enabling the Timer1 overflow interrupt (so you don’t get an erroneous initial interrupt). This is accomplished by writing a logical 1 to the TOV1 bit of TIFR1, not by writing a logical 0 (I know, it’s confusing, but that’s the convention used by AVRs and I believe it exists to prevent you from accidentally clearing flags you might not want cleared). You should change this line to:

TIFR1 |= 1 << TOV1;

So in short, here’s the code I would try:

#include <avr/io.h>
#include <avr/interrupt.h>

ISR(TIMER1_OVF_vect)
{
  // toggle the LED pin to make the LED blink
  PORTB ^= 1 << PB5;  // the same as digitalWrite(13, HIGH if LOW or LOW if HIGH)
}

void setup()
{
  DDRB |= 1 << PB5;  // the same as pinMode(13, OUTPUT);
  TCCR1A = 0;  // this should fix your problem
  TCCR1B=5;  // prescaler to divide clock by 1024 (starts timer counting)
  TCNT1=0;  // clear timer1 counter
  TIFR1 |= (1<<TOV1);  // clear the overflow flag
  TIMSK1 |= (1<<TOIE1);  // enable timer1 overflow interrupts
}

void loop()
{
  // nothing to see here, move along
}

The problem: Your specific problem is being caused by the function init() from wiring.c, which is called by the Arduino pre-compiler before your setup(). init() configures Timer1 to run in 8-bit phase-correct PWM mode by using TCCR1A, which means it overflows after TCNT1 = 0xFF, not when TCNT1 = 0xFFFF as you are expecting. To fix this problem, you need to add the line

TCCR1A = 0;

to your setup().

  • Ben

Thanks Ben, as usual a very clear and comprehensive explanation. I had bets with a colleague of mine that you would add a reply...and thankfully I was right.
I like your solution, it is both concise and clean and I will definitely try it out when I have a few minutes. I am also pleased that you were able to explain the behaviour I was seeing with the overly fast ticking, viz. the 8bit counter - I doubt I wouldn't have nailed that one in a month of Sundays without your help.

Again, many thanks for helping this newbie along.

I’m happy to help. Now I just hope it actually works!

  • Ben

Hi Ben, both code snippets worked just fine; yours straight off, and mine (with the suggested mod). So, I’m now reasonably happy that I understand timer basics. I’ve now moved on to generating interrupts using an external signal. I’ve managed to get samples using INT0 and INT1 working without too much trouble, but I’ve not had any similar success with interrupts generated from pin changes.

The simple sketch below is attempting to recognise when digital pin 7 changes state and then toggle the state of the LED (on pin 13).
The circuit I am using to change the state has +5v connected to Pin 7 by means of a push-switch. I’ve also got the pin pulled down by a 10K resistor so that when the switch is not pressed, it should be reading LOW. Unfortunately, pushing the switch (or directly applying +5v) to pin 7 does not appear to be generating the required interrupt.

Any ideas on what I’m doing wrong here?

#include <avr/io.h>
#include <avr/interrupt.h>

// whenever pin change is detected, toggle the test LED
ISR(PCINT23_vect) {
  PORTB ^= 1 << PB5;  // the same as digitalWrite(13, HIGH if LOW or LOW if HIGH)
}

void setup()
{
  DDRB |= 1 << PB5;     // the same as pinMode(13, OUTPUT)
  DDRD &= ~(1<<PD7);    // pinMode(7, INPUT)
  
  PCMSK2 |= (1<<PCINT23);
  PCICR |= 1<<PCIE2;       //enable interrupt on pin change, PORTD
  sei();                  //enable global interrupts
  PORTB |= 1 << PB5;  // turn on the LED
} 
 
void loop()
{
  // nothing to see here, move along
}

Unfortunately I don't have enough time to reply in detail now, but one thing I can point out is that pin change interrupts occur for an entire port, so to speak, and you mask for the bits that are allowed to trigger the interrupt. You are are trying to create an interrupt for a single pin (the one that corresponds to pin-change 23, but you need to create the interrupt for the pin-change bank that contains PCINT23 and use the pin-change mask to enable interrupts from pin 23.

If this doesn't make sense I can try to give you a code example, but that probably won't happen until tonight. First I'm off to a local robotics competition, and then I need to get some sleep to make up for a long night of robot-building.

  • Ben

Thanks for the tip Ben. I'll dig into it a bit more and see if I can figure it out for myself from what you've said. Good luck with the comp!!!

Well, that took all of 5 minutes to figure out once I had your tip. As you hinted at, I was trying to get a pin specific interrupt, and had set up an ISR for an event that didn't even exist, i.e. PCINT23_vect. What I really wanted was an ISR for PCINT2_vect. Once I made that small change, my circuit began to work just how I wanted it to, i.e. LED turns on/off every time the state of pin 7 changes.

Many thanks once more for your help.