Undefined behavior when using interrupts with ATmega48PA

Question

I am building a simple Timer/Counter application, that generates a delay using the normal mode in Atmel's ATmega48PA, using Timer1, toggling an LED in a constant time interval. What happens is when using the interrupt, the LED toggles for a definite amount of time, then the toggling effect halts, keeping the LED always ON! I believe there is something with the sei() function or enabling the global interrupt in SREG, as I had experienced such behavior with the same microcontroller before when using interrupts.

Here is a code snippet provided with my question, although anybody will see this code as very normal and have to be working correctly!

#include <avr/io.h>
#include <atmel_start.h>
#include <util/delay.h>
#include <math.h>
#include <clock_config.h>
#include "avr/iom48pa.h"

#define LOAD_VALUE      49911UL

#define SET_BIT( REG, BIT )                 REG |= ( 1 << BIT )
#define CLR_BIT( REG, BIT )                 REG &= ~( 1 << BIT )
#define TOG_BIT( REG, BIT )                 REG ^= ( 1 << BIT )

void Timer16_Init( void );
void Timer16_DelayMS( unsigned short delayMS );

unsigned short delayMS = 50;

ISR( TIMER1_OVF_vect ){
    Disable_global_interrupt();
    TOG_BIT( PORTC, 2 );
    TOG_BIT( PORTC, 3 );
    TCNT1 = ( ( 4194304 - delayMS ) * 1000 ) / 64;
    Enable_global_interrupt();
}

int main( void ){
    
    /* Initializes MCU, drivers and middle ware */
    atmel_start_init();
     
     /* configure pin 2 and pin 3 in PORTC as output */
     SET_BIT( DDRC, 3 );
     SET_BIT( DDRC, 2 );
     
     Enable_global_interrupt();
     Timer16_Init();
     TCNT1 = ( ( 4194304 - delayMS ) * 1000 ) / 64;

    while( 1 ){
        
    }
}
void Timer16_Init( void ){
    SET_BIT( TCCR1B, CS10 );
    SET_BIT( TCCR1B, CS12 );
    SET_BIT( TIMSK1, TOIE1 );
}

I just want to know, what in the world happens right here ?!

Answer 1

Well at the first look, there is no special problem in your code. So let's check the possibilities:

First you have done some 32-bit long calculations and put the result in the 16-bit register:

TCNT1 = ( ( 4194304 - delayMS ) * 1000 ) / 64; it results in an unpredictable value that has been entered in the register. so I recommend you to use appropriate values or using (long) and (int) to your code to prevent data overflow.

Second You have not entered the correct data line order:

Enable_global_interrupt();
Timer16_Init();
TCNT1 = ( ( 4194304 - delayMS ) * 1000 ) / 64;

you enabled the interrupts, then initialized the timer, and then applied the value of timer. this is incorrect since the timer runs and is able to make an interrupt, but its value has not been set yet. the order must be like this:

TCNT1 = ( ( 4194304 - delayMS ) * 1000 ) / 64;    
Enable_global_interrupt();
Timer16_Init();

Third you have used the timer in normal mode, entering overflow interrupt and setting the timer value inside of interrupt routine. I highly recommend you to use the compare mode instead, since it does not require the timer value setting in the interrupt routine.