0

I have 5 LDR and Arduino Mega. I connect them and get light level.

void loop() {
// print the analog output in milli volts
int w=analogRead(west);
Serial.print("west = ");

Serial.print(w);
Serial.println(" mv");

int e=analogRead(east);
Serial.print("east = ");

Serial.print(e);
Serial.println(" mv");

int n=analogRead(north);
Serial.print("north = ");

Serial.print(n);
Serial.println(" mv");

int s=analogRead(south);
Serial.print("south = ");

Serial.print(s);
Serial.println(" mv");

int c=analogRead(control);
Serial.print("control = ");

Serial.print(c);
Serial.println(" mv");

delay(1000);

if(e > w)
Serial.println("west");

else if(e < w)
Serial.println("east");

else if(s > n)
Serial.println("south");

else(s < n);
Serial.println("north");

Serial.println("\n");
}

This is my loop and I always see east-north. When west is bigger than east I also see east. What

  • 1
    What indeed @user3748265, what indeed. – CharlieHanson Feb 14 '16 at 22:38
  • It would help to post (under your code) what you see in the serial prints. – Nick Gammon Feb 14 '16 at 22:46
  • You should write out in plain English/language what you want to happen and then translate that to code. Your current program doesn't seem to follow any logic and it's hard for us to know your intentions. For example, your first if statement says if E is greater than W then print *west* - that's why you see "west" when E is bigger, but you are saying you don't want that. – CharlieHanson Feb 14 '16 at 22:49
3

Nick Gammon correctly points out a problem due to an extra semicolon, but you should also note what the construct else(s < n) is doing. When the if condition that this else gives an alternative to is false, the statement (s < n) gets executed. That statement is an expression statement, and its value is the evaluation of s<n, either true or false.

If the semicolon after (s < n) weren't there, you'd get a compile error because (s < n) serial.println("north") is not a valid statement. You may have meant to say else if(s < n) instead of else(s < n).

Note, in the sequence

if (e > w)
   Serial.println("west");
else if (e < w)
   Serial.println("east");
else if (s > n)
   ...

that last else typically is reached only rarely, because the only condition to get to it is e and w being equal. Due to sensor noise and signal processing noise, the chance of e and w being equal is probably at most a few percent.

If your north/south test sequence is supposed to be independent of your east/west test sequence, leave out the second if and second else . That is, instead say:

if (e > w)
   Serial.println("west");
else
   Serial.println("east");
if (s > n)
   ...
  • There is another issue I didn't want to comment on. E and W are read from different sensors, so you might conceivably have E == 1023 and W == 1023 which doesn't make a lot of sense. That's why I asked to see the actual serial output. – Nick Gammon Feb 15 '16 at 3:03
  • @NickGammon, I think user3748265 hasn't given enough info for us to say if equal values do or don't make sense. In a suntracker, for instance, equal values are desired. (If you're spotlighting maxed-out sensor values, I agree that's usually a problem.) – James Waldby - jwpat7 Feb 15 '16 at 8:04
  • I won't contradict you. :) – Nick Gammon Feb 15 '16 at 10:09
2

Here:

else(s < n);
-----------^

That is wrong. See Arduino programming traps, tips and style guide.


Trap #13: Don't put a semicolon at the end of every line

Semicolons end statements. However if, for and while are compound statements, so they should not have a semicolon after them like this:

Wrong! ...

  if (temperature > 40);    // <----- this semicolon is incorrect!
    {
    digitalWrite (furnaceControl, LOW);
    digitalWrite (warningLight, HIGH);
    }

  for (int i = 0; i < 10; i++);   // <----- this semicolon is incorrect!
    digitalWrite (i, HIGH);

The semicolons above (as indicated) terminate the if and for so they don't do anything useful. The lines following them are just unconditional blocks of code which are always executed, once.

Correct:

  if (temperature > 40)
    {
    digitalWrite (furnaceControl, LOW);
    digitalWrite (warningLight, HIGH);
    }

  for (int i = 0; i < 10; i++)
    digitalWrite (i, HIGH);

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.