My interrupt works, but if i #ifdef something seemingly unrelated it stops working

Question

working on an ATTINY13a. My interrupt stops working when I comment out a #define used for testing. I can get the interrupt to trigger and keep the led on my board lit while holding down the button, but only with #define BLINK_TEST active.

Both versions compile fine and upload to the board, but without the led blinking I can't get it to light up with the ISR.

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

#define LED1   PB0
#define LED2   PB1
#define BUTTON PB2
#define DEBOUNCE_TIME (400) // in _delay_us
#define BLINK_TEST
//#define BUTTON_TEST

//---------------------------Declare Func----------------------
void testBlink ();
void isButtonPressed ();
void led2_on ();
void led2_off ();

//---------------------------Declare Variables-----------------
uint8_t button_pressed = 0;


int main(void) {

//--------------------------IO Setup------------------------------
DDRB |= (1 << LED1);
DDRB |= (1 << LED2);
DDRB &= ~(1 << BUTTON); //set pushbuton as input
PORTB |= (1 << BUTTON); //enable pullup

//-------------------Pin Change Interrupt Setup-------------------
MCUCR |= (1 << ISC01);  //set interrupt to occur on falling edge
GIMSK |= (1 << INT0);   //external interrupt enable
PCMSK |= (1 << PCINT2); //set button pin(PB2) to accept interrupts

sei(); //enable global interrupts

while (1) {

//----------------------Sanity Check Blinky-------------------------
#ifdef BLINK_TEST
testBlink();
#endif 

#ifdef BUTTON_TEST
isButtonPressed();

if (button_pressed) {
    led2_on();
} else { 
    led2_off();
    }
#endif


}
    return 0;
}

//---------------------------Button Press ISR----------------------------
ISR (INT0_vect) {
    if (!(PINB & (1 << BUTTON))) {
        _delay_us (DEBOUNCE_TIME);
           if (!(PINB & (1 << BUTTON))) { 
                led2_on();
           } else {
                led2_off();
             }
    }
}

//--------------------------blinktest function---------------------------
#ifdef BLINK_TEST
void testBlink () {
    if (button_pressed == 0) {
        led2_on();
        _delay_ms (200);
        led2_off();
        _delay_ms (200);
    }  
} 
#endif

//--------------------------Detect Button Press--------------------------
#ifdef BUTTON_TEST
void isButtonPressed () {

    if (!(PINB & (1 << BUTTON))) {
    _delay_us(DEBOUNCE_TIME);
        if (!(PINB & (1 << BUTTON))) {
        button_pressed = 1;
        } else {
            button_pressed = 0;
        }
    }
}
#endif

//----------------------LED2 ON-----------------------
void led2_on () {
    PORTB |= (1 << LED2);
}
//----------------------LED2 OFF-----------------------
void led2_off () {
    PORTB &= ~(1 << LED2);
}

I tried changing the interrupt vector to (PCINT0) because the data sheet seems to show that where it compares C to assembly, but that didn't work. I've tried to change code inside my ISR but that doesn't seem to work either, but it does interrupt my little blink function when #define BLINK_TEST is active.

Answer 1

It looks like you're calling tesBlink() each iteration of the while loop. button_pressed doesn't get set, so you're just continually turning the LED off.

I would bet your ISR is working, but it's only turning the LED on for an unnoticeable amount of time

Answer 2

As noted in comments, having a busy-wait delay inside an ISR is a cardinal sin. ISRs should be written as quick and small as possible.

Additionally, you cannot debounce buttons by introducing a busy-wait. Because if you have a simple program like this, then probably 99% of the time spent by the program is spent busy-waiting. During which it will not register button presses in this case.

Instead you should have a timer ISR trigger cyclically, lets say every 5ms or so depending on "bounce" characteristics (400us sounds a fair bit too short for most generic switches/buttons). Pseudo code of the simplest possible debouncing algorithm:

volatile bool button_pressed = false;

ISR // triggered by timer, cyclically every x ms
{
  static bool prev_press;

  uint8_t new_press = PORTX;
  if(new_press == prev_press)
  {
    button_pressed = new_press;
  }

  prev_press = new_press;
}

volatile is required to prevent incorrect compiler optimizations in case a certain compiler does not realize that ISRs are functions called by hardware, not by software.

---

In general, busy-wait delays have no business at all in a correctly designed embedded systems. Use hardware peripheral timers for everything and wrap them in abstraction layers if necessary.