Interrupt on button press + debouncing

Question

I want to have an interrupt function executed whenever a button is pressed. The button is connected to pin 2 and the GND. Therefore, the pin is turned to LOW whenever the button is pressed. In addition to that, proper debouncing should be used As a proof of concept, the interrupt function should just toggle the BUILTIN_LED whenever the button is pressed.

I have tried many different approaches but i cannot make it work.

This is my last iteration:

bool led_status = LOW; // current state of output pin
int buttonState; // the current reading from the input pin
int lastButtonState = HIGH;  //the previous reading from the input pin

// the following variables are unsigned longs because the time, measured in
// milliseconds, will quickly become a bigger number than can be stored in an int.
unsigned long lastDebounceTime = 0;  // the last time the output pin was toggled
unsigned long debounceDelay = 50;    // the debounce time; increase if the output flickers

int button_switch = 2; // external interrupt pin
bool initialisation_complete = false; // inhibit any interrupts until initialisation is complete



// ISR for handling interrupt triggers arising from associated button switch
// check to see if you just pressed the button
// (i.e. the input went from LOW to HIGH), and you've waited long enough
// since the last press to ignore any noise:
void button_interrupt_handler()
{
  //static long int elapse_timer;
  
  if (initialisation_complete == true) //only able to run the ISR when arduino has finished initialization
  {
    // new interrupt so okay start a new button read process -
    // now need to wait for button release plus debounce period to elapse
    // this will be done in the button_read function

    int reading = digitalRead(button_switch);
    if ( reading != lastButtonState) // If the switch changed, due to noise or pressing:
    {
      lastDebounceTime = millis(); // reset the debouncing timer
    }

    // whatever the reading is at, it's been there for longer than the debounce
    // delay, so take it as the actual current state:
    if ( (millis() - lastDebounceTime) > debounceDelay)
    {
      if (reading != buttonState) // if the button state has changed:
      {
        buttonState = reading;
        if (buttonState == HIGH) // only toggle the LED if the new button state is HIGH
        {
          led_status = !led_status;
        }
      }
    }
    digitalWrite(LED_BUILTIN, led_status); //set the LED
    lastButtonState = reading; // save the reading. Next time through the loop, it'll be the lastButtonState:
  }
} // end of button_interrupt_handler


void setup()
{
  pinMode(LED_BUILTIN, OUTPUT);
  pinMode(button_switch, INPUT_PULLUP); //no res = change to INPUT_PULLUP
  attachInterrupt(digitalPinToInterrupt(button_switch), button_interrupt_handler, FALLING); //no res = change to FALLING
  digitalWrite(LED_BUILTIN, led_status);
  initialisation_complete = true; // open interrupt processing for business
}

void loop()
{
  //do nothing  
}

I am not sure what I am doing wrong or why this does not work.

EDIT: When I press the button, nothing happens at all.

Answer 1

Here is a non-blocking debouncing solution which is suitable also for electrically noisy environments. For example where there are long wires to the buttons which could pick up spurious signals. In this case, you cannot rely on short pulses being the direct result of button activity and must check that the button state has been stable for X ms before accepting it. It also handles potential bounces on the release of the button. It uses a simple finite state machine approach and does not use interrupts.

It can be tried also in a simulator: https://wokwi.com/projects/351021820324872788

const uint8_t ledPin = 13 ;
const uint8_t button_switch = 2;  // the button is wired between this pin and ground (LOW side)
const unsigned long debounceDelay = 100 ;    // the debounce time; increase if the output flickers

enum ButtonState  { PENDING_PRESS, IN_PRESS, PENDING_RELEASE} ;
ButtonState buttonState = ButtonState::PENDING_PRESS ;
uint32_t stateEnteredAtMs = millis() ;


void setup() {
  Serial.begin(115200);
  pinMode( ledPin, OUTPUT) ;
  pinMode( button_switch, INPUT_PULLUP) ;
  
}

void loop() {
  uint32_t ms = millis() ;
  static uint32_t lastButtonCheckAtMs = 0 ;

  if( ms - lastButtonCheckAtMs > 10 ) {

    switch ( buttonState ) {
 
     case ButtonState::PENDING_PRESS : {
        if ( digitalRead(button_switch)  == LOW ) {
          buttonState = ButtonState::IN_PRESS ;
          stateEnteredAtMs = ms ;
        }
        break ;
     }

     case ButtonState::IN_PRESS : {
        if ( digitalRead(button_switch) == HIGH )  {  
          if ( ms - stateEnteredAtMs < debounceDelay ) {
            stateEnteredAtMs = ms ;
            buttonState = ButtonState::PENDING_PRESS ;
          } 
          else {
            stateEnteredAtMs = ms ;
            buttonState = ButtonState::PENDING_RELEASE ;
            digitalWrite( ledPin, ! digitalRead(ledPin)) ;  // toggle led
          }
        }
        break ;
     }

     case ButtonState::PENDING_RELEASE : {
        if ( ms - stateEnteredAtMs > debounceDelay ) {
            stateEnteredAtMs = ms ;
            buttonState = ButtonState::PENDING_PRESS ;
          } 
        break ;
     }
    }
    lastButtonCheckAtMs = ms ;
  }
}

Answer 2

In almost any run of the ISR it will read the button state as LOW, because the ISR is triggered by the falling edge. The time from the falling edge to the reading is very short, about microseconds, and therefore a level change is improbable.

As others already suggest, you can make the ISR "deaf" for some time after the accepted falling edge. You can do this inside the ISR. The approach is to accept subsequent triggers only if they are long enough after the former one.

Please be aware that some buttons bounce even when released. You might experience a trigger in such cases.

Anyway, do not base a decision on the reading of button state after its falling edge.

---

The most simple solution to read buttons is a cyclic timer with a period of several milliseconds. Take a sample each 50ms or so, and detect the button change in software. You don't even need to add debouncing code, if the period is longer than the bouncing time of your button.

Since this timer will commonly trigger an ISR, you have already an interrupt as source of the button state for other parts of your software.

If you want an extra interrupt, trigger some unused interrupt in software.

Answer 3

Take a look at this line in your code - the very first thing that is done in the ISR,

int reading = digitalRead(button_switch);

I think that the reason that your program does not “do anything” is because you are reading the button state inside the ISR and, because you trigger an interrupt on a falling edge, you will ALWAYS see 0 as the button state at that point. It can never change when you read it once at the beginning of the ISR, it will always =0, unless you stay in the ISR and wait for a change, which is a very, very, bad idea.

The rest of your comparisons and detections all fail because within the ISR, the button read is always going to be the same and = 0, because that is how you got to the ISR. As far as your program is concerned, the button has never been pressed or depressed.

It is true, depending on how you configure the button; a single state change will be detected because of initialization, but that is besides the point.

You can test this yourself by moving all of your variables to volatile globals and then in your loop run this:

void loop()
{
Serial.print("lastButtonState ");
Serial.println(lastButtonState);
Serial.print("buttonState ");
Serial.println(buttonState);
Serial.print("reading ");
Serial.println(reading);
}

You may have other problems, but that one stuck out to me.

I would suggest that you take a look at this project and see how he is doing it - https://create.arduino.cc/projecthub/ronbentley1/button-switch-using-an-external-interrupt-7879df

He does read the switch within the interrupt [I don't see why he needs to test the value since in his case it will always be HIGH]. The difference is that he uses the ISR only as a trigger. The rest of the reading of the switch to detect when there are state changes, is all done outside of the ISR.

Additionally, once an interrupt occurs, a triggered status is set and, thereafter the ISR just leaves until the status is changed to "not triggered" and that is done outside of the ISR. The accompanying article explains what he is doing in the code and why.

You are doing, literally nothing [after setup()] outside of the ISR and I think maybe you should re-think that approach.

I tested his program and it works fine. I note that he functionally defines a "press" as pressing and releasing the button (with associated debouncing). So, you don't see the led toggle until you press and release the button. That's not always how I would like it, but it is also besides the point.

Two other points...

Keep your ISR as short as you can. I don't have any hard core evidence, but, right or wrong, yours looks longer than I would be comfortable with.

Finally, I think you would enjoy this excellent thread on millis() inside an ISR. Using millis() and micros() inside an interrupt routine

To be sure, you are NOT using delays inside an ISR and there is nothing wrong with using millis() inside an ISR, but you need to be aware of the behavior.

That's my two cents - hope it helps.