AVR global variable modified by interrupt not persistant

Question

Using an ATmega328p, I'm running into a problem when I make an interrupt (INT0 or INT1) active, the code will execute just fine (MusicOnLed and FadeOnLed functions will execute), however as soon as I deactivate the interrupt, the program will do nothing until another interrupt is active.

The global volatile display_select is being changed, as it allows those functions to run, but only when the interrupt is active. When the interrupt is deactivated, either display_select is being changed back to 0xFF, or it is hanging up somewhere.

Ideally, the interrupt fires, changes the display_select value, then goes back to main. In main it evaluates display_select, and sees that it is changed (per the interrupt changing it), and keeps executing that loop.

This was done this way to get out of having to use nested interrupts, as I was running into issues with those.

#define F_CPU 16000000L
#include <avr/io.h>
#include <stdio.h>
#include <util/delay.h>
#include <avr/interrupt.h>
#include "ADC.c"
#include "serial.c"                 //only needs to be included if using stdio or serial functions

//Preprocessor Macros
#define EQ_RESET 6                  //msgeq-7 reset on portd bit 6
#define STROBE 5                    //strobe pin on portd bit 5
#define SW0 2                       //SW0 on PORTD bit 2 (INT0)
#define SW1 3                       //SW1 on PORTD bit 3 (INT1)
#define SW2 4                       //SW2 on PORTD bit 4 (PCINT20)

#define GREEN OCR1A                 //GREEN LED is PWM Output Compare A for timer1
#define RED OCR1B                   //RED LED is PWM Output Compare B for timer1
#define BLUE OCR2A                  //BLUE LED is PWM Output Compare A for timer2

//Global audioband array
uint8_t AudioLevel[7] = {0};        //init 7 position AudioLevels array to 0

//Globals for storing average output values 
uint8_t out_low;
uint8_t out_mid;
uint8_t out_high;

//Fade delay function declaration
const int led_delay = 50;

//Selector switch global initialized at 0xFF
volatile uint8_t display_select = 0xFF;         

//Function Prototypes
void GetAudioBandLevel(void);                           //Gets audio levels, stores in global AudioLevels array
void MusicOnLed(void);                                  //Music display function on LEDs
void FadeOnLed(void);                                   //Fade display function on LEDs
void StrobeOnLed(uint8_t, uint8_t, uint8_t);            //Strobe display function on LEDs
void IncrementUp(uint8_t, uint8_t);                     //Increments specific LED up by specified delay cycle time (const int led_delay)
void IncrementDown(uint8_t, uint8_t);                   //Increments specific LED down by specified delay cycle time (const int led_delay)
void Init_Timer1_PWM(void);                             //Timer1 for PWM on BASS and MID
void Init_Timer2_PWM(void);                             //Timer2 for PWM on TREBLE
void Init_IO(void);                                     //See actual function for IO Port Description
void SolidColorOnLed(void);                             //Changes between colors on LED

int main(void)
{
    init_ADC();                     //init ADC for channel 0, 8 bit resolution, ADC clock = clk/128
    Init_Timer1_PWM();              //timer1 and 2 as 8 bit output compares
    Init_Timer2_PWM();
    Init_IO();                      //enables things and interrupts 
    sei();                          //enables global interrupts

    while(1)
    {
        while(display_select == 0xFF)
        {
            //do nothing while waiting for an interrupt
        }
        while(display_select == 0x00)
        {
            MusicOnLed();
        }
        while(display_select == 0x01)
        {
            FadeOnLed();    
        }       
    }
}

void IncrementUp(uint8_t led, uint8_t max_val)
{
    uint8_t i = 0;      //LCV

    for(i = 0; i < max_val; i++)
    {
        switch(led)
        {
            case 1: 
                RED     = i;
            break;          
            case 2:
                GREEN   = i;
            break;          
            case 3:
                BLUE    = i;
            break;
        }
        _delay_ms(led_delay);
    }
}

void IncrementDown(uint8_t led, uint8_t max_val)
{
    uint8_t i;      //LCV

    for(i = 0; i < max_val; i++)
    {
        switch(led)
        {
            case 1:
            RED     = max_val - i;
            break;
            case 2:
            GREEN   = max_val - i;
            break;
            case 3:
            BLUE    = max_val - i;
            break;
        }
        _delay_ms(led_delay);
    }
}

void GetAudioBandLevel(void)
{
    uint8_t audio_band = 0;
    DDRD    |=  (1 << STROBE)|(1 << EQ_RESET);              //PORTD bit strobe and reset pins output
    PORTD   &=  ~((1 << STROBE)|(1 << EQ_RESET));           //sets strobe and reset low
    PORTD   |=  (1 << EQ_RESET);                            //reset goes high
    _delay_us(100);                                         //delay 100usec for setup time req
    PORTD   |=  (1 << STROBE);                              //strobe goes high
    _delay_us(50);                                          //strobe delays
    PORTD   &=  ~(1 << STROBE);                             //strobe goes low
    _delay_us(50);                                          //strobe delays
    PORTD   &=  ~(1 << EQ_RESET);                           //reset reset
    PORTD   |=  (1 << STROBE);                              //strobe goes high
    _delay_us(50);              

    for(audio_band = 0; audio_band < 7; audio_band++)
    {
        PORTD   &=  ~(1 << STROBE);             //resets (set strobe pin low (active))
        _delay_us(30);                          //setup time for capture
        AudioLevel[audio_band] = read_ADC();    //reads 8 bit resolution audio level from audio bandpass filter
        PORTD   |=  (1 << STROBE);                  //sets (set strobe pin high again)
        _delay_us(50);                          //not sure if needed - check datasheet
    }
}

void MusicOnLed(void)
{
    GetAudioBandLevel();
    out_low     = (AudioLevel[0] + AudioLevel[1]) / 2;                      // Average of two Lowest Bands
    out_mid     = (AudioLevel[2] + AudioLevel [3] + AudioLevel[4]) / 3;     // Average of three Middle Bands
    out_high    = (AudioLevel[5] + AudioLevel[6]) / 2;                      // Average of two Highest Bands

    uint8_t led_out_low     =   out_low * 1.2;
    uint8_t led_out_mid     =   out_mid * 1.2;
    uint8_t led_out_high    =   out_high * 1.2;

    if(out_low >= 212)
    {
        led_out_low = 255;      //set to max brightness if overflow condition would otherwise occur
    }
    if(out_mid >= 212)
    {
        led_out_mid = 255;      //set to max brightness if overflow condition would otherwise occur
    }
    if(out_high >= 212)
    {
        led_out_high = 255;     //set to max brightness if overflow condition would otherwise occur
    }

    //assign to outputs
    GREEN   = led_out_low;
    RED     = led_out_mid;
    BLUE    = led_out_high;
    //_delay_ms(10);            //old delay before menu implementation
    _delay_ms(1);

}

void Init_IO(void)
{
    DDRB = 0x3F;                                            //portb as output

    DDRD    &= ~(1 << SW0) | ~(1 << SW1) | ~(1 << SW2);     //portd bits 2, 3, and 4 as inputs (pushbuttons)
    PORTD   |= (1 << SW0) | (1 << SW1) | (1 << SW2);        //turn pullups on only on pushbutton input pins

    EICRA   = 0x0;                                          //Enables active LOW triggering on ext. interrupts on INT0 and INT1 pins (SW0 and SW1)
    EIMSK   = 0x3;                                          //Enables external interrupts on INT0 and INT1 (SW0 and SW1)

    /*
    PCMSK2  = 0x10;                                         //Enable interrupts on PCINT20 (SW2)
    PCICR   |= (1 << 2);                                    //Enables interrupts on pin change request 2 reg
    */

    DDRC    = 0x00;                                         //port c as input
    PORTC   = 0x3F;                                         //pullups on
}

void Init_Timer1_PWM(void)
{
    TCCR1A = 0b10100001;        // Timer1 in fast mode PWM
    TCCR1B = 0b00000100;        //
    TIMSK1 = 0b00000110;    
}

void Init_Timer2_PWM(void)
{
    TCCR2A = 0b10100001;        // Timer2 in fast mode PWM
    TCCR2B = 0b00000110;        //
    TIMSK2 = 0b00000100;
}

void FadeOnLed(void)
{
    GREEN   = 0;
    BLUE    = 0;
    RED     = 90;

    //see led_delay for led delay time
    //RED is defined as 1, GREEN as 2, BLUE as 3 when passing values to function
    while (1)
    {
        IncrementUp(2, 90);
        IncrementDown(1, 90);
        IncrementUp(3, 90);
        IncrementUp(1, 90);
        IncrementDown(2, 90);
        IncrementDown(1, 90);
        IncrementDown(3, 90);
        IncrementUp(1, 90);
        IncrementUp(2, 90);
        IncrementDown(2, 90);
    }
}

void StrobeOnLed(uint8_t red, uint8_t green, uint8_t blue)
{
    RED     = red;
    GREEN   = green;
    BLUE    = blue;
    _delay_ms(100);
}

void SolidColorOnLed(void)
{
    RED = 0;
    GREEN = 100;
    BLUE = 0;
}

ISR(INT0_vect)
{
    display_select = 0x00;
}

ISR(INT1_vect)
{
    display_select = 0x01;
}

/*
ISR(PCINT2_vect)        //PCINT20
{
    display_select++;
}
*/

---

EDIT:

I modified main() to:

int main(void)
{
    init_ADC();                     //init ADC for channel 0, 8 bit resolution, ADC clock = clk/128
    Init_Timer1_PWM();              //timer1 and 2 as 8 bit output compares
    Init_Timer2_PWM();
    Init_IO();                      //enables things and interrupts 
    sei();                          //enables global interrupts
    display_select = 0xFF;

    while(1)
    {
        switch(display_select)
        {
            case 0x00:
                    MusicOnLed();
                break;
            case 0x01:
                    //FadeOnLed();
                break;
            case 0xFF:
                    RED = 0;
                    GREEN = 50;
                    BLUE = 0;
                break;
        }   

What is happening is when INT0 stays in the low state (triggered), MusicOnLed() keeps functioning as long as INT0 is in the low state. As soon as INT0 goes from low to high (triggered to not triggered), display_select should still be 0x00 (allowing MusicOnLed() to keep running) but is somehow being reset to 0xFF. This obviously causes the GREEN LEDs to become lit, and the other LEDs to turn off.

---

EDIT 2

I've changed the code around to poll the display_select variable during the execution of both FadeOnLed() and MusicOnLed() functions. I am able to now interrupt the functions.

I have also changed the DDRD and PORTD values around to reflect what I have learned online

My new issue is that INT1 always seems to be active. In the simulator, PIND bits 2 and 4 get set, while the others remain reset. I am wondering if this causes INT1 to fire continuously (being a level interrupt).

Please see the following following changed functions:

---

int main(void)
{
    sei();                          //enables global interrupts
    init_ADC();                     //init ADC for channel 0, 8 bit resolution, ADC clock = clk/128
    Init_Timer1_PWM();              //timer1 and 2 as 8 bit output compares
    Init_Timer2_PWM();
    Init_IO();                      //enables things and interrupts 

    while(1)
    {       
        switch(display_select)
        {
            case 0x00:
                    while (display_select == 0x00)
                    {
                        MusicOnLed();
                    }
                break;
            case 0x01:
                    FadeOnLed();
                break;
            case 0xFF:          //standby glow faint purple
                RED = 1;
                GREEN = 0;
                BLUE = 1;
            break;
        }   
    }
}

void IncrementUp(uint8_t led, uint8_t max_val)
{
    uint8_t i = 0;      //LCV

    while((i < max_val) & (display_select == 0x01))
    {
        switch(led)
        {
            case 1:
            RED     = i;
            break;
            case 2:
            GREEN   = i;
            break;
            case 3:
            BLUE    = i;
            break;
        }
        i++;
        _delay_ms(led_delay);
    }
}
void IncrementDown(uint8_t led, uint8_t max_val)
{
    uint8_t i = 0;      //LCV

    while((i < max_val) & (display_select == 0x01))
    {
        switch(led)
        {
            case 1:
            RED     = max_val - i;
            break;
            case 2:
            GREEN   = max_val - i;
            break;
            case 3:
            BLUE    = max_val - i;
            break;
        }
        i++;
        _delay_ms(led_delay);
    }
}

void Init_IO(void)
{
    DDRB = 0x3F;                                            //portb as output

    SMCR |= (1 << SM0);                                     //Sets sleep mode 'ADC Noise Reduction'

    //DDRD  &= ~(1 << SW0) | ~(1 << SW1) | ~(1 << SW2);     //portd bits 2, 3, and 4 as inputs (pushbuttons)
    DDRD    |= (1 << SW0) | (1 << SW1) | (1 << SW2);        //ddrd set high
    PORTD   |= (1 << SW0) | (1 << SW1) | (1 << SW2);        //turn pullups on only on pushbutton input pins

    EICRA   = 0x0;                                          //Enables active LOW triggering on ext. interrupts on INT0 and INT1 pins (SW0 and SW1)
    EIMSK   |= (1 << INT1) | (1 << INT0);                   //Enables external interrupts on INT0 and INT1 (SW0 and SW1)    

    //PCMSK2    = 0x10;                                         //Enable interrupts on PCINT20 (SW2)
    //PCICR |= (1 << 2);                                    //Enables interrupts on pin change request 2 reg


    DDRC    = 0x00;                                         //port c as input
    PORTC   = 0x3F;                                         //pullups on
}

Answer 1

There is an infinite loop inside FadeOnLED(). Once INT1 is triggered, it will take your main() inside FadeOnLED() and as it contains while(1) there is no way out of it. So your main program flow will be stuck inside the infinite loop of FadeOnLED(). After that, other interrupts can shorty interrupt the work of this infinite loop(where your main program flow is), but will not get out (break) of it.

One possible solution (not very nice, but just to show the idea) is to use the following:

void FadeOnLed(void)
{
    GREEN   = 0;
    BLUE    = 0;
    RED     = 90;

    //see led_delay for led delay time
    //RED is defined as 1, GREEN as 2, BLUE as 3 when passing values to function
    while (display_select == 0x01)
    {
        ...
    }
}