Am i using && correctly in my (basic) rule?

here’s my rule, you can see ive added a couple of sendMails to help with debugging. I always get the first mail, never the second, and the rule doesn’t trigger.

 when
     Item Powermax_zone6_status changed to ON
 then
 {sendMail("colin@.com", "zone 6 triggered", "zone 6 triggered")}
     if (NightTime.state == ON && Light_GF_Hall_Lamp.state == OFF)
     {sendMail("colin@.com", "zone 6 triggered step 2", "zone 6 triggered step2 ")
         sendCommand(Light_GF_Hall_Lamp, ON)
        Thread::sleep(200000)
             sendCommand(Light_GF_Hall_Lamp, OFF)            
     }
 end

The curly braces on the first line after then are unneeded and might be causing a problem.

Also, I think that

(NightTime.state == ON && Light_GF_Hall_Lamp.state == OFF)

should have a following format:

((NightTime.state == ON) && (Light_GF_Hall_Lamp.state == OFF))

Best regards,
Davor

Also, if “NightTime” is a string item, you might need to change the line to:

((NightTime.state.toString == “ON”) && (Light_GF_Hall_Lamp.state == OFF))

Thanks all for your comments :slight_smile:
NightTime is a switch item - i struggled with it being a string, but seems like a have the answer to that too now.

Time to do some testing

thanks

HI - i thought curly braces were needed around actions?
Colin

Curly braces are a way to define a context. One is not required to define a new context around a single line of code and they are certainly not required to call an action. So at best the curly braces are unnecessary and at worse they could be causing problems if the Rules interpreter is not expecting them to be there.

In addition to curly braces, indentation is also used to visually indicated context. So the code that is in the curly braces that are part of your if statement should all be indented and all indented at the same level.

     if (NightTime.state == ON && Light_GF_Hall_Lamp.state == OFF)
     {
         sendMail("colin@.com", "zone 6 triggered step 2", "zone 6 triggered step2 ")
         sendCommand(Light_GF_Hall_Lamp, ON)
         Thread::sleep(200000)
         sendCommand(Light_GF_Hall_Lamp, OFF)            
     }

This is not functional but it really really makes it easier to read the code (kind of like when they decided to put spaces between words in medieval Latin).

1 Like