State machine question: L->H->L

The purpose of this program is

  1. to detect a L->H on an input, then wait for a debounce time,
  2. then verify if the pin is still H,
  3. then wait for the pin to go back to L,
  4. and only then to take action.
    If however the pin was falsily detected H, then nothing should happen.

I have my doubts the following code will do what is required, but I can't "put my finger on it".

const int buttonPin = 2;    // the number of the pushbutton pin
const int ledPin = 12;      // the number of the LED pin
int ledState = LOW;         // the current state of the output pin
int buttonState = LOW;             // the current reading from the input pin
int lastButtonState = LOW;   // the previous reading from the input pin
unsigned long lastDebounceTime = 0;  // the last time the output pin was toggled
int debounceDelay = 50;    // the debounce time; increase if the output flickers

void setup() {
  Serial.begin(115200);
  pinMode(buttonPin, INPUT_PULLUP);
  digitalWrite(buttonPin, HIGH);
  pinMode(ledPin, OUTPUT);
  digitalWrite(ledPin, LOW);   // set initial LED state
}

void loop() {
  // read the state of the switch into a local variable:
  buttonState = digitalRead(buttonPin);
  if (buttonState == HIGH && lastButtonState == LOW) {
    while (lastDebounceTime == 0) {
      lastDebounceTime = millis();
    }
    if ((millis() - lastDebounceTime) > debounceDelay) // uitvoeren na debounce
    {
      while (buttonState != lastButtonState)
      {
        buttonState = digitalRead (buttonPin);
      }
      lastButtonState = !buttonState;
    }
  }

  if (buttonState == LOW && lastButtonState == HIGH)
  {
    digitalWrite (ledPin, HIGH);
    delay (500);
    digitalWrite (ledPin, LOW);
    lastButtonState = buttonState;
    lastDebounceTime = 0;
  }
}

Hello
The combination of the functions delay() and millis() is not usefull.
Take a look into the IDE examples to find a proper soultion for your project.
Have a nice day and enjoy coding in C++.

The delay() in my sample is just for demo purposes. The issue at stake here is the nesting of a while in an if loop, and whether the preset conditions can be met by this program.

So you're looking to detect a button press, but only action when the button is released?

i find a simple 10 msec delay adequate for debouncing

// check multiple buttons and toggle LEDs

enum { Off = HIGH, On = LOW };

byte pinsLed [] = { 10, 11, 12 };
byte pinsBut [] = { A1, A2, A3 };
#define N_BUT   sizeof(pinsBut)

byte butState [N_BUT];

// -----------------------------------------------------------------------------
int
chkButtons ()
{
    for (unsigned n = 0; n < sizeof(pinsBut); n++)  {
        byte but = digitalRead (pinsBut [n]);

        if (butState [n] != but)  {
            butState [n] = but;

            delay (10);     // debounce

            if (On == but)
                return n;
        }
    }
    return -1;
}

// -----------------------------------------------------------------------------
void
loop ()
{
    switch (chkButtons ())  {
    case 2:
        digitalWrite (pinsLed [2], ! digitalRead (pinsLed [2]));
        break;

    case 1:
        digitalWrite (pinsLed [1], ! digitalRead (pinsLed [1]));
        break;

    case 0:
        digitalWrite (pinsLed [0], ! digitalRead (pinsLed [0]));
        break;
    }
}

// -----------------------------------------------------------------------------
void
setup ()
{
    Serial.begin (9600);

    for (unsigned n = 0; n < sizeof(pinsBut); n++)  {
        pinMode (pinsBut [n], INPUT_PULLUP);
        butState [n] = digitalRead (pinsBut [n]);
    }

    for (unsigned n = 0; n < sizeof(pinsLed); n++)  {
        digitalWrite (pinsLed [n], Off);
        pinMode      (pinsLed [n], OUTPUT);
    }
}
1 Like

or how about this approach

#define DebounceMsec    10
int
chkButtons ()
{
    static unsigned long  msecLst;
           unsigned long  msec = millis ();

    if (msecLst && (msec - msecLst) < DebounceMsec)
        return -1;

    msecLst = 0;

    for (unsigned n = 0; n < sizeof(pinsBut); n++)  {
        byte but = digitalRead (pinsBut [n]);

        if (butState [n] != but)  {
            butState [n] = but;

            msecLst = msec ? msec : 1;      // make sure non-zero

            if (On == but)
                return n;
        }
    }
    return -1;
}
1 Like

Yes: looking for button press, action only after release.

Great! And I notice no while, I already had suspicions about its use.
Thanks a lot for those examples gcjr!

I like a simple button class like so:

(Note! This is with INPUT_PULLUP) so it's an active LOW setup. Meaning you should connect your button to ground and a digital pin. No external resistors.)

"Button.h"

#ifndef Button_h
#define Button_h

#include <Arduino.h>

class Button {
  public:
    Button(const uint8_t inputPin, unsigned long debounce = 10)
      : inputPin(inputPin), debounce(debounce) {
        pinMode(inputPin, INPUT_PULLUP);
      }

    bool Button::read() {
      bool currentState = digitalRead(inputPin);
      now = millis();

      if (currentState != lastState) {
        lastState = currentState;
        if (!currentState)
          timeCapture = now;
        else if ((now - timeCapture) > debounce)
          return true;
      }
      return false;
    }

  private:
    const uint8_t inputPin;
    unsigned long debounce;
    bool lastState = HIGH;
    uint32_t timeCapture;
    uint32_t now;
};
#endif

"Main.ino"

#define arrSize(X) sizeof(X) / sizeof(X[0])
#define BTN0_PIN 10
#define BTN1_PIN 9
#define BTN2_PIN 8

#include "Button.h"

Button buttons[] = { BTN0_PIN, BTN1_PIN, BTN2_PIN };

void setup() {
  Serial.begin(115200);
}

void loop() {
  for (byte i = 0; i < arrSize(buttons); i++)
    if (buttons[i].read())
      Serial.println((String)"Button " + i + " Pressed"); // Debugging only. Don't cast to Strings!
}

Could be worth pulling the pinMode inside Button.h into its own 'begin' function, and then calling that on all buttons with a loop in setup. Personally, I've not had any issues with this approach, but others do advise that!

1 Like

That's not expressed as a proper state machine. A state machine would be more like:

  1. if (input reads HIGH) startTime=millis(); state = 2;
  2. if (millis() - startTime >= debounceTime) state = 3; if (input reads LOW) state = 1;
  3. if (input reads LOW) state = 4
  4. take action. state = 1;

In code, that would be:

const byte ButtonPin = 4;
int State = 1;
unsigned long StartTime;
const unsigned long DebounceTime = 10;

void setup()
{
  pinMode(ButtonPin, INPUT);
}

void loop()
{
  switch (State)
  {
    case 1:
      if (digitalRead(ButtonPin) == HIGH)
      {
        StartTime = millis();
        State = 2;
      }
      break;

    case 2:
      if (millis() - StartTime >= DebounceTime)
        State = 3;  // Still HIGH at end of wait

      if (digitalRead(ButtonPin) == LOW)
        State = 1;  // Went LOW before end of wait
      break;

    case 3:
      if (digitalRead(ButtonPin) == LOW)
        State = 4;  // Button released
      break;

    case 4:
      // take action
      
      // Go back to waiting for a button press
      State = 1;
      break;
  }
}
1 Like

Top! Thank you johnwasser, use of case, and ditch my intial while.

Somewhere I found Robin2 stating that a while and for should not be used if condition fulfilment takes more than a few miliseconds.

But yet this on my initial code: a while resides inside an if loop where the while gets resolved after a short time but the if loop condition is not valid anymore... what happens then?

This topic was automatically closed 180 days after the last reply. New replies are no longer allowed.