Pages: [1]   Go Down
Author Topic: Need help with an "IF" statement ...  (Read 554 times)
0 Members and 1 Guest are viewing this topic.
Offline Offline
Full Member
***
Karma: 0
Posts: 125
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

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 !
Logged

Massachusetts, USA
Offline Offline
Tesla Member
***
Karma: 180
Posts: 8103
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

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

Send Bitcoin tips to: 1L3CTDoTgrXNA5WyF77uWqt4gUdye9mezN
Send Litecoin tips to : LVtpaq6JgJAZwvnVq3ftVeHafWkcpmuR1e

Sydney, Australia
Offline Offline
Full Member
***
Karma: 3
Posts: 230
Arduino rocks
View Profile
WWW
 Bigger Bigger  Smaller Smaller  Reset Reset

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?
Logged

Is life really that serious...??!

0
Offline Offline
Shannon Member
****
Karma: 162
Posts: 10496
Arduino rocks
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

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

[ I won't respond to messages, use the forum please ]

nr Bundaberg, Australia
Offline Offline
Tesla Member
***
Karma: 121
Posts: 8458
Scattered showers my arse -- Noah, 2348BC.
View Profile
WWW
 Bigger Bigger  Smaller Smaller  Reset Reset

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
Logged

Rob Gray aka the GRAYnomad www.robgray.com

Pages: [1]   Go Up
Jump to: