Button handling class

Hi there,
I’ve made following class to handle some push buttons.

The output of the “handle” routine is a byte telling me following states of the button: Off, On, On for a long (predefined) time, positive edge and negative edge.

I expect following sequence of states when pressing a button

Off, PosEdge, On, OnLong (when pressed long enough), NegEdge, Off again.
What I get is
Off, On (shortly), PosEdge, On, OnLong (when pressed long enough), NegEdge, Off again.

I can’t see where the first “On” is coming from :confused:

Does any of you ?
Thanks for your inputs!

Here is the code (sorry some comment & naming is in Dutch)
Some declarations

enum { ev_Off=0, ev_On, ev_OnLong, ev_PosEdge, ev_NegEdge };
byte _EventRood = 0;
byte _EventZwart = 0;

The class

class ButtonHandler {
  public:
    // Constructor
    ButtonHandler(int Pin, long DelayDebounce, long DelayLongPress);

    // Initialization done after construction, to permit static instances
    void init();

    // Handler, to be called in the loop()
    int handle();

  protected:
    boolean     _Latch;               // For PosEdge event
    long        _DelayDebounce;       // Setpoint for debounce button
    long        _DelayLongPress;      // Setpoint for long press of button
    
    const int   Pin;                  // Arduino pin number
    const long  DelayDebounce;        // Value to add to millis() when button pressed
    const long  DelayLongPress;       // Value to add to millis() when button pressed
};

ButtonHandler::ButtonHandler(int p, long ddb, long dlp)
: Pin(p), DelayDebounce(ddb), DelayLongPress(dlp) {
}

void ButtonHandler::init() {
  pinMode(Pin, INPUT_PULLUP);
  _DelayDebounce = 0;
  _DelayLongPress = 0;
  _Latch = false;
}

int ButtonHandler::handle() {
  byte _Event;
  
  // By default set event to "Off"
  // Code will then change it when needed
  //
  _Event = ev_Off;
  
  if (digitalRead(Pin)) {
    // Button NOT pressed = one shot reset variables and set event NegEdge
    //
    if (_DelayDebounce > 0) {
        _Event = ev_NegEdge;    // One shot event NegEdge
        _DelayDebounce = 0;     // reset variables
        _DelayLongPress = 0;
        _Latch = false;
    }
  } else {
    // Button IS pressed = one shot set delay times
    // 
    if (_DelayDebounce == 0) {
      _DelayDebounce = millis() + DelayDebounce;
      _DelayLongPress = millis() + DelayLongPress;
      _Latch = false;
    }

    // Check debounce time over, need of _Latch for PosEdge event
    //
    if (millis() > _DelayDebounce && !_Latch) _Event = ev_PosEdge;
    else _Event = ev_On;
    if (millis() > _DelayDebounce) _Latch = true;

    
    // Check button pressed for a looooong time
    //
    if (millis() > _DelayLongPress) _Event = ev_OnLong;    
  }
  
  // Set return value
  return _Event;
}



// Init buttons classes
// ButtonHandler(int pin, long DelayDebonce, long DelayLongPress);
ButtonHandler ToetsRood(P06_ToetsRood, 150, 4000);
ButtonHandler ToetsZwart(P07_ToetsZwart, 150, 4000);

In routine “setup”

  // Init buttons
  ToetsRood.init();
  ToetsZwart.init();

In routine “loop”

  // Handle buttons
  _EventRood = ToetsRood.handle();
  _EventZwart = ToetsZwart.handle();

To evaluate the class I send the status to an LCD, this code is also in the “loop” routine

    lcd.setCursor(LCDLine_3, 10);
    lcd.print("Rood ");  
  switch (_EventRood) {
    case ev_Off:
      lcd.print("Off  ");
      break;
    case ev_On:
      lcd.print("On   ");
      break;
    case ev_OnLong:
      lcd.print("Long ");
      break;
    case ev_PosEdge:
      lcd.print("PEdge");
      delay(750);
      break;
    case ev_NegEdge:
      lcd.print("NEdge");
      delay(750);
      break;
    default:
      lcd.print("?????");
  }


  lcd.setCursor(LCDLine_4, 9);
  lcd.print("Zwart ");
  switch (_EventZwart) {
    case ev_Off:
      lcd.print("Off  ");
      break;
    case ev_On:
      lcd.print("On   ");
      break;
    case ev_OnLong:
      lcd.print("Long ");
      break;
    case ev_PosEdge:
      lcd.print("PEdge");
      delay(750);
      break;
    case ev_NegEdge:
      lcd.print("NEdge");
      delay(750);
      break;
    default:
      lcd.print("?????");
  }

I looks like you need || (or) instead of &&

if (_DelayDebounce == 0) {
      _DelayDebounce = millis() + DelayDebounce;
      _DelayLongPress = millis() + DelayLongPress;
      _Latch = false;
    }

    // Check debounce time over, need of _Latch for PosEdge event
    //
    if (millis() > _DelayDebounce && !_Latch) _Event = ev_PosEdge;
    else _Event = ev_On;

If you have just set *```* *[color=blue]_DelayDebounce[/color]* *```* to *```* *[color=blue]millis() + DelayDebounce;[/color]* *```* (i.e. grater than millis()), then there is little to no chance that
* *[color=blue]millis() > _DelayDebounce[/color]* *
is true, hence the else being selected.

Then 150ms later it is true, and you get *```* *[color=blue]_Event = ev_PosEdge;[/color]* *```*

Thank you for your input pYro_65!
Indeed OR is the one I need to prevent the “On” event of showing up before the debouncing time.
In the mean time I changed to code to this, no need to double check the millis.

    // Check debounce time over, need of _Latch for PosEdge event
    //
    if (millis() > _DelayDebounce) {
      if(!_Latch) _Event = ev_PosEdge;
      else        _Event = ev_On;
      _Latch = true;
    }