Sketch will not loop.

Using Arduino Programming Questions
#1

I am fairly new to Ardunio and although doing a thorough search I have a problem I cannot solve.

Code below does not loop and and the last sub loop does not seem to work. Appreciate any help or guidance.

Regards

Ken

/*
Light LEDs on pins 2 to 7 sequentially, each after short delay and leaving all on.
Then turn all Leds off.
Follow by a reverse sequence (eg. Led on pin 7 down to led on pin 2)
Turn all Leds off.
Then loop to start.
*/

int timerS = 30 ;
int timerL = 300 ;

void setup() {
// use as loop to initialize each pin as an output:
for (int thisPin = 2; thisPin < 8; thisPin++) {
pinMode(thisPin, OUTPUT);
}
}
void loop() {

// Light up pin 2 to pin 7 with short delay:

for (int thisPin = 2; thisPin <= 8; thisPin++) {
// turn the pin on:
digitalWrite(thisPin, HIGH);
// delay lighting the next LED:
delay(timerS);
}
//When all LEDS are HIGH, switch to LOW after long delay:

delay(timerL);
for (int thisPin = 2; thisPin <= 8; thisPin++) {
digitalWrite (thisPin, LOW);
}
// After long delay, switch LEDS on one at a time in reverse order:

delay(timerL);
for (int thisPin = 7; thisPin <= 8; thisPin–) {
// turn the pin on:
digitalWrite (thisPin, HIGH);
// delay lighting the next LED:
delay(timerS);
}
//Turn all LEDs LOW). NOTE: This sub loop below does not do what is intended and all Leds remain HIGH:

for (int thisPin = 2; thisPin <= 8; thisPin++) {
// turn the pin off:
digitalWrite(thisPin, LOW);
// Return to restart Loop ():
}
}

#2

kchunter:
Code below does not loop and and the last sub loop does not seem to work. Appreciate any help or guidance.

I don’t know what you mean by that. Please tell us what it actually does and what you want it to do that is different.

And please modify your post and use the code button </> so your code looks like this and is easy to copy to a text editor. See How to use the Forum

…R

#3 
// After long delay, switch LEDS on one at a time in reverse order:

  delay(timerL);
  for (int thisPin = 7; thisPin <= 8; thisPin--) {
    // turn the pin on:
    digitalWrite (thisPin, HIGH);
    // delay lighting the next LED:
    delay(timerS);
  }

This is not doing what you think it is doing (and some of your other “for” loops also have incorrect “conditions”).

#4

Thanks Robin

Sorry, still learning.

I have reformatted the original code as suggested.

The basic functionality is as below.

The problem is that the final sub-loop which turns all LEDs to LOW before it should return to the start of the loop does not appear to work.

Basically, it is intended to work as follows:

  1. Light LEDs on pins 2 to 7 sequentially, each after short delay and leaving all on.
  2. Then turn all Leds off.
  3. Follow by a reverse sequence (eg. Led on pin 7 down to led on pin 2)
  4. Turn all Leds off.
  5. Then loop to start.

Steps 4 & 5 are not working:

I very much appreciate your guidance.

Regards

Ken

#5

try this out:

/*
  Light LEDs on pins 2 to 7 sequentially, each after short delay and leaving all on.
  Then turn all Leds off.
  Follow by a reverse sequence (eg. Led on pin 7 down to led on pin 2)
  Turn all Leds off.
  Then loop to start.
*/

int timerS = 30 ;
int timerL = 300 ;


void setup() {
  // use as loop to initialize each pin as an output:
  for (int thisPin = 2; thisPin < 8; ++thisPin) {
    pinMode(thisPin, OUTPUT);
  }
}
void loop() {
int thisPin; //since u are using this variable repeatedly maybe better to declare it at beginnig of loop

  // Light up pin 2 to pin 7 with short delay:

  for (thisPin = 2; thisPin < 8; ++thisPin) { //no need to used <=
    // turn the pin on:
    digitalWrite(thisPin, HIGH);
    // delay lighting the next LED:
    delay(timerS);
  }
  //When all LEDS are HIGH, switch to LOW after long delay:

  delay(timerL);
  for (thisPin = 2; thisPin < 8; thisPin++) {
    digitalWrite (thisPin, LOW);
  }
  // After long delay, switch LEDS on one at a time in reverse order:

  delay(timerL);
  for (thisPin = 7; thisPin >1; --thisPin) { // since decrementing should be >= not <=
    // turn the pin on:
    digitalWrite (thisPin, HIGH);
    // delay lighting the next LED:
    delay(timerS);
  }
  //Turn all LEDs LOW).  NOTE: This sub loop below does not do what is intended and all Leds remain HIGH:

  for (thisPin = 2; thisPin < 8; thisPin++) {
    // turn the pin off:
    digitalWrite(thisPin, LOW);
    // Return to restart Loop ():
  }
}
#6

This code

 // Light up pin 2 to pin 7 with short delay:

  for (int thisPin = 2; thisPin <= 8; thisPin++) {

will operate on pins 2 to 8 but your comment says 2 to 7 - which is correct?

This code

 for (int thisPin = 7; thisPin <= 8; thisPin--) {

goes from London to Bristol (or New York to Chicago) via China.

It should be

 for (int thisPin = 7; thisPin >=2; thisPin--) {

And for values such as pin numbers that are always in the range 0-255 use a byte rather than an int

…R

#7

Your pinMode sets pins 2..7 to output.

In loop(), you also write pin 8 which is an input.

#8

Thanks to all for your generous advice. As advised by most, there was a logic error and all is well.

Regards

Ken

#9