Problem With My Code

Using Arduino Programming Questions
#1

I am just getting started with Arduino's. So, please excuse me if I have missed something blatantly obvious. So, I am trying to make my own sort of combination lock using four tactile switches. Write now I am just laying it on a breadboard. I am using an Uno clone and have also tried using a Mega, but I have the same problem both times. I created an integer and set it equal to 1. When a button is pressed, the integer is changed in different ways depending on which button is pressed, and the integer is displayed on the serial monitor. I plan to do a lot more in the future, but I don't want to continue with servos and lock mechanisms when this isn't working now. When I start the program, the program loops through changing the integer and displaying information on the serial monitor when I haven't pressed any buttons. The buttons are not faulty, and I have tested the connections. I have detached everything from the Uno and Mega. There are no wires connected to the boards, but the program keeps on looping as if there are no if statements at all. The code is below.

int num = 1;
int button2 = 0;
int button3 = 0;
int button4 = 0;
int button5 = 0;


void setup() {
  Serial.begin(9600);
  pinMode(2, INPUT);
  pinMode(3, INPUT);
  pinMode(4, INPUT);
  pinMode(5, INPUT);
}

void loop() {
  button2 = digitalRead(2);
  button3 = digitalRead(3);
  button4 = digitalRead(4);
  button5 = digitalRead(5);
  if (button2 == HIGH) {
    num = num * 7;
    delay(250);
    Serial.println(num);
  }
  if (button3 == HIGH) {
    num = num * 9;
    delay(250);
    Serial.println(num);
  }
  if (button4 == HIGH) {
    num = num / 2;
    delay(250);
    Serial.println(num);
  }
  if (button5 == HIGH) {
    num = num - 1000;
    delay(250);
    Serial.println(num);
  }
}
#2

How are the switches wired? You are not using the internal pullup resistors, so you must have external pullup or pulldown resistors.

You need to look at the state change detection example, too. You do NOT want to do something when the switch IS pressed. You want to do something when the switch BECOMES pressed.

#3

In this shorter code I moved 2 lines to the end of loop() - then you only need them once because they are used every time. There is a good programming maxim Don’t Repeat Yourself (DRY). If you only have one version it is easier to find mistakes and harder to make them.

Try extending the delay to 1000 and see what happens - as well as @PaulS’s advice. Using INPUT_PULLUP always seems easiest to me.

void loop() {
  button2 = digitalRead(2);
  button3 = digitalRead(3);
  button4 = digitalRead(4);
  button5 = digitalRead(5);
  if (button2 == HIGH) {
    num = num * 7;
  }
  if (button3 == HIGH) {
    num = num * 9;
  }
  if (button4 == HIGH) {
    num = num / 2;
  }
  if (button5 == HIGH) {
    num = num - 1000;
  }
  delay(250);
  Serial.println(num);
}

…R

#4

Thank you both! Your help was appreciated. I used both of your advice, and shortened the code and used INPUT_PULLUP. It works flawlessly now. Thanks! :slight_smile:

#5