How to add debounce to a 4x1 keypad input function

I looked at some of the debounce posts, but they seemed complicated for my use. I have a 4x1 keypad and want to make sure I only record each keystroke once. I am currently using a time delay, but its not very accurate. Any suggestions would be appreciated (and yeah, I don't need all the print statements. They were there for debug purposes)

Thank you

//*
//* This is the Arduino code for 4 keys Keypad.
//* watch the video for details and demo https://youtu.be/BiOh_Z2N7kk
//*  * 
//* Written by Ahmad Nejrabi for Robojax Video channel www.Robojax.com
//* Date: Dec 28, 2017, in Ajax, Ontario, Canada
//* Permission granted to share this code given that this
//* note is kept with the code.
//* Disclaimer: this code is "AS IS" and for educational purpose only.
//* this code has been downloaded from http://robojax.com/learn/arduino/
// Recorded Dec 28, 2017 for Robojax.com

#define key1 5 //connect wire 1 to pin 2
#define key2 4 //connect wire 2 to pin 3
#define key3 3 //connect wire 3 to pin 4
#define key4 2 //connect wire 4 to pin 5

void setup() {
 Serial.begin(9600);
 pinMode(key1, INPUT_PULLUP);// set pin as input
 pinMode(key2, INPUT_PULLUP);// set pin as input
 pinMode(key3, INPUT_PULLUP);// set pin as input
 pinMode(key4, INPUT_PULLUP);// set pin as input  
}

  int keypressed = 0;
  int keystroke_counter = 0;  // counts how many key strokes have been recorded
  int keypad_total = 0; // this is the total of the keypad entries added as a two digit value
  
void loop() {
  int key1S = digitalRead(key1);// read if key1 is pressed
  int key2S = digitalRead(key2);// read if key1 is pressed
  int key3S = digitalRead(key3);// read if key1 is pressed
  int key4S = digitalRead(key4);// read if key1 is pressed


  
 // Code written for Robojax.com video Tutorial & modified by Dan Manson
     if(!key1S){
      Serial.println("key 1 is pressed");
      keypressed = 1;
      keystroke_counter = keystroke_counter +1;
     }
     if(!key2S){
      keypressed = 2;
      Serial.println("key 2 is pressed");
      keystroke_counter = keystroke_counter +1;
     }
     if(!key3S){
      keypressed = 3;
      Serial.println("key 3 is pressed");
      keystroke_counter = keystroke_counter +1;
     }
     if(!key4S){
      keypressed = 4;
      Serial.println("key 4 is pressed");
      keystroke_counter = keystroke_counter +1;
     }  
     Serial.println("keystroke_counter total is ");
     Serial.println(keystroke_counter);             
     if (keystroke_counter == 1) {
        keypad_total = keypressed*10;
     }
     else if(keystroke_counter == 2) {
          keypad_total = keypad_total + keypressed;
     }
     else {
          Serial.println("keypad total is ");
          Serial.println(keypad_total);
     }


     if (keystroke_counter == 2) {
         Serial.println("keypad total is ");
         Serial.println(keypad_total);
         delay(5000);
         keystroke_counter = 0; // 2 Digit keystoke received.  Reset counter to zero
         keypad_total = 0; //// 2 Digit keystoke received.  Reset total to zero
     } 
     delay(150);
}

I am not clear that you are experiencing button bounce. You are not reading a button state change, and its possible that what you are seeing is the result of inadvertantly holding a button down through multiple loop cycles. Here is your code converted to reading when the button becomes pressed (state change) rather than when it is pressed. This code is more efficiently written with arrays.

#define key1 5 //connect wire 1 to pin 2
#define key2 4 //connect wire 2 to pin 3
#define key3 3 //connect wire 3 to pin 4
#define key4 2 //connect wire 4 to pin 5

void setup() {
  Serial.begin(9600);
  pinMode(key1, INPUT_PULLUP);// set pin as input
  pinMode(key2, INPUT_PULLUP);// set pin as input
  pinMode(key3, INPUT_PULLUP);// set pin as input
  pinMode(key4, INPUT_PULLUP);// set pin as input
}

int keypressed = 0;
int keystroke_counter = 0;  // counts how many key strokes have been recorded
int keypad_total = 0; // this is the total of the keypad entries added as a two digit value

void loop() {
  static byte last_key1S = 1;
  static byte last_key2S = 1;
  static byte last_key3S = 1;
  static byte last_key4S = 1;
  byte key1S = digitalRead(key1);// read key1
  byte key2S = digitalRead(key2);// read key2
  byte key3S = digitalRead(key3);// read key3
  byte key4S = digitalRead(key4);// read key4

  if (key1S != last_key1S) {
    if (key1S == 0) { //went from HIGH to LOW
      Serial.println("key 1 is pressed");
      keypressed = 1;
      keystroke_counter = keystroke_counter + 1;
    }
  }
  last_key1S = key1S;

  if (key2S != last_key2S) {
    if (key2S == 0) { //went from HIGH to LOW
      Serial.println("key 2 is pressed");
      keypressed = 2;
      keystroke_counter = keystroke_counter + 1;
    }
  }
  last_key2S = key2S;

  if (key3S != last_key3S) {
    if (key3S == 0) { //went from HIGH to LOW
      Serial.println("key 3 is pressed");
      keypressed = 3;
      keystroke_counter = keystroke_counter + 1;
    }
  }
  last_key3S = key3S;

  if (key4S != last_key4S) {
    if (key4S == 0) { //went from HIGH to LOW
      Serial.println("key 1 is pressed");
      keypressed = 4;
      keystroke_counter = keystroke_counter + 1;
    }
  }
  last_key4S = key4S;

  Serial.println("keystroke_counter total is ");
  Serial.println(keystroke_counter);
  if (keystroke_counter == 1) {
    keypad_total = keypressed * 10;
  }
  else if (keystroke_counter == 2) {
    keypad_total = keypad_total + keypressed;
  }
  else {
    Serial.println("keypad total is ");
    Serial.println(keypad_total);
  }
  
  if (keystroke_counter == 2) {
    Serial.println("keypad total is ");
    Serial.println(keypad_total);
    //delay(5000);
    keystroke_counter = 0; // 2 Digit keystoke received.  Reset counter to zero
    keypad_total = 0; //// 2 Digit keystoke received.  Reset total to zero
  }
  //delay(150);
}

Thank you very much! It works like a charm. I tried to click the buttons really fast and no errors.

Would you mind explain a little more how your changes work?
Looks like declaring the variables as byte and static byte was a key factor, but I haven't used those types before.

cattledog:
This code is more efficiently written with arrays.

Code below is my array version which I use as a template for cases like that. Just put the pin numbers in the array:

byte buttonPins[] = {8, 9, 10};

There’s a blink-without-delay on the builtin led to prove it doesn’t block. You can press and hold a button and press and release a different one while first one is held. You can do that with as many buttons at once as you have fingers :wink:

// state change detect on a button array
// 30 august 2019
// has blink-without-delay on pin13 to prove no blocking

// the buttons
byte buttonPins[] = {8, 9, 10}; //the buttons must be wired from pin to ground, pinmodes are input_pullup
const byte howManyButtons(sizeof(buttonPins) / sizeof(byte));
bool buttonStates[howManyButtons];         // current state of the button
bool lastButtonStates[howManyButtons];     // previous state of the button
bool buttonIsNewlyPressed[howManyButtons];
bool buttonIsNewlyReleased[howManyButtons];

//the bwod led
int bwodLedInterval = 500;
unsigned long previousMillisBwod;
bool bwodState = false;

void setup()
{
  // initialize serial communication:
  Serial.begin(9600);
  Serial.println("setup() ... ");
  Serial.println(".... state change detect on a button array ....");
  Serial.print("Compiler: ");
  Serial.print(__VERSION__);
  Serial.print(", Arduino IDE: ");
  Serial.println(ARDUINO);
  Serial.print("Created: ");
  Serial.print(__TIME__);
  Serial.print(", ");
  Serial.println(__DATE__);
  Serial.println(__FILE__);
  Serial.println(" ");

  // initialize the button pins as input with pullup so active low
  //    make sure the button is from pin to ground
  Serial.println("Should show button, pin, 1100:");
  for (int i = 0; i < howManyButtons; i++)
  {
    pinMode(buttonPins[i], INPUT_PULLUP);
    //initialize button states
    buttonStates[i] = digitalRead(buttonPins[i]);
    lastButtonStates[i] = buttonStates[i];
    buttonIsNewlyPressed[i] = 0;
    buttonIsNewlyReleased[i] = 0;
    Serial.print(i);
    Serial.print(", ");
    Serial.print(buttonPins[i]);
    Serial.print(", ");
    Serial.print(buttonStates[i]);
    Serial.print(lastButtonStates[i]);
    Serial.print(buttonIsNewlyPressed[i]);
    Serial.print(buttonIsNewlyReleased[i]);  //should show button, pin, then 1100
    Serial.println("");
  }
  //initialise pulse led
  pinMode(LED_BUILTIN, OUTPUT);
  digitalWrite(LED_BUILTIN, bwodState);

  Serial.println("setup() done");
  Serial.println("Press a button....");
  Serial.println(" ");
}

void loop()
{
  bwod();
  checkForButtonStateChange();
  doSomeThingWithANewlyPressedButton();
  doSomeThingWithANewlyReleasedButton();
} //loop

void checkForButtonStateChange()
{
  for (int i = 0; i < howManyButtons; i++)
  {
    buttonStates[i] = digitalRead(buttonPins[i]);
    // compare the buttonState to its previous state
    if (buttonStates[i] != lastButtonStates[i]) // means it changed... but which way?
    {
      if (buttonStates[i] == LOW)  // changed to pressed
      {
        Serial.print(i);
        Serial.println(" newly pressed");
        buttonIsNewlyPressed[i] = true;
      }
      else  // changed to released
      {
        // if the current state is HIGH then the button was released
        Serial.print("   ");
        Serial.print(i);
        Serial.println(" newly released");
        buttonIsNewlyReleased[i] = true;
      }
      // ersatz de-bounce
      delay(50);
    }
    // save the current state as the last state, for next time through the loop
    lastButtonStates[i] = buttonStates[i];
  }
} // checkForButtonStateChange()

void doSomeThingWithANewlyPressedButton()
{
  for (int i = 0; i < howManyButtons; i++)
  {
    if (buttonIsNewlyPressed[i])
    {
      Serial.print("Using the new press of button ");
      Serial.println(i);
      buttonIsNewlyPressed[i] = false;
    }
  }
}//doSomeThingWithANewlyPressedButton

void doSomeThingWithANewlyReleasedButton()
{
  for (int i = 0; i < howManyButtons; i++)
  {
    if (buttonIsNewlyReleased[i])
    {
      Serial.print("   Using the new release of button ");
      Serial.println(i);
      buttonIsNewlyReleased[i] = false;
    }
  }
}//doSomeThingWithANewlyReleasedButton

void bwod()
{
  if (millis() - previousMillisBwod >= bwodLedInterval)
  {
    previousMillisBwod = millis();
    bwodState = !bwodState;
    digitalWrite(LED_BUILTIN, bwodState);
  }
} //bwod

edit… sample output:

0 newly pressed
Using the new press of button 0
2 newly pressed
Using the new press of button 2
   2 newly released
   Using the new release of button 2
1 newly pressed
Using the new press of button 1
   0 newly released
   Using the new release of button 0
   1 newly released
   Using the new release of button 1

Looks like declaring the variables as byte and static byte was a key factor, but I haven't used those types before.

Minimizing variable size and memory usage is an important habit to develop with small microprocessors like the Arduino. See the reference on byte and int

The static qualifier lets you declare the variable in loop rather than as a global.

The important thing to understand is the concept of state change. Detecting when a button becomes pressed rather than is pressed. Comparing present state to past state the way it is done.