Presence trigger firing off piston when false?

chadbaldwin · 2017-12-19 04:12:53 UTC

1) Give a description of the problem
In the list of triggers for this piston, one of them is

execute
    if
        Presence Sensor 2's presence changes to not present (line 36)

2) What is the expected behavior?
That nothing happens when Presence Sensor 2’s presence changes to present, execution only occurs if statement is true. But it’s continuing execution even when false? Looking at my piston, I see that I can move all of my commands at the bottom to inside the THEN section of the main IF condition and that will solve my problem. But I’m also posting this to see if this is how it’s supposed to be working…or a bug?

3) What is happening/not happening?
(this just happened a few minutes ago) my girlfriend came home from work while I was already home, and the piston ran because her presence sensor changed to present.

4) Post a Green Snapshot of the piston

5) Attach any logs (From ST IDE and by turning logging level to Full)
(too long, will post if really needed). Logs show this line:
+2ms ╔Received event [Presence Sensor 2].presence = present with a delay of 137ms

chadbaldwin · 2017-12-19 04:25:14 UTC

I’ve implemented this solution to fix the problem. Again, this post is more to see if this is working as intended or if it’s a bug.

Solution:

michicago · 2017-12-19 05:38:09 UTC

All your actions, with the exception setting variables, are outside your IF statements. Therefore they will execute every time the piston runs (every time presence changes whether present or not present) because there are no conditions to evaluate true or false.

It’s working correctly, not a bug. Just have to learn how pistons execute and build from there. It takes some trial and error!

Robin · 2017-12-19 10:27:25 UTC

I would use a switch case:

same triggers as in your example
THEN

switch {$timeNow}

case between 05:00 and Sunrise
do stuff

case between Sunrise and Sunset
do stuff

case between Sunset and 08:30PM
do stuff

etc. etc.

chadbaldwin · 2017-12-19 12:01:58 UTC

@michicago, I understand how it works, please see my comment before yours where I’ve posted the solution to fix this from happening. I was just asking because it seemed odd to me and thought I would ask just in case.

@Robin using a case statement would remove one of the intended features, which is the ability to recover the lights at any point in time. If one of the lights are changed to an undesired state, this piston can be run to restore all of the lights to their correct state. Also a case statement would cause all other lights whose variable is not set to turn off since most of the initialization values are off. To get around that, I would have to set the value of every variable at every case. The between statements also remove the restore feature and they make maintenance more difficult if you want to add or remove new trigger times. If I want to add a new trigger time, all I have to do is add a new statement and set the variables and drag it to the right spot. But with between statements, you’d have to modify the two cases that overlap before and after your new statement you are adding.

Robin · 2017-12-19 12:13:12 UTC

Your’re setting the same variables multiple times in a single execution by doing it with a series of IF time is after... statements… masivley extends the execution time with possible race conditions and errors.

For example, if you run your piston after 10pm, {DrivewayLightsSwitch} gets quickly set set to On>OFf>On>Off before the light is eventually set to ‘Off’

Why set the variables and then immediately set the light to the variable??? just set the lights directly???

Using a switch case with between statements, you can run the piston at any time using your RT routine and it will pick the correct case and only run that case.

chadbaldwin · 2017-12-19 15:43:22 UTC

How is there a race condition? Isn’t everything executed in series? (I don’t know much about race conditions)

Also, if I were to use the case statements, what happens when if the porch light is manually turned off, and I run the piston at 8:35. The only case that would be true is “between 8:30pm and 10:00pm”, all that case does is turn off the fish tank. So the porch light will continue to be off until the next case where porch light is included.

I would also have to change the initialization values of all the variables to point at their devices value instead of defaulting to off. Otherwise the case statement method will turn off everything except what is in that case.

I also don’t care how long it takes to run… Even if it takes a whole minute… If it takes an extra few seconds to turn on my porch light, big whoop.

Robin · 2017-12-19 17:11:32 UTC

Variables are stored by SmartThings, lag and possible race conditions may occur when changing it in quick succession.

You’re over thinking it, trying to make it look short on the screen, when in actual fact, with switch statements, only a small portion of the piston will run. Just create a large block, setting every light in every case.

You don’t need to use variables, just put the device commands directly in each case.

If your way works then no issue… it’s just more complicated to work out what you did a year down the line.

chadbaldwin · 2017-12-19 19:24:17 UTC

Ah okay. Yeah, I see where you are getting at then. Ditch the variables, swap in a case statement and then control every device in every case statement and then switch my conditions to use “time between”. That would definitely work, but I personally see that as more work to manage as new devices are added to my network, or I move times around.

It’s been working fine for over a month, so I haven’t seen any issues regarding a race condition error. However, I could always throw in a 1-2 second wait for each condition to give it some room. Like I mentioned earlier, I’m not too worried about how long it takes to run.

The reason I structured it this way is because it’s the easiest structure I’ve found for management. If I want to add a new time trigger, or change a time trigger, or if I want to change the time a device is turned on or off, it’s very easy with this method.

I’ve tried using the method you’re suggesting, and it’s more work to manage. Because I would have conditions like:
BETWEEN 5AM AND Sunrise
BETWEEN Sunrise and Sunset
BETWEEN Sunset and 8:30PM
BETWEEN 8:30PM and 10:00PM
BETWEEN 10:00PM and 11:30PM

If I wanted to add a condition between Sunrise and Sunset, for 3PM, I would have to break “BETWEEN Sunrise and Sunset” into two conditions, populate all of the device commands into each one, and set the condition times to:
BETWEEN Sunrise and 3PM
BETWEEN 3PM and Sunset

But with the current design, if I wanted to add a 3PM condition, then I add the “Time is after 3PM” condition, drag it in between Sunrise and Sunset, and then add the one device I want to control.

Also, with case statements, if I add a new device to my network, then I would have to go and add it to every single case statement.

I’ve already thought through a lot of this stuff, and there aren’t a lot of designs that are able to do what this piston design is able to do…even if it is a bit inefficient.