Pushbuttons read as LOW although they are activated

Hello,

I have 5 pushbuttons in my circuit with an LCD screen. I am trying to read the voltage of each button and through if-statements complete certain tasks. In my if-statement, button 5 is used as the end to the user input, basically, the user has to choose between four drinks, being able to mix any and after button 5 is pressed the selection is over. When I run this code and click any mixture of the buttons and click button 5 last, only button 5 is set to "HIGH" and it seems that all the other buttons are set to "LOW" although they were pressed on. I think it has to do with the fact that button 5 is pressed last, and that through every loop the values of buttons 1 to 4 are set to "LOW" somehow, even though the buttons should not be affected by the code through each loop.

The code:

Blockquote

#include <LiquidCrystal.h>

LiquidCrystal lcd(12, 11, 5, 4, 3, 2);

int buttonState1;
int buttonState2;
int buttonState3;
int buttonState4;
int buttonState5;

const int button1 = 1;

const int button2 = 6;

const int button3 = 7;

const int button4 = 8;

const int button5 = 9;

void setup() {

lcd.begin(16, 2);

pinMode(button1, INPUT_PULLUP);
pinMode(button2, INPUT_PULLUP);
pinMode(button3, INPUT_PULLUP);
pinMode(button4, INPUT_PULLUP);
pinMode(button5, INPUT_PULLUP);

lcd.print("Choose drinks");

lcd.setCursor(0,1);

lcd.print("please!");

}

void loop() {

buttonState1 = digitalRead(button1);
buttonState2 = digitalRead(button2);
buttonState3 = digitalRead(button3);
buttonState4 = digitalRead(button4);
buttonState5 = digitalRead(button5);

if (buttonState5 == HIGH)
{

lcd.setCursor(0,0);

lcd.print("You chose drinks:");

lcd.setCursor(0,1);

lcd.print(" ");

lcd.setCursor(0,1);

if (buttonState1 == HIGH)
{

lcd.print("1  ");

}

if (buttonState2 == HIGH)
{

lcd.print("2   ");

}

if (buttonState3 == HIGH)
{

lcd.print("3   ");

}

if (buttonState4 == HIGH)
{

lcd.print("4   ");

}

}

}

Hi, @icantcode3241
Welcome to the forum.
Please read the post at the start of any forum , entitled "How to use this Forum".

This will show you how to post your code in a scrolling window.
Before you post it , press < CTRL > < T > in the IDE to Auto format your code to make it easier to read.
A circuit diagram will help, as we need to see how you have wired your buttons.

What model Arduino are you using?

Tom.. :grinning: :+1: :coffee: :australia:

Hi,
Can I suggest you write some simple code to read your buttons and display the results in the IDE serial monitor.

It looks like you have written all your code in one go, instead of stages.
Each stage getting a particular piece of hardware working , then combining them one at a time.

Tom... :grinning: :+1: :coffee: :australia:

Hey,

Sorry its 4 AM here, trying to finish this for a project and I'm not laser-focused, I put in a screenshot of the circuit diagram, and a reformatted code below. The Arduino used is Uno R3 and I'm using TinkerCad to simulate it before doing it, I don't know if that could be the problem.

#include <LiquidCrystal.h>

LiquidCrystal lcd(12, 11, 5, 4, 3, 2);

int buttonState1;
int buttonState2;
int buttonState3;
int buttonState4;
int buttonState5;

const int button1 = 1;

const int button2 = 6;

const int button3 = 7;

const int button4 = 8;

const int button5 = 9;

void setup() {

lcd.begin(16, 2);

pinMode(button1, INPUT_PULLUP);
pinMode(button2, INPUT_PULLUP);
pinMode(button3, INPUT_PULLUP);
pinMode(button4, INPUT_PULLUP);
pinMode(button5, INPUT_PULLUP);

lcd.print("Choose drinks");

lcd.setCursor(0, 1);

lcd.print("please!");

}

void loop() {

buttonState1 = digitalRead(button1);
buttonState2 = digitalRead(button2);
buttonState3 = digitalRead(button3);
buttonState4 = digitalRead(button4);
buttonState5 = digitalRead(button5);

if (buttonState5 == HIGH)
{

lcd.setCursor(0, 0);

lcd.print("You chose drinks:");

lcd.setCursor(0, 1);

lcd.print("                              ");

lcd.setCursor(0, 1);

if (buttonState1 == HIGH)
{

  lcd.print("1  ");
}

if (buttonState2 == HIGH)
{

  lcd.print("2   ");
}

if (buttonState3 == HIGH)
{

  lcd.print("3   ");
}

if (buttonState4 == HIGH)
{

  lcd.print("4   ");


}

}

}

Hi,
Here is your code in scrolling window and Auto Formated.

#include <LiquidCrystal.h>

LiquidCrystal lcd(12, 11, 5, 4, 3, 2);

int buttonState1;
int buttonState2;
int buttonState3;
int buttonState4;
int buttonState5;

const int button1 = 1;
const int button2 = 6;
const int button3 = 7;
const int button4 = 8;
const int button5 = 9;

void setup()
{
  lcd.begin(16, 2);
  pinMode(button1, INPUT_PULLUP);
  pinMode(button2, INPUT_PULLUP);
  pinMode(button3, INPUT_PULLUP);
  pinMode(button4, INPUT_PULLUP);
  pinMode(button5, INPUT_PULLUP);
  lcd.print("Choose drinks");
  lcd.setCursor(0, 1);
  lcd.print("please!");
}

void loop()
{
  buttonState1 = digitalRead(button1);
  buttonState2 = digitalRead(button2);
  buttonState3 = digitalRead(button3);
  buttonState4 = digitalRead(button4);
  buttonState5 = digitalRead(button5);
  if (buttonState5 == HIGH)
  {
    lcd.setCursor(0, 0);
    lcd.print("You chose drinks:");
    lcd.setCursor(0, 1);
    lcd.print("                              ");
    lcd.setCursor(0, 1);
    if (buttonState1 == HIGH)
    {
      lcd.print("1  ");
    }
    if (buttonState2 == HIGH)
    {
      lcd.print("2   ");
    }
    if (buttonState3 == HIGH)
    {
      lcd.print("3   ");
    }
    if (buttonState4 == HIGH)
    {
      lcd.print("4   ");
    }
  }
}

I suggest you get out the hardware and make it.

HINT;
Write code that just reads your buttons to check that you can, use the IDE serial monitor to check.
Write code that JUST puts Hello World into your LCD to prove it works.
Then think about combining them.

I troubleshoot and develop equipment for a living, even I start in small steps.

Tom... :grinning: :+1: :coffee: :australia:

Hey,

I’ve tested it in small steps, every one of them work, but it just seems when i want it to run in these if statements it just doesn’t.

Just ta add my 2 cents to @TomGeorge 's advice: Make sure you use arrays and functions, they will clear up your code quite a bit.

Hi,
You will have problems using pin 1 as a button pin.
Pin 0 and pin 1 are used for serial comms and programming.
Use another pin.

As you are using pullups, you buttons are between the gnd and the digital input, I hope.
So when you activate the button, a zero or LOW appears on your input, releasing the button and the pullup resistor pulls the digital input to 5V or HIGH.

Why do you have 10K resistors connected to each button if you already have pullups in your code?

PLEASE post a proper hand drawn circuit.

Tom.... :grinning: :+1: :coffee: :australia:

I put the code (#5 version) in the wokwi.com simulator.

This

 buttonState1 = digitalRead(button1);
 buttonState2 = digitalRead(button2);
 buttonState3 = digitalRead(button3);
 buttonState4 = digitalRead(button4);
 buttonState5 = digitalRead(button5);

when changed to reflect the way you've wired (we think) the buttons thusly

 buttonState1 = !digitalRead(button1);
 buttonState2 = !digitalRead(button2);
 buttonState3 = !digitalRead(button3);
 buttonState4 = !digitalRead(button4);
 buttonState5 = !digitalRead(button5);

made the program do something a bit more like maybe it is supposed to. Basically logic tipped upside down, pressed is LOW for pulled up buttons.

Now: Buttons 1-4 do nothing (provide no feedback to having done anything)

Press them all day long.

And Button 5 changes the screen to "You chose drinks:" and the program does nothing further (provides no evidence of doing anything any more).

Please describe exactly what we are supposed to be seeing.

a7

  1. You don't need resistors if you use INPUT_PULLUP. Just connect the switch between GND and your pin. This means a value of LOW indicates the button is PRESSED.
  2. Do not use pin 1 for a button
  3. Look at the following code. I have adjusted the logic so that LOW is pressed. Note that button 1-4 will NOT be checked unless button 5 is pressed AT THE SAME TIME. Is this what you want? I think you are confused between determining the button IS pressed vs. the button HAS BEEN pressed.
void loop()
{
  buttonState1 = digitalRead(button1);
  buttonState2 = digitalRead(button2);
  buttonState3 = digitalRead(button3);
  buttonState4 = digitalRead(button4);
  buttonState5 = digitalRead(button5);
  if (buttonState5 == LOW)
  {
    lcd.setCursor(0, 0);
    lcd.print("You chose drinks:");
    lcd.setCursor(0, 1);
    lcd.print("                              ");
    lcd.setCursor(0, 1);
    if (buttonState1 == LOW)
    {
      lcd.print("1  ");
    }
    if (buttonState2 == LOW)
    {
      lcd.print("2   ");
    }
    if (buttonState3 == LOW)
    {
      lcd.print("3   ");
    }
    if (buttonState4 == LOW)
    {
      lcd.print("4   ");
    }
  }
}

I attached button 1 to pin 10 now, and the reason I have resistors is that before writing all this code the only way the buttons would work is with the resistors in the circuit. I made the mistake of including my code with "INPUT_PULLUP" when initially I started with just "INPUT" and have changed it back. When it comes to drawing the hand drawing the circuit, I, unfortunately, don't know-how.

Hey,
I need all buttons to provide feedback. More specifically I need buttons 1-4 to be pressed before button 5, and after button 5 is pressed I need the Arduino to read whether buttons 1-4 were pressed before 5 or not and depending on which ones were pressed, the LCD prints the drinks chosen (which are chosen using button 1-4). After I figure this out in each if-statement (for buttons 1-4) I will include a command to run motors 1-4 (not included yet), so I really need the Arduino to capture whether 1-4 were pressed and store this information so that it can run the proper motors later on, for now, I just want it to print onto the screen to see if it works, which it clearly doesn't.

Hey,

My bad using "INPUT_PULLUP", I changed them to "INPUT" and kept the resistors because, without the resistors, none of the buttons work whether or not I use "INPUT" or "INPUT_PULLUP". I explained in the reply to @alto777 that I need button the Arduino to determine whether or not buttons 1-4 have been pressed before button 5 or not, and depending on whether they did or not, for the screen to print which buttons have been pressed.

For example, if buttons 2 and 4 were pressed and then button 5, the expected output should be:

You chose drinks:
2 4

However right now even if buttons 2 and 4 were pressed before 5, the output is:

You chose drinks:

This means that buttons 2 and 4 were not registered as HIGH.

Are you holding down all three buttons at the same time? If not then it will not work with the logic you have.

Hey,

I think that's exactly the problem, I need a pushbutton switch not a regular pushbutton, to my logic I just regarded the button as a switch however it clearly isn't. I think if I replace the pushbuttons with a pushbutton switch the code will run perfectly fine. I appreciate all the help.

Not necessarily. You just need to check when buttons 1-4 become pressed, remember which drinks were chosen, and then when button 5 becomes pressed then display the drinks chosen and clear the chosen drinks for the next time.

This tutorial should help:

StateChangeDetection

I still recommend getting rid of the resistors. You don't need them if you use INPUT_PULLUP.

Here is a version that works with momentary switches and no resistors. I printed to serial monitor rather than LCD but you can add that:

const int NUM_DRINKS = 4;
const int drinkButton[NUM_DRINKS] = {6,7,8,9};
const int drinksSelectedButton = 10;
bool drinkSelected[NUM_DRINKS];
byte lastDrinkButtonStatus[NUM_DRINKS];
  
void setup()
{
  Serial.begin(115200);
  pinMode(drinksSelectedButton, INPUT_PULLUP);
  for (int i = 0; i < NUM_DRINKS; i++)
  {
	drinkSelected[i] = false;
    pinMode(drinkButton[i], INPUT_PULLUP);
    lastDrinkButtonStatus[i] = digitalRead(drinkButton[i]);
  }
}

void loop() 
{
  static byte lastDrinksSelectedStatus = digitalRead(drinksSelectedButton);
  byte drinksSelectedStatus = digitalRead(drinksSelectedButton);

  // Check to see if any drinks have been selected
  for (int i = 0; i < NUM_DRINKS; i++)
  {
    byte drinkButtonStatus = digitalRead(drinkButton[i]);
    if (drinkButtonStatus != lastDrinkButtonStatus[i])
    {
      delay(50); // debounce
      if (drinkButtonStatus == LOW)
      {
        drinkSelected[i] = true;
      }
      lastDrinkButtonStatus[i] = drinkButtonStatus;
    }
  }
  
  if (drinksSelectedStatus != lastDrinksSelectedStatus)
  {
    delay(50); // debounce
    if (drinksSelectedStatus == LOW)
    {
      Serial.println("Drinks Selected:");
      displayDrinks();
    }
    lastDrinksSelectedStatus = drinksSelectedStatus;
  }
}

void displayDrinks()
{
  for (int i = 0; i < NUM_DRINKS; i++)
  {
    if (drinkSelected[i])
    {
      Serial.print(i+1);
      Serial.print(" ");
      drinkSelected[i] = false;
    }
  }
  Serial.println("");
}

Haha, yes you did, but you took the hard way. Since the OP had all the digitalRead in one place, why not just invert them at that point?, no need to look and be sure you got all the HIGHs and LOWs strewn about correctly changed.

The code in #17 works OK.

There is some debouncing of the selecting buttons, this is not necessary. The logic below functions identically because the code can only select, so selecting multiple times even if that's what would happen is not a problem.

    for (int i = 0; i < NUM_DRINKS; i++)
        if (!digitalRead(drinkButton[i]))
            drinkSelected[i]) = true;

But it is good that you did a debounced logic for the selection buttons, as we can exploit that and change the action to a toggle of the selected status for the button.
 for (int i = 0; i < NUM_DRINKS; i++) {
    byte drinkButtonStatus = digitalRead(drinkButton[i]);
    if (drinkButtonStatus != lastDrinkButtonStatus[i]) {
      delay(50); // debounce
      if (drinkButtonStatus == LOW) {
        if (!drinkSelected[i]) {
          drinkSelected[i] = true;

          Serial.print("mark selected ");
          Serial.println(i);
        }
        else {
          drinkSelected[i] = false;

          Serial.print("unmark ");
          Serial.println(i);
        }        
      }
      lastDrinkButtonStatus[i] = drinkButtonStatus;
    }
 }

I too did not put anything on the LCD. All this relies on the resetting of the drinkSelected array, which could be done when the drink is dispensed.

Sorry about the messy wiring:

a7

Correct, especially with the debounce. I wrote this code literally while talking on the phone so it was very quick and dirty. My main point was to illustrate the state detection method and saving of the drink selection.

Thank you all, the problem has been solved.