My code isnt working?

Using Arduino Programming Questions
1

For some reason my code isn't working? I am fairly new to Arduino and am using it for a school assignment. But I need to make 6 leds turn on 1 at a time when the button is pressed, then stop when the button is pressed again. This is my code:
int buttonPin = 7;
int ledPin1 = 13;
int ledPin2 = 12;
int ledPin3 = 11;
int ledPin4 = 10;
int ledPin5 = 9;
int ledPin6 = 8;
int buttonState = 0;
int ledState = 0;
int ledState1 = 0;
int ledState2 = 0;
int ledState3 = 0;
int ledState4 = 0;
int ledState5 = 0;

void setup(){
pinMode(ledPin1, OUTPUT);
pinMode(ledPin2, OUTPUT);
pinMode(ledPin3, OUTPUT);
pinMode(ledPin4, OUTPUT);
pinMode(ledPin5, OUTPUT);
pinMode(ledPin6, OUTPUT);
pinMode(buttonPin, INPUT);
}
void loop(){
digitalWrite (ledPin1, ledState);
digitalWrite (ledPin2, ledState1);
digitalWrite (ledPin3, ledState2);
digitalWrite (ledPin4, ledState3);
digitalWrite (ledPin5, ledState4);
digitalWrite (ledPin6, ledState5);
buttonState = digitalRead(buttonPin);

if (ledState==1)
{
if (buttonState==1)
{ledState=0;
{ledState1=0;
{ledState2=0;
{ledState3=0;
{ledState4=0;
{ledState5=0;
delay (400);
}
}
}
}
}
}else{
if (buttonState ==1)
(ledState =1);
(ledState1 =0);
(ledState2 =0);
(ledState3 =0);
(ledState4 =0);
(ledState5 =0);
delay(400);
(ledState =0);
(ledState1 =1);
(ledState2 =0);
(ledState3 =0);
(ledState4 =0);
(ledState5 =0);
delay (400);
(ledState =0);
(ledState1 =0);
(ledState2 =1);
(ledState3 =0);
(ledState4 =0);
(ledState5 =0);
delay (400);
(ledState =0);
(ledState1 =0);
(ledState2 =0);
(ledState3 =1);
(ledState4 =0);
(ledState5 =0);
delay (400);
(ledState =0);
(ledState1 =0);
(ledState2 =0);
(ledState3 =0);
(ledState4 =1);
(ledState5 =0);
delay (400);
(ledState =0);
(ledState1 =0);
(ledState2 =0);
(ledState3 =0);
(ledState4 =0);
(ledState5 =1);
delay (400);
}
}
}

2

Please follow the advice given in the link below when posting code . Use code tags when posting code here to make it easier to read and copy for examination

3 
  buttonState = digitalRead(buttonPin);
  if (ledState == 1)
  {
    if (buttonState == 1)
    {
      ledState = 0;
      {
        ledState1 = 0;
        {
          ledState2 = 0;
          {
            ledState3 = 0;
            {
              ledState4 = 0;
              {
                ledState5 = 0;
                delay (400);
              }
            }
          }
        }
      }
    }

Why all the curly braces ?

    else
    {
          if (buttonState == 1)
        (ledState = 1);
      (ledState1 = 0);
      (ledState2 = 0);
      (ledState3 = 0);
      (ledState4 = 0);
      (ledState5 = 0);
      delay(400);
      (ledState = 0);
      (ledState1 = 1);
      (ledState2 = 0);
      (ledState3 = 0);
      (ledState4 = 0);
      (ledState5 = 0);
      delay (400);
      (ledState = 0);
      (ledState1 = 0);
      (ledState2 = 1);
      (ledState3 = 0);
      (ledState4 = 0);
      (ledState5 = 0);

Which lines of code are executed when buttonState equals 1 and which are executed unconditionally ?

What do you expect to happen by setting some variables to values and then delaying for a while ? Setting the ledState variables to a value will not affect the LEDs unless you use digitalWrite() to write those states to the LEDs

4

The button input should be declared with pin mode INPUT_PULLUP.

You need to use the technique of the state-change-detection example to detect when the button becomes pressed, so that your state-change code only executes once for each button press, not thousands of times a second during the button press.

You need to advance the state variable when the button is pressed to the next state.

When changing state you only have to turn off the previous LED and turn on the new LED - you don't have to write to every single LED every time(!)

Once its working I'd suggest looking at combining most of the state transitions into the same bit of code, as there is no need to code such similar actions separately for each LED - you parameterize the code with the LED number to do this. Arrays will be useful here (hint).

5

here is the code more properly formatted

int buttonPin = 7;
int ledPin1 = 13;
int ledPin2 = 12;
int ledPin3 = 11;
int ledPin4 = 10;
int ledPin5 = 9;
int ledPin6 = 8;
int buttonState = 0;
int ledState = 0;
int ledState1 = 0;
int ledState2 = 0;
int ledState3 = 0;
int ledState4 = 0;
int ledState5 = 0;

void setup(){
    pinMode(ledPin1, OUTPUT);
    pinMode(ledPin2, OUTPUT);
    pinMode(ledPin3, OUTPUT);
    pinMode(ledPin4, OUTPUT);
    pinMode(ledPin5, OUTPUT);
    pinMode(ledPin6, OUTPUT);
    pinMode(buttonPin, INPUT);
}

void loop(){
    digitalWrite (ledPin1, ledState);
    digitalWrite (ledPin2, ledState1);
    digitalWrite (ledPin3, ledState2);
    digitalWrite (ledPin4, ledState3);
    digitalWrite (ledPin5, ledState4);
    digitalWrite (ledPin6, ledState5);

    buttonState = digitalRead(buttonPin);

    if (ledState==1)
    {
        if (buttonState==1) {
            ledState=0;
            ledState1=0;
            ledState2=0;
            ledState3=0;
            ledState4=0;
            ledState5=0;
            delay (400);
        }
        else {
            if (buttonState ==1)
                (ledState =1);

            (ledState1 =0);
            (ledState2 =0);
            (ledState3 =0);
            (ledState4 =0);
            (ledState5 =0);
            delay(400);
            (ledState =0);
            (ledState1 =1);
            (ledState2 =0);
            (ledState3 =0);
            (ledState4 =0);
            (ledState5 =0);
            delay (400);
            (ledState =0);
            (ledState1 =0);
            (ledState2 =1);
            (ledState3 =0);
            (ledState4 =0);
            (ledState5 =0);
            delay (400);
            (ledState =0);
            (ledState1 =0);
            (ledState2 =0);
            (ledState3 =1);
            (ledState4 =0);
            (ledState5 =0);
            delay (400);
            (ledState =0);
            (ledState1 =0);
            (ledState2 =0);
            (ledState3 =0);
            (ledState4 =1);
            (ledState5 =0);
            delay (400);
            (ledState =0);
            (ledState1 =0);
            (ledState2 =0);
            (ledState3 =0);
            (ledState4 =0);
            (ledState5 =1);
            delay (400);
        }
    }
}

the bulk of the code is conditional (will execute when the condition is true) when "ledState" equals 1. but "ledState" is initialized to zero and i don't see where it's value is changed outside of the condition.

inside that condition, i see ledStateNs being set but don't led pins being set

it's conventional to connect a button switch between the pin and ground, and configure the pin as INPUT_PULLUP so that when the button is pressed, it pulls the pin LOW.

the button pin is configured as INPUT

inside the "ledState" condition there is a conditional test for buttonState. the else condition (buttonState != 0) also tests for buttonState == 1, which it can't be if that condition is executed.

it appears the intended operation is to monitor the button, and as soon as it goes LOW when being pressed, to execute the ledState == 1 condition, sequencially turning on/off each LED and otherwise (the else) turning all the LEDs off.

i see no need for ledStates

think it over

6

i disagree

7

Hello
You may inserted some serial.println()´s at POI to see what happens.

8

I used so many curly brackets because the program told me I was missing some? I just put more in until it was happy.

9

Hi, @oliviabingus
Welcome to the forum.
Can you please post a copy of your circuit diagram showing your LEDs and button connections?

It llok like you have your button wired from the Arduino input pin to 5V.
You will need a pull down resistor of 10K between the input pin and gnd, this is so the input pin is grounded when the switch is open.
OR
Wire your button from digital input to gnd and in your setup() add,

pinMode(buttonPin, INPUT_PULLUP);

This will turn the internal pullup resistor ON.
You will have to edit your code for the opposite digital input pin operation.

It might be worth you looking at switch.. case function;

https://www.arduinoplatform.com/using-switch-statements-in-arduino/

Tom... :smiley: :+1: :coffee: :australia:

10

There are literally an infinite number of non-working programs that follow the rules of C/C++ syntax, and compile with no errors.

The compiler can catch obvious and some less obvious errors, but it doesn't have a brain. When it does catch an error, you need to analyze the code and decide on a cause and look for a solution.

11

a colleague told me on one project, a team member said he completed his code when it compiled. he never understood that it needed to be tested

12

"the eagle has landed" 1969 from Mon :grin:

13

Make that "an infinite number of possible non-working programs". In actual existence there's only ever a finite number of programs of any kind :slight_smile:

14

Try to delete this line:

if (ledState==1){

and delete last "}"

I hope this will help :slight_smile:

15

The universe is infinite. Therefore it contains an infinite number of civilizations capable of building computers. If each civilization had only one programmer that wrote one meaningless program (perhaps falling asleep on the keyboard?), there would still be an infinite number of programs.

16

The observable universe (aka reality) is not infinite though...

1 Like
17

This topic was automatically closed 120 days after the last reply. New replies are no longer allowed.