Trying to turn button code into a function so it works with multiple buttons

I'm new to defining my own functions and not good at coding.

I took tutorial code I found for making a long-press and short-press button and made it work to increase a counter when the button is pressed or held down. I wanted to have another two buttons (one that I haven't gotten around to yet and one) to subtract when pressed or held. I duplicated the variables needed by the button detection function and made the function take some values to determine which variables to use so each button would have its own variables. The problem is that the code doesn't work (the counter does not appear on the serial monitor) and I'm not sure where it's gone wrong.

If I insert somewhere into the loop Serial.println(digitalRead(3)) then I can see that the button is working (the value responds to my presses) but if I change that to Serial.println(currentStatePlus) then the value is stuck at 0 regardless whether the button is pushed. It's only supposed to be 0 when I push the button.

Below are two lots of code: the first one for one button which works and the second lot of code is my attempt to make it work for two buttons; one adding and one subtracting from the counter.

Could anyone tell me why my code makes currentStatePlus get stuck at 0 when it should follow digitalRead(3)? Thanks for any insight.

// constants won't change. They're used here to set pin numbers:
const int BUTTON_PIN = 3; // the number of the pushbutton pin
const int SHORT_PRESS_TIME = 1000; // 1000 milliseconds
const int LONG_PRESS_TIME  = 1000; // 1000 milliseconds

// Variables will change:
int lastState = LOW;  // the previous state from the input pin
int currentState;     // the current reading from the input pin
unsigned long pressedTime  = 0;
unsigned long releasedTime = 0;
bool isPressing = false;
bool isLongDetected = false;

// counter
unsigned char counter = 0;

void setup() {
  Serial.begin(9600);
  pinMode(BUTTON_PIN, INPUT_PULLUP);
}

void loop() {
  buttonPressDetection();
  counterTask();
  }

void counterTask() {
  static unsigned char counterCheck = counter;
  static unsigned long previousMillis = 0;
  static unsigned long currentMillis;

  if (isLongDetected == true) {
    currentMillis = millis();
    if (currentMillis - previousMillis < 150) return;
    counter++;
  }
  previousMillis = currentMillis;

  if (counterCheck != counter) {
    Serial.println(counter);
  }
  counterCheck = counter;
}

void buttonPressDetection() {
  // read the state of the switch/button:
  currentState = digitalRead(BUTTON_PIN);

  if (lastState == HIGH && currentState == LOW) {       // button is pressed
    pressedTime = millis();
    isPressing = true;
    isLongDetected = false;
  } else if (lastState == LOW && currentState == HIGH) { // button is released
    isPressing = false;
    releasedTime = millis();

    long pressDuration = releasedTime - pressedTime;

    if ( 10 < pressDuration && pressDuration < 1000 )
      //Serial.println("A short press is detected");
      counter++;
    isLongDetected = false;
  }

  if (isPressing == true && isLongDetected == false) {
    long pressDuration = millis() - pressedTime;

    if ( pressDuration > LONG_PRESS_TIME ) {
      //Serial.println("A long press is detected");
      isLongDetected = true;
    }
  }

  // save the the last state
  lastState = currentState;
}

and

// constants won't change. They're used here to set pin numbers:
const int BUTTON_PIN_PLUS = 3;
const int BUTTON_PIN_MINUS = 4; // the number of the pushbutton pin
const int SHORT_PRESS_TIME = 1000; // 1000 milliseconds
const int LONG_PRESS_TIME  = 1000; // 1000 milliseconds

// Variables will change:
int lastStateMinus = LOW;  // the previous state from the input pin
int currentStateMinus;     // the current reading from the input pin
unsigned long pressedTimeMinus = 0;
unsigned long releasedTimeMinus = 0;
bool isPressingMinus = false;
bool isShortDetectedMinus = true;
bool isLongDetectedMinus = false;
int lastStatePlus = LOW;  // the previous state from the input pin
int currentStatePlus;     // the current reading from the input pin
unsigned long pressedTimePlus = 0;
unsigned long releasedTimePlus = 0;
bool isPressingPlus = false;
bool isShortDetectedPlus = true;
bool isLongDetectedPlus = false;

// counter
unsigned char counter = 0;

void setup() {
  Serial.begin(9600);
  pinMode(BUTTON_PIN_MINUS, INPUT_PULLUP);
  pinMode(BUTTON_PIN_PLUS, INPUT_PULLUP);
}

void loop() {
  buttonPressDetection(BUTTON_PIN_MINUS, lastStateMinus, currentStateMinus, pressedTimeMinus, releasedTimeMinus, isPressingMinus, isShortDetectedMinus, isLongDetectedMinus);
  buttonPressDetection(BUTTON_PIN_PLUS, lastStatePlus, currentStatePlus, pressedTimePlus, releasedTimePlus, isPressingPlus, isShortDetectedPlus, isLongDetectedPlus);
  counterTask();
}

void counterTask() {
  static unsigned char counterCheck = counter; // this will check for changes
  static unsigned long previousMillis = 0;
  static unsigned long currentMillis;

  if (isShortDetectedPlus == true) {
    counter++;
    isShortDetectedPlus = false;
  }
  if (isShortDetectedMinus == true) {
    counter--;
    isShortDetectedMinus = false;
  }

  if (isLongDetectedPlus == true) {
    currentMillis = millis();
    if (currentMillis - previousMillis < 150) return;
    counter++;
  }
  if (isLongDetectedMinus == true) {
    currentMillis = millis();
    if (currentMillis - previousMillis < 150) return;
    counter--;
  }
  previousMillis = currentMillis;

  if (counterCheck != counter) { // check whether counter has changes since last function call
    Serial.println(counter);
  }
  counterCheck = counter;
}

void buttonPressDetection(const int BUTTON_PIN, int lastState, int currentState, unsigned long pressedTime, unsigned long releasedTime, bool isPressing, bool isShortDetected, bool isLongDetected) {
  // read the state of the switch/button:
  currentState = digitalRead(BUTTON_PIN);

  if (lastState == HIGH && currentState == LOW) {       // button is pressed
    pressedTime = millis();
    isPressing = true;
    isLongDetected = false;
  } else if (lastState == LOW && currentState == HIGH) { // button is released
    isPressing = false;
    releasedTime = millis();

    long pressDuration = releasedTime - pressedTime;

    if ( 10 < pressDuration && pressDuration < 1000 )
      //Serial.println("A short press is detected");
      isShortDetected = true;
    isLongDetected = false;
  }

  if (isPressing == true && isLongDetected == false) {
    long pressDuration = millis() - pressedTime;

    if ( pressDuration > LONG_PRESS_TIME ) {
      //Serial.println("A long press is detected");
      isLongDetected = true;
    }
  }

  // save the the last state
  lastState = currentState;
}

Integers are passed by value. The function gets a COPY of the value and can't change the variable being passed. For lastState, currentState, pressedTime, releasedTime, isPressing, isShortDetected, and isLongDetected you probably want to use references so the function can change the values of those variables.

void buttonPressDetection(const int BUTTON_PIN, int &lastState, int &currentState, unsigned long &pressedTime, unsigned long &releasedTime, bool &isPressing, bool &isShortDetected, bool &isLongDetected) {
1 Like

I know that you know but to clarify a bit to @badrequest

When you call a function, the arguments are copied on the so-called stack. The function will use those copies and wil modify those copies, not the originals. When the function ends, the copies of the arguments are removed from the stack.

1 Like

Thank you both very much for taking the time to look at my code and respond!