[SOLVED] Button press short and long works not at all times

I got this working so that a short press toggles one LED on and off, and a long press toggles another LED on and off. However, when I short press once and then long press once, and both LEDs are on as expected, a long press sometimes toggles both LEDs off.

So, there is something wrong in my logic here. What mistake am I making?

// Variables that remain constant
const byte pinSwitch = 8;
const byte pinLED1 = 3;
const byte pinLED2 = 6;
const int timeShortPress = 200;

// Variables that can change
byte LED1state = LOW;
byte LED2state = LOW;
byte lastSwitchState = HIGH;
unsigned long timePressed  = 0;
unsigned long timeReleased = 0;

void setup ()
{
  pinMode (pinSwitch, INPUT_PULLUP);

  pinMode(pinLED1, OUTPUT);
  pinMode(pinLED2, OUTPUT);
}

void loop ()
{
  byte switchState = digitalRead (pinSwitch);

  if (switchState != lastSwitchState)
  {
    if (switchState == LOW)
    {
      timePressed = millis();
    }

    else
    {
      timeReleased = millis();

      long pressDuration = timeReleased - timePressed;

      if (pressDuration < timeShortPress )
      {
        LED1state = !LED1state;

        digitalWrite(pinLED1, LED1state);
      }

      else
      {
        LED2state = !LED2state;

        digitalWrite(pinLED2, LED2state);
      }
    }

    lastSwitchState =  switchState;
  }
}

Debouncing. Sometimes when you press for a long time you are really pressing a short and a long press.

The momentary switch is hardware debounced. Forgot to mention that. I followed the suggestions here "Hardware debouncing an SPST switch with an RC network", and to date this has always worked well.

It sounds like bouncing. Try putting a short dirty delay and see if it works. If you have an oscilloscope you could check for it

Put some debug code in like serial print short press and long press

Thanks! When I add serial printing, and I click as much as I can short and long for a minute, both LEDs are toggled without a single fault.

I don't understand why that single line of code seems to make a difference. I don't have an oscilloscope at hand, but other projects with that button and hardware debouncing work reliably.

      long pressDuration = timeReleased - timePressed;
      Serial.println(pressDuration); // The only thing I changed in loop()

Does it print to serial monitor when it happens on both sides of the if else?

It prints always exactly once, when the button is released. I meanwhile suspect a dodgy switch. I have others I will try.

Here's a WOKWi simulation of your code adapted to use the Toggle library.

Works as it should with all other switches, so code was ok.

You need a state machine.
Assume, you have 'debounced" the mechanical switches. They provide for a longer period if time an "uncertain" result.

You write a program as (hints):

  • wait X ms for short press: if it is still the same state of switch - at least a short button press happened

  • wait now Y ms (much longer): it this is also the same state of switch - it might have pressed longer

And short and long press are exclusive: if "long press" is realized, it cannot be a short press.
OK, here, the delay to distinguish between short and long is always the period for long (you cannot decide if it was short when not making sure it was not a long). Both will have a similar delay to realize (decide).

The "only problem": if you press the button twice, it was released after the short period - you get it as long pressed. It looks like a long press at the end.

I kept what I had and simply defined the minimum duration of what is a long press, and then tested for that. That provides a "dead band" of 200 milliseconds to differentiate between short and long presses. I might add another test for a "very long press", if I can find a use for that.

// Variables that remain constant
const byte pinSwitch = 8;
const byte pinLED1 = 3;
const byte pinLED2 = 6;
const int timeShortPress = 200;
const int timeLongPress = 400;

// Variables that can change
byte LED1state = LOW;
byte LED2state = LOW;
byte lastSwitchState = HIGH;
unsigned long timePressed  = 0;
unsigned long timeReleased = 0;

void setup ()
{
  pinMode (pinSwitch, INPUT_PULLUP);

  pinMode(pinLED1, OUTPUT);
  pinMode(pinLED2, OUTPUT);
}

void loop ()
{
  byte switchState = digitalRead (pinSwitch);

  if (switchState != lastSwitchState)
  {
    if (switchState == LOW)
    {
      timePressed = millis();
    }

    else
    {
      timeReleased = millis();

      long pressDuration = timeReleased - timePressed;

      if (pressDuration < timeShortPress )
      {
        // aFunction(); that does someting
        
        LED1state = !LED1state;

        digitalWrite(pinLED1, LED1state);
      }

      else if (pressDuration > timeLongPress )
      {
        // anotherFunction();
        
        LED2state = !LED2state;

        digitalWrite(pinLED2, LED2state);
      }
    }

    lastSwitchState =  switchState;
  }
}

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