Debouncing with interrupts on a Mega

I am building this 2 channel pulse counter and I have to use an ISR to count button pushes.
I used the Debounce.h library but it does not work if two events (Change of state) happen at the same time.
I hope to have better luck with interrupts.

So I built a code that will count pulses if they stay longer than 100mS, to start with.

Works great. It debouces my limit switch perfectly, before I had a mess...

However: It also counts a pulse when I release the Switch, meaning from Low to hi.

Any ideas?

Thanks, Mitch

#define BUTTON_PIN 21 // Blue - pluse of the timer comes from the cord
#define Second_Button 20 //Wire to the Reject switch
#define KnobPushButton 15 // White
#define KeySwPin 16 //Brown

volatile int Good;
float GoodPercentage;
volatile int Rejects;

void setup() {
Serial.begin(9600);
attachInterrupt(2, GoodFun, FALLING); //interrupt 2 is pin 21 on mega-Button

attachInterrupt(3, GoodFun, FALLING); //interrupt 3 is pin 20 on mega-Second Button
pinMode(BUTTON_PIN, INPUT_PULLUP);
pinMode(Second_Button, INPUT_PULLUP);

Serial.print ("READY");
}

void GoodFun()
{
static unsigned long last_interrupt_time = 0;
unsigned long interrupt_time = millis();
// If interrupts come faster than 200ms, assume it's a bounce and ignore
if (interrupt_time - last_interrupt_time > 100)
{
Good++;
last_interrupt_time = interrupt_time;
}

}

void loop() {

Serial.print ("Good=");
Serial.println (Good);
//Serial.println (digitalRead (BUTTON_PIN));

}

laptophead:
However: It also counts a pulse when I release the Switch, meaning from Low to hi.

Any ideas?

if you hold

isn't it bouncing on the way up too?

I'm guessing that you are pressing for more than 200 mSecs, maybe?

Yes, many switches will give bounces upon release. With just a FALLING interrupt, you can't distinguish those.

Interrupts are not the solution for buttons pushed by humans. If you think it's the solution then you have something else to solve first.

Next time use [ code ] tags.

MorganS:
Interrupts are not the solution for buttons pushed by humans.

Unless you have some blocking code that you simply cannot get away from. Yes, it happens...

Show me a non-trivial example.

Hey, I write blocking code all the time. Some device needs a "startup" command sent and then you have to wait a second for it to start, so delay(1000) is absolutely the right solution for that job. But you won't see delays in the loop, where it is expected to respond to button presses and other useful events.

MorganS:
Show me a non-trivial example.

Hey, I write blocking code all the time. Some device needs a "startup" command sent and then you have to wait a second for it to start, so delay(1000) is absolutely the right solution for that job. But you won't see delays in the loop, where it is expected to respond to button presses and other useful events.

delay() isn't the only form of blocking.

there are real world examples of code that blocks for an unavoidable reason, like an internet server parsing out a web page or certain sensors that can take up to a second to respond.

There's a lot of sensor libraries that wait for the sensor to respond. The Dallas temperature library is a good example. Now I won't say that it's bad because it makes it very easy to work with those sensors but it's not going to fly in my code.

BulldogLowell:
delay() isn't the only form of blocking.

there are real world examples of code that blocks for an unavoidable reason, like an internet server parsing out a web page or certain sensors that can take up to a second to respond.

That's possibly not a global block if it's done right. The fact that a consumer process depends on a producer process output (e.g. waiting for a web page) and therefore has to stop while the producer stops, need not affect other processes. I actually have non-blocking code that parses a web page. It operates similarly to Robin's serial input examples in how it avoids blocking. Some code that uses processor cycles in a direct way, such as for timing, must block and that is a reason why there are so many hardware assisted peripheral solutions.

Thanks everyone.

Yes, it seems that I have a dirty signal from LOW TO HIGH and that registers as a new "FALLING"

So I tried to debounce it on the way up. See the code.
It did not work, still shows a new count when I release the limit switch.

Where am I going wrong??

InterruptStudy.ino (1.67 KB)

Please post your code inline like this instead of as an attachment unless it's too large. Auto format before posting also helps.

#define BUTTON_PIN 21  // Blue - pluse of the timer comes from the cord
#define Second_Button 20 //Wire to the Reject switch
#define KnobPushButton 15 // White
#define KeySwPin 16  //Brown 

volatile int Good;
float GoodPercentage;
volatile int Rejects;


void setup() {
  Serial.begin(9600);
  attachInterrupt(2, GoodFun, FALLING); //interrupt 2 is pin 21 on mega-Button

  // attachInterrupt(3, RjctFun, FALLING); //interrupt 3 is pin 20 on mega-Second Button
  pinMode(BUTTON_PIN, INPUT_PULLUP);
  pinMode(Second_Button, INPUT_PULLUP);


  Serial.print ("READY");
}

void GoodFun()
{
  static unsigned long last_interrupt_time = 0;
  unsigned long interrupt_time = millis();
  // If interrupts come faster than 10ms, assume it's a bounce and ignore
  if (interrupt_time - last_interrupt_time > 10)
  {
    Good++;
    last_interrupt_time = interrupt_time;
    //Button is LOW, Preparing for the Raising condition
    detachInterrupt(2);
    attachInterrupt(2, GoodFun, RISING);
  }

}

void GoodFunRISING()
{
  static unsigned long last_interrupt_time = 0;
  unsigned long interrupt_time = millis();
  // If interrupts come faster than 10ms, assume it's a bounce and ignore
  if (interrupt_time - last_interrupt_time > 10)
  {

    Good = Good; // dont want to ad to value.
    // Button Goes to HIGH and Nothing should happen for 10mS Till the Switch settles down.
    // then we prepare for the next Falling condition
    last_interrupt_time = interrupt_time;
    detachInterrupt(2);
    attachInterrupt(2, GoodFun, FALLING);
  }


}

void loop() {

  Serial.print ("Good=");
  Serial.print (Good);
  Serial.print (" . ");
  Serial.print ("Rejects=");
  Serial.print (Rejects);
  Serial.println (digitalRead (BUTTON_PIN));

}

Shuffling interrupt modes and detaching/attaching interrupts is rarely good practice. This might be a good time for hardware debounce with a capacitor or RC network.

You are also not attaching your RISING ISR. with

attachInterrupt(2, GoodFun, RISING);

Try

attachInterrupt(2, GoodFunRISING, RISING);

If you kept the state in a variable, you could debounce both the press and the release, but only trigger on one of them.

laptophead:
I am building this 2 channel pulse counter and I have to use an ISR to count button pushes.
I used the Debounce.h library but it does not work if two events (Change of state) happen at the same time.
I hope to have better luck with interrupts.

You are doing it the hard way.

Think about what bounce IS:

Your switch is open, you flip it to closed and the N.O. contact begins to move toward the COM contact.

When they touch, the pads on the contacts compress slightly like springs, storing the kinetic energy of the moving N.O. contact, then when the kinetic energy of the moving contact is completely stored in the compressed contact pads, movement stops and the stored (potential) energy in the compressed contact pads is released, pushing the contacts apart.

This cycle repeats several times (4 or 5 times or as many as 25 times) and each bounce is seen by the (relatively) high speed electronics as individual switch actuations.

What's important to know is how LONG a bounce event lasts (which is usually a millisecond or less with "ordinary sized" switches such as used in Arduino projects. Snap action microswitches bounce a LOT more (more times and for longer), but even this is only 3 to 5 milliseconds.

To optimize your software, you could put an oscilloscope on the switch, actuate it and measure the bounce time. Or, you can just choose an absurdly long time that's more than good enough for ANY switch.

So, we need a piece of software that detects a switch closure, then begins a time countdown. If the switch bounces, reset the count down value and keep on checking. When the switch is stable, exit with the switch status.

HOW to do it? Something like this (not actual copy-able code, but it should give you an idea (code assumes a switch that pulls an Arduino pin to ground, and the pin is pulled up high with a resistor or by [b]pinMode(INPUT_PULLUP)[/b] :

uint8_t debounce (int pin)
{
    uint8_t deb = 100; // debounce counter loaded with arbitrary value
    if (digitalRead (pin) == HIGH) {
        return 0; // if switch isn't closed, just exit with code
    }
    while (deb--) {
        if (digitalRead (pin) == HIGH) {
            deb = 100; // switched bounced, reset deb
        }
    }
    return 1; // switch was stable for "deb" counts, so accept it.
}

Hope this helps.

You're pretty close. Try this:

#define BUTTON_PIN 21  // Blue - pluse of the timer comes from the cord
#define Second_Button 20 //Wire to the Reject switch
#define KnobPushButton 15 // White
#define KeySwPin 16  //Brown 

volatile int Good;
float GoodPercentage;
volatile int Rejects;


void setup() {
  Serial.begin(9600);
  
  pinMode(BUTTON_PIN, INPUT_PULLUP);
  attachInterrupt(digitalPinToInterrupt(BUTTON_PIN), GoodFun, CHANGE);  // Interrupt on rising or falling.

  pinMode(Second_Button, INPUT_PULLUP);


  Serial.print ("READY");
}

void GoodFun()
{
  static unsigned long last_interrupt_time = 0;
  unsigned long interrupt_time = millis();
  // If interrupts come faster than 10ms, assume it's a bounce and ignore
  if (interrupt_time - last_interrupt_time > 10)
  {
    if (digitalRead(BUTTON_PIN) == LOW)      // Only count presses -- not releases
      Good++;
    else
      ++Rejects;
    last_interrupt_time = interrupt_time;  // This will make sure we ignore bounce on release.
  }
  else
    ++Rejects;
}


void loop() {
  int storeGood;
  int storeRejects;

  // Bracket this in a critical section.  Otherwise we might get interrupted in the middle and get weird data.
  cli();
  storeGood = Good;
  storeRejects = Rejects;
  sei();
  Serial.print ("Good=");
  Serial.print (storeGood);
  Serial.print (" . ");
  Serial.print ("Rejects=");
  Serial.print (storeRejects);
  Serial.println (digitalRead (BUTTON_PIN));

}

Jimmus:
You're pretty close. Try this:

#define BUTTON_PIN 21  // Blue - pluse of the timer comes from the cord

#define Second_Button 20 //Wire to the Reject switch
#define KnobPushButton 15 // White
#define KeySwPin 16  //Brown

volatile int Good;
float GoodPercentage;
volatile int Rejects;

void setup() {
 Serial.begin(9600);
 
 pinMode(BUTTON_PIN, INPUT_PULLUP);
 attachInterrupt(digitalPinToInterrupt(BUTTON_PIN), GoodFun, CHANGE);  // Interrupt on rising or falling.

Why advise him to use interrupts for this? It's OVERLY complex, totally un-necessary and consumes resources that don't need to be used, slowing down the program.

Why advice him to use interrupts for this? It's OVERLY complex, totally un-necessary and consumes resources that don't need to be used, slowing down the program.

That may be. I'm not sure why he was advised to use interrupts. He didn't say. He only said, "I have to use an ISR...". Maybe this is a school assignment and that was part of the constraints. Maybe this is to be part of a larger project and it needs near real-time responses. Maybe he tried other ways and they didn't work, and he thinks using an interrupt will work.
He didn't seem inclined to elaborate. But his code is very close. With a couple of small adjustments I think it will work. The only complexities I added were making the ISR fire on rising edge as well and adding the critical section. I didn't even add a state variable, since the time stamp he already implemented will work fine for that. In any case, I'm not the one who gets to decide whether it is overly complex. Neither are you.

Perhaps he will choose to use your method. It looks like it will work. Although your suggested spin wait loop will most likely slow down the program way more than any interrupt.

I don't expect that performance is the big issue here. I don't expect complexity is the big issue here, either. I expect he wants to get his code working. Now he has three options.

Thanks everyone,
To the member Cattledog, I did your correction, "attachInterrupt(2, GoodFunRISING, RISING);" you were right.

The results improved, but I still get a count on the RISING condition intermittently. Where am I going wrong?

Regarding why use interrupts?

The results of these counts will be displayed on a TFT that plugs in the MEGA.

https://www.amazon.com/kuman-Display-Screen-Socket-Arduino/dp/B01J3B08P8/ref=sr_1_3?ie=UTF8&qid=1513100324&sr=8-3&keywords=3.5+tft+arduino

The pertinent libraries are:
#include <Adafruit_GFX.h> // Core graphics library
#include <Adafruit_TFTLCD.h> // Hardware-specific library

The letters and numbers to be dislpayed take a while, about 400mS.

If I dont use interrupts and use digitalRead:

While the display is refreshing the digitalread does not register. the Mega is busy and skips it.

Also if I do read 2 channels and the pulse comes a the same time, only one is read, intermittently.

So this does work but for a very slow counter.

If I do use interrupts:
Both channels register, even if they come in the same time. Even if the display is refreshing, I dont miss a count. Nice. The display refresh is stopped maybe for a mS , not perceptible.

The problem: My counts are not accurate. A correct debounced count is going on FALLING But another one is registered on RISING>

Can we please build this on the ISR procedure?

Thanks

Here is my latest code.

#define BUTTON_PIN 21 // Blue - pluse of the timer comes from the cord
#define Second_Button 20 //Wire to the Reject switch
#define KnobPushButton 15 // White
#define KeySwPin 16 //Brown

volatile int Good;
float GoodPercentage;
volatile int Rejects;

void setup() {
Serial.begin(9600);
attachInterrupt(2, GoodFun, FALLING); //interrupt 2 is pin 21 on mega-Button

// attachInterrupt(3, RjctFun, FALLING); //interrupt 3 is pin 20 on mega-Second Button
pinMode(BUTTON_PIN, INPUT_PULLUP);
pinMode(Second_Button, INPUT_PULLUP);

Serial.print ("READY");
}

void GoodFun()
{
static unsigned long last_interrupt_time = 0;
unsigned long interrupt_time = millis();
// If interrupts come faster than 10ms, assume it's a bounce and ignore
if (interrupt_time - last_interrupt_time > 20)
{
Good++;
last_interrupt_time = interrupt_time;
//Button is LOW, Preparing for the Raising condition
detachInterrupt(2);
attachInterrupt(2, GoodFunRISING, RISING);
}

}

void GoodFunRISING()
{
static unsigned long last_interrupt_time = 0;
unsigned long interrupt_time = millis();
// If interrupts come faster than 10ms, assume it's a bounce and ignore
if (interrupt_time - last_interrupt_time > 20)
{

//Good=Good; // dont want to ad to value.
// Button Goes to HIGH and Nothing should happen for 10mS Till the Switch settles down.
// then we prepare for the next Falling condition
last_interrupt_time = interrupt_time;
detachInterrupt(2);
attachInterrupt(2, GoodFun, FALLING);
}

}

void loop() {

Serial.print ("Good=");
Serial.print (Good);
Serial.print (" . ");
Serial.print ("Rejects=");
Serial.print (Rejects);
Serial.println (digitalRead (BUTTON_PIN));

}

So here is what I would do, and why.

You need to capture the rising edge of the interrupt. Otherwise, when you release the button, if it bounces, you miss the rising edge but catch the falling edge of the bounce. You would not be able to tell the difference between that and a button press. If you catch the rising edge, you timestamp that and then if you catch the falling edge of the bounce, you can safely label it as a bounce and ignore it.

It is easy to catch both edges of the interrupt in the same ISR. You just attach with CHANGE instead of FALLING or RISING. I would not trust attaching and de-attaching inside the ISR. It is simple to tell if you caught the rising edge or the falling edge by checking the current state of the pin. You can use digitalRead() or fastDigitalRead() or check the PINx pseudo variable directly.

If you really want to count ignored interrupts, that can be done as well, but you have to actually increment the count.

With that in mind, I took your code and modified it a little bit. Maybe this will work for you:

#define BUTTON_PIN 21  // Blue - pluse of the timer comes from the cord
#define Second_Button 20 //Wire to the Reject switch
#define KnobPushButton 15 // White
#define KeySwPin 16  //Brown 

volatile int Good;
float GoodPercentage;
volatile int Rejects;


void setup() {
  Serial.begin(9600);
  
  pinMode(BUTTON_PIN, INPUT_PULLUP);
  attachInterrupt(digitalPinToInterrupt(BUTTON_PIN), GoodFun, CHANGE);  // Interrupt on rising or falling.

  pinMode(Second_Button, INPUT_PULLUP);


  Serial.print ("READY");
}

void GoodFun()
{
  static unsigned long last_interrupt_time = 0;
  unsigned long interrupt_time = millis();
  // If interrupts come faster than 10ms, assume it's a bounce and ignore
  if (interrupt_time - last_interrupt_time > 10)
  {
    if (digitalRead(BUTTON_PIN) == LOW)      // Only count presses -- not releases
      Good++;
    else
      ++Rejects;
    last_interrupt_time = interrupt_time;  // This will make sure we ignore bounce on release.
  }
  else
    ++Rejects;
}


void loop() {
  int storeGood;
  int storeRejects;

  // Bracket this in a critical section.  Otherwise we might get interrupted in the middle and get weird data.
  cli();
  storeGood = Good;
  storeRejects = Rejects;
  sei();
  Serial.print ("Good=");
  Serial.print (storeGood);
  Serial.print (" . ");
  Serial.print ("Rejects=");
  Serial.print (storeRejects);
  Serial.println (digitalRead (BUTTON_PIN));

}

Jimmus,
I appreciate your help. However, out of 10 clicks I still go 15 "Good" counts.

What I think happens:
if (digitalRead(BUTTON_PIN) == LOW) // Only count presses -- not releases

Your condition makes sense, but on the release, the button makes a HIGH And a LOW and a High again and that is enough to trigger a count.

The rejects will be assigned its own button, "Second_Button" and we'll make its own function.

So I removed the Reject count from the good function.

Please stick with me, I think we're close.

void GoodFun()
{
static unsigned long last_interrupt_time = 0;
unsigned long interrupt_time = millis();
// If interrupts come faster than 10ms, assume it's a bounce and ignore
if (interrupt_time - last_interrupt_time > 20)
{
if (digitalRead(BUTTON_PIN) == LOW) // Only count presses -- not releases
Good++;
last_interrupt_time = interrupt_time; // This will make sure we ignore bounce on release.
}

}

Oops. Try this:

void GoodFun()
{
  static unsigned long last_interrupt_time = 0;
  unsigned long interrupt_time = millis();
  // If interrupts come faster than 10ms, assume it's a bounce and ignore
  if (interrupt_time - last_interrupt_time > 10)
  {
    if (digitalRead(BUTTON_PIN) == LOW)      // Only count presses -- not releases
      Good++;
  }
  last_interrupt_time = interrupt_time;  // This will make sure we ignore bounce on release.
}

Jim, I tried it,

I get the same results, every other release registers as a Good.

Any ideas?