[Solved] (noob) void loop () acts different each time - led patterns

Hi Everyone

I'm brand new to programming so there may well be some obvious errors in my code, but its baffling me.

I have tried to play around with an example which turns one LED on then, turns on the next one and turns off the former (one LED will look like it is scrolling along the line).
my goal is to have the LEDs scroll in (as before) and 'fill up' (stay on), then scroll off again to 'empty' (turn off).

the code seems to run fine for a few loops but on the 'fill' function the 5th loop LEDs 1,2,4 and 5 turn on before the 'empty' function starts. Then the whole code seems to stop after the 'empty' on the 7th loop when LEDs 1,2 and 3 stay on.

I added the delays into the void loop() to check if all the LEDs were getting reset properly, and strangly the code breaks even sooner (the start of the 4th loop) if the delays are taken out.

any help would be appreciated, and feel free to let me know if there is a better way to write the code.

Thanks

int ledpins[] = {2,3,4,5,6,7,8,9}; //array to hold led pin #s
                                   //i.e. ledpin #0 is pin 2 on board and so on
                                   
void setup ()
{
  for(int i = 0; i < 8; i++){
    pinMode(ledpins[i],OUTPUT);
  }
}

void loop()
{
  fill(); //leds 'move' left to right (low to high pin #s)
          // and stay on right to left (one led per loop)
         
  delay (1000);    //added to troubleshoot    
  empty(); //leds 'move' left to right, starting one lower each time)    
  delay (1000);    //added to troubleshoot
}

void fill(){
  int delayTime = 100;
  
  for (int x = 7; x >= 0; x--){ //decrease x per loop 
                                //to lower # of leds activated per loop 
                                //so once filled the leds stay on
    for (int i = 0; i <= x; i++){
      int offled = i - 1;
     
    digitalWrite (ledpins[i], HIGH);
    digitalWrite (ledpins[offled], LOW);
    delay(delayTime);    
    }}
}   

void empty(){
  int delayTime = 100;
  
  for (int x = 7; x >= 0; x--){ //decrease x per loop 
                                //works its way back down the row
    for (int i = x; i <= 7; i++){
     int onled = i + 1;
     
    digitalWrite (ledpins[i], LOW);
    digitalWrite (ledpins[onled], HIGH);
    delay(delayTime);    
    }}
}
   for (int i = 0; i <= x; i++){
     int offled = i - 1;

    digitalWrite (ledpins[i], HIGH);
    digitalWrite (ledpins[offled], LOW);

You are turning off the pin indexed by -1, that won't be a big success.

Thanks for the heads up Nick

the example I copied had

int offLED = i - 1; 
    if(i == 0) {     
      offLED = 7;   
    }

since i dont want to turn off LED 7 when 1 turns on i deleted the 'if' part and assumed the code would ignore the reference to pin -1.

any advice on how to fix this? i just tried adding if (i == 0){} in the hopes this would mean "if i = 0, do nothing" but the code ran exactly the same.

thanks

int ledpins[] = {2,3,4,5,6,7,8,9}; //array to hold led pin #s
                                   //i.e. ledpin #0 is pin 2 on board and so on
                                   
void setup ()
{
  for(int i = 0; i < 8; i++){
    pinMode(ledpins[i],OUTPUT);
  }
}

void loop()
{
  fill(); //leds 'move' left to right (low to high pin #s)
          // and stay on right to left (one led per loop)
         
  delay (1000);    //added to troubleshoot    
  empty(); //leds 'move' left to right, starting one lower each time)    
  delay (1000);    //added to troubleshoot
}

void fill(){
  int delayTime = 100;
  
  for (int x = 7; x >= 0; x--){ //decrease x per loop 
                                //to lower # of leds activated per loop 
                                //so once filled the leds stay on
    for (int i = 0; i <= x; i++){
      int offled = (i==0) ? 0: (i - 1);
     
    digitalWrite (ledpins[i], HIGH);
    digitalWrite (ledpins[offled], LOW);
    delay(delayTime);    
    }}
}   

void empty(){
  int delayTime = 100;
  
  for (int x = 7; x >= 0; x--){ //decrease x per loop 
                                //works its way back down the row
    for (int i = x; i <= 7; i++){
     int onled = (i==7) ? 7 : (i + 1);
     
    digitalWrite (ledpins[i], LOW);
    digitalWrite (ledpins[onled], HIGH);
    delay(delayTime);    
    }}
}

This should fix your "pointer" on and off problem. But I think that the last and first LED won't be visible. So that's not the solution.

But why not:

void fill(){
  int delayTime = 100;
  
  for (int x = 7; x >= 0; x--){ //decrease x per loop 
                                //to lower # of leds activated per loop 
                                //so once filled the leds stay on
    for (int i = 0; i <= x; i++){
      int offled = i - 1;
     
    digitalWrite (ledpins[i], HIGH);
    if (offled >0) 
        digitalWrite (ledpins[offled], LOW);
    delay(delayTime);    
    }}
}   

void empty(){
  int delayTime = 100;
  
  for (int x = 7; x >= 0; x--){ //decrease x per loop 
                                //works its way back down the row
    for (int i = x; i <= 7; i++){
     int onled = i + 1;
     
    digitalWrite (ledpins[i], LOW);
    if (onled <7) 
        digitalWrite (ledpins[onled], HIGH);
    delay(delayTime);    
    }}
}

??

Spond:
i just tried adding if (i == 0){} in the hopes this would mean "if i = 0, do nothing" but the code ran exactly the same.

Yes, that "did nothing" for a split-second, and then it went ahead and did what it was going to do anyway.

Quick question ..., is this the transition you're expecting to see?

........
o.......
.o......
..o.....
...o....
....o...
.....o..
......o.
.......o
o......o
.o.....o
..o....o
...o...o
....o..o
.....o.o
......oo
......oo
o.....oo
.o....oo
..o...oo
...o..oo
....o.oo
.....ooo
o....ooo
.o...ooo
..o..ooo
...o.ooo
....oooo
o...oooo
.o..oooo
..o.oooo
...ooooo
o..ooooo
.o.ooooo
..oooooo
o.oooooo
.ooooooo
oooooooo
ooooooo.
oooooo.o
oooooo..
ooooo.o.
ooooo..o
ooooo...
oooo.o..
oooo..o.
oooo...o
oooo....
ooo.o...
ooo..o..
ooo...o.
ooo....o
ooo.....
oo.o....
oo..o...
oo...o..
oo....o.
oo.....o
oo......
o.o.....
o..o....
o...o...
o....o..
o.....o.
o......o
o.......
.o......
..o.....
...o....
....o...
.....o..
......o.
.......o
........

@ lloyddean

If you wrote that by hand I envy your patience.

A quick simple C++ console program!

Thanks bubulindo,

I added your suggestions and the code ran great, the only issue was that the 1st led stayed on during the whole 'fill' function, and the 8th one stays off during the 'empty' function.

I changed the if statements to look at 'i' instead of 'offled' and 'onled' which fixed it.

for reference:
the full code below displays the led pattern that lloyddean wrote above.

int ledpins[] = {2,3,4,5,6,7,8,9}; //array to hold led pin #s
                                   //i.e. ledpin #0 is pin 2 on board and so on
                                   
void setup ()
{
  for(int i = 0; i < 8; i++){
    pinMode(ledpins[i],OUTPUT);
  }
}

void loop()
{
  fill(); //leds 'move' left to right (low to high pin #s)
          // and stay on right to left (one led per loop)
         
  delay (1000);    //added to troubleshoot    
  empty(); //leds 'move' left to right, starting one lower each time)    
  delay (1000);    //added to troubleshoot
}

void fill(){
  int delayTime = 100;
  
  for (int x = 7; x >= 0; x--){ //decrease x per loop 
                                //to lower # of leds activated per loop 
                                //so once filled the leds stay on
    for (int i = 0; i <= x; i++){
      int offled = i - 1;
     
    digitalWrite (ledpins[i], HIGH);
    if (i >0) 
        digitalWrite (ledpins[offled], LOW);
    delay(delayTime);    
    }}
}   

void empty(){
  int delayTime = 100;
  
  for (int x = 7; x >= 0; x--){ //decrease x per loop 
                                //works its way back down the row
    for (int i = x; i <= 7; i++){
     int onled = i + 1;
     
    digitalWrite (ledpins[i], LOW);
    if (i <7) 
        digitalWrite (ledpins[onled], HIGH);
    delay(delayTime);    
    }}
}

Spond:
Thanks bubulindo,

I added your suggestions and the code ran great, the only issue was that the 1st led stayed on during the whole 'fill' function, and the 8th one stays off during the 'empty' function.

I changed the if statements to look at 'i' instead of 'offled' and 'onled' which fixed it.

for reference:
the full code below displays the led pattern that lloyddean wrote above.

int ledpins[] = {2,3,4,5,6,7,8,9}; //array to hold led pin #s

//i.e. ledpin #0 is pin 2 on board and so on
                                   
void setup ()
{
  for(int i = 0; i < 8; i++){
    pinMode(ledpins[i],OUTPUT);
  }
}

void loop()
{
  fill(); //leds 'move' left to right (low to high pin #s)
          // and stay on right to left (one led per loop)
         
  delay (1000);    //added to troubleshoot   
  empty(); //leds 'move' left to right, starting one lower each time)   
  delay (1000);    //added to troubleshoot
}

void fill(){
  int delayTime = 100;
 
  for (int x = 7; x >= 0; x--){ //decrease x per loop
                                //to lower # of leds activated per loop
                                //so once filled the leds stay on
    for (int i = 0; i <= x; i++){
      int offled = i - 1;
     
    digitalWrite (ledpins[i], HIGH);
    if (i >0)
        digitalWrite (ledpins[offled], LOW);
    delay(delayTime);   
    }}
}

void empty(){
  int delayTime = 100;
 
  for (int x = 7; x >= 0; x--){ //decrease x per loop
                                //works its way back down the row
    for (int i = x; i <= 7; i++){
     int onled = i + 1;
     
    digitalWrite (ledpins[i], LOW);
    if (i <7)
        digitalWrite (ledpins[onled], HIGH);
    delay(delayTime);   
    }}
}

I love it when the day is productive. LOL