next topic »

[on:]

Hi all,

I need someone's help for resolving a little problem in one of my IF statements ...

There comes the code :

Code:

void WeatherDayControl(){
  // Conditions for ENABLING aeration
  if(ActiveAerationState == 0){
    if((dht_IN_Temp >= Temp_IN_Value + Temp_IN_Value_Threshold && dht_IN_Temp - dht_EXT_Temp >= 5) || dht_IN_Hum - dht_EXT_Hum <= -15 || millis() - ActiveAerationPreviousStateChangeMillis >= ActiveAerationOffDuration){
      ActiveAerationOn();
    }
  }
  // Conditions for DISABLING aeration
  if(ActiveAerationState == 1 && millis() - ActiveAerationPreviousStateChangeMillis >= ActiveAerationOnDuration){
    if((dht_IN_Temp <= Temp_IN_Value - Temp_IN_Value_Threshold && dht_IN_Temp - dht_EXT_Temp <= 2 ) || dht_IN_Hum - dht_EXT_Hum >= 5){
      ActiveAerationOff();
    }
  }
}

Where "ActiveAerationPreviousStateChangeMillis" corresponds to this :

Code:

void ActiveAerationOn(){
  if(ActiveAerationState == 0){
    ActiveAerationPreviousStateChangeMillis = millis();
    ActiveAerationState = 1;
  }
  FanOUTon();
  FanINon();
  ServoOUTopen();
  ServoINopen();
}

Code:

void ActiveAerationOff(){
  if(ActiveAerationState == 1){
    ActiveAerationPreviousStateChangeMillis = millis();
    ActiveAerationState = 0;
  }
  ServoINclose();
  ServoOUTclose();
  FanINoff();
  FanOUToff();
}

In fact, it works great except the (dht_IN_Temp >= Temp_IN_Value + Temp_IN_Value_Threshold && dht_IN_Temp - dht_EXT_Temp >= 5) statement which appears not to be executed.

Is there something to do when mixing AND and OR arguments inside an IF statement ?


Thanks !

[Reply #1 on:]

In fact, it works great except the (dht_IN_Temp >= Temp_IN_Value + Temp_IN_Value_Threshold && dht_IN_Temp - dht_EXT_Temp >= 5) statement which appears not to be executed.

Is there something to do when mixing AND and OR arguments inside an IF statement ?

Sometimes the order of execution of operations is not what you might expect.  I think it may be doing comparisons before addition.  To force it to do what you want, try parenthesis:

((dht_IN_Temp >= (Temp_IN_Value + Temp_IN_Value_Threshold)) && ((dht_IN_Temp - dht_EXT_Temp) >= 5))

[Reply #2 on:]

Addition has higher order of precedence, so the compiler shouldn't be getting that mixed up (http://en.wikipedia.org/wiki/Operators_in_C_and_C%2B%2B#Operator_precedence) - however the brackets are a good idea for clarity anyway.

What type are your variables?

[Reply #3 on:]

For clarity put only one comparison operator per line!  You might then just see the problem.

[Reply #4 on:]

A line like

Code:

if((dht_IN_Temp >= Temp_IN_Value + Temp_IN_Value_Threshold && dht_IN_Temp - dht_EXT_Temp >= 5) || dht_IN_Hum - dht_EXT_Hum <= -15 || millis() - ActiveAerationPreviousStateChangeMillis >= ActiveAerationOffDuration){

Is so confusing I don't think there's a chance it's doing the right thing. And if it is it's totally unmaintainable.

I'd at least try to put the tests on separate lines

 

Code:

if( (dht_IN_Temp >= (Temp_IN_Value + Temp_IN_Value_Threshold) &&
(dht_IN_Temp - dht_EXT_Temp >= 5) ||
(dht_IN_Hum - dht_EXT_Hum <= -15) ||
(millis() - ActiveAerationPreviousStateChangeMillis >= ActiveAerationOffDuration)
{

But it's not clear if is should be

a && (b || c || d)

or

(a && b || c) || d
 
or whatever. Now I'm sure you could look into the operator precedence but you shouldn't rely on that anyway. I'd be included to split the calculations from the tests

Code:

int a = Temp_IN_Value + Temp_IN_Value_Threshold;
int diff_temp = dht_IN_Temp - dht_EXT_Temp ;
int diff_hum = dht_IN_Hum - dht_EXT_Hum ;
int d = millis() - ActiveAerationPreviousStateChangeMillis ;

   if(dht_IN_Temp >= a && diff_temp >= 5 || diff_hum <= -15 || d >= ActiveAerationOffDuration) {

This needs better variable names and still needs ()s to force the order of tests but I think it's a lot clearer.

______
Rob