3 out of 4 buttons behaving strangely (Button 1 and Joystick work flawlessly)

I had 8 buttons originally but removed 4 and added the joystick. All 8 buttons worked but most of them had to be held down to get them to work (except Button 1 which worked perfectly). They're all coded and wired the same way so they should only have to be pressed to give an input. Now, some of the buttons have to be held for a few seconds just for the input to register.

I attached a pic of the schematic. The buttons are each using a 10k resistor. I don't understand why they're behaving stangely.

#include <Joystick.h>

const int Button1 = 3; //Right side - bottom
const int Button2 = 4; //Right side - right
const int Button3 = 5; //Right side - top
const int Button4 = 6; //Right side - left

const int X_pin = A0; //X-Axis 
const int Y_pin = A1; //Y-Axis
const int SW_pin = 2; //SW button (Click Joystick)

int buttonState = 0;         // variable for reading the pushbutton status
int x,y,z;

long lastDebounceTime = 0;
long debounceDelay = 50;


void setup() {
  Serial.begin(9600);
 
  pinMode(Button1, INPUT);
  pinMode(Button2, INPUT);
  pinMode(Button3, INPUT);
  pinMode(Button4, INPUT);
  pinMode(SW_pin, INPUT);
  digitalWrite(SW_pin, HIGH);

}

void loop() {

  x = analogRead(X_pin);
  if (x == 1023)
  {
    Serial.println("Up:");
  }
  else

  x = analogRead(X_pin);
  if (x == 0)
  {
    Serial.println("Down:");
  }
  else

  y = analogRead(Y_pin);
  if (y == 1023)
  {
    Serial.println("Right:");
  }


  y = analogRead(Y_pin);
  if (y == 0)
  {
    Serial.println("Left:");
  }


  int z = digitalRead(SW_pin);
  if (z == 0)
  {
    Serial.println("Enter:");
  }

    if ( (millis() - lastDebounceTime) > debounceDelay) {
  
  buttonState = digitalRead(Button1);// read the state of the pushbutton value:
  if (buttonState == HIGH) { // check if the pushbutton is pressed. If it is, the buttonState is HIGH:
    lastDebounceTime = millis();
    Serial.print("1:");
  }
else {
    lastDebounceTime = millis();
    }
  }
  
    if ( (millis() - lastDebounceTime) > debounceDelay) {
  
  buttonState = digitalRead(Button2);// read the state of the pushbutton value:
  if (buttonState == HIGH) { // check if the pushbutton is pressed. If it is, the buttonState is HIGH:
    lastDebounceTime = millis();
    Serial.print("2:");
  } 
else {
    lastDebounceTime = millis();
    }
  }
  
    if ( (millis() - lastDebounceTime) > debounceDelay) {
  
  buttonState = digitalRead(Button3);// read the state of the pushbutton value:
  if (buttonState == HIGH) { // check if the pushbutton is pressed. If it is, the buttonState is HIGH:
    lastDebounceTime = millis();
    Serial.print("3:");
  } 
else {
    lastDebounceTime = millis();
    }
  }
  
    if ( (millis() - lastDebounceTime) > debounceDelay) {
  
  buttonState = digitalRead(Button4);// read the state of the pushbutton value:
  if (buttonState == HIGH) { // check if the pushbutton is pressed. If it is, the buttonState is HIGH:
    lastDebounceTime = millis();
    Serial.print("4:");
  } 
else {
    lastDebounceTime = millis();
    }
  }
}

(deleted)


Can you please re-draw that, but accurately? You cannot push 2 wires into the same breadboard hole. I don't know why Fritzing even allows you to draw it like that, it must be a bug.

You don't need those 10K resistors anyway, you can just use the built-in pull-ups.

It's ok to use the same variables to debounce all the buttons, if you do it the right way, which is to read all the button pins together, but only do it 20~50 times per second.

I wrote this as demonstration code for debouncing buttons, in the example below it debounces 4 buttons. You can modify it to do what you want.

PS, I don't understand why you are using analogue read for button inputs.

/* Simple button debounce for 4 buttons. Increments a count and sends the updated count to the serial monitor once per button press */
/* Tested on an Uno */
/* Connect simple push to make buttons between 0V and pin 2, 0V and pin 3, 0V and pin 4 and between 0V and pin 5 */

#define noOfButtons 4     //Exactly what it says; must be the same as the number of elements in buttonPins
#define bounceDelay 20    //Minimum delay before regarding a button as being pressed and debounced
#define minButtonPress 3  //Number of times the button has to be detected as pressed before the press is considered to be valid

const int buttonPins[] = {2, 3, 4, 5};      // Input pins to use, connect buttons between these pins and 0V
uint32_t previousMillis[noOfButtons];       // Timers to time out bounce duration for each button
uint8_t pressCount[noOfButtons];            // Counts the number of times the button is detected as pressed, when this count reaches minButtonPress button is regared as debounced 
uint8_t testCount[noOfButtons];             //Test count, incremented once per button press

void setup() {
  uint8_t i;
  uint32_t baudrate = 115200;
  Serial.begin(baudrate);
  Serial.println("");
  Serial.print("Serial port connected: ");
  Serial.println(baudrate);
  for (i = 0; i < noOfButtons; ++i) {
    pinMode(buttonPins[i], INPUT_PULLUP);
    Serial.print("Testcount ");
    Serial.print(i);
    Serial.print(" = ");
    Serial.println(testCount[i]);
  }
}

void loop() {
  debounce();
  delay(10);     //Your other code goes here instead of this delay. DO NOT leave this delay here, it's ONLY for demonstration.
}

void debounce() {
  uint8_t i;
  uint32_t currentMillis = millis();
  for (i = 0; i < noOfButtons; ++i) {
    if (digitalRead(buttonPins[i])) {             //Input is high, button not pressed or in the middle of bouncing and happens to be high
        previousMillis[i] = currentMillis;        //Set previousMillis to millis to reset timeout
        pressCount[i] = 0;                        //Set the number of times the button has been detected as pressed to 0
      } else {
      if (currentMillis - previousMillis[i] > bounceDelay) {
        previousMillis[i] = currentMillis;        //Set previousMillis to millis to reset timeout
        ++pressCount[i];
        if (pressCount[i] == minButtonPress) {
          doStuff(i);                             //Button has been debounced. Call function to do whatever you want done.
        }
      }
    }
  }
}

// Function to do whatever you want done once the button has been debounced.
// In this example it increments a counter and send the count to the serial monitor.
// Put your own functions here to do whatever you like.
void doStuff(uint8_t buttonNumber) {
  ++testCount[buttonNumber];
  Serial.print("Button ");
  Serial.print(buttonNumber);
  Serial.print(" testcount = ");
  Serial.println (testCount[buttonNumber]);
}

PaulRB:


Can you please re-draw that, but accurately? You cannot push 2 wires into the same breadboard hole. I don't know why Fritzing even allows you to draw it like that, it must be a bug.

You don't need those 10K resistors anyway, you can just use the built-in pull-ups.

It's ok to use the same variables to debounce all the buttons, if you do it the right way, which is to read all the button pins together, but only do it 20~50 times per second.

It's accurate. You can have a resistor wire and a regular wire in the same hole. They were exactly like that but are soldered now. And I know I can do the Pull-up but I already had it working this way so I just left it.

PerryBebbington:
I wrote this as demonstration code for debouncing buttons, in the example below it debounces 4 buttons. You can modify it to do what you want.

PS, I don't understand why you are using analogue read for button inputs.

/* Simple button debounce for 4 buttons. Increments a count and sends the updated count to the serial monitor once per button press */

/* Tested on an Uno /
/
Connect simple push to make buttons between 0V and pin 2, 0V and pin 3, 0V and pin 4 and between 0V and pin 5 */

#define noOfButtons 4    //Exactly what it says; must be the same as the number of elements in buttonPins
#define bounceDelay 20    //Minimum delay before regarding a button as being pressed and debounced
#define minButtonPress 3  //Number of times the button has to be detected as pressed before the press is considered to be valid

const int buttonPins[] = {2, 3, 4, 5};      // Input pins to use, connect buttons between these pins and 0V
uint32_t previousMillis[noOfButtons];      // Timers to time out bounce duration for each button
uint8_t pressCount[noOfButtons];            // Counts the number of times the button is detected as pressed, when this count reaches minButtonPress button is regared as debounced
uint8_t testCount[noOfButtons];            //Test count, incremented once per button press

void setup() {
  uint8_t i;
  uint32_t baudrate = 115200;
  Serial.begin(baudrate);
  Serial.println("");
  Serial.print("Serial port connected: ");
  Serial.println(baudrate);
  for (i = 0; i < noOfButtons; ++i) {
    pinMode(buttonPins[i], INPUT_PULLUP);
    Serial.print("Testcount ");
    Serial.print(i);
    Serial.print(" = ");
    Serial.println(testCount[i]);
  }
}

void loop() {
  debounce();
  delay(10);    //Your other code goes here instead of this delay. DO NOT leave this delay here, it's ONLY for demonstration.
}

void debounce() {
  uint8_t i;
  uint32_t currentMillis = millis();
  for (i = 0; i < noOfButtons; ++i) {
    if (digitalRead(buttonPins[i])) {            //Input is high, button not pressed or in the middle of bouncing and happens to be high
        previousMillis[i] = currentMillis;        //Set previousMillis to millis to reset timeout
        pressCount[i] = 0;                        //Set the number of times the button has been detected as pressed to 0
      } else {
      if (currentMillis - previousMillis[i] > bounceDelay) {
        previousMillis[i] = currentMillis;        //Set previousMillis to millis to reset timeout
        ++pressCount[i];
        if (pressCount[i] == minButtonPress) {
          doStuff(i);                            //Button has been debounced. Call function to do whatever you want done.
        }
      }
    }
  }
}

// Function to do whatever you want done once the button has been debounced.
// In this example it increments a counter and send the count to the serial monitor.
// Put your own functions here to do whatever you like.
void doStuff(uint8_t buttonNumber) {
  ++testCount[buttonNumber];
  Serial.print("Button ");
  Serial.print(buttonNumber);
  Serial.print(" testcount = ");
  Serial.println (testCount[buttonNumber]);
}

It's only Analog for the X and Y axis. The buttons are Digital. I'm gonna try out what you posted. Thanks.

PerryBebbington:
I wrote this as demonstration code for debouncing buttons, in the example below it debounces 4 buttons. You can modify it to do what you want.

PS, I don't understand why you are using analogue read for button inputs.

/* Simple button debounce for 4 buttons. Increments a count and sends the updated count to the serial monitor once per button press */

/* Tested on an Uno /
/
Connect simple push to make buttons between 0V and pin 2, 0V and pin 3, 0V and pin 4 and between 0V and pin 5 */

#define noOfButtons 4    //Exactly what it says; must be the same as the number of elements in buttonPins
#define bounceDelay 20    //Minimum delay before regarding a button as being pressed and debounced
#define minButtonPress 3  //Number of times the button has to be detected as pressed before the press is considered to be valid

const int buttonPins[] = {2, 3, 4, 5};      // Input pins to use, connect buttons between these pins and 0V
uint32_t previousMillis[noOfButtons];      // Timers to time out bounce duration for each button
uint8_t pressCount[noOfButtons];            // Counts the number of times the button is detected as pressed, when this count reaches minButtonPress button is regared as debounced
uint8_t testCount[noOfButtons];            //Test count, incremented once per button press

void setup() {
  uint8_t i;
  uint32_t baudrate = 115200;
  Serial.begin(baudrate);
  Serial.println("");
  Serial.print("Serial port connected: ");
  Serial.println(baudrate);
  for (i = 0; i < noOfButtons; ++i) {
    pinMode(buttonPins[i], INPUT_PULLUP);
    Serial.print("Testcount ");
    Serial.print(i);
    Serial.print(" = ");
    Serial.println(testCount[i]);
  }
}

void loop() {
  debounce();
  delay(10);    //Your other code goes here instead of this delay. DO NOT leave this delay here, it's ONLY for demonstration.
}

void debounce() {
  uint8_t i;
  uint32_t currentMillis = millis();
  for (i = 0; i < noOfButtons; ++i) {
    if (digitalRead(buttonPins[i])) {            //Input is high, button not pressed or in the middle of bouncing and happens to be high
        previousMillis[i] = currentMillis;        //Set previousMillis to millis to reset timeout
        pressCount[i] = 0;                        //Set the number of times the button has been detected as pressed to 0
      } else {
      if (currentMillis - previousMillis[i] > bounceDelay) {
        previousMillis[i] = currentMillis;        //Set previousMillis to millis to reset timeout
        ++pressCount[i];
        if (pressCount[i] == minButtonPress) {
          doStuff(i);                            //Button has been debounced. Call function to do whatever you want done.
        }
      }
    }
  }
}

// Function to do whatever you want done once the button has been debounced.
// In this example it increments a counter and send the count to the serial monitor.
// Put your own functions here to do whatever you like.
void doStuff(uint8_t buttonNumber) {
  ++testCount[buttonNumber];
  Serial.print("Button ");
  Serial.print(buttonNumber);
  Serial.print(" testcount = ");
  Serial.println (testCount[buttonNumber]);
}

I tried using that but I got A LOT of errors. It made my working code have issues so I'm not sure.

I tried using that but I got A LOT of errors.

Did you just paste my code into yours or did you modify it to what you are doing? It was not intended as a direct copy and paste, it was intended as something to modify to suit your code.

Try my code on its own with buttons as per the instructions.

SOLVED:

I took your guys' advice and gave each button it's own debounce. I added 3 more and gave each button it's own. Super simple.

long lastDebounceTime1 = 0;
long lastDebounceTime2 = 0;
long lastDebounceTime3 = 0;
long lastDebounceTime4 = 0;
#include <Joystick.h>


const int Button1 = 3; //Right side - bottom
const int Button2 = 4; //Right side - right
const int Button3 = 5; //Right side - top
const int Button4 = 6; //Right side - left

const int X_pin = A0; //X-Axis 
const int Y_pin = A1; //Y-Axis
const int SW_pin = 2; //SW button (Click Joystick)
const long debounceDelay = 50;

// variables will change:
int buttonState = 0;         // variable for reading the pushbutton status
int x,y,z;

long lastDebounceTime1 = 0;
long lastDebounceTime2 = 0;
long lastDebounceTime3 = 0;
long lastDebounceTime4 = 0;

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

 pinMode(Button1, INPUT_PULLUP);
 pinMode(Button2, INPUT_PULLUP);
 pinMode(Button3, INPUT_PULLUP);
 pinMode(Button4, INPUT_PULLUP);
 pinMode(SW_pin, INPUT);
 digitalWrite(SW_pin, HIGH);

}

void loop() {

 x = analogRead(X_pin);
 if (x == 1023)
 {
   Serial.println("Up:");
 }
 else

 x = analogRead(X_pin);
 if (x == 0)
 {
   Serial.println("Down:");
 }
 else

 y = analogRead(Y_pin);
 if (y == 1023)
 {
   Serial.println("Right:");
 }


 y = analogRead(Y_pin);
 if (y == 0)
 {
   Serial.println("Left:");
 }


 int z = digitalRead(SW_pin);
 if (z == 0)
 {
   Serial.println("Enter:");
 }

   if ( (millis() - lastDebounceTime1) > debounceDelay) {
 
 buttonState = digitalRead(Button1);// read the state of the pushbutton value:
 if (buttonState == LOW) { // check if the pushbutton is pressed. If it is, the buttonState is HIGH:
   lastDebounceTime1 = millis();
   Serial.print("1:");
 } else {
   lastDebounceTime1 = millis();
   }
 }
 
   if ( (millis() - lastDebounceTime2) > debounceDelay) {
 
 buttonState = digitalRead(Button2);// read the state of the pushbutton value:
 if (buttonState == LOW) { // check if the pushbutton is pressed. If it is, the buttonState is HIGH:
   lastDebounceTime2 = millis();
   Serial.print("2:");
 } else {
   lastDebounceTime2 = millis();
   }
 }
 
   if ( (millis() - lastDebounceTime3) > debounceDelay) {
 
 buttonState = digitalRead(Button3);// read the state of the pushbutton value:
 if (buttonState == LOW) { // check if the pushbutton is pressed. If it is, the buttonState is HIGH:
   lastDebounceTime3 = millis();
   Serial.print("3:");
 } else {
   lastDebounceTime3 = millis();
   }
 }
 
   if ( (millis() - lastDebounceTime4) > debounceDelay) {
 
 buttonState = digitalRead(Button4);// read the state of the pushbutton value:
 if (buttonState == LOW) { // check if the pushbutton is pressed. If it is, the buttonState is HIGH:
   lastDebounceTime4 = millis();
   Serial.print("4:");
 } else {
   lastDebounceTime4 = millis();
   }
 }
}

It's accurate. You can have a resistor wire and a regular wire in the same hole.

That's breadboard abuse. You will soon have to throw that breadboard away because it won't be able to make reliable connections any more.

Super simple.

Its about 4 times more complex than it needs to be.

You can have a resistor wire and a regular wire in the same hole.

But why? The whole point of breadboard having 5 holes in a row is so that you don't ever have to even think of doing that.

long lastDebounceTime1 = 0;
long lastDebounceTime2 = 0;
long lastDebounceTime3 = 0;
long lastDebounceTime4 = 0;
...
pinMode(Button1, INPUT_PULLUP);
pinMode(Button2, INPUT_PULLUP);
pinMode(Button3, INPUT_PULLUP);
pinMode(Button4, INPUT_PULLUP);

[crystal ball]There are arrays in your future.[/crystal ball]

(deleted)