Debounce Function

I tried making a variation of the website's button debounce program, but I couldn't get it to work. I wanted to use a debounce function but its making my led act all funky. Sometimes it turns off, sometimes it doesn't. I'm assuming there is a problem with my debouncing but I can't figure it out. Here is my code.

boolean lastButtonState=LOW;
long lastDebounceTime=0;
boolean ledState=HIGH;
boolean currentButtonState=LOW;
const int button = 2;
const int LED = 4;

void setup() {
  // put your setup code here, to run once:
pinMode(button,INPUT);
pinMode(LED,OUTPUT);
}

boolean debounce(boolean lastButton, long lastDebounce, int BUTTON){
  boolean current = digitalRead(BUTTON);
  if (current != lastButton && millis()-lastDebounce > 50){
    current = digitalRead(BUTTON);
  }
  return current;
}

void loop() {
  // put your main code here, to run repeatedly:
currentButtonState = debounce(lastButtonState, lastDebounceTime, button);
if (lastButtonState==LOW && currentButtonState==HIGH){
  ledState= !ledState;
  lastDebounceTime=millis();
}
lastButtonState=currentButtonState;

digitalWrite(LED, ledState);
}

DRLDavis: I tried making a variation of the website's button debounce program, but I couldn't get it to work. I wanted to use a debounce function but its making my led act all funky. Sometimes it turns off, sometimes it doesn't. I'm assuming there is a problem with my debouncing but I can't figure it out.

This:

boolean debounce(boolean lastButton, long lastDebounce, int BUTTON){
  boolean current = digitalRead(BUTTON);
  if (current != lastButton && millis()-lastDebounce > 50){
    current = digitalRead(BUTTON);
  }
  return current;
}

is essentially the same as this:

boolean debounce(boolean lastButton, long lastDebounce, int BUTTON){
  boolean current = digitalRead(BUTTON);
  return current;
}

Hence, your problem.

DRLDavis: I'm assuming there is a problem with my debouncing but I can't figure it out. Here is my code.

You know what they say about assumptions...

Do you have a pull down resistor on the input pin in your circuit?

If not, pinMode(button,INPUT); Will leave the input pin 'floating' and your LED will act all funky.

If you do not have a pull down resistor, you can enable the internal pull up resistor with pinMode(button, INPUT_PULLUP); However, the button state will be inverted.

Well, he at least has a problem with his debounce code as mentioned above. As written, it does not do any debouncing.

OP, your debounce function blocks execution until it is done.

Code that blocks generally wastes huge numbers of cycles (16000 per milli) doing nothing. That may be okay for single-task code but it can totally screw multi-tasking on Arduinos that lets you have things like status lights (the blink tells you the sketch is running) and always-responsive buttons.

Here is an example of multi-tasking button & led code ready to add more (user serial commands?). Note that the main parts in loop() are separate and act independently but NONE of them block.

// All you need to do this example is an Arduino and a jumper in pin 2.
// Push and let go the button once, led blinks. Again, led turns off.
// Button is debounced and wired directly to ground or, the button can 
// be a jumper from pin 2 grounded on USB connector. Tested with jumper.
// By GoForSmoke for free public use. Using Arduino 1.0.5-r2

/*
This sketch uses millis() timing but only the low bytes. Unsigned math
 lets this work just as well as with all 32 bits.
 This sketch has 3 separated code blocks. 
 --- 1 for the button that changes buttonState only after a debounced change.
 --- 1 that processes changes in buttonState to control led blink.
 --- 1 that blinks the led when it's supposed to.
 Each code block could be replaced by another with minimal change to the others.
 More blocks can be added, they don't have to deal with the led but they could.
 If you want to merge sketches then here is one strategy to do it.
 */

// Look Ma, no extra libraries!

const byte ledPin =  13;      // this is the onboard led pin
const byte buttonPin = 2;


byte ledPinState = 0;
byte blinkLed = 0; // led stays off when 0, blinks when 1
const word ledBlinkMs = 1000U; // blink interval
word ledBlinkNowMs = 0U; // low 16 bits of millis()
word ledBlinkStartMs = 0U; // 16 bit blink (on or off) time ms

// button variables
byte buttonRead;  // wired for pullup -- if down then LOW, if up then HIGH 
byte lastButtonRead = HIGH; // so debounce knows previous read
byte checkDebounce = 0; // only checks decounce after every button pin change
byte lastButtonState = 0; // last stable button state
byte buttonState = 0;  // stable button state
// 0 = button is up after debounce
// 1 = button is down after debounce
// button debounce timing variables
const word debounceMs = 300; // debounced when reads for debounceMs ms are the same
word debounceMillisLowByte = 0; // only need to count to 10, don't need 32 bits
word msNow = 0; // don't worry, unsigned rollover makes it work 


byte processState = 0;  // the button gets pushed and released twice
// 0 = 1st press has not happened, led blink is OFF, looking for press
// 1 = 1st press is down, led blink is ON, looking for release
// 2 = 1st push relesased, led blink stays ON, looking for 2nd press
// 3 = 2nd press is down, led blink is OFF, looking for release
// there is no state 4, 2nd push release changes state to 0


void setup() 
{
  Serial.begin( 115200 );
  Serial.println( F( "\n Blinking Status Light with ON/OFF Button" ));
  Serial.println( F( "Ground pin 2 to press the button else not pressed\n" ));

  pinMode( ledPin, OUTPUT );  // default is LOW 
  pinMode( buttonPin, INPUT_PULLUP ); // my button connects to ground, not 5V
  // however that means that when the button is pressed the pin is LOW.
}

void loop() // make sure that loop() runs fast and stays in the "now".
{

  // BUTTON CODE BLOCK, it handles debouncing. 
  // the task is to set the variable buttonState when a Stable Button State is reached.
  // other sensor code could change the same variable if desired

  // read the pin which may be changing fast and often as the jumper contacts the housing
  buttonRead = digitalRead( buttonPin ); // momentary state

  // msNow gets the low bytes of millis() to time the very short debounce time
  msNow = (word)( millis() & 0xFFFF ); // set once, used in 2 places

  if ( buttonRead != lastButtonRead )
  {
    debounceMillisLowByte = msNow;
    checkDebounce = 1;
  }
  else if ( checkDebounce )
  { 
    if ( msNow - debounceMillisLowByte >= debounceMs ) // stable button state achieved
    {
      buttonState = !buttonRead; // mission accomplished, button is stable
      // note that buttonState is opposite buttonRead
      checkDebounce = 0; // stop debounce checking until pin change
    }
  }
  lastButtonRead = buttonRead;
  //
  // End of the BUTTON CODE BLOCK

  //==================================================================================

  // LED CONTROL CODE BLOCK that uses buttonState and processState
  if ( lastButtonState != buttonState )
  {
    lastButtonState = buttonState;

    // debug type prints -- also testing some loading, does it alter the blink much?
    Serial.println( F( "============================================================" ));    
    Serial.print( F( "processState " ));
    Serial.print( processState );
    Serial.print( F( "  buttonState " ));
    Serial.println( buttonState );
    Serial.print( F( "  millis " ));
    Serial.println( msNow ); // the low 16 bits of millis, rolls over at 65535+1
    Serial.println( F( "============================================================" ));    

    switch ( processState )
    {
    case 0: // 0 = 1st press has not happened, led is OFF, looking for press
      if ( buttonState == 1 ) // button is pressed 
      {
        processState = 1;
        blinkLed = 1;
        ledPinState = 0;
        ledBlinkStartMs = (word) ( millis() & 0xFFFF )- ledBlinkMs; 
      }
      break; // note that until the 1st press, this case runs over and over

    case 1: // 1 = 1st press is down, led is ON, looking for release
      if ( buttonState == 0 ) // button is released 
      {
        processState = 2;
      }
      break; // note that until the 1st release, this case runs over and over

    case 2: // 2 = 1st push relesased, led stays ON, looking for 2nd press
      if ( buttonState == 1 ) // button is pressed 
      {
        processState = 3;
        blinkLed = 0;
      }
      break; // note that until the 2nd press, this case runs over and over

    case 3: // 3 = 2nd press is down, led is OFF, looking for release
      if ( buttonState == 0 ) // button is released 
      {
        processState = 0; // back to the start
      }
      break; // note that until the 2nd release, this case runs over and over
    } 
  }
  // End of the LED CONTROL CODE

  //==================================================================================

  // LED BLINK CODE BLOCK
  if ( blinkLed )
  { 
    ledBlinkNowMs = (word) ( millis() & 0xFFFF );

    if ( ledBlinkNowMs - ledBlinkStartMs >= ledBlinkMs )
    { 
      ledPinState = !ledPinState;
      digitalWrite(ledPin, ledPinState );
      ledBlinkStartMs = ledBlinkNowMs;
    }
  }
  else
  {
    ledPinState = LOW;
    digitalWrite(ledPin, LOW );
  }
  // End of the LED BLINK CODE

  //==================================================================================

  // Want to add serial commands and args input? 
  // this is a good spot. Serial is so slow it can usually go last.
}

GoForSmoke: OP, your debounce function blocks execution until it is done.

No, it doesn't.

Actually, the way it is at the moment, it does nothing except read the button.

arduinodlb: Well, he at least has a problem with his debounce code as mentioned above. As written, it does not do any debouncing.

I do not doubt it. You covered the code. I thought it worth not assuming the hardware works any better.

arduinodlb: No, it doesn't.

Actually, the way it is at the moment, it does nothing except read the button.

Right, I didn't read that code right at all.

I hope my old code shows something that will help. I have newer library objects that do a bit better job, debounce jumpers making contact in 2 ms.

I appreciate everyone's help. I just don't understand why the debounce doesn't work. I tried changing a few things but its still not working...

boolean debounce(boolean lastButton, long lastDebounce, int BUTTON){
  boolean current = digitalRead(BUTTON);
  if (current != lastButton){
    lastDebounceTime=millis();
  if ((millis()-lastDebounce) > 50){
    current = digitalRead(BUTTON);
  }
  }
  return current;
}

void loop() {
  // put your main code here, to run repeatedly:
currentButtonState = debounce(lastButtonState, lastDebounceTime, button);
if (lastButtonState==LOW && currentButtonState==HIGH){
  ledState= !ledState;
}
lastButtonState=currentButtonState;

digitalWrite(LED, ledState);

DRLDavis: I appreciate everyone's help. I just don't understand why the debounce doesn't work. I tried adding

if (current != lastButton){
    lastDebounceTime=millis();

before

 if (millis()-lastDebounce > 50){
    current = digitalRead(BUTTON);
  }

but that didn't help...

What is the difference between:

     int value = 20;
     return value;

and

    int value = 20;
    if (true)
        value = 20;
    return value;

answer: nothing.

but, make your changes, and then post your whole code so we can look at it again, even if you think it's wrong.

Ok here is my new code

boolean lastButtonState=LOW;
long lastDebounceTime=0;
boolean ledState=HIGH;
boolean currentButtonState=LOW;
const int button = 2;
const int LED = 4;

void setup() {
  // put your setup code here, to run once:
pinMode(button,INPUT);
pinMode(LED,OUTPUT);
}

boolean debounce(boolean lastButton, long lastDebounce, int BUTTON){
  boolean current = digitalRead(BUTTON);
  if (current != lastButton){
    lastDebounceTime=millis();
  if ((millis()-lastDebounce) > 50){
    current = digitalRead(BUTTON);
  }
  }
  return current;
}

void loop() {
  // put your main code here, to run repeatedly:
currentButtonState = debounce(lastButtonState, lastDebounceTime, button);
if (lastButtonState==LOW && currentButtonState==HIGH){
  ledState= !ledState;
}
lastButtonState=currentButtonState;

digitalWrite(LED, ledState);
}

I thought that

if (current != lastButton){
    lastDebounceTime=millis();

Would reset the debouncing timer every time the state of the switch changes. This way the state of the button wouldn't change until 50 millisecods after the last bounce in the button.

Here is how they do it in the debounce tutorial on this website

const int buttonPin = 2;    // the number of the pushbutton pin
const int ledPin = 13;      // the number of the LED pin

// Variables will change:
int ledState = HIGH;         // the current state of the output pin
int buttonState;             // the current reading from the input pin
int lastButtonState = LOW;   // the previous reading from the input pin

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

void setup() {
  pinMode(buttonPin, INPUT);
  pinMode(ledPin, OUTPUT);

  // set initial LED state
  digitalWrite(ledPin, ledState);
}

void loop() {
  // read the state of the switch into a local variable:
  int reading = digitalRead(buttonPin);

  // 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:

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

  if ((millis() - lastDebounceTime) > debounceDelay) {
    // whatever the reading is at, it's been there for longer
    // than the debounce delay, so take it as the actual current state:

    // if the button state has changed:
    if (reading != buttonState) {
      buttonState = reading;

      // only toggle the LED if the new button state is HIGH
      if (buttonState == HIGH) {
        ledState = !ledState;
      }
    }
  }

  // set the LED:
  digitalWrite(ledPin, ledState);

  // save the reading.  Next time through the loop,
  // it'll be the lastButtonState:
  lastButtonState = reading;
}

I was trying to essentially the same thing but using a function. I realize I could just do it this way, but I really want to understand why mine isn't working.

DRLDavis:
Ok here is my new code

I thought that

if (current != lastButton){

lastDebounceTime=millis();




Would reset the debouncing timer every time the state of the switch changes. This way the state of the button wouldn't change until 50 millisecods after the last bounce in the button.

I was trying to essentially the same thing but using a function. I realize I could just do it this way, but I really want to understand why mine isn't working.
boolean debounce(boolean lastButton, long lastDebounce, int BUTTON){
  boolean current = digitalRead(BUTTON);
  if (current != lastButton){
    lastDebounceTime=millis();
  if ((millis()-lastDebounce) > 50){
    current = digitalRead(BUTTON);
  }
  }
  return current;
}

Your current problem is not your timer.

Your problem is your “if” condition does nothing different.

You are essentially doing this:

    if (some condition)
       return current_button;
    else
       return current_button;

so the “if” condition does nothing.

DRLDavis: Ok here is my new code

boolean debounce(boolean lastButton, long lastDebounce, int BUTTON){
  boolean current = digitalRead(BUTTON);
  if (current != lastButton){
    lastDebounceTime=millis();
  if ((millis()-lastDebounce) > 50){
    current = digitalRead(BUTTON);
  }
  }
  return current;
}

Here is how they do it in the debounce tutorial on this website

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

  if ((millis() - lastDebounceTime) > debounceDelay) {     // whatever the reading is at, it's been there for longer     // than the debounce delay, so take it as the actual current state:

    // if the button state has changed:     if (reading != buttonState) {       buttonState = reading;

      // only toggle the LED if the new button state is HIGH       if (buttonState == HIGH) {         ledState = !ledState;       }     }   }

Notice how in the tutorial code the debounce is done by two separate if () {} statements? They both run, regardless.

Notice how in your code the second if () {} is inside the first? It only runs when the pin state changes (and the timer reset), not after.

I tried changing the if statement but that didn't help.

boolean debounce(boolean lastButton, long lastDebounce, int BUTTON){
  boolean current = digitalRead(BUTTON);
  if (current != lastButton){
    lastDebounceTime=millis();
    }
  if ((millis()-lastDebounce) > 50){
    current = digitalRead(BUTTON);}
  return current;
}

I really don't see what the difference is between my code and this

void loop() {
  // read the state of the switch into a local variable:
  int reading = digitalRead(buttonPin);


  if (reading != lastButtonState) {
  
    lastDebounceTime = millis();
  }

  if ((millis() - lastDebounceTime) > debounceDelay) {
    

    if (reading != buttonState) {
      buttonState = reading;

   
      if (buttonState == HIGH) {
        ledState = !ledState;

Yet for some reason this one ^ works...

DRLDavis:
I tried changing the if statement but that didn’t help.

I really don’t see what the difference is between my code and this

The difference is that the:

      // only toggle the LED if the new button state is HIGH
      if (buttonState == HIGH) {
        ledState = !ledState;

happens INSIDE the:

  if ((millis() - lastDebounceTime) > debounceDelay) {
    // whatever the reading is at, it's been there for longer
    // than the debounce delay, so take it as the actual current state:

    // if the button state has changed:
    if (reading != buttonState) {
      buttonState = reading;

      // ...WE ARE DOING OUR LED TOGGLE HERE...., inside the IF

      // only toggle the LED if the new button state is HIGH
      if (buttonState == HIGH) {
        ledState = !ledState

Work out what each line of your code is doing, following through the code in your mind and what happens with the "if"s, and you will understand why your code does not work.

It's pretty easy to lose track of braces and not see what logic is or is not inside of other logic.

To that end and to older eyes in general,

while (expression) { if (expression) { statement statement if (expression) { statement statement } } }

is a bit harder to read and match open and close braces than

while (expression) { if (expression) { statement statement if (expression) { statement statement } } }

after a few pages of searching for an error, the second style will leave you less tired out.

When you do make a structure,

while (expression) { }

Type both braces in before you fill the middle. Get the logical structure in and right before you bog down on details. But remember that whatever you do, it's going to need checking and testing before the lot is done.

With AVR's you best once-it's-running debug tool is Serial.print(). But don't print too much, it can mess with your timing.

GoForSmoke: To that end and to older eyes in general,

while (expression) {
  if (expression) {
    statement();
  }
}

/* is a bit harder to read and match open and close braces than */

while (expression) {   if (expression)   {     statement();   } }




after a few pages of searching for an error, the second style will leave you less tired out.

The problem with a generalised assertion is when someone comes along and contradicts you, the assertion is proved wrong.

My eyes were pretty rubbish when I was born and now I'm 49. I can not stand the second style.

MattS-UK:
The problem with a generalised assertion is when someone comes along and contradicts you, the assertion is proved wrong.

My eyes were pretty rubbish when I was born and now I’m 49. I can not stand the second style.

My “assertion” that one style of coding “is a bit harder to read and match open and close braces than” another?

My “assertion” is a suggestion to make coding easier which I state and show.

Your dislike only “proves” your dislike, not that what I showed has no merit.
I note that you make zero attempt at “disproving” the one point that the braces are easier to match.
Oh no, you just can’t stand it so… disproved!

I wrote brick-code for a couple of years before figuring out that hunting for braces is a lot easier when they’re on the same level, period. It’s a matter of form fitting function, not style for the sake of style or sticking with what you were taught because anything else would be “wrong”.

I hope that what I showed can help the OP who is having trouble with braces and which code is inside of other code. I want the OP to gain a clear structural view of his code that is less clear when the lines are bunched up. That’s just one part he needs to learn and this will help make it easier for the OP.

Thank you everyone for your patience with me, but I am still really confused. I tried following arduinodlb's advice, but I still couldn't figure out the problem. Everything seems ok to me. I commented out my code so you can see where my thinking is going wrong. Thanks again everyone for your help.

boolean debounce(boolean lastButton, long lastDebounce, int BUTTON){
  boolean current = digitalRead(BUTTON);
  if (current != lastButton){ //if the button state has changed
    lastDebounceTime=millis();  //reset timer
    }
  if ((millis()-lastDebounce) > 5){  //if 50 milliseconds has passed since last bounce
    current = digitalRead(BUTTON);} //read value again now that bouncing is over
  return current;
}

void loop() {
  
currentButtonState = debounce(lastButtonState, lastDebounceTime, button); //currentButtonState equals returned value of debounce function
if (lastButtonState==LOW && currentButtonState==HIGH){  //if state of button has changed from low to high
  ledState= !ledState;  //reverse button state
}
lastButtonState=currentButtonState; //reset last button state

digitalWrite(LED, ledState);  //power on LED
}