Button Read Optimizations

Projects Programming
1

I have a button reading code with the functionality of returning a short press or a long press.

When I post this I'm certain I'll get jumped on for it being blocking code and not using millis. And so I'm wondering, what's the shortest code I could have here that give me this functionality?

Here's what I've got so far:
The read function increases a counter while the switch is low, then as soon as it goes high it returns either an action type enum (which is used elsewhere). If the loop count is below the short press threshold it returns NOTHING (no action), else it returns either a SHORT_PRESS or a LONG_PRESS depending on how high the loopCount got.

ActionType Footswitch::read()
{
    loopCount = 0;

    while (digitalRead(inputPin) == LOW)
        loopCount++;

    if (loopCount < SHORT_PRESS_TIME)
        return NOTHING;
   

    return loopCount < LONG_PRESS_TIME ? SHORT_PRESS : LONG_PRESS;
}

For context here are the other declarations used:

#ifndef LONG_PRESS_TIME
const uint32_t LONG_PRESS_TIME = 60000;
#endif

#ifndef SHORT_PRESS_TIME
const uint32_t SHORT_PRESS_TIME = 5000;
#endif

enum ActionType
{
    NOTHING = 0,
    INCREASE = 1,
    DECREASE = 2,
    SHORT_PRESS = 3,
    LONG_PRESS = 4
};

And the input type an INPUT_PULLUP

2

I do understand that blocking is bad, and know how to use millis (capture the time at the press and compare it to the total elapsed time) but also (so far!) I've not had any issues with blocking causing me trouble.

But still, I'm wondering if there are more optimal ways of writing this same piece of functionality?

3

i believe a better approach is to recognize both an early release indicating a short press and a timeout indicating a long press. capturing the time (millis()) the button is pressed and monitoring buttons avoid blocking

4 
#ifndef LONG_PRESS_TIME
const uint32_t LONG_PRESS_TIME = 60000;
#endif

Will that work as @anon46966594 intended? Does #ifndef recognise C variable names? I thought it would only recognise symbols defined with #define.

5

My MobaTools library contains a class to read several buttons with edge detection, short/longpres and more. If you don't want simply to use it, you can have a look into MoToButtons.h to see how it is done ( witout blocking ). The complete class definition is contained in this file.

6

consider

enum { ButNone, ButShort, ButLong };

struct Button {
    const unsigned long MsecShort = 1000;
    byte _pin;
    byte _state;
    unsigned long _msec;

  public:
    Button (int pin)  {
        _pin = pin;
        pinMode (_pin, INPUT_PULLUP);
        _state = digitalRead (_pin);
    }

    int read (void) {
        byte but = digitalRead (_pin);
        if (_state != but)  {
            _state = but;
            delay (10);     // debounce

            if (LOW == but)  {
                _msec = millis();
                _msec = 0 ==_msec ? 1 : _msec;
            }
            // released before timeout
            else if (0 != _msec) { 
                _msec = 0;
                return ButShort;
            }
        }

        // timeout
        if (0 != _msec && (millis() - _msec) > MsecShort)  {
            _msec = 0;
            return ButLong;
        }

        return ButNone;
    }
};

Button but (A1);

// -----------------------------------------------------------------------------
void loop()
{
    switch (but.read ())  {
    case ButShort:
        Serial.println ("short");
        break;

    case ButLong:
        Serial.println ("long");
        break;
    }
}

void setup()
{
    Serial.begin (9600);
}
1 Like
7

My take on reading buttons is here: Buttons and other electro-mechanical inputs (introduction)

More examples here, including short press / long press detection: Buttons and other electro-mechanical inputs (advanced)

8

This is quite nice actually! I had to 'translate' it into my own style of writing code before it made a bit more sense to me but that does actually do the same thing as my code for not that many lines of extra code and is non-blocking.

Happy to mark this as the solution as it does improve on my code but keeps the same functionality (I wasn't looking to use a library but to get better at writing/understanding this for myself)... unless anyone has improvements on gcjr's code?

This was my translation which was mostly giving my own variable names and changing '0 != variable', to 'variable != 0' :smiley: I also added the non-blocking proof so I could double check that long pressing the button didn't hold up the rest of the loop code, which was my own critique of my original post.

enum ActionType
{
    NOTHING = 0,
    INCREASE = 1,
    DECREASE = 2,
    SHORT_PRESS = 3,
    LONG_PRESS = 4
};

class Button {
  public:
  
    static const unsigned long longPressTimeout = 1000;
    byte _pin;
    byte lastState;
    unsigned long timeCapture;

  
    Button (int pin)  {
        _pin = pin;
        pinMode (_pin, INPUT_PULLUP);
        lastState = digitalRead (_pin);
    }

int read () 
    {

        byte currentState = digitalRead (_pin);

        if (lastState != currentState)  
        	{
              lastState = currentState;
              delay (10);     // debounce

              if (currentState == LOW)
                  {
                  timeCapture = millis();
                  timeCapture = (timeCapture == 0 ? 1 : timeCapture);
                  }
              // released before timeout
              else if (timeCapture != 0) 
                  { 
                  timeCapture = 0;
                  return SHORT_PRESS;
                  }
        	}

        // timeout
        if (timeCapture !=  0 && (millis() - timeCapture) > Button::longPressTimeout)  
        	{
              timeCapture = 0;
              return LONG_PRESS;
        	}

   	return NOTHING;
  
    } // End of read()
  
};

Button button(2);

// -----------------------------------------------------------------------------

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

void loop()
{
  

static int count;

  Serial.println((String)"Non-blocking proof: " + count++);
  
    switch (button.read ())  {
    
    case SHORT_PRESS:
        Serial.println ("Short Pressed");
      	count=0;
        break;

    case LONG_PRESS:
        Serial.println ("Long Pressed");
      count=0;
        break;
    }
}
9

I put the code in #6 in the wokwi.

It works well.

I guess I pay less for switches, so I need more than 10 ms debouncing.

But there is no need for the delay, so with minimal damage except for obvious style differences in my additions, I have amended @gcjr's code to provide a few more messages.

Believe it or not, 10 ms can in some circumstances be an eternity.

enum { ButNone, ButDown, ButStill, ButShort, ButLong };

# define DEBOUNCE 100   // absurd to demonstrate use 50 or wahtever you get away with

struct Button {
    const unsigned long MsecShort = 1000;
    byte _pin;
    byte _state = HIGH;
    unsigned long _msec;

  public:
    Button (int pin)  {
        _pin = pin;
        pinMode (_pin, INPUT_PULLUP);
        _state = digitalRead (_pin);
    }

    int read (void) {
    
    unsigned long now = millis();
    
    if (now - _msec < DEBOUNCE) {
    	if (_state == LOW)
        return (ButStill);
      else
        return (ButNone);
    }

 // we can look at the button again
    
    byte but = digitalRead (_pin);
    
    byte returnValue;
    
    if (_state != but)  {
        _state = but;
        
//            delay (10);     // debounce

        if (LOW == but)  {
            
            returnValue = ButDown;
        }
        else
            returnValue = (now - _msec > 1000) ? ButLong : ButShort;
    
      _msec = now;
    }
    else returnValue = (_state == LOW) ? ButStill : ButNone;

    return returnValue;
}
};


Button but (A1);

// -----------------------------------------------------------------------------
void loop()
{
  static byte printedStill = false;

    switch (but.read ())  {
    case ButDown :
    	Serial.println("button pressed");
    	break;
    	
	case ButStill :

    if (!printedStill) {
		  Serial.println(" and is being held");
      printedStill = true;
    }

		break;

    case ButShort:
        Serial.println ("short");
        printedStill = false;
        break;

    case ButLong:
        Serial.println ("long");
        printedStill = false;
        break;
    }
}

void setup()
{
    Serial.begin (9600);
    Serial.println("Hello ButtonThingWorld!\n");
}

See it here, I have only "before caffeine" tested this, but it does seem to function:

a7

10

guess my code is "flawed"

1 Like
11

I didn't say it was flawed, that's why I marked it as the solution.

'0 != variable' is just backwards from how my brain like to reads it.
It feels like I'm reading the code in a mirror :smiley:

But now I think about it, if I've learned anything from this forum (aside from posting with tags) it's that any and all use of delay() is punishable by death and that regardless of how small, it is blocking :stuck_out_tongue:

12

Don't be silly, death is too good for anyone using delay!

1 Like
13

comment to 777

14

It used delay(), it didn't have to. Nnot a flaw, very commonly seen.

10 ms is a long time in the context of loops running at, say 50 Hz.

I don't want to waste half my time denouncing a button. You can call it a flaw if that helps.

There is no need to wait for the button to settle. Deglitching noisy signals is another thing entirely, even so delay() doesn't have to be part of the solution.

And sure, getting rid of delay() on principal can be good for the brain.

a7

1 Like
15

i found a need. i saw multiple short button presses being reported

16

Would it then be better to have a minimum threshold for short press time too? To save having to use delay? Similar to my original post

17

I agree. The only thing you have to do, is to read the button not too often. The intervall between 2 read statements must be longer than the bouncetime of the button. This is sufficent to debounce it.

18

TBC the method in #6 needs the delay. Not sure I feel any better about it finding that it was introduced to fix an observed problem.

In my adjustment, shall we say, to that struct class object, there is no need to delay().

There is no need to delay reporting that the button went down.

And a short button press is fine and recognized after the debounce interval. No need to filter "too short", buttons do not bounce closed or open capriciously, they always do it in the context of getting to a final stable state.

I only added messages and removed delay(). Imma call that an improvement, and this is a struct class object that can and will be useful to me.

I do not have much experience with the struct class object thing, I like how it works and is used here. I appreciate the opportunity to learn about them in these fora, and here in this thread, and I hope my injection of some things I have learned about debouncing are useful.

BTW what do you call it not "struct class object" when you along about such things?

a7

1 Like
19

Yes. At the top of my method, I look to see if it is "too soon to even look at the button".

a7

20

by adding a delay ?
why not add a delay only when needed