Counting Lights

Hi everyone, If i could get some help with this it would be great!

Problem:

All of the LEDs should start turned off. Pick one pushbutton, it will be the increment button. Every time the increment button is pressed, the first LED in the row should be turned on. If it is already turned on, turn it off instead and turn on the second LED. If the second LED is also already on, turn it off too, and turn on the third LED. If it too is already on, keep repeating this procedure until an LED can be turned on, or you run out of LEDs (thus all of the LEDs should be off). When the increment button is pressed again the whole procedure is followed from scratch.

Here is my code so far:

#include <Arduino.h>

int LEDpins[5] = {9, 10, 11, 12, 13};

int incrementButtonPin = 7;
int incrementButtonValue;

void setup()
{
   for (int i=0; i<5; i++)
  {
    pinMode(LEDpins[i], OUTPUT);
   digitalWrite(LEDpins[i], LOW);
  }
  pinMode(incrementButtonPin, INPUT);
  digitalWrite(incrementButtonPin, HIGH);
}

void loop()
{
incrementButtonValue = digitalRead(incrementButtonPin);
  for (int i=0; i<5; i++)
 {
     if (incrementButtonValue == LOW)
    {
         if (digitalRead(LEDpins[i]) == LOW)
        {
               digitalWrite(LEDpins[i], HIGH);
               continue;
        }
        else if (digitalRead(LEDpins[i]) == HIGH)
       {
               digitalWrite(LEDpins[i], LOW);
       }
   }
 }
}

I think I did almost everything right, but I can't seem to get it work. Only the first LED lights up.

You didn't read the instructions on how to post, did you?

See what happens when you don't read the instructions?

Go back and modify your post, highlighting the code and using the code widget "</>" above the submission window to mark it as code so it is correctly displayed.

Then we can inspect it more easily.

You might try:
Use the increment button to increase an int counter.
Then
digitalWrite(Light1, ( ( counter & 1 ) == 1 ));
digitalWrite(Light2, ( ( counter & 2 ) == 2 ));
digitalWrite(Light3, ( ( counter & 4 ) == 4 ));

At least I think that will work.
Dwight

@Dwight
I don't quite understand the code you wrote. Can you explain a bit more as to how that would fit into my code? Since I am already using a for loop.
Thank you!

dwightthinker:
You might try:
Use the increment button to increase an int counter.
Then
digitalWrite(Light1, ( ( counter & 1 ) == 1 ));
digitalWrite(Light2, ( ( counter & 2 ) == 2 ));
digitalWrite(Light3, ( ( counter & 4 ) == 4 ));

At least I think that will work.
Dwight

Don't use a for loop. Just use the
main void loop { ......};
The counter has a binary number being the number of times
the switch has been pressed.
Just increment is with the proper debouncing.
If you really wanted to use the for...loop, you might try

for ( ... ) {
digitalWrite(LEDpins*, ( (counter & (( 1 << i )==( 1 << i ) ) ) );*
}
I hope I have enough )s there. I really don't like C.
Dwight

Ok, there are more than one way to skin..............

Try these fixes:

#include <Arduino.h>

int LEDpins[5] = {9, 10, 11, 12, 13};

int incrementButtonPin = 7;
int incrementButtonValue;

void setup()
{
   for (int i=0; i<5; i++)
  {
    pinMode(LEDpins[i], OUTPUT);
   digitalWrite(LEDpins[i], LOW); // if resistor and LED to ground else
                                  // HIGH for resistor and LED to VCC
  }
//  pinMode(incrementButtonPin, INPUT);
//  digitalWrite(incrementButtonPin, HIGH); // isn't what you want
  pinMode(incrementButtonPin, INPUT_PULLUP); // this is what you want
}


void loop()
{
incrementButtonValue = digitalRead(incrementButtonPin);
 if (incrementButtonValue == LOW) {
    for (int i=0; i<5; i++)
 {
 //    if (incrementButtonValue == LOW) // you don't want to test in each for loop
 //   {
         if (digitalRead(LEDpins[i]) == LOW)
        {
               digitalWrite(LEDpins[i], HIGH);
//               continue;   // it will still do the other i values
                 break; // no carry to other lights so no more loops
        }
 //       else if (digitalRead(LEDpins[i]) == HIGH) // not needed, else is understood by break
 //      {
         digitalWrite(LEDpins[i], LOW);
 //      }
//    }
   }
 }
delay(5); // debounce switch
 while ( incrementButtonValue == LOW ){  // wait until switch is high again
   incrementButtonValue = digitalRead(incrementButtonPin);
 }
}

Dwight

Why use variable type int for things that are well under 255 and could be declared as byte instead?
That wasted memory (2 bytes vs 1 for a number that needs <= 8 bits) could come back to haunt you as your program grows.

I was trying to do a minimum of changes to his code.
I don't think I'd have ever done it his way but it should work.
Point about wasted space is a good one though.
Dwight

thank you @dwightthinker, the revised code works, and I see where I went wrong! :slight_smile:

Not too bad when you consider I've only been programming
in C now for 3 weeks and hating it. I'd bought an arduino copy
board and then I didn't need it for the intended application.
Now I'm repurposing it.
I have been programming for over 37 years though.
Dwight