Issue with If statement and boolean logic

Question

I'm trying to control motor based on rain sensor and my own build anemometer. It should only controlling motor clockwise/CW for open the roof if there are no rain (indicated by value above 900) AND the wind speed below 4 km/h. If there are raining (analog value below 650) and/or wind speed above 4 km/h the motor should spin counterclockwise/CCW. I succeed to rotate the motor CCW if rain and/or high wind speed happen, but when the motor hit the limit switch for stopping the motor, motor only stop for maybe 2 seconds and then back rotating CW (open the roof), even the wind still above 4 km/h. The roof should be open only when there is no rain (analog value above 900) and the wind speed below 2 km/h. Here is my If Statement logic program:

if ((analogRead(rain) < 650 || (Wind > 4)){
 motor_closed();
}
else if (analogRead(rain) < 650 && (Wind > 4)){
 motor_closed();
}
else if (analogRead(rain) < 650 && (Wind < 2)){
 motor_closed();
}
else if (analogRead(rain) > 900 && Wind > 4){
 motor_closed();
}
else if (analogRead(rain) > 900 && Wind < 2){
 motor_open();
}

Did I make mistake when using && boolean operator? Or its something else?

• Ignacio Vazquez-Abrams: Why do you call analogRead() multiple times? Do you expect the result to change that quickly?
• Majenko: Only the first and last options will be possible there.
• Avamander: Use analogRead() once and reuse the variable, also what about nesting if statements instead? I think that would improve readability as you can first check if it's raining and then check if it's windy, follows human pattern of thought a bit more than two comparisons at once. Another thing I would avoid is separating else statements from the if statement with a newline, harder to read.
• rofimu: I don't expect for fast result change, only want that the motor respond accordingly. I'm thought it would work with multiple if statements and raining and windy comparisons.
• Majenko: You only need two tests: "is it too windy or too wet?" Yes, close it. Otherwise, "is it not windy enough and not rainy enough?" Yes, open it.

Answer 1

I ran a test with your original code, with rain and wind varying over all the range:

(rain, wind) -> action
------------------------
(600, 1) -> motor_closed
(600, 2) -> motor_closed
(600, 3) -> motor_closed
(600, 4) -> motor_closed
(600, 5) -> motor_closed
(650, 1) -> 
(650, 2) -> 
(650, 3) -> 
(650, 4) -> 
(650, 5) -> motor_closed
(700, 1) -> 
(700, 2) -> 
(700, 3) -> 
(700, 4) -> 
(700, 5) -> motor_closed
(750, 1) -> 
(750, 2) -> 
(750, 3) -> 
(750, 4) -> 
(750, 5) -> motor_closed
(800, 1) -> 
(800, 2) -> 
(800, 3) -> 
(800, 4) -> 
(800, 5) -> motor_closed
(850, 1) -> 
(850, 2) -> 
(850, 3) -> 
(850, 4) -> 
(850, 5) -> motor_closed
(900, 1) -> 
(900, 2) -> 
(900, 3) -> 
(900, 4) -> 
(900, 5) -> motor_closed
(950, 1) -> motor_open
(950, 2) -> 
(950, 3) -> 
(950, 4) -> 
(950, 5) -> motor_closed

So your logical expression reduces to this:

if (analogRead(rain) < 650 || Wind > 4) {motor_close();}
else if (analogRead(rain) > 900 and Wind == 1) {motor_open();}

It's that what you want?

What's that "limiting switch"? May be it's a physical problem (like installing sensor under de roof)

• next-hack: you can improve your code by declaring a temporary variable, e.g. ar in this way ar = analogRead(rain); and then perform your code checking ar instead of analogRead(rain).
• rofimu: The concept is when its raining or strong wind the motor will closing the roof, and when no rain and no strong wind then the motor will open the roof. For controlling motor I use 2 limit switches for stopping the motor when it reach the edge of the roof, 1 for stopping from closing, 1 for stopping from opening. There no issue with limit switches, everything looks good.
• user31481: @rofimu. Look at test I ran. What is missing or superfluous in that?
• rofimu: actually, my first code before I use with multiple If statements is similar like this. But I don't know why every limit switch active (pressed), the motor stop (roof closed), but maybe 1-2 seconds later it turn the opposite way and open the roof although in strong wind condition, but able to stop completely after pressing the second limit switch. I still prefer with else if (analogRead(rain) > 900 and Wind < 2) {motor_open();}. Is it and or && for the operators?
• Majenko: In C it is && and || for logical "and" and "or" operations.

Answer 2

Personally I like to force precedence in my if statements to ensure that things always work precisely as I intend. Group your logical predicates with brackets:

if ((analogRead(rain) < 650) || (Wind > 4)) {
 motor_close();
} else if ((analogRead(rain) > 900) and (Wind < 2)) {
 motor_open();
}

Using intermediate variables would make it more readable:

bool tooWindy = (Wind > 4);
bool notWindy = (Wind < 2);
bool tooRainy = (analogRead(rain) < 650);
bool notRainy = (analogRead(rain) > 900);
if (tooWindy || tooRainy) {
 motor_close();
} else if (notWindy && notRainy) {
 motor_open();
}

Giving names to the logical conditions makes it far more obvious what is going on:

• If it is too window or it is too rainy then close the roof.
• Otherwise, if it's not too windy and it's not too rainy then you can open the roof.

And of course you have the time when it's between too rainy and not too rainy, or between too windy and not too windy, in which case nothing should happen. This is called hysteresis and is good to have since it stops the roof from opening and then immediately wanting to close.

You could also improve the logic of the system even more and implement a small Finite State Machine:

enum {
 CLOSED, OPENING, OPEN, CLOSING
};
int state = CLOSED;
switch (state) {
 case CLOSED:
 if ((analogRead(rain) > 900) && (Wind < 2)) {
 motor_open();
 state = OPENING;
 }
 break;
 case OPENING:
 if (digitalRead(limit_open) == HIGH) {
 motor_stop();
 state = OPEN;
 }
 break;
 case OPEN: 
 if ((analogRead(rain) < 650) || (Wind > 4)) {
 motor_closed();
 state = CLOSING;
 }
 break;
 case CLOSING:
 if (digitalRead(limit_close) == HIGH) {
 motor_stop();
 state = CLOSED;
 }
 break;
}

In that you only attempt to open the roof (and even check to see if you should open the roof) if the roof is actually closed. Similarly for checking for and closing the roof - it only happens when the roof is actually open. Nothing happens in the way of checks while the roof is in the process of opening or closing, so it has to complete its open or close operation first before any more decisions are made as to whether the roof should be open or closed.

Answer 3

Did I make mistake when using && boolean operator? Or its something else?

use brackets liberally.

rather than

if (x > 65 && y < 10)

instead,

if ((x > 65) && (y < 10))