0

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
  • 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) – Wouter van Ooijen Mar 30 '18 at 15:30
  • @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. – Katie Mar 30 '18 at 15:34
  • 1
    @WoutervanOoijen == takes precedence over && - but I too consider it bad practice to rely on C++ operator precedence. – Arsenal Mar 30 '18 at 15:35
  • 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 – Peter Smith Mar 30 '18 at 15:36
  • @PeterSmith To make sure all arguments are read, should I use nested for loops? or is that going to be too messy? – Katie Mar 30 '18 at 15:38
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.

  • 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 Apr 4 '18 at 12:03
  • 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? – Katie Apr 4 '18 at 15:43
  • If 'N' means neither button, then sure. – Nick Gammon Apr 4 '18 at 22:18
  • You still aren't looking at the warnings. Your new function ButtonOne does not return anything when it "falls out" the bottom. – Nick Gammon Apr 4 '18 at 22:37
  • See amended answer with some suggested code changes. – Nick Gammon Apr 4 '18 at 22:46
0

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”

  • Yes, I've modified to use debouncing, but have run into issues with this. – Katie Apr 3 '18 at 18:43
  • @Katie there is not in fact any debouncing in your code. Debouncing is fundamentally a matter of time. – Chris Stratton Apr 4 '18 at 16:44
-1

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

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.