Short/Long button press

Hi, I am trying to detect short/long press on a single button, the thing is, my code works, for some time, but when I don't touch it for a while, and then press the button for a short time, it detects a long press instead.

Can someone tell me what am I doing wrong? Thanks

int IsSwitchEnabled(int nPin){
  if (digitalRead(nPin)==LOW){
    delayMicroseconds(50);      //debounce
    int time = millis();        //start time
    while(digitalRead(nPin)==LOW){    //button presses
      if((millis() - time) >= 3000){
       return 2;                  //long press
     }
     delayMicroseconds(50);
    }     
      return 1;                  //short press
  }
  return 0;
}

Despite the comments in the code, you are not actually debouncing the button. Think step-by-step through your code again. Keep in mind that button contacts can be a few microseconds, with bounces of milliseconds.

There are a couple of button libraries that distinguish between long and short presses.

Well, I came up with this (piece of cr*p) :smiley:

I tried to use some libraries, but they totally break my timing (which is pretty important in my code)
Is there some way to make this work?

The problem really only occurs if the button is untouched for some time.

int IsSwitchEnabled(int nPin){
  int time = 0;
  if (digitalRead(nPin)==LOW){
    delay(5); //debounce
  if (digitalRead(nPin)==LOW){
    for(time = millis();millis()-time <3000;){
      if(digitalRead(nPin)==HIGH){
          return 1;
      }
      delay(5); 
    }
      return 2; 
  }
  }
  return 0;
}

Weirdly, this seems to work …
but I can’t increase the time the button is supposed to be pressed

int IsSwitchEnabled(int nPin){
  if (digitalRead(nPin) == LOW){
   int time = millis();
   delay(200); //debounce
   // check if the switch is pressed for longer than 1 second.
    if(digitalRead(nPin) == LOW && time - millis() >1000) 
     {
       return 2;
     } else
     return 1; 
    }
    return 0;
}
   int time = millis();

Always use unsigned long variables when dealing with millis().

time - millis() >1000

Two problems here. time - millis() is negative (alternatively a very large unsigned long integer), and 1000 is a 16 bit integer.

You want something like:

unsigned long time = millis();
... some code ...
if (millis() - time > 1000 UL) {

this handles click, double-click, long press, very long press:

// for types of button presses for the Arduino: click, double-click, long press (and release), very long press 
// might work with other controllers. modified from: http://pastebin.com/gQLTrHVF
/*
  i had to make a few changes (update it?) to make the code work. main changes:
  1. REMOVED: #include "WProgram.h" (no longer required).
  2. CHANGED: bpUP TO bp.
  3. ADDED: event listeners for ALL four types of presses in the sample code.
  4. CHANGED: time intervals for single, double, long clicks and for long press. these feel more intuitive to me.
  5. CHANGED: event OnLongPress is raised ONLY after the button is released. this, again, feels more intuitive. code is tested and at http://pastebin.com/87cCn6h9
*/

//
// jb_exe
// Class from the code of JEFF'S ARDUINO BLOG
// http://jmsarduino.blogspot.com/2009/10/4-way-button-click-double-click-hold.html
// 
// the modified version at http://pastebin.com/gQLTrHVF
// further modified by mahesh [at] tinymogul.com


// How to use me :
// "FWB_Project.pde"
// #include "FWB.h"
// #define BP 0 //the pin where your button is connected
//
// FWB bp;
//
// void OnClick(int pin) {
//   //Your code here
// }
//
// void OnDblClick(int pin) {
//   //Your code here
// }
//
// void OnLongPress(int pin) {
//   //Your code here
// }
//
// void OnVLongPress(int pin) {
//   //Your code here
// }
//
// void setup()
// {
//   // errors in code fixed here. empty event handlers added.
//   bp.Configure(BP);
//   bp.OnClick = OnClick;
//   bp.OnDblClick = OnDblClick;
//   bp.OnLongPress = OnLongPress;
//   bp.OnVLongPress = OnVLongPress;
// }
//
// void loop()
// {
//  // Test button state
//  bp.CheckBP();
// }

#define PULL_UP 1
#define PULL_DOWN 0

class FWB
{
private:
  int _pin;
  boolean _pullMode;

  // Properties //
  ////////////////

  // Debounce period to prevent flickering when pressing or releasing the button (in ms)
  int Debounce;
  // Max period between clicks for a double click event (in ms)
  int DblClickDelay;
  // Hold period for a long press event (in ms)
  int LongPressDelay;
  // Hold period for a very long press event (in ms)
  int VLongPressDelay;

  // Variables //
  ///////////////

  // Value read from button
  boolean _state;
  // Last value of button state
  boolean _lastState;
  // whether we're waiting for a double click (down)
  boolean _dblClickWaiting;
  // whether to register a double click on next release, or whether to wait and click
  boolean _dblClickOnNextUp;
  // whether it's OK to do a single click
  boolean _singleClickOK;

  // time the button was pressed down
  long _downTime;
  // time the button was released
  long _upTime;

  // whether to ignore the button release because the click+hold was triggered
  boolean _ignoreUP;
  // when held, whether to wait for the up event
  boolean _waitForUP;
  // whether or not the hold event happened already
  boolean _longPressHappened;
  // whether or not the long hold event happened already
  boolean _vLongPressHappened;

  public:
  void (*OnClick)(int pin);
  void (*OnDblClick)(int pin);
  void (*OnLongPress)(int pin);
  void (*OnVLongPress)(int pin);

  FWB()
  {
    // Initialization of properties
    Debounce = 20;
    DblClickDelay = 250;
    LongPressDelay = 750;
    // LongPressDelay = 1000;    
    VLongPressDelay = 3500;
    // VLongPressDelay = 3000;

    // Initialization of variables
    _state = true;
    _lastState = true;
    _dblClickWaiting = false;
    _dblClickOnNextUp = false;
    _singleClickOK = false; //Default = true
    _downTime = -1;
    _upTime = -1;
    _ignoreUP = false;
    _waitForUP = false;
    _longPressHappened = false;
    _vLongPressHappened = false;
  }
  void Configure(int pin, int pullMode = PULL_DOWN)
  {
    _pin = pin;
    _pullMode = pullMode;
    pinMode(_pin, INPUT);
  }

  void CheckBP(void)
  {
    int resultEvent = 0;
    long millisRes = millis();
    _state = digitalRead(_pin) == HIGH;

    // Button pressed down
    if (_state != _pullMode && _lastState == _pullMode && (millisRes - _upTime) > Debounce)
    {
      //Serial.println("button down");
      _downTime = millisRes;
      _ignoreUP = false;
      _waitForUP = false;
      _singleClickOK = true;
      _longPressHappened = false;
      _vLongPressHappened = false;
      if ((millisRes - _upTime) < DblClickDelay && _dblClickOnNextUp == false && _dblClickWaiting == true)
        _dblClickOnNextUp = true;
      else
        _dblClickOnNextUp = false;
      _dblClickWaiting = false;
    }
    // Button released
    else if (_state == _pullMode && _lastState != _pullMode && (millisRes - _downTime) > Debounce)
    {
      //Serial.println("button up");
      if (_ignoreUP == false) //Replace "(!_ignoreUP)" by "(not _ignoreUP)"
      {
        _upTime = millisRes;
        if (_dblClickOnNextUp == false) _dblClickWaiting = true;
        else
        {
          resultEvent = 2;
          _dblClickOnNextUp = false;
          _dblClickWaiting = false;
          _singleClickOK = false;
        }
      }
    }

    // Test for normal click event: DblClickDelay expired
    if (_state == _pullMode && (millisRes - _upTime) >= DblClickDelay && _dblClickWaiting == true && _dblClickOnNextUp == false && _singleClickOK == true && resultEvent != 2)
    {
      resultEvent = 1;
      _dblClickWaiting = false;
    }

    // added code: raise OnLongPress event when only when the button is released
    if (_state == _pullMode && _longPressHappened && !_vLongPressHappened) {
      resultEvent = 3;
      _longPressHappened = false;
    }

    // Test for hold
    if (_state != _pullMode && (millisRes - _downTime) >= LongPressDelay)
    {
      // Trigger "normal" hold
      if (_longPressHappened == false)
      {
        // resultEvent = 3;
        _waitForUP = true;
        _ignoreUP = true;
        _dblClickOnNextUp = false;
        _dblClickWaiting = false;
        //_downTime = millis();
        _longPressHappened = true;
      }
      // Trigger "long" hold
      if ((millisRes - _downTime) >= VLongPressDelay)
      {
        if (_vLongPressHappened == false)
        {
          resultEvent = 4;
          _vLongPressHappened = true;
          //_longPressHappened = false;
        }
      }
    }

    _lastState = _state;
    //if (resultEvent!=0)
    //  Serial.println((String)"resultEvent: " + (String) resultEvent);

    if (resultEvent == 1 && OnClick) OnClick(_pin);
    if (resultEvent == 2 && OnDblClick) OnDblClick(_pin);
    if (resultEvent == 3 && OnLongPress) OnLongPress(_pin);
    if (resultEvent == 4 && OnVLongPress) OnVLongPress(_pin);
    //  if (resultEvent != 0)
    //    Usb.println(resultEvent);
  }

};

Have you considered using Switch Manager by Nick Gammon.

 /*SwitchManager skeleton 
 
 This sketch is to introduce new people to the SwitchManager library written by Nick Gammon
 
 The library handles switch de-bouncing and provides timing and state change information in your sketch.
 The SwitchManager.h file should be placed in your libraries folder, i.e.
 C:\Users\YourName\Documents\Arduino\libraries\SwitchManager\SwitchManager.h
 You can download the library at:
 http://gammon.com.au/Arduino/SwitchManager.zip    Thank you Nick!
 
 In this example we have 2 normally open (N.O.) switches connected to the Arduino - increment and decrement.
 The increment switch will also be used as a "Reset" switch if pressed for more than two seconds.
 The two switches are connected between GND (0 volts) and an Arduino input pin.
 The library enables pull-up resistors for your switch inputs.
 Pushing a switch makes its pin LOW. Releasing a switch makes its pin HIGH.
 
 The SwitchManager library provides 10ms de-bounce for switches. 
 i.e. enum { debounceTime = 10, noSwitch = -1 };
 If you need more time, edit the SwitchManager.h file
 i.e. enum { debounceTime = 50, noSwitch = -1 }; //here it is changed to 50ms
 */

#include <SwitchManager.h>             
//object instantiations
SwitchManager myIncSwitch;
SwitchManager myDecSwitch;

unsigned long currentMillis;
unsigned long heartBeatMillis;
unsigned long heartFlashRate  = 500UL; // time the led will change state       
unsigned long incShortPress   = 500UL; // 1/2 second
unsigned long incLongPress    = 2000UL;// 2 seconds 
unsigned long decShortPress   = 500UL; // 1/2 second

const byte heartBeatLED       = 13;
const byte incSwitch          = 4; //increment switch is on Arduino pin 4
const byte decSwitch          = 5; //decrement switch is on Arduino pin 5

int myCounter;

//======================================================================

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

  //gives a visual indication if the sketch is blocking
  pinMode(heartBeatLED, OUTPUT);  

  myIncSwitch.begin (incSwitch, handleSwitchPresses); 
  myDecSwitch.begin (decSwitch, handleSwitchPresses);
  //the handleSwitchPresses() function is called when a switch changes state

} //                   E N D  O F  s e t u p ( )

//======================================================================

void loop()
{
  //leave this line of code at the top of loop()
  currentMillis = millis();

  //***************************
  //some code to see if the sketch is blocking
  if (CheckTime(heartBeatMillis, heartFlashRate, true))
  {
    //toggle the heartBeatLED
    digitalWrite(heartBeatLED,!digitalRead(heartBeatLED));
  }

  //***************************
  //check to see what's happening with the switches
  //"Do not use delay()s" in your sketch as it will make switch changes unresponsive 
  //Use BlinkWithoutDelay (BWD) techniques instead.
  myIncSwitch.check ();  
  myDecSwitch.check (); 

  //***************************
  //put other non-blocking stuff here


} //                      E N D  O F  l o o p ( )


//======================================================================
//                          F U N C T I O N S
//======================================================================


//                        C h e c k T i m e ( ) 
//**********************************************************************
//Delay time expired function
//parameters:
//lastMillis = time we started
//wait = delay in ms
//restart = do we start again  

boolean CheckTime(unsigned long  & lastMillis, unsigned long wait, boolean restart) 
{
  //has time expired for this task?
  if (currentMillis - lastMillis >= wait) 
  {
    //should this start again? 
    if(restart)
    {
      //yes, get ready for the next iteration
      lastMillis += wait;  
    }
    return true;
  }
  return false;

} //                 E N D   o f   C h e c k T i m e ( )


//                h a n d l e S w i t c h P r e s s e s( )
//**********************************************************************

void handleSwitchPresses(const byte newState, const unsigned long interval, const byte whichPin)
{
  //  You get here "ONLY" if there has been a change in a switches state.

  //When a switch has changed state, SwitchManager passes this function 3 arguments:
  //"newState" this will be HIGH or LOW. This is the state the switch is in now.
  //"interval" the number of milliseconds the switch stayed in the previous state
  //"whichPin" is the switch pin that we are examining  

  switch (whichPin)
  {
    //***************************
    //are we dealing with this switch?
  case incSwitch: 

    //has this switch gone from LOW to HIGH (gone from pressed to not pressed)
    //this happens with normally open switches wired as mentioned at the top of this sketch
    if (newState == HIGH)
    {
      //The incSwitch was just released
      //was this a short press followed by a switch release
      if(interval <= incShortPress) 
      {
        Serial.print("My counter value is = ");
        myCounter++;
        if(myCounter > 1000)
        {
          //limit the counter to a maximum of 1000
          myCounter = 1000; 
        }
        Serial.println(myCounter);
      }

      //was this a long press followed by a switch release
      else if(interval >= incLongPress) 
        //we could also have an upper limit
        //if incLongMillis was 2000UL; we could then have a window between 2-3 seconds
        //else if(interval >= incLongMillis && interval <= incLongMillis + 1000UL) 
      {
        //this could be used to change states in a StateMachine
        //in this example however, we will just reset myCounter
        myCounter = 0;
        Serial.print("My counter value is = ");
        Serial.println(myCounter);
      }

    }

    //if the switch is a normally closed (N.C.) and opens on a press this section would be used
    //the switch must have gone from HIGH to LOW 
    else 
    {
      Serial.println("The incSwitch was just pushed");
    } 

    break; //End of case incSwitch

    //*************************** 
    //are we dealing with this switch?
  case decSwitch: 

    //has this switch gone from LOW to HIGH (gone from pressed to not pressed)
    //this happens with normally open switches wired as mentioned at the top of this sketch
    if (newState == HIGH)
    {
      //The decSwitch was just released
      //was this a short press followed by a switch release
      if(interval <= decShortPress) 
      {
        Serial.print("My counter value is = ");
        myCounter--;
        if(myCounter < 0) 
        {
          //don't go below zero
          myCounter = 0;
        }
        Serial.println(myCounter);
      }

    }

    //if the switch is a normally closed (N.C.) and opens on a press this section would be used
    //the switch must have gone from HIGH to LOW
    else 
    {
      Serial.println("The decSwitch switch was just pushed");
    } 

    break; //End of case decSwitch

    //*************************** 
    //Put default stuff here
    //default:
    //break; //END of default

  } //End switch (whichPin)

} //      E n d   o f   h a n d l e S w i t c h P r e s s e s ( )


//======================================================================
//                      E N D  O F  C O D E
//======================================================================

Another take (the long and short of it):

int ledPin = 13;
int switchPin = 7;
byte switchState = 1;
byte lastState = 1;
word shortDuration = 100;
word longDuration = 2000;
unsigned long switchTime = 0;

void setup() {
  pinMode(switchPin, INPUT_PULLUP);
  pinMode(ledPin, OUTPUT);
}

void loop() {
  switchState = digitalRead(switchPin);

  if (!switchState && lastState) {  // just pressed
    lastState = switchState;
    switchTime = millis(); // start timer
  }
  if (switchState && !lastState) {   // just released
    if (((millis() - switchTime) <= shortDuration) && ((millis() - switchTime) <= longDuration)) { // short press
      digitalWrite(ledPin, HIGH);
      delay(shortDuration);
      digitalWrite(ledPin, LOW);
      lastState = switchState;
    }
    if ((millis() - switchTime) > longDuration) { // long press
      digitalWrite(ledPin, HIGH);
      delay(longDuration);
      digitalWrite(ledPin, LOW);
      lastState = switchState;
    }
  }
}

I was looking for that issue. I found my own solution. Not brilliant but working

  while (digitalRead(buton3) == HIGH) {
    t++;
    delay(1);
  }
  if (t > 500) {
    analogWrite(buzzerPin, 50);
    delay(100);
    analogWrite(buzzerPin, 0);
    t = 0;
  }
  if (t > 50) {
    analogWrite(buzzerPin, 200);
    delay(100);
    analogWrite(buzzerPin, 0);
    t = 0;
  }