Sometimes missing return value from function

Hi all,

I've been trying to troubleshoot 'inconsistent button presses' for a few days. I finally narrowed it down to the fact that I'm not receiving the return value from a function and for the life of me I can't figure out why.

I've re-written my code ~8 times but I can't seem to make it work reliably. I would like to determine if a button was pressed(<600ms) or held(>600ms).

I'm not really sure what I'm doing wrong and any search terms I can use are too vague to return anything relevant.

The board I'm using is a NodeMCU(ESP-32S) from AliExpress(I think).
Sketch:

#define BUTTON_PRESSED LOW
#define BUTTON_RELEASED HIGH

#define BUTTON_SHORT_PRESS 1
#define BUTTON_LONG_PRESS 2

const int ledPin = 12; //LED_BUILTIN;

typedef class BUTTON_CLASS {
  public:
  bool state = BUTTON_RELEASED;
  bool longPressSent = false;
  bool shortPress = false;
  int pin = 13;
  unsigned long pushTime = 0;
  unsigned long holdTime = 0;
  unsigned long longPressTime = 600;
  int debounceTime = 20;

  int status() {
    if (state == BUTTON_PRESSED) {
      digitalWrite(ledPin, HIGH);
      if (pushTime == 0) {
        pushTime = millis();
        } else {
          holdTime = millis() - pushTime;
          if ((holdTime > longPressTime) && (longPressSent == false)) { longPressSent = true; return BUTTON_LONG_PRESS; }
        }
    }
    if (state == BUTTON_RELEASED) {
      digitalWrite(ledPin, LOW);
      if (pushTime != 0){
        holdTime = millis() - pushTime;
        if ((holdTime > debounceTime) && (longPressSent == false)) {
          pushTime = 0;
          Serial.println("\nabout to return short-press");
          return BUTTON_SHORT_PRESS;
        }
        pushTime = 0;
      }
    longPressSent = false;
    }
    return 0;
    }
};
BUTTON_CLASS button;


void setup() {
  Serial.begin(115200);
  Serial.println("\n\nStarting buttonClass...");
  pinMode (button.pin, INPUT);
  pinMode (ledPin, OUTPUT);
  digitalWrite(ledPin, LOW);
  attachInterrupt(digitalPinToInterrupt(button.pin), buttonInterrupt, CHANGE);
}

void loop() {
 
  if (button.status() == BUTTON_SHORT_PRESS) {Serial.println(" returned button short press");}
  if (button.status() == BUTTON_LONG_PRESS) {Serial.print("\nButton long press");}
}

void buttonInterrupt() {
  if (digitalRead(button.pin) == BUTTON_PRESSED) {
    button.state = BUTTON_PRESSED;
    } else {
    button.state = BUTTON_RELEASED;
    }
}

Output:

Starting buttonClass...

about to return short-press

about to return short-press

about to return short-press
 returned button short press

about to return short-press

about to return short-press

about to return short-press

about to return short-press

If anyone could help point me in the right direction, I'd appreciate it. I'm out of ideas :slight_smile:

Thanks,
Jason

You have not identified which function is not returning a value. Nor have you told us how you know that.

Paul

You need to declare "state" as volatile

Nor have you included the code where the problem actually is....

Regards,
Ray L.

arduino_new:
You need to declare "state" as volatile

First, it is "volatile", not "violate". Second, you cannot possibly know that from the snippet that was posted. There is no indication in that code that interrupts are involved.

Regards,
Ray L.

Paul_KD7HB:
You have not identified which function is not returning a value. Nor have you told us how you know that.

Paul

The output I posted shows that the status() function is about to return the value("about to return short-press") and also shows that it only got to the point "returned button short press" in the main loop.

Jason

Paul_KD7HB:
You have not identified which function is not returning a value. Nor have you told us how you know that.

Paul

RayLivingston:
Nor have you included the code where the problem actually is....

Regards,
Ray L.

Hey Paul,
The specific lines that the problem resides in are:

return BUTTON_SHORT_PRESS;

and

 if (button.status() == BUTTON_SHORT_PRESS) {Serial.println(" returned button short press");}

I guarantee that the function is returning a value, otherwise the compiler would complain. That the value may not be what you expect / want is a different problem.

RayLivingston:
There is no indication in that code that interrupts are involved.

Regards,
Ray L.

Are we seeing different code?

gfvalvo:
I guarantee that the function is returning a value, otherwise the compiler would complain. That the value may not be what you expect / want is a different problem.

I'm sure you're right but if the code gets to the line containing "about to return short-press" and the next line is "return BUTTON_SHORT_PRESS;" and "BUTTON_SHORT_PRESS" is a define(value 1), how is "button.status() == BUTTON_SHORT_PRESS" not true?

jasonzx:
I'm sure you're right but if the code gets to the line containing "about to return short-press" and the next line is "return BUTTON_SHORT_PRESS;" and "BUTTON_SHORT_PRESS" is a define(value 1), how is "button.status() == BUTTON_SHORT_PRESS" not true?

The way to debug your problem is Serial.println() the value before you return it and then again after the function return.

Paul

Interrupts can happen at any time.

  if (button.status() == BUTTON_SHORT_PRESS) {
    Serial.println(" returned button short press");
  }

// <------- Button is released after a short press right here.
//          Next call to status() prints "about to return short-press" 
//          But, 'if' statement only checks for BUTTON_LONG_PRESS

  if (button.status() == BUTTON_LONG_PRESS) {
    Serial.print("\nButton long press");
  }

gfvalvo:
Interrupts can happen at any time.

  if (button.status() == BUTTON_SHORT_PRESS) {

Serial.println(" returned button short press");
  }

// <------- Button is released after a short press right here.
//          Next call to status() prints "about to return short-press"
//          But, 'if' statement only checks for BUTTON_LONG_PRESS

if (button.status() == BUTTON_LONG_PRESS) {
    Serial.print("\nButton long press");
  }

Yes but I thought that after the interrupt the program would go back to where it left off but maybe I was mistaken?

jasonzx:
Yes but I thought that after the interrupt the program would go back to where it left off...

Exactly. And, here's the next statement to be executed after the ISR exits:

  if (button.status() == BUTTON_LONG_PRESS) {
    Serial.print("\nButton long press");

It doesn't check for BUTTON_SHORT_PRESS.

gfvalvo:
Exactly. And, here's the next statement to be executed after the ISR exits:

  if (button.status() == BUTTON_LONG_PRESS) {

Serial.print("\nButton long press");



It doesn't check for BUTTON_SHORT_PRESS.

Craaaap. Yeah. I see where I went wrong now.
I'm calling and comparing the function twice when I should be calling it once, saving it to a variable and comparing the variable. :confused:

Thanks everyone.

Also, as noted in Reply #2, state need to be declared 'volatile'.