Timer 1 interrupt doesn't work

I'm using an Arduino mega and the Arduino IDE. I've been trying to setup CTC mode on Timer 1 with a 256 pre-scaler to get 1 second sampling rate. If I use the Arduino IDE, the interrupt gets triggered in less than a second but if I use my avr toolchain, the same code works fine. Do any of you know why?

Arduino code

static bool flag =0;


void setup() {
  // put your setup code here, to run once:
  TCCR1B = (1<<WGM12)|(1<<CS12);
  OCR1A = 0xFFFF;
  TIMSK1 = (1<<OCIE1A);
  sei();
  TCNT1 =0;

  pinMode(LED_BUILTIN, OUTPUT);

}

void loop() {
  // put your main code here, to run repeatedly:

}

ISR(TIMER1_COMPA_vect){
  
  cli();
  if(flag == 0)
  {
    digitalWrite(LED_BUILTIN,HIGH);
    flag =1;
  }
  else
  {
    digitalWrite(LED_BUILTIN,LOW);
    flag=0;
  }
  sei();
}

AVR code

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

void init();

int main(){
    init();
    while(1);
    return 1;
}

void init(){
    ioinit();
    TCCR1B = (1<<WGM12)|(1<<CS12);

    OCR1A = 0xFFFF;
    TIMSK1 = (1<<OCIE1A);
    sei();
    TCNT1=0;
    DDRB |= (1<<PB7);
}

ISR(TIMER1_COMPA_vect){
    sei();
    printf("In here\n");
    PORTB ^= (1<<PB7);
    cli();
}

Hint, look at your sei() and cli() in both codes. Spot anything about them?

@OP

Many things are wrong in your codes, which are due to your incomplete understanding of the principles of CTC Mode operation of TC1 Module.

1. Your Arduino sketch is uploaded in my UNO but not running. The problem has disappeared when I have included and deleted the following codes:

TCCR1A = 0x00;   //included in the setup() 

cli();   //deleted from the ISR() 
sei();  //deleted from the ISR()

2. The L (built-in LED) blinks at about 1.05 sec interval; whereas, you are expecting to see it blinking at 1-sec interval. The problem is the wrong value you have loaded into OCR1A register. How much should you load into OCR1A Register?
(1) The following is the Mode-4 CTC (you have set Mode-4 in your sketch) timing diagram of TC1.
tc1ctcx.png
Figure-1: Mode-4 CTC timing diagram of TC1

(2) TCNT1 begins counting up alone path AB of Fig-1 with initial value of 0. You want that match with the content of OCR1A register should happen after 1-sec time period. The TCNT1 is being driven by a clock frequency of 62500 Hz (16000000/256). Therefore, the OCR1A Regsister must be loaded with 62500 (0xF424) and not with 0xFFFF (65536 65535 Edit: vide @Post6) for the L to blink (state change) at 1-sec interval. (65536 65535 gives 65536 65535/62500 ~= 1.05 sec time delay.)

3. The corrected program of your's

static bool flag =0;


void setup() {
  // put your setup code here, to run once:
  TCCR1A = 0x00;     //this register contains WGM11, WGM10 bits
  TCCR1B = (1<<WGM12)|(1<<CS12);
  //OCR1A = 0xFFFF;
  OCR1A = 0xF424;
  TIMSK1 = (1<<OCIE1A);
  sei();
  TCNT1 =0;

  pinMode(LED_BUILTIN, OUTPUT);

}

void loop() {
  // put your main code here, to run repeatedly:

}

ISR(TIMER1_COMPA_vect){
  
  //cli();  no need; the interrupt has already been disabled by the MCU
  if(flag == 0)
  {
    digitalWrite(LED_BUILTIN,HIGH);
    flag =1;
  }
  else
  {
    digitalWrite(LED_BUILTIN,LOW);
    flag=0;
  }
  //sei();  no need; the interrupt logic will be automatically enabled while returning form ISR
}

tc1ctcx.png

TCCR1A = 0x00;   //included in the setup()

The reason to add this to setup with the Arduino IDE code is that the timers are preset for PWM outputs. If you don't clear the presets, you will not be in the mode you are trying to set.

cattledog:

TCCR1A = 0x00;   //included in the setup()

The reason to add this to setup with the Arduino IDE code is that the timers are preset for PWM outputs. If you don't clear the presets, you will not be in the mode you are trying to set.

I was thinking why should I put this line (TCCR1A = 0x00) -- whereas, the default value is 0x00? Thanks with + for reading the thread and providing the answer what I have been longing -- does telepathy exist?

cattledog:

TCCR1A = 0x00;   //included in the setup()

The reason to add this to setup with the Arduino IDE code is that the timers are preset for PWM outputs. If you don't clear the presets, you will not be in the mode you are trying to set.

That tortured me for several days 'til I stumbled upon it!

GolamMostafa:
and not with 0xFFFF (65536) for the L to blink (state change) at 1-sec interval. (65536 gives 65536/62500 ~= 1.05 sec time delay.)

0xFFFF = 65535
0x10000 = 65536

Grumpy_Mike:
Hint, look at your sei() and cli() in both codes. Spot anything about them?

Do you just mean that i have them swapped in my AVR code?

GolamMostafa:
@OP

Many things are wrong in your codes, which are due to your incomplete understanding of the principles of CTC Mode operation of TC1 Module.

1. Your Arduino sketch is uploaded in my UNO but not running. The problem has disappeared when I have included and deleted the following codes:

TCCR1A = 0x00;   //included in the setup() 

cli();   //deleted from the ISR()
sei();  //deleted from the ISR()




**2.** The L (built-in LED) blinks at about 1.05 sec interval; whereas, you are expecting to see it blinking at 1-sec interval. The problem is the wrong value you have loaded into OCR1A register. How much should you load into OCR1A Register?
**(1)** The following is the Mode-4 CTC (you have set Mode-4 in your sketch) timing diagram of TC1.
![tc1ctcx.png|547x157](upload://nng7e4kXfHB6QACoyIOIwU9y5Pc.png)
Figure-1: Mode-4 CTC timing diagram of TC1

**(2)** TCNT1 begins counting up alone path AB of Fig-1 with initial value of 0. You want that match with the content of OCR1A register should happen after 1-sec time period. The TCNT1 is being driven by a clock frequency of 62500 Hz (16000000/256). Therefore, the OCR1A Regsister must be loaded with 62500 (0xF424) and not with 0xFFFF (~~65536~~ 65535 Edit: vide @Post6) for the L to blink (state change) at 1-sec interval. (~~65536~~ 65535 gives ~~65536~~ 65535/62500 ~= 1.05 sec time delay.)

**3.** The corrected program of your's


static bool flag =0;

void setup() {
 // put your setup code here, to run once:
 TCCR1A = 0x00;     //this register contains WGM11, WGM10 bits
 TCCR1B = (1<<WGM12)|(1<<CS12);
 //OCR1A = 0xFFFF;
 OCR1A = 0xF424;
 TIMSK1 = (1<<OCIE1A);
 sei();
 TCNT1 =0;

pinMode(LED_BUILTIN, OUTPUT);

}

void loop() {
 // put your main code here, to run repeatedly:

}

ISR(TIMER1_COMPA_vect){
 
 //cli();  no need; the interrupt has already been disabled by the MCU
 if(flag == 0)
 {
   digitalWrite(LED_BUILTIN,HIGH);
   flag =1;
 }
 else
 {
   digitalWrite(LED_BUILTIN,LOW);
   flag=0;
 }
 //sei();  no need; the interrupt logic will be automatically enabled while returning form ISR
}

Thanks for taking the time to write all that. I didn't bother calculating the value since 1.05 s was close enough.

cattledog:

TCCR1A = 0x00;   //included in the setup()

The reason to add this to setup with the Arduino IDE code is that the timers are preset for PWM outputs. If you don't clear the presets, you will not be in the mode you are trying to set.

Does Arduino document this?

Upballoon:
Does Arduino document this?

Not that I'm aware of - but it doesn't cost much to clear the register (just one clock cycle to perform this instruction at start of program), and it makes it explicit that the register is zeroed. Even if that was the case already. It's like setting pins to pinMode INPUT in setup(), even though that's their default state.

Does Arduino document this?

Yes, but ..

It's all in C:\Program Files (x86)\Arduino\hardware\arduino\avr\cores\arduino

In the IDE, there is a hidden main() function which calls init().

init() is defined in wiring.c and it sets up all the timers for standard pwm with analogWrite().

The issue is exacerbated by the fact that the waveform generation mode bits are across the two control registers A and B.

In your case, the CTC modes don't use the Register A bits, and while you think you are setting the mode in Register B, the preset bit for timer 1 in 8-bit phase correct pwm mode is alive and well in Register A.