Debouncing Multiple Buttons

Question

I'm creating a binary calculator with my Arduino. This will take two different binary numbers and add them together. I am focusing right now on the Byte One Input screen. For this, I can use two buttons, the left to change to selected bit to either 0 or 1 when pressed, and the right button to move to the next bit, or when at the end, to move to the next screen. I am having difficulty with the debouncing of the buttons.

Right now, the debouncing is not working and the position changes from 1 to 8 or 16 instead of to the second bit when I hit the right button.

Any help would be great!

Fixed Code: See accepted answer below.

 #include <LiquidCrystal.h>
/*(RS, E, D4, D5, D6, D7)*/
LiquidCrystal lcd(12,11,5,4,3,2); 
#include <Button.h>
int page = 0; 
int binary[16] = {0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0};
int con = 80;
char btn_push;
int pos = 0;
const int button1Pin = 8; 
const int button2Pin = 9;
int button1State = HIGH;
int button2State = HIGH;
int lastButton1State = HIGH;
int lastButton2State = HIGH;
unsigned long debounceDelay = 50;
//****************************************************************setup*****************************************
void setup() {
 pinMode(button1Pin, INPUT_PULLUP);
 pinMode(button2Pin, INPUT_PULLUP);
 Serial.begin(9600);
 analogWrite(6,con);
 mylcd.begin();
 lcd.print("Welcome");
}
//************************************************************loop************************************************
void loop() { 
 int reading1 = digitalRead(button1Pin); 
 int reading2 = digitalRead(button2Pin); 
 if (reading1 == lastButton1State &&
 reading2 == lastButton2State)
 return; // do nothing if no change
 delay (debounceDelay); // debounce
 // re-read so we can catch both buttons down at once
 reading1 = digitalRead(button1Pin); 
 reading2 = digitalRead(button2Pin); 
 // save for next time
 lastButton1State = reading1;
 lastButton2State = reading2;
 if (reading1 == LOW && reading2 == HIGH) // first one down
 btn_push = 'D';
 else if (reading1 == HIGH && reading2 == LOW) // second one down
 btn_push = 'U';
 else if (reading1 == LOW && reading2 == LOW) // both
 btn_push = 'S';
 else
 return; // nothing interesting
 Serial.print("btn_push = ");
 Serial.println(btn_push);
 // page select
 switch (page) {
 case 0:
 page0();
 break;
 case 1:
 page1();
 break;
 case 2:
 page2();
 break;
 case 3:
 page3();
 break;
 case 4:
 page4();
 break;
 }
}
// The rest is the same, excluding the functions that are no longer needed

• Wouter van Ooijen: Are you sure those last two if's are interpreted by the compiler the way you mean? I would use some more ()'s ! And instead of foo() == false you can use ! foo(), which makes things more readable. (at least to me)
• Katie: @WoutervanOoijen You are right about readability, I will change it to !foo(). But regarding if the computer is reading the last two if's correctly, that's what I'm trying to figure out. I don't think it is, but I don't know how else I could code it so it works correctly.
• Arsenal: @WoutervanOoijen == takes precedence over && - but I too consider it bad practice to rely on C++ operator precedence.
• Peter Smith: Note that == has a higher precedence than the logical and (&&), so in the first test only the button.2() statement is evaluated. en.cppreference.com/w/c/language/operator_precedence
• Katie: @PeterSmith To make sure all arguments are read, should I use nested for loops? or is that going to be too messy?

Answer 1

Your first problem is here:

char ReadKeypad(){
 if(button1.isPressed() && !button2.isPressed()){
 return 'D';
 }
 else if(button2.isPressed() && !button1.isPressed()){
 return 'U';
 }
 else if (!button1.isPressed() && !button2.isPressed()){
 return 'N';
 }
 else if (button1.isPressed() && button2.isPressed()){
 return 'S';
 }
}

The compiler gives a warning, which you should pay attention to:

In function 'char ReadKeypad()':
warning: control reaches end of non-void function [-Wreturn-type]
 }
 ^

The warning basically is that if no button is pressed you are returning an undefined value. It would be better to add to the end:

return 0; // no button pressed

---

Then later on in page1 function:

 for (int i; i<16; i++){
 lcd.print(binary[i]);
 }

Another compiler warning:

In function 'void page1()':
warning: 'i' may be used uninitialized in this function [-Wmaybe-uninitialized]
 for (int i; i<16; i++){

You haven't initialized i so it could have any value. That should read:

 for (int i = 0; i<16; i++){
 lcd.print(binary[i]);
 }

Ditto further down.

---

If I add some debugging here:

void MainmenuBtn(){
 btn_push = ReadKeypad();
 Serial.print ("btn_push = ");
 Serial.println (btn_push);
 // WaitBtnRelease();
}

And then press a button, I see:

btn_push = D
btn_push = D
btn_push = D
btn_push = D
btn_push = D
btn_push = D
btn_push = D
btn_push = D
btn_push = D
btn_push = D
btn_push = D
btn_push = D

So that is registering as many presses, not just one. You should test for "has the button state changed?". That is:

• Is the button closed now?
• Was it closed before?
• If the two are different, then the button has just been pressed.

---

Adding debugging prints is very helpful in general when things aren't going to plan.

If you had added those extra prints to your MainmenuBtn function you would immediately have seen that it looked like the button was pressed hundreds of times (until you let go) rather than once.

the position changes from 1 to 8 or 16 instead of to the second bit when I hit the right button

Exactly. Since it is treating a button press as a multiple press you would expect exactly that to happen. Effectively you have built in a very fast auto-repeat (unintentionally).

---

Edited to add:

Your code is so confusing I've rewritten the state change stuff.

#include <LiquidCrystal.h>
/*(RS, E, D4, D5, D6, D7)*/
LiquidCrystal lcd(12,11,5,4,3,2); 
#include <Button.h>
int page = 0; 
int binary[16] = {0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0};
int con = 80;
char btn_push;
int pos = 0;
const int button1Pin = 8; 
const int button2Pin = 9;
int button1State = HIGH;
int button2State = HIGH;
int lastButton1State = HIGH;
int lastButton2State = HIGH;
unsigned long debounceDelay = 50;
//****************************************************************setup*****************************************
void setup() {
 pinMode(button1Pin, INPUT_PULLUP);
 pinMode(button2Pin, INPUT_PULLUP);
 Serial.begin(9600);
 analogWrite(6,con);
 mylcd.begin();
 lcd.print("Welcome");
}
//************************************************************loop************************************************
void loop() { 
 int reading1 = digitalRead(button1Pin); 
 int reading2 = digitalRead(button2Pin); 
 if (reading1 == lastButton1State &&
 reading2 == lastButton2State)
 return; // do nothing if no change
 delay (debounceDelay); // debounce
 // re-read so we can catch both buttons down at once
 reading1 = digitalRead(button1Pin); 
 reading2 = digitalRead(button2Pin); 
 // save for next time
 lastButton1State = reading1;
 lastButton2State = reading2;
 if (reading1 == LOW && reading2 == HIGH) // first one down
 btn_push = 'D';
 else if (reading1 == HIGH && reading2 == LOW) // second one down
 btn_push = 'U';
 else if (reading1 == LOW && reading2 == LOW) // both
 btn_push = 'S';
 else
 return; // nothing interesting
 Serial.print("btn_push = ");
 Serial.println(btn_push);
 // page select
 switch (page) {
 case 0:
 page0();
 break;
 case 1:
 page1();
 break;
 case 2:
 page2();
 break;
 case 3:
 page3();
 break;
 case 4:
 page4();
 break;
 }
}
// The rest is the same, excluding the functions that are no longer needed

I made the buttons INPUT_PULLUP so that they normally read HIGH if not pressed, thus LOW means pressed (therefore the switches should be wired from the digital pin on one side to Gnd on the other side).

It still isn't perfect but that should give you something to play with. Add some more debugging prints to see what is going on in the other functions.

• Katie: Wow! Thank you for catching all of my errors, I guess I've just been looking at the code too long. I agree I should have used debugging prints. I am working right now to figure out debouncing for the buttons to see if the state has changed. Just a question about that first problem you found, are you saying I should return 0 at the end of the function or instead of returning 'N' if neither button is pressed?
• Katie: I've updated my code in an attempt to implement debouncing and detect a state change, but it seems I've only made the loop worse. Any ideas on what I'm doing wrong?
• Nick Gammon: If 'N' means neither button, then sure.
• Nick Gammon: You still aren't looking at the warnings. Your new function ButtonOne does not return anything when it "falls out" the bottom.
• Nick Gammon: See amended answer with some suggested code changes.

Answer 2

The button stays down long enough for you to cycle through more than once, first setting binary[i] to 1 then to 0, etc. you need to add some logic to say "this is the first time I’ve seen this button state"

• Katie: Yes, I've modified to use debouncing, but have run into issues with this.
• Chris Stratton: @Katie there is not in fact any debouncing in your code. Debouncing is fundamentally a matter of time.

Answer 3

As an alternative to using two inputs, consider using a single analogue input with several momentary buttons - each with differing resistor values in series - this will let you avoid debouncing altogether. Just 5/3V3->resistor->btn->analogInput.

At worst, you will need to smoothe out jitter on analog reads. Have a look at https://www.codeproject.com/Tips/709109/ADKeyboard-Library-for-Arduino