3
\$\begingroup\$

I have 8 LEDs lined up on a breadboard, and I am trying to make them run from one end to the other and back, sorta like the lights on KITT (from night rider - hope that clarifies). the LEDs are all set up and correct and work.

I have set up my code like so:

int ledPins[]={2,3,4,5,6,7,8,9};

void setup()
{
  for (int i =0; i <8; i++)
  {
    pinMode(ledPins[i],OUTPUT);
  }
}

void loop()
{
  for(int i =0; i<8; i++)
  {
    digitalWrite(ledPins[i],HIGH);
    delay(100);
    digitalWrite(ledPins[i],LOW);
  }

  for(int i =7; i<1; i--)
  {
    digitalWrite(ledPins[i],HIGH);
    delay(100);
    digitalWrite(ledPins[i],LOW);
  }
}

but it doesn't cycle the LEDs from right to left and then back like I was aiming for.

EDIT: What it does do - is light the LEDs from right to left, but doesn't light them the opposite way (left to right). It just keeps going from right to left, right to left

I am using an Arduino UNO, and basing my circuit off the experimentation kit for Arudino CIRC-02

EDIT #2: So I have changed the for-loops to while loops:

int ledPins[]={2,3,4,5,6,7,8,9};

void setup()
{
  for (int i =0; i <8; i++)
  {
    pinMode(ledPins[i],OUTPUT);
  }
}

void loop()
{
  int i = 0;
  while (i<7)
  {
    {
      digitalWrite(ledPins[i],HIGH);
      delay(100);
      digitalWrite(ledPins[i],LOW);
      i++;
    }
  }

  i = 7;
  while (i >0)
  {
    digitalWrite(ledPins[i],HIGH);
    delay(100);
    digitalWrite(ledPins[i],LOW);
    i--;
  }
}

and it works as intended! However, I am still interested in where my for-loop went wrong, if anybody has any ideas

\$\endgroup\$
  • \$\begingroup\$ Saying what it DOES do may help. \$\endgroup\$ – Russell McMahon Aug 9 '11 at 13:36
3
\$\begingroup\$

In the second 'for' loop, i is immediately not <1, so the loop never executes.

for(int i =7; i<1; i--)

should say

for(int i =7; i>=0; i--)
\$\endgroup\$
  • \$\begingroup\$ looks like I need to revise my for-loops! fantastic, thanks! \$\endgroup\$ – snowflakekiller Aug 9 '11 at 15:37
2
\$\begingroup\$
int ledPins[]={2,3,4,5,6,7,8,9,10,11,12};

void setup()
{
  for (int i =0; i <11; i++)
  {
    pinMode(ledPins[i],OUTPUT);
  }
}

void loop()
{
  for(int i =0; i<11; i++)
  {
    digitalWrite(ledPins[i],HIGH);
    delay(30);
    digitalWrite(ledPins[i],LOW);
  }

  for(int i =10; i>1; i--)
  {
    digitalWrite(ledPins[i],HIGH);
    delay(80);
    digitalWrite(ledPins[i],LOW);
  }
}
\$\endgroup\$
  • 1
    \$\begingroup\$ I cleaned up your code a bit. In addition to that, when the OP asks for an explanation why he was wrong, it's at the very least a bit... odd, to answer with just code, and no explanation whatsoever. \$\endgroup\$ – Keelan Aug 31 '14 at 21:13
1
\$\begingroup\$

(a) I'm not 100% familiar with what your language demands but, are your "for" tests of the form (start, while, action) or (start, until, action) ?

If the latter then your "<8" tests should be be ">7"

If tests are start/while then OK as is.

(b) In the second LED lighting loop (the downcount) LOW and HIGH should be swapped - going backwards does not change the way you want the LED to light.

\$\endgroup\$
  • \$\begingroup\$ it's (start,while,action) and yeah the 2nd loop's LOW and HIGH were the wrong way round, I've swapped them (edited the question too) but the for loop still isn't working \$\endgroup\$ – snowflakekiller Aug 9 '11 at 13:51
  • \$\begingroup\$ ahh swapping the for-loops for while loops makes it work. I should have tried this first. Grr! Thanks though \$\endgroup\$ – snowflakekiller Aug 9 '11 at 13:56
1
\$\begingroup\$

Count-down loops are more traditionally done with pre-decrement:

for(i=8; i>0; --i)
{
  ...
}

Would count from 7 to 0. The --i decreases i by 1 as the very first thing that gets done after the comparison.

\$\endgroup\$
  • \$\begingroup\$ for loops in C are entirely equivalent to "expression1; while( expression2 != 0 ){ loop-body-code; expression3 }". I.e., the final expression (whether it's pre- or post- increment or decrement, or something else, is the last thing done after executing the loop-body code. So your expression would execute for values of i of [8,7,...,1]. In this context, --i and i-- are equivalent and you can choose one or the other to aid readability. I usually write standalone increment or decrement expressions as pre-(whichevers) because they read the way I say them, e.g., "decrement i". \$\endgroup\$ – JRobert Aug 9 '11 at 16:14
  • 1
    \$\begingroup\$ ... One of many references: cprogramminglanguage.net/c-for-loop-statement.aspx \$\endgroup\$ – JRobert Aug 9 '11 at 16:14
-1
\$\begingroup\$

Commented example code:

void setup() {
  // initialize digital pin 13 to 10 as an output.
  pinMode(13, OUTPUT);
  pinMode(12, OUTPUT);
  pinMode(11, OUTPUT);
  pinMode(10, OUTPUT);

}

// the loop function runs over and over again forever
void loop() {
  digitalWrite(13, HIGH);   // turn the LED on (HIGH is the voltage level)
  delay(50);              // wait for a second
  digitalWrite(13, LOW);   // turn the LED off by making the voltage LOW
  delay(50);             // wait for a second
  digitalWrite(12, HIGH);   // turn the LED on (HIGH is the voltage level)
  delay(50);              // wait for a second
  digitalWrite(12, LOW);   // turn the LED off by making the voltage LOW
  delay(50); 
  digitalWrite(11, HIGH);   // turn the LED on (HIGH is the voltage level)
  delay(50);              // wait for a second
  digitalWrite(11, LOW);   // turn the LED off by making the voltage LOW
  delay(50);             // wait for a second
  digitalWrite(10, HIGH);   // turn the LED on (HIGH is the voltage level)
  delay(50);              // wait for a second
  digitalWrite(10, LOW);   // turn the LED off by making the voltage LOW
  delay(1000);
  digitalWrite(10, HIGH);
  delay(50);
  digitalWrite(10, LOW);   // turn the LED off by making the voltage LOW
  delay(50);
  digitalWrite(11, HIGH);   // turn the LED on (HIGH is the voltage level)
  delay(50);              // wait for a second
  digitalWrite(11, LOW);   // turn the LED off by making the voltage LOW
  delay(50);
  digitalWrite(12, HIGH);   // turn the LED on (HIGH is the voltage level)
  delay(50);              // wait for a second
  digitalWrite(12, LOW);   // turn the LED off by making the voltage LOW
  delay(50); 
   digitalWrite(13, HIGH);   // turn the LED on (HIGH is the voltage level)
  delay(50);              // wait for a second
  digitalWrite(13, LOW);   // turn the LED off by making the voltage LOW
  delay(1000);

}
\$\endgroup\$
  • \$\begingroup\$ rule of thumb: if you have that many lines in your program that look almost the same, there is most likely room for improvement :) \$\endgroup\$ – 0x6d64 Dec 7 '15 at 19:53

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.