if else question

Hi,

I am trouble with the "if, else" coding. I am trying to use a RF remote to light up a simple PingPong sequence. The problem is that the coding needs to be split before the "else" statement and the board will not allow this. I have tried removing the curly brackets but then the code doesn't work correctly. Below is what I am working with:

void pingPong()
{
int index;
int delayTime = 100; // milliseconds to pause between LEDs

// step through the LEDs, from 0 to 7

for(index = 0; index <= 4; index++)
{
if (button2State == HIGH) {
digitalWrite(ledPins[index], HIGH); // turn LED on
delay(delayTime); // pause to slow down
digitalWrite(ledPins[index], LOW); // turn LED off
}

// step through the LEDs, from 7 to 0

for(index = 4; index >= 0; index--)
{
digitalWrite(ledPins[index], HIGH); // turn LED on
delay(delayTime); // pause to slow down
digitalWrite(ledPins[index], LOW); // turn LED off
}
else {
// turn LED off:
digitalWrite(ledPin, LOW);
}
}
}

Is there any way to do this?

Thanks!!

First you need to organize your code better. You have an option in the "Tools" menu to "Auto-Format" (or something like this, I've not the program in English).
Then you need to paste the code to this post box properly. You have a button in the toolbar menu with a icon ' # ', to do that. You must end with something like this:

void pingPong()
{
  int index;
  int delayTime = 100; // milliseconds to pause between LEDs

  // step through the LEDs, from 0 to 7

  for (index = 0; index <= 4; index++)
  {
    if (button2State == HIGH) {
      digitalWrite(ledPins[index], HIGH);  // turn LED on
      delay(delayTime);                    // pause to slow down
      digitalWrite(ledPins[index], LOW);   // turn LED off
    }

    // step through the LEDs, from 7 to 0

    for (index = 4; index >= 0; index--)
    {
      digitalWrite(ledPins[index], HIGH);  // turn LED on
      delay(delayTime);                    // pause to slow down
      digitalWrite(ledPins[index], LOW);   // turn LED off
    }
    else {
      // turn LED off:
      digitalWrite(ledPin, LOW);
    }
  }
}

Now you can see better that you don't have an if-else but an if-for-else (whatever is that!).

plus, you are incrementing and decrementing your nested for loops with the same variable

look at alternative like this:

void pingPong()
{
  int delayTime = 100; // milliseconds to pause between LEDs
  // step through the LEDs, from 0 to 7

  for(int index = 0; index <= 4; index++)
  {
    if (button2State == HIGH) 
    {
      digitalWrite(ledPins[index], HIGH);  // turn LED on
      delay(delayTime);                    // pause to slow down
      digitalWrite(ledPins[index], LOW);   // turn LED off
      // step through the LEDs, from 7 to 0
      for(int index2 = 4; index2 >= 0; index2--) //
      {
        digitalWrite(ledPins[index2], HIGH);  // turn LED on
        delay(delayTime);                    // pause to slow down
        digitalWrite(ledPins[index2], LOW);   // turn LED off
      }
    }
    else 
    {
      // turn LED off:
      digitalWrite(ledPin, LOW);  
    }
  }
}

Thank you luisilva, I am still very new to coding and even getting onto the forums. Now I know!

BulldogLowel - you are close! I uploaded your code to my Arduino and it lights up the LEDs in one direction but doesn't go back. It also doesn't stop when I left go of the button (I have a momentary switch setup).

Try this:

void pingPong()
{
  int index;
  int delayTime = 100; // milliseconds to pause between LEDs

  // step through the LEDs, from 0 to 7

  if (button2State == HIGH) {
    for (index = 0; index <= 4; index++)
    {
      digitalWrite(ledPins[index], HIGH);  // turn LED on
      delay(delayTime);                    // pause to slow down
      digitalWrite(ledPins[index], LOW);   // turn LED off
    }

    // step through the LEDs, from 7 to 0

    for (index = 4; index >= 0; index--)
    {
      digitalWrite(ledPins[index], HIGH);  // turn LED on
      delay(delayTime);                    // pause to slow down
      digitalWrite(ledPins[index], LOW);   // turn LED off
    }
  }
  else {
    // turn LED off:
    digitalWrite(ledPin, LOW);
  }
}

BTW, I don't understand what you're doing in the else part.

BulldogLowel - Thank you, I moved the first "for" state after the "if" coding. Now it turns off when I release the button but the LEDs are now closer to the actual sequence. Here is what I did:

void pingPong();
{
  int index;
  int delayTime = 100; // milliseconds to pause between LEDs
  // step through the LEDs, from 0 to 7

  
  {
    if (button4State == HIGH) 
    { for(index = 0; index <= 4; index++)
      digitalWrite(ledPins[index], HIGH);  // turn LED on
      delay(delayTime);                    // pause to slow down
      digitalWrite(ledPins[index], LOW);   // turn LED off
      // step through the LEDs, from 7 to 0
      for(index = 4; index >= 0; index--)
      {
        digitalWrite(ledPins[index], HIGH);  // turn LED on
        delay(delayTime);                    // pause to slow down
        digitalWrite(ledPins[index], LOW);   // turn LED off
      }
    }
    else 
    {
      // turn LED off:
      digitalWrite(ledPins[index], LOW);  
    }
  }
}

luisilva!! You did it!!! THANK YOU!! For my else part, that is turning the LEDs off when I release the button on the RF remote.

Rubrburnr:
luisilva!! You did it!!! THANK YOU!! For my else part, that is turning the LEDs off when I release the button on the RF remote.

What LED's? All the 5 LED's? If is that, you need something like:

(...)
  else {
    // turn ALL LED's off:
    for (index = 0; index <= 4; index++)
    {
      digitalWrite(ledPins[index], LOW); 
    }
  }

luisilva (and anybody else who wants a challenge), feel like being tested more? Here is my next button I was trying get the code to work for. It partially works with what everybody has just corrected on here.

Problems:

  1. It seems to go through the sequence ok, but doesn't speed up like its supposed to
  2. It does not turn off right away when I release the button

For the PingPong code I like more of the first version (I don't like to see all the digitalWrite instead of only one pre for).

For your questions:

Rubrburnr:

  1. It seems to go through the sequence ok, but doesn't speed up like its supposed to

You don't tell him to speed up. Because you have this:

      timer = 130;

The first for will be a little more fast, but nothing more.

Rubrburnr:
2. It does not turn off right away when I release the button

Yes, because it don't work that way. As long as you have the for cycles and the delay's you can't (at least easily) do that. It must end the for (actualy the 2 for's), to check again the key, and then turn off the LED (do you understand?). For the original code (the last that I post), I think that this should work:

void pingPong()
{
  int index;
  int delayTime = 100; // milliseconds to pause between LEDs
  boolean turnOff = false;
  // step through the LEDs, from 0 to 7

  if (button2State == HIGH) {
    for (index = 0; index <= 4; index++)
    {
      digitalWrite(ledPins[index], HIGH);  // turn LED on
      delay(delayTime);                    // pause to slow down
      digitalWrite(ledPins[index], LOW);   // turn LED off
      if (button2State == LOW) {
        turnOff = true;
        break; // exit from this for loop
      }
    }
  }
  // step through the LEDs, from 7 to 0
  if ( !turnOff && button2State == HIGH) {
    for (index = 4; index >= 0; index--)
    {
      digitalWrite(ledPins[index], HIGH);  // turn LED on
      delay(delayTime);                    // pause to slow down
      digitalWrite(ledPins[index], LOW);   // turn LED off
      if (button2State == LOW) {
        turnOff = true;
        break; // exit from this for loop
      }
    }
  }

  if (turnOff) {
    // turn ALL LED's off:
    for (index = 0; index <= 4; index++)
    {
      digitalWrite(ledPins[index], LOW);
    }
  }
}

As you see, the code is much more complicated. And you must wait until the delay end to turn off the LED's. If you don't what that, then there is a solution but a little more complex.

EDIT: 1 more thing that I forget to write: I hope that button2State actually check the button like the digitalRead(buttonPin) would do.

Your last edit helped tremendously and it did the trick! Thank you so much! Muito obrigado!

So, in theory, if all my buttons (4) have a certain sequence coded to them, am I able to have one code running as soon as I power on the Arduino? Here is what I was thinking:

Power on Arduino board, it automatically runs a sequence. Then press a button - the Arduino stops running the initial sequence and starts running the coded sequence for the button.

What do you think?

everything placed in the setup() function will run once...

you can call another function from setup()...

Rubrburnr:
(...)
Power on Arduino board, it automatically runs a sequence. Then press a button - the Arduino stops running the initial sequence and starts running the coded sequence for the button.

What happens before that? If you press other button starts another sequence or only changes the sequence after restart?

If is only after restart you can use the suggestion of BulldogLowell, I think is the best way to do it.

Sorry, should have been more detailed.

Power on Arduino board, it automatically runs a sequence. Then press a button - the Arduino stops running the initial sequence and starts running the coded sequence for the button.

Then I'll release the button and the initial sequence starts over until I press another button (or the same one again). This was the code I was going to have as the initial sequence:

void oneOnAtATime()
{
  int index;
  int delayTime = 100; // milliseconds to pause between LEDs
                       // make this smaller for faster switching
  
  // step through the LEDs, from 0 to 7
  
  for(index = 0; index <= 4; index++)
  {
    digitalWrite(ledPins[index], HIGH);  // turn LED on
    delay(delayTime);                    // pause to slow down
    digitalWrite(ledPins[index], LOW);   // turn LED off
  }
}

Inside the for loop you must check all the buttons to see if you must to start other sequence. Maybe the easiest way is do all the sequences this way.

What I was thinking yesterday was to input oneOnAtATime code in the else coding instead of having it turn off the LEDs. It didn't go too well for me...

Yes, is an option too, but must be used with caution. You can't call one function inside another and then another (and so on). You must let the functions return. (I don't know if the problem was this, but it can be)

That makes sense. ok, I will try putting the code in the setup () function. Thanks again!!

Ok, I have tried putting it in the setup() function but it needs to keep running. I was thinking a do while function would work best but I am getting numerous errors. Here is the code I entered:

 do
  {
   int index;
  int delayTime = 100; // milliseconds to pause between LEDs
                       // make this smaller for faster switching
  
  // step through the LEDs, from 0 to 7
  
  for(index = 0; index <= 4; index++)
  {
    digitalWrite(ledPins[index], HIGH);  // turn LED on
    delay(delayTime);                    // pause to slow down
    digitalWrite(ledPins[index], LOW);   // turn LED off
  }
   while ((button1State == LOW);
         (button2State == LOW);
         (button3State == LOW);
         (button4State == LOW));
  }

So it should be lighting the LEDs with none of the buttons are pushed, but once one of them is pushed, it stops the initial sequence and starts the sequence for the button.

Yes, you need to do other loop to make the sequence to repeat, but this:

   while ((button1State == LOW);
         (button2State == LOW);
         (button3State == LOW);
         (button4State == LOW));

is not do this way. you can do it:

   while ((button1State == LOW) && (button2State == LOW) && 
                 (button3State == LOW) && (button4State == LOW));

with only one' ; ' and this symbols '&&' meaning "AND", so you can this in English like:
"repeat while button1State is low and button2State is low and... "

The other problem that you have here is the same that you have at the beginning. This only stops at the end of the sequence (outside the for loop). To make it stop anywhere (in the sequence) you need to to the same that we did in the other example. Makes sense?