How To Control Do...While structures.

Hello,

I am trying to count the number of times two individual switches are pressed. There is a button1 and a button2. I would like to keep track of how many times each button was pushed and report this to the serial monitor.

I have two ‘do…while’ loops that should run a debounce function whenever an individual switch goes LOW, but it seems to run both do keywords whenever either button is pushed. I’m pretty new to arduino. Let me know if you need any other information. I left all of my debugging Serial.print() in the code // commented out. The code is attached.

All the best. Any help is greatly appreciated.
Kyle H

/*
  Original debounce example from Adafruit, modified to count button presses
  from two individial tactile switches. 
 
  This example code is in the public domain.
 
  http://www.arduino.cc/en/Tutorial/Debounce
*/
 
const int button1Pin = 2;     // button pin 2
const int button2Pin = 3;     // button pin 3

int buttonState = HIGH;       // the current reading from the input pin
int lastButtonState = HIGH;   // the previous reading from the input pin
 
// the following variables are unsigned longs because the time, measured in
// milliseconds, will quickly become a bigger number than can be stored in an int.

unsigned long lastDebounceTime = 0;  // the last time a output pin was toggled
unsigned long debounceDelay = 50;    // the debounce time; increase if the output flickers
 
void setup() {
  pinMode(button1Pin, INPUT);
  pinMode(button2Pin, INPUT);
  digitalWrite(button1Pin, HIGH);
  digitalWrite(button2Pin, HIGH);
  
  Serial.begin(9600);
}
 
void loop() {
  // static int button1Clicked = 0;
  // static int button2Clicked = 0;
  static int lastButton1Clicked = 0;
  static int lastButton2Clicked = 0;
  //Serial.println(digitalRead(button1Pin));
  //Serial.println(digitalRead(button2Pin));
  
do {
  int button1Clicked = debounce(button1Pin);
    // Serial.println("step one running");
    if (button1Clicked != lastButton1Clicked) {
    Serial.print("Button one clicked ");
    Serial.print(button1Clicked);
    Serial.println(" times.");
    lastButton1Clicked = button1Clicked;
  }
 } while (digitalRead(button1Pin) == LOW);

do {
  int button2Clicked = debounce(button2Pin);
      // Serial.println("step two running");
      if (button2Clicked != lastButton2Clicked) {
      Serial.print("Button two clicked ");
      Serial.print(button2Clicked);
      Serial.println(" times.");
      lastButton2Clicked = button2Clicked;
     }
   } while (digitalRead(button2Pin) == LOW);
}

int debounce(int debouncePin) {
  static int count = 0;
  int reading = digitalRead(debouncePin);     // read buttonPin
  if (reading != lastButtonState) {           // check if reading has changed
    lastDebounceTime = millis();              // set lastDeboucnceTime to time since program started
                                              // Serial.println("lastDebounceTime = millis()");
  }
  
  if ((millis() - lastDebounceTime) > debounceDelay) {    // If button has retained the same state for more than 50 milliseconds
    if (reading != buttonState) {             // and if there is a new buttonState
      Serial.println("ran");
      buttonState = reading;                  // set button state to the new buttonState
      if (buttonState == HIGH) {              // if button is no longer pressed - button goes low when pressed
        count++;
        Serial.println("count increased");
      }
    }
  }
  lastButtonState = reading;                  // write lastButtonState to whatever the digitalReading of buttonPin is, no matter how long button was pressed 
  return count;   
}                                             // repeat loop

debounce-example-mod-2.ino (3.09 KB)

A do-while is simply the wrong tool for the job. While you're debouncing one switch you can't read the other. Let the loop do the looping and handle both switches in loop with if statements.

not sure if you're using external pullup/pulldown and whether the depressed switch is LOW/HIGH. It's common to configure the pin w/ pullup, INPUT_PULLUP and the button tied to gnd making the input LOW and depressed.

your while will loop when the button is depressed may terminate after the first bounce. But there's no benefit for doing this.

your debounce() is overly complicated. a 10msec delay in loop() would be sufficient to detect a change in button state but not check again soon enough to see the bounce.

keeping track of the last button state, checking for a change And recognizing the state when the button is depressed is a common approach. return a "1" when pressed (i.e. return ! butState).

you could create a button press function that is passed a pin and a pointer to the last State (or an array index). it could be called for each button and the result ("1" when pressed) added to the corresponding button count variable.

Find some guidance in the first five demos in IDE → file,examples,digital. Keep timer variables for each button unique.

Small sketches can be shown to us using code tags.

Use CTRL T to format your code.
Attach your ‘complete’ sketch between code tags, use the </> icon in the posting menu.
[code]Paste your sketch here[/code]

Thank you for your responses. I am setting the pullup resistors for the two buttonPins by setting digitalWrite(buttonPin, HIGH). I think that is the older way of doing this in arduino. The buttons do go LOW when pressed.

The debounce code is from Adafruit, I agree its a little complicated. It does a very good job and is very reliable. It is also about the same as the default Arduino debounce example.

The main reason I am writing this program is to practice writing encapsulated functions that are recyclable. I am also practicing passing perimeters to functions.

I did originally use if() statements rather than do...while. I was able to get the do...while more stable. But still both buttons are incrementing each time either button is pressed. It seems the second do...while should not run because button2Pin is HIGH. Would if() statements be fundementally better? Is it possible to do with do...while?

DougP - By timer variables do you mean the **millis() or the count++; ? **

gcjr - I am not sure what is meant by 'a pointer to the last State (or an array index)'. Would the pointer be another variable for each button? Would you happen to have an example? Thank you for your help so far. You are helping me think through this.

kahaubrich:
DougP - By timer variables do you mean the **millis() or the count++; ? **

count++. Don't touch millis, it's only for reading.

kahaubrich:
I did originally use if() statements rather than do...while. I was able to get the do...while more stable.

What do you mean by more stable? This doesn't make any sense.

gcjr - I am not sure what is meant by 'a pointer to the last State (or an array index)'. Would the pointer be another variable for each button? Would you happen to have an example? Thank you for your help so far. You are helping me think through this.

instead of either a pointer or use of arrays, you can pass the button state variable by reference
consider

// -----------------------------------------------------------------------------
int
butPress (
    byte  pin,
    byte &last )
{
    byte state = digitalRead (pin);
    if (last != state)  {
         last = state;
         return !state;
    }
    return 0;
}

// ---------------------------------------------------------
void loop() {
    static int button1Clicked = 0;
    static int button2Clicked = 0;
    static byte butLast1 = 0;
    static byte butLast2 = 0;

    delay (10);     // debounce
    button1Clicked += butPress (button1Pin, butLast1);
    button2Clicked += butPress (button2Pin, butLast2);

    // display counts once every second
    static unsigned long  msecLst = 0;
           unsigned long  msec    = millis ();
    if (msec - msecLst > 1000)  {
        msecLst = msec;
        char s [40];
        sprintf (s, " but1 %2d, but2 %3d", button1Clicked, button2Clicked);
        Serial.println (s);
    }
}

count++.

So maybe I should be passing a second variable to the debounce function and have that variable + 1 returned?

What do you mean by more stable?

As far as the if() statement not working, I probably had some other part of the program set it up incorrectly, by the time I tried a do...while loop I probably had some other parts fixed.

Code should (almost) never ever be blocking. A while loop to wait for a human to press a button is just silly. Let the loop be the loop. Handle the same exact logic as the do-while structure, but let the rest of the loop function run.

Because I'm feeling particularly generous today here's an example. With mine, the blinking light will blink at 1Hz no matter how long you hold down a button. With your do-while loops that is impossible. You wouldn't be able to keep a light blinking while the user held down a button without rewriting your blink code into every do-while loop.

(Compiled but untested)(intentionally not using arrays for illustrative purposed, sorry for repeated code)

int oldState1 = HIGH;
int oldState2 = HIGH;

unsigned long pressStart1 = 0;
unsigned long pressStart2 = 0;
unsigned int debounceTime = 100;  //  100ms debounce

int buttonPin1 = 2;
int buttonPin2 = 3;

boolean button1Pressed = false;
boolean button2Pressed = false;

int button1Count = 0;
int button2Count = 0;

int heartBeatPin = 4;
int heartBeatDelay = 500;  // 1 hz blinking light
unsigned long lastBeatTime = 0;

void setup(void) {
  Serial.begin(115200);
  pinMode(buttonPin1, INPUT_PULLUP);
  pinMode(buttonPin2, INPUT_PULLUP);
}


void loop(void) {
///  BUTTON 1
  int state1 = digitalRead(buttonPin1);

  if(state1 != oldState1){
    if(state1 == LOW){   // press just now starting 
      pressStart1 = millis();
    } else {
      button1Pressed = false;  // reading HIGH - button bouncing or released
    }
  }
  
  //if the button is down AND has been long enough AND we haven't reacted to this press yet
  if((state1 == LOW) && (!button1Pressed) && (millis() - pressStart1 >= debounceTime)){
    button1Pressed = true;
    Serial.print("Button 1 pressed ");
    Serial.print(++button1Count);
    Serial.println(" times.");
  }
  oldState1 = state1;

///  BUTTON 2
  int state2 = digitalRead(buttonPin2);

  if(state2 != oldState2){
    if(state2 == LOW){   // press just now starting
      pressStart2 = millis();
    } else {
      button2Pressed = false;  // reading HIGH button bouncing or released
    }
  }

  //if the button is down AND has been long enough AND we haven't reacted to this press yet
  if((state2 == LOW) && (!button2Pressed) && (millis() - pressStart2 >= debounceTime)){
    button2Pressed = true;
    Serial.print("Button 2 pressed ");
    Serial.print(++button2Count);
    Serial.println(" times.");
  }
  oldState2 = state2;

///  Heartbeat
  if(millis() - lastBeatTime >= heartBeatDelay){
    digitalWrite(heartBeatPin, !digitalRead(heartBeatPin));
    lastBeatTime = millis();  
  }
}

Thanks Delta G. I appreciate you taking the time. It seemed the serial output was a little funky, but it may be my machine.

I rewrote my code without the do...while statements and without if statements to call the single decounce runction. Rather the debounce functions run at each loop as suggested. I also abandoned my DRY code and made two instanced of the debounce function with separate variables for button1 and button2. I wonder if it was the global variables that were causing the unusual results of both buttons needing to be pressed at the same time for anything to write to the serial monitor?

Also, I am curious about DRY, encapsulated code VS code that just works? Maybe this is an oversimplification on my part to think this might be a debate. Maybe I should be more carful to use internal variables rather than global variables. Anyway, here is some code that works. Sorry, GCJR, I had a hard time understanding your code. I could not get it to work. I do appreciate the use of sprintf(), that one was new to me.

Does anyone know if I can retitle a post? This thread may not be much about do..while statements.

Thanks everyone for your help,
Kyle H

const int button1Pin = 2;
const int button2Pin = 3;

int button1State = HIGH;
int lastButton1State = HIGH;
int button2State = HIGH;
int lastButton2State = HIGH;

unsigned long lastDebounce1Time = 0;
unsigned long debounce1Delay = 50;
unsigned long lastDebounce2Time = 0;
unsigned long debounce2Delay = 50;

void setup() {
  pinMode(button1Pin, INPUT);
  pinMode(button2Pin, INPUT);
  digitalWrite(button1Pin, HIGH);
  digitalWrite(button2Pin, HIGH);

  Serial.begin(9600);
}

void loop() {

  static int lastButton1Count = 0;
  static int lastButton2Count = 0;
  
  int button1Count = debounce1(button1Pin, lastButton1Count);
  if (button1Count != lastButton1Count) {
    Serial.print("Button one clicked ");
    Serial.print(button1Count);
    Serial.println(" times.");
    lastButton1Count = button1Count;
  }
  
  int button2Count = debounce2(button2Pin, lastButton2Count);
  if (button2Count != lastButton2Count) {
    Serial.print("Button two clicked ");
    Serial.print(button2Count);
    Serial.println(" times.");
    lastButton2Count = button2Count;
  }
}

int debounce1(int debounce1Pin, int button1Inc) {
  
  int reading = digitalRead(debounce1Pin);     // read buttonPin
  if (reading != lastButton1State) {           // check if reading has changed
    lastDebounce1Time = millis();              // set lastDeboucnceTime to time since program started
                                              // Serial.println("lastDebounceTime = millis()");
  }
  
  if ((millis() - lastDebounce1Time) > debounce1Delay) {    // If button has retained the same state for more than 50 milliseconds
    if (reading != button1State) {             // and if there is a new buttonState
      Serial.println("ran1");
      button1State = reading;                  // set button state to the new buttonState
      if (button1State == HIGH) {              // if button is no longer pressed - button goes low when pressed
        button1Inc = button1Inc + 1;
        Serial.println("count increased");
      }
    }
  }
  lastButton1State = reading;                  // write lastButtonState to whatever the digitalReading of buttonPin is, no matter how long button was pressed 
  return button1Inc;   
}    

int debounce2(int debounce2Pin, int button2Inc) {
  
  int reading = digitalRead(debounce2Pin);     // read buttonPin
  if (reading != lastButton2State) {           // check if reading has changed
    lastDebounce2Time = millis();              // set lastDeboucnceTime to time since program started
                                              // Serial.println("lastDebounceTime = millis()");
  }
  
  if ((millis() - lastDebounce2Time) > debounce2Delay) {    // If button has retained the same state for more than 50 milliseconds
    if (reading != button2State) {             // and if there is a new buttonState
      Serial.println("ran2");
      button2State = reading;                  // set button state to the new buttonState
      if (button2State == HIGH) {              // if button is no longer pressed - button goes low when pressed
        button2Inc = button2Inc + 1;
        Serial.println("count increased");
      }
    }
  }
  lastButton2State = reading;                  // write lastButtonState to whatever the digitalReading of buttonPin is, no matter how long button was pressed 
  return button2Inc;   
}

You can edit your posts.