Arduino Code - simple if, else statement?

Question

Basically, I have two led lights connected to pin 2 (led 1) and pin 3 (led 2), and I want led 2 to light up every time led 1 lights up, and turn off every time led 1 turns off.

My code:

int led1=2;
int led2=3;
void setup()
{
 pinMode(led1, OUTPUT);
 pinMode(led2, OUTPUT);
}
void loop()
{
 if (digitalRead(led1 == HIGH))
 {
 digitalWrite(led2, HIGH);
 }
 else
 {
 digitalWrite(led2, LOW); //This line won't work.
 } 
 digitalWrite(led1, HIGH);
 delay(5000);
 digitalWrite(led1, LOW);
 delay(3000);
}

So after I upload this code, led 2 lights up whenever led 1 lights up, but led 2 wont turn off when led 1 turns off.

Answer 1

With the if() statement fixed as mentioned in another answer, it still won't work because you turn led1 on, then off at the end of the loop, so when the program gets back to the top of the loop, led1 is always off.

To get the effect you want, you need to do the if/else both after turning led1 on, and after turning led1 off.

Answer 2

This line:

if (digitalRead(led1 == HIGH))

Is incorrect. This should be:

if (digitalRead(led1) == HIGH)

Because you want to check the return value of digitalRead(). However, this doesn't cause the problem.

You can see the real problem when you try to think as the microcontroller. It does these steps (I start at digitalWrite(led1, HIGH)):

1. Set LED1 high
2. Wait
3. Set LED1 low
4. Wait
5. Check:
   1. If LED1 is high, set LED2 high
   2. If LED1 is low, set LED2 low
6. Go to 1.

When 5 is executed, the value of LED1 is always low. 5.2 will always be executed, 5.1 never. If you want to achieve this programmatically, you could use something like this:

void loop() {
 digitalWrite(led1, HIGH);
 digitalWrite(led2, HIGH);
 delay(5000);
 digitalWrite(led1, LOW);
 digitalWrite(led2, LOW);
 delay(3000);
}

If you want to do this with an if, you can either do something like this:

void calcLed2() {
 if (digitalRead(led1) == HIGH) {
 digitalWrite(led2, HIGH);
 } else {
 digitalWrite(led2, LOW);
 }
}
void loop() {
 digitalWrite(led1, HIGH);
 calcLed2();
 delay(5000);
 digitalWrite(led1, LOW);
 delay(3000);
}

Or put the if loop in the loop() (i.e. without a function call).

It would also be good practice if you set the initial state of the machine in your setup(). For example, set both LEDs high or both LEDs low. If you don't do this, it will work, but might give unexpected results in the first few seconds.

• Amos: Are you sure LED1 is always high when 5 is executed? Surely LED1 is always low?

Answer 3

your if statement is incorrect, what you want is

if (digitalRead(led1) == HIGH)
{
 digitalWrite(led2, HIGH);
}
else
{
 digitalWrite(led2, LOW); //This line won't work.
} 

However, as Keelan has already pointed out, there are other issues as well.

Your original "if" statement would get evaluated as follows: First, the "led1 == HIGH" statement gets evaluated. Since you defined led1 as 2, and "HIGH" is (presumably) defined as 1, this should result in a boolean FALSE, or the value 0. This result (the value 0) is then passed as input to the digitalRead function. Since this function expects an Arduino pin number as input, it effectively tries to read the value of pin 0 (which I don't think is defined). This nonetheless appears to results in a value greater than one, so that the body of the if clause is executed as your LED2 is evidently turned on.

As Keelan already pointed out, with only the change in the if statement, your LED2 would never actually turn on, since at the end of the loop(), LED1 would always be off/low. So if all you want to do is turn on or off several pins, you could just use two digitalWrite() calls. If you do need to check if one pin is low or high (e.g. because other parts of the code can change its state), then you do need to make sure to do the comparison the against the output of digitalRead(), an not within its input.

• user17592: That is true, but not what causes the problem.
• fm_andreas: No, it's not the only problem, but that was the reason it never went into the else part. Since our answers overlapped, I'm happy to withdraw mine.
• user17592: Look at my answer. It is significantly different from yours. With your code, LED2 wouldn't get low either, because the if loop is only executed when LED1 is high. If our answers were essentially the same, I'd have withdrawn mine because you were earlier, however, your answer doesn't answer the question.
• Tony Yi: Thank you Camil and fm, but once i change my code to if (digitalRead(led1) == HIGH); led 2 won't even turn on. what's going on?

Answer 4

int led1=2;
int led2=3;
void setup()
{
 pinMode(led1, OUTPUT);
 pinMode(led2, OUTPUT);
}
void loop()
{
 if (digitalRead(led1 == HIGH))
 {
 digitalWrite(led2, HIGH);
 }
 else if (digitalWrite(led1, LOW) )
 {
 digitalWrite(led2, LOW);
 } 
}

• Nick Alexeev: Please explain your code. Consider also that this is a 2 year old question.