Go Down

Topic: Need help with an "IF" statement ... (Read 1 time) previous topic - next topic

simkard

Hi all,

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

There comes the code :
Code: [Select]

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: [Select]

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

Code: [Select]

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 !

johnwasser


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))
Send Bitcoin tips to: 1G2qoGwMRXx8az71DVP1E81jShxtbSh5Hp

pocketscience

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?

MarkT

For clarity put only one comparison operator per line!  You might then just see the problem.
[ I will NOT respond to personal messages, I WILL delete them, use the forum please ]

Graynomad

A line like

Code: [Select]
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: [Select]
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: [Select]
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
Rob Gray aka the GRAYnomad www.robgray.com

Go Up