C for AVR Application - ISR Repetition

Question

I'm trying to get a simple interrupt routine to work on an ATMega328P. There's an LED connected to PD6 and a built-in button on PB7. The LED should blink normally until the button is pressed and then go solid for 1.5s before going back to blinking. Here's the code:

#define F_CPU 16000000UL
#include <avr/io.h>
#include <avr/interrupt.h>
#include <util/delay.h>

int main(void)
{
    // Enable pull-ups and set pin directions
    MCUCR |= (1<<PUD);
    PORTD &= ~(1<<PORTD6);
    DDRD |= (1<<DDD6);
    PORTB |= (1<<PORTB7);
    DDRB &= ~(1<<DDB7);

    // Enable pin change interrupt
    PCICR = 0x01;
    PCMSK0 = 0x80;
    sei();

    while (1) 
    {
        // Blink LED at standard rate
        _delay_ms(500);
        PORTD ^= (1<<PORTD6);
        _delay_ms(500);
        PORTD ^= (1<<PORTD6);
    }
}

ISR(PCINT0_vect,ISR_BLOCK)
{
    PORTD &= ~(1<<PORTD6);
    _delay_ms(500);
    PORTD |= (1<<PORTD6);
    _delay_ms(1500);
    PORTD &= ~(1<<PORTD6);
}

The interrupt triggers correctly, however the ISR routine loops twice. I imagine it's some sort of button bounce issue but I'm unfamiliar with how to deal with it. I tried introducing the 500ms delay at the beginning and I've also tried clearing the pin change interrupt flag within the ISR so that it doesn't trigger again but it still does. Thanks in advance for any help!

Answer 1

Let's work on the basis that you are happy to ignore any button presses while the LED is on for 1.5 seconds. You could write your interrupt handler like this:

ISR(PCINT0_vect,ISR_BLOCK)
{
    button_pressed = 1;
}

and put this next the top of your code:

volatile int button_pressed = 0;

(See this page for information on what volatile is all about and why it's needed here.)

Then your main loop can look like this:

while (1) 
{
    // Blink LED on and off

    PORTD |= (1<<PORTD6);   // Turn LED on.
    if (button_pressed) {
        _delay_ms(1500);    // Long delay if button was pressed.
        button_pressed = 0;
    } else {
        _delay_ms(500);     // Regular delay otherwise.
    }

    PORTD &= ~(1<<PORTD6);  // Turn LED off.
    _delay_ms(500);
}

---

Notes for advanced readers:

1. volatile int button_pressed = 0; could actually be just volatile int button_pressed;, as static ints at file scope are initialised to 0, but it's a lot clearer to initialise explicitly.

2. C programs often use for (;;) as the idiom for "loop forever" instead of while (1).