Timing push button code

Question

I am trying to time how long button is held for. The code seems to work but with two problems:

1. the millis values are being read wrongly. If you hold the button for 1 second, it returns holdTime of about 3000 ms. A quick click returns holdTime of about 1000 ms.

2. Each successive button press adds to the previous millis value so holdTime just gets higher and higher and doesn't measure how long the button has just been pressed for

Any ideas or advice for newbie welcome!

#include <LiquidCrystal.h>

const int button1Pin = 1;  // pushbutton 1 pin
const int ledPin = 13;     // LED pin

int button1State, button2State;  // variables to hold the pushbutton states
LiquidCrystal lcd(12, 11, 5, 4, 3, 2);
int startPressed = 0;
int endPressed = 0;
int holdTime = 0;

void setup() {
  // Set up the pushbutton pins to be an input:
  pinMode(button1Pin, INPUT);

  // Set up the LED pin to be an output:
  pinMode(ledPin, OUTPUT);   
  digitalWrite(ledPin, LOW);    
\
}

void loop() {

  while (digitalRead(button1Pin) == LOW) {}

  button1State = digitalRead(button1Pin);
  startPressed = 0;
  endPressed =0;
  holdTime = 0;

  // if button1 or button 2 are pressed (but not both)
  if (button1State == LOW) {
    digitalWrite(ledPin, LOW);   // turn the LED on
    startPressed = millis();
  } else {
    digitalWrite(ledPin, HIGH);  // turn the LED off
    endPressed = millis();
    holdTime = endPressed - startPressed;
  }
  lcd.print("You pressed for");
  lcd.setCursor(0, 1);
  lcd.print(holdTime);
  lcd.print("ms");
  while (digitalRead(button1Pin) == LOW) {}
  lcd.clear();
  startPressed =0;
  endPressed =0;
  holdTime = 0;
}

Answer 1

Your source has several flaws, and one error that generates the output you see. This is the analysis of the error.

In loop() you start by waiting for the button pin to get HIGH, following by another read of the pin's state that will most probably return the same, HIGH. Note: Only if your button bounces with just the right timing, this last read will return LOW.

while (digitalRead(button1Pin) == LOW) {}
button1State = digitalRead(button1Pin);

Next you check the pin's state, and let's assume that it is HIGH. Only the else part is executed, and it calculates the holdtime as the difference of endPressed and startPressed. The former has the current value of millis(), but the latter is 0 as you initialized it.

That's why you're getting always growing values.

How to solve?

Think about the while loops, do you need them? Remember that loop() is already called repeatedly.

You may need a state variable that stores the last pin's state, so you can detect changes.