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?

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)

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))