[solved] how to do proper ISR debouncing

For a test project, I would like to test out how ISR (Interrupt Service Routine) could be used.
The first thing I thought of was a button being pressed. Here below I have made basic code to make it work.

#define Button 3
volatile bool buttonPressed = false;
volatile unsigned long LastMicros;

void ISR() {
//Serial.println("interupt called");
  if ((long)(micros() - LastMicros) >= 20 * 1000) {
    if (digitalRead(Button) == LOW) {
      buttonPressed = true;
    }
    LastMicros = micros();
  }
}
void setup() {
  Serial.begin(9600);
  delay(1000);
  Serial.println("Beginning Main Setup");

  pinMode(Button, INPUT_PULLUP);
  attachInterrupt(Button, ISR, FALLING);

  Serial.println("Main Setup Done");
}

void loop() {
  // put your main code here, to run repeatedly:
  if (buttonPressed) {
    Serial.println("button was pressed");
    buttonPressed = false;
  }
}

But what I came across was that de debouncing didn't work properly.

First thing what happens is when I press and hold the button the ISR triggers only once (that how it should be) and then I release the button and sometimes it will also trigger for a second time. (that shouldn't happen)

I can't quite tell why it would trigger twice.
Could it be because of the "timeout" in the ISR has expired?

don't you want to check for a state change, not simply that the button is pressed?

gcjr:
don't you want to check for a state change, not simply that the button is pressed?

sorry for if that bit was unclear. I simply want to know if the button is pressed.

The thing that is a "bug" that it could and would sometimes trigger a second time when I would release the button. That shouldn't happen.

sorry, i didn't read you response correctly

the ISR is only triggered on the falling edge. so when the button is released, the interrupt is not triggered on the first rising edge and LastMicros was set long ago.

if there is a bounce, the ISR is triggered and long after LastMicros so it set buttonPressed.

thanks for your reply! I get what you're saying.
Now the big question how could I fix this?

I had the idea of lengthening the "timeout delay" but then the ISR could lockup other tasks or add a boolean check if the bool is false. like here below:

 if (((long)(micros() - LastMicros) >= 20 * 1000) && !buttonPressed) {

It could reduce the chance of triggering a second time but not completely.

if ((long)(micros() - LastMicros) >= 20 * 1000) {

Why are you casting to (long)? All micros-related math should be done with unsigned long.

gfvalvo:
Why are you casting to (long)? All micros-related math should be done with unsigned long.

well, I forgot to add unsigned. :sweat_smile:
and it didn't remove triggering for a second time but made the code more stable.

sebasdt1:
Now the big question how could I fix this?

trigger on all changes, not just falling

thanks a lot for your help!
it makes more sense now.

You forgot "digitalPinToInterrupt()" to translate the pin number to the interrupt number.

On my Arduino UNO the name 'ISR' is defined to be a string constant somewhere so it can't be used as a function name.

No reason to use micros() to measure millis() so I switched to millis().

const byte ButtonPin = 3;
volatile bool ButtonPressed = false;


void setup()
{
  Serial.begin(115200);
  delay(1000);
  Serial.println("Beginning Main Setup");


  pinMode(ButtonPin, INPUT_PULLUP);
  attachInterrupt(digitalPinToInterrupt(ButtonPin), ButtonISR, FALLING);


  Serial.println("Main Setup Done");
}


// Called each time ButtonPin goes from HIGH to LOW.
void ButtonISR()
{
  static unsigned long lastPressTime = 0;
  // Only process the press if it's been more than 20
  // milliseconds since the last press (to filter out bounces)
  if (millis() - lastPressTime >= 20)
  {
    ButtonPressed = true; 
    lastPressTime = millis();
  }
}


void loop()
{
  // put your main code here, to run repeatedly:
  if (ButtonPressed)
  {
    Serial.println("button was pressed");
    ButtonPressed = false;
  }
}