Lights not working as planned (SOLVED)

Projects Programming
1

The aim of this project was to make a random number picker, with 10 lights, and each light representing a number from 1 to 10. It would change the light being lit along the line- lighting each light one by one- and when it lands on the randomly generated number- it would stop. however- the lighting no longer initiates, while I was able to see that beforehand, nothing is happening now. I've also attached a picture of the project to show what I'm trying to do.
Any help would be greatly appreciated.

//includes entropy
#include <Entropy.h>

//chooses random number
uint8_t random_byte;

// A variable to set a delay time between each LED
int delayTime = 40;

// A variable to store which LED we are currently working on
int currentLED = 4;

// A variable to store the direction of travel of LEDs
int dir = 1;

// A variable to store the last time we changed something
unsigned long timeChanged = 0;

// Create an array to hold the value for each LED pin
byte ledPin[] = {4, 5, 6, 7, 8, 9, 10, 11, 12, 13};

void setup() {

choosing();
// Set all pins for OUTPUT
for (int x=0; x<10; x++) {
pinMode(ledPin[x], OUTPUT);
}

// Set up the
timeChanged = millis();
}

void loop() {

// Check whether it has been long enough
if ((millis() - timeChanged) > delayTime) {

// Turn off all of the LEDs
for (int x=0; x<10; x++) {
digitalWrite(ledPin[x], LOW);
}

// Turn on the current LED
digitalWrite(ledPin[currentLED], HIGH);

// Increment by the direction value
currentLED += dir;

// If we are at the end of a row, change direction
if (currentLED == 9) {
dir = -1;
}
if (currentLED == 0) {
dir = 1;
}

// Store the current time as the time we last changed LEDs
timeChanged = millis();

}

}

void choosing() {
delay(2000);
random_byte = Entropy.random(1,11); // returns a value from 1 to 10
}

IMG_0042.jpg

2

nothing is happening now.

What did you change ?

3

I took out all of the bounces stuff, to simplify things, and moved the Entropy stuff to it’s own void.

4

how about you flash the led's within setup() just to make sure they are actually working:

  // Set all pins for OUTPUT
  for (int x=0; x<10; x++) {
    pinMode(ledPin[x], OUTPUT);
    digitalWrite(ledPin[x],HIGH); 
  }
  choosing();
  for (int x=0; x<10; x++) {
    digitalWrite(ledPin[x],LOW); 
  }

Also i would turn off just one led not all of them, but that shouldn't matter for the moment.

5

Ok I've made a couple of changes as you said, and ended up with the following, but it's still not compiling, and giving the same error message. :expressionless:

// Random number picker
// Sequentially lights up leds


// A variable to set a time between LEDs in milliseconds
int delayTime = 40;

// A variable to choose the LED
int currentLED = 4;

// Choosing the direction of travel
int dir = 1;

// A variable to say how long it's been since we changed something
long timeChanged = 0;

// An array to hold the value for each LED pin
byte ledPin[] = {4, 5, 6, 7, 8, 9, 10, 11, 12};

void setup() {

#include <Entropy.h>
int rannumber = Entropy.random(9);

  
  //set all pins for OUTPUT
  for (int i=0; i<10; i++) {
    pinMode(ledPin[1], OUTPUT);
    digitalWrite(ledPin[currentLED], HIGH);
  }
  choosing();
     for (int i=0; i<10; i++) {
      digitalWrite(ledPin[i], LOW);
  // Record the time once the setup has completed
  timeChanged = millis();

}
void loop() {
  
  // Check wether it has been long enough
  if ((millis() - timeChanged) > delayTime) {
   currentLED = currentLED + dir;
    }
    

 
    
    // Store the current time as we last changed LEDs
    timeChanged = millis();
  

delayTime = analogRead(0);
delay(5);
}
6 
    pinMode(ledPin[1], OUTPUT);

This will not cause a compiler error but neither will it set all of the LED pins to OUTPUT. This is one of the perils of using i as a for loop variable

    digitalWrite(ledPin[currentLED], HIGH);

What is the value of currentLED at this point in the for loop ?

  choosing();

Do you have a choosing() function in your program ?

Where does the setup() function end ?

I stopped looking at that point because you did not put your code in code tags and a lot of it was turned into italics as a result

7

Put your code in code tags.

This will happen every trip through loop(), not just when you changed LEDs.

   // Store the current time as we last changed LEDs
    timeChanged = millis();

No need to put the digitalWrite in this for loop in setup(). currentLED == 4 every iteration.

  //set all pins for OUTPUT
  for (int i=0; i<10; i++) {
    pinMode(ledPin[1], OUTPUT);  //note the 1 instead of i
    digitalWrite(ledPin[currentLED], HIGH);
  }

Put the digitalWrite after the for loop.

8

I am with Bob on the matter, if you don't use [/] code-tags we can't copy your code into the IDE, and there is not much i can do to help.

9

There- the code has been edited into a tag. Also I'm pretty sure that the initial value of currentLED is 4

10

For the sake of all concerned here is the code from post #4 Auto Formatted in the IDE

// Random number picker
// Sequentially lights up leds


// A variable to set a time between LEDs in milliseconds
int delayTime = 40;

// A variable to choose the LED
int currentLED = 4;

// Choosing the direction of travel
int dir = 1;

// A variable to say how long it's been since we changed something
long timeChanged = 0;

// An array to hold the value for each LED pin
byte ledPin[] = {4, 5, 6, 7, 8, 9, 10, 11, 12};

void setup()
{
#include <Entropy.h>
  int rannumber = Entropy.random(9);
  //set all pins for OUTPUT
  for (int i = 0; i < 10; i++)
  {
    pinMode(ledPin[1], OUTPUT);
    digitalWrite(ledPin[currentLED], HIGH);
  }
  choosing();
  for (int i = 0; i < 10; i++)
  {
    digitalWrite(ledPin[i], LOW);
    // Record the time once the setup has completed
    timeChanged = millis();
  }
  void loop()
  {
    // Check wether it has been long enough
    if ((millis() - timeChanged) > delayTime)
    {
      currentLED = currentLED + dir;
    }
    // Store the current time as we last changed LEDs
    timeChanged = millis();
    delayTime = analogRead(0);
    delay(5);
  }

My questions from post #5 are all still relevant plus now that the code is formatted it is obvious that the setup() function is missing its closing }

In addition currentLed is updated in loop() but its value is never used

11

I've added the end bracket to setup(), but don't understand the way in which I should use the value of currentLED, I meant for it's appearance in the loop to be the way in which it's used.

12

don't understand the way in which I should use the value of currentLED, I meant for it's appearance in the loop to be the way in which it's used.

You set currentLed to 4 when it is declared and set ledPin[currentLed] to HIGH in the for loop in setup() without changing the value of currentLed so all the for loop does is to set the pin in the fifth entry in the ledPins array HIGH several times.

The in loop() you change the value of currentLed but never do anything with the variable

13

You did have this line in the original code you posted

// Turn on the current LED
    digitalWrite(ledPin[currentLED], HIGH);

actually that code seemed more or less error free (it alos had a section changing direction)

 // Turn on the current LED
    digitalWrite(ledPin[currentLED], HIGH);

    // Increment by the direction value
    currentLED += dir;

    // If we are at the end of a row, change direction
    if (currentLED == 9) {
      dir = -1;
    }
    if (currentLED == 0) {
      dir = 1;
    }

and as far as i can tell it did have the closing brace for loop() :wink:

14

In that case, here's what I've got now, moving a bunch of stuff into the "if enough time has changed" loop.
(the same message is there)

// Random number picker
// Sequentially lights up leds


// A variable to set a time between LEDs in milliseconds
int delayTime = 40;

// A variable to choose the LED
int currentLED = 4;

// Choosing the direction of travel
int dir = 1;



// A variable to say how long it's been since we changed something
long timeChanged = 0;

// An array to hold the value for each LED pin
byte ledPin[] = {4, 5, 6, 7, 8, 9, 10, 11, 12};

void setup() {

#include <Entropy.h>
int rannumber = Entropy.random(11);

timeChanged = millis();
  
}

void loop() {
  
  // Check wether it has been long enough
  if ((millis() - timeChanged) > delayTime) {

//set all pins for OUTPUT
  for (int i=0; i<10; i++) {
    pinMode(ledPin[i], OUTPUT);
  }

     for (int i=0; i<10; i++) {
      digitalWrite(ledPin[i], LOW);
// Record the time once the setup has completed
  }

//turn it on
digitalWrite(ledPin[currentLED], HIGH);
currentLED += dir;

// If we are at the end of a row, change direction
    if (currentLED == 11) {
      dir = -1;
      }
    
    if (currentLED == 0) {
      dir = 1;
    }

// Store the current time as we last changed LEDs
timeChanged = millis();
  
delayTime = analogRead(0);
delay(5);
  
}
15

Where is the end of the loop() function ?
Auto Format the code and note that the final { is not on the left margin as it should be. Why not ?
Check that every { has a corresponding } in the right place

    //set all pins for OUTPUT
    for (int i = 0; i < 10; i++)
    {
      pinMode(ledPin[i], OUTPUT);
    }
    for (int i = 0; i < 10; i++)
    {
      digitalWrite(ledPin[i], LOW);
      // Record the time once the setup has completed
    }

Why is this code in loop() ? How many times do you need to set the pinMode of the LEDs and should it depend on the timing period elapsing as it does in your code ?

16 
//set all pins for OUTPUT
  for (int i=0; i<10; i++) {
    pinMode(ledPin[i], OUTPUT);
  }

i think this belongs in setup() (not in loop() ) and it should be

for (int i=0; i<9; i++) {
    pinMode(ledPin[i], OUTPUT);
  }

and i think this

if (currentLED == 11) {
      dir = -1;
      }

should be

if (currentLED == 8) {
      dir = -1;
      }

It is quite important that you do not read beyond the size of the array (as this will cause unexpected results, not as bad as writing beyond i though) your array for ledPin is 9 bytes (0 -8)
and until you have it all going properly i would not alter the delayTime (also if you want to read analog pin 0 you should specify it as A0

// delayTime = analogRead(A0);
17

I set all of the LEDs to turn off before the next one turns on, so that the last one isn't left turned on. If I set all the LEDs to OUTPUT in the start will it still work?
Also- I know I should have mentioned this, but the two other LEDs now work, so the total is 10 instead of 8 now.

18

I set all of the LEDs to turn off before the next one turns on,

That explains the digitalWites()s but what about the pinMode()s ?

If I set all the LEDs to OUTPUT in the start will it still work?

Yes. Do it once in setup()

19

I've fixed the pinMode() thing, but still can't find the missing } after looking for ages.

// Random number picker
// Sequentially lights up leds


// A variable to set a time between LEDs in milliseconds
int delayTime = 40;

// A variable to choose the LED
int currentLED = 4;

// Choosing the direction of travel
int dir = 1;



// A variable to say how long it's been since we changed something
long timeChanged = 0;

// An array to hold the value for each LED pin
byte ledPin[] = {4, 5, 6, 7, 8, 9, 10, 11, 12};

void setup() {

  for (int i=0; i<11; i++) {
    pinMode(ledPin[i], OUTPUT);
  }
  
#include <Entropy.h>
int rannumber = Entropy.random(11);

timeChanged = millis();
  
}

void loop() {
  
  // Check wether it has been long enough
  if ((millis() - timeChanged) > delayTime) {

//set all pins for OUTPUT


     for (int i=0; i<11; i++) {
      digitalWrite(ledPin[i], LOW);
   }
  }

//turn it on
digitalWrite(ledPin[currentLED], HIGH);
currentLED += dir;

// If we are at the end of a row, change direction
    if (currentLED == 11) {
      dir = -1;
      }
    
    if (currentLED == 0) {
      dir = 1;
    }

// Store the current time as we last changed LEDs
timeChanged = millis();
  
delayTime = analogRead(0);
delay(5);
  
 }
20

I've fixed the pinMode() thing, but still can't find the missing } after looking for ages.

There isn't one, except that your #include statement#include <Entropy.h>is in the wrong spot, it shouldn't be within setup() that is what is causing the issue, move it to the top of the sketch.
And fix the other things i suggested.