Motion driven rule, works mostly but fails when brightness is over 31

[dastrix80 (Kris K)]

Hi All

The intention was for this rule to not be active when I set the brightness above 31%, however it keeps switching the lights off after motion inactivity and appears to ignore the brightness.

Any suggestions as to why? I just set the Dim to 40% and its gone off as an example

smarthome:status FrontRoomDim1
40

var Timer FrontRoom_Timer = null
val Integer FrontRoom_TimeOut = 2


rule "Front Room motion detection turns ON Front Room Lights when Lux is less than 20, with a 2 Minute Inactivity Timer"
when
       Item FrontRoomMotion changed to ON
then
      if(FrontRoomMotion_Armed.state == ON && Motion_PartyMode.state != ON){
         var int Dim_FrontRoom = 0
           try {Dim_FrontRoom = Integer::parseInt(FrontRoomDim1.state.toString)} catch (Exception e) {}
               if (FrontRoomLux.state < 20 && Dim_FrontRoom < 31) {
                if (FrontRoom_Timer !== null) {
                        FrontRoomDim1.sendCommand("30")
                        logInfo("FrontRoom_Motion","Front Room Motion Timer rescheduled for " + Eye1_TimeOut + " minutes")
                        FrontRoom_Timer.reschedule(now.plusMinutes(FrontRoom_TimeOut))
                } else {
                        logInfo("FrontRoom_Motion", "Front Room Motion Detected! Turning ON Front Room Lights")
                        FrontRoomDim1.sendCommand("30")
                        logInfo("FrontRoom_Motion","Front Room Timer created with " + Eye1_TimeOut + " minutes")
                        FrontRoom_Timer = createTimer(now.plusMinutes(FrontRoom_TimeOut))
                        [ |
                                if (FrontRoomMotion.state ==  ON) {
                                        logInfo("FrontRoom_Motion","Front Room Motion triggered, but rescheduled again for " + FrontRoom_TimeOut + " minutes")
                                        FrontRoom_Timer.reschedule(now.plusMinutes(FrontRoom_TimeOut))
                                } else {
                                        logInfo("FrontRoom_Motion", "Front Room, No Motion Detected! Turning OFF Front Room Lights")
                                        FrontRoomSw1.sendCommand("OFF")
                                        FrontRoom_Timer = null
                                }
                        ]
                }
        }
}
end

Thank you!

[rossko57]

What is the value of that key variable Dim_FrontRoom? Bit of an odd parsing at work there.

You probably need a bit of a logic restructure here.
Though you only start off or reschedule the off-timer if dim<31, once it’s started timing it’s started, and when expires will turn off lights regardless.

I’d be inclined to just have the timer do its start/reschedule thing without taking any notice of ‘dim’. The only time that is important is when deciding to turn the lights off.

[dastrix80 (Kris K)]

How do I find that out when its set inside the rule rossko57? the vbalue of Dim_FrontRoom

Sometimes you want the lights on manual control, which I like doing by telling Alexa to set the brightness manually.

[rossko57]

@dastrix80

How do I find that out when its set inside the rule

Isn’t this an Item? Aren’t you already parsing this ‘dim’ value from Item state in your rule?

@dastrix80

Sometimes you want the lights on manual control, which I like doing by telling Alexa to set the brightness manually.

Sure. What problem do you anticipate with this?

[dastrix80 (Kris K)]

No, it’s not an item, its a variable set in the rule.

The item is

FrontRoomDim1

Dim_FrontRoom, is not an item. It’s parsed and theres never any issues in getting the right Dim value of the item

Problem do i anticipate? I’m not following you.

[rossko57]

@dastrix80

How do I find that out when its set inside the rule

Okay, to answer that, here is how you do it now
var Dim_FrontRoom = Integer::parseInt(FrontRoomDim1.state.toString)
Here is how you would do it inside the Timer code, which is I think what you are asking
var Dim_FrontRoom = Integer::parseInt(FrontRoomDim1.state.toString)

[dastrix80 (Kris K)]

They are exactly the same? scratching head

[rossko57]

Yep. I’m suggesting that you don’t test for dim<31 when starting or managing the Timer. Who cares, its only a timer. It doesn’t do anything until/unless it expires.

You’re more interested in the code that runs when the Timer expires, and would normally turn the lights off. Here, you test if dim<31 to decide if you should turn the lights off or not.

This saves you the problem of working out what to do if the timer is already running and then you change to dim<31.

[dastrix80 (Kris K)]

But that doesnt solve my issue. I want to manually set my dim via voice, and ensure the lights dont turn off when its set to that value, or more

[rossko57]

@dastrix80

I want to manually set my dim via voice, and ensure the lights dont turn off when its set to that value, or more

Okay. Which part of this is your issue?

I’m not clear why this is not a solution -

@rossko57

when the Timer expires, and would normally turn the lights off. Here, you test if dim<31 to decide if you should turn the lights off or not.

Do you need something else to tell if dim was set by voice or not?

[dastrix80 (Kris K)]

Let me try and explain it a bit clearer.

The lights are motion driven, they work 100% fine.
What fails, is when I set the dim value to something higher than 31%, the lights still go out.

The intention, was that if the Dim was higher than 31% then the lighting rule would not kick in. I.e it would not be motion driven, rather if it was above 31% it would remain ON indefinitely, unless A) I lowered the dim and motion was triggered, or B) I pressed the light switch

[bob_dickenson]

Try using the Expire binding for timer control, then checking the Dim value at timer expiration to conditionally turn light off or reset timer. Also see this thread – Automation/Orchestration Design Patterns

[rossko57]

I’d go one step at a time. There’s no need to change the way the timer works in order to add a conditional check before turning lights off.

@dastrix80

The intention, was that if the Dim was higher than 31% then the lighting rule would not kick in.

Missing from your requirement is what you would like to happen if the motion timer is already running when you change the dim to >30.

[dastrix80 (Kris K)]

Hi Rossko57,

OK. If the timer is already running, cancel it, because I’d like the light to remain on, as the dim is in lets call it, ‘manual mode’

Cheers

[rossko57]

@dastrix80

If the timer is already running, cancel it

That’s easy, make a rule that triggers on dim changing, does your condition check, sees if a timer exists, and kills it.

Don’t be afraid to have many small rules doing little event-driven jobs.

If you use the expire technique for timing, you can’t cancel that.
You can make your expire-triggered rule check the conditions at its appointed time, and avoid turning off the light.

Same as I’ve been suggesting you do with the existing timer. (That would avoid any need to cancel that too.)
There is more than one way to do most things.

A question that may help choices: have you any use for “occupancy sensing”?
If so, you may want to use use the same start/reschedule/timeout process for that as well, whether dark or daylight or manual mode.
The process would of course have conditional checks to see if should do lighting actions at each stage, but that becomes almost like an add-on to the main flow.

[dastrix80 (Kris K)]

Could I not just modify the existing rule to this, doing a check if the DIM is lower than 31 before switching off?

EDIT: That doesnt work, accepts the code but brings up big errors

var Timer FrontRoom_Timer = null
val Integer FrontRoom_TimeOut = 2


rule "Front Room motion detection turns ON Front Room Lights when Lux is less than 20, with a 2 Minute Inactivity Timer"
when
       Item FrontRoomMotion changed to ON
then
      if(FrontRoomMotion_Armed.state == ON && Motion_PartyMode.state != ON){
         var int Dim_FrontRoom = 0
           try {Dim_FrontRoom = Integer::parseInt(FrontRoomDim1.state.toString)} catch (Exception e) {}
               if (FrontRoomLux.state < 20) {
                if (FrontRoom_Timer !== null) {
                        FrontRoomDim1.sendCommand("30")
                        logInfo("FrontRoom_Motion","Front Room Motion Timer rescheduled for " + Eye1_TimeOut + " minutes")
                        FrontRoom_Timer.reschedule(now.plusMinutes(FrontRoom_TimeOut))
                } else {
                        logInfo("FrontRoom_Motion", "Front Room Motion Detected! Turning ON Front Room Lights")
                        FrontRoomDim1.sendCommand("30")
                        logInfo("FrontRoom_Motion","Front Room Timer created with " + Eye1_TimeOut + " minutes")
                        FrontRoom_Timer = createTimer(now.plusMinutes(FrontRoom_TimeOut))
                        [ |
                                if (FrontRoomMotion.state ==  ON) {
                                        logInfo("FrontRoom_Motion","Front Room Motion triggered, but rescheduled again for " + FrontRoom_TimeOut + " minutes")
                                        FrontRoom_Timer.reschedule(now.plusMinutes(FrontRoom_TimeOut))
                                } 
                                else {
                                        if(FrontRoomDim1 < 31)
                                        logInfo("FrontRoom_Motion", "Front Room, No Motion Detected! Turning OFF Front Room Lights")
                                        FrontRoomSw1.sendCommand("OFF")
                                        FrontRoom_Timer = null
                                }
                        ]
                }
        }
}
end

[rossko57]

@dastrix80

Could I not just modify the existing rule to this, doing a check if the DIM is lower than 31 before switching off?

Well, yes. This has been suggested before, once or twice.

Remember you will need to fetch/calculate current “dim” in the timer code, at the time the timer runs.

if(FrontRoomDim1 < 31)
Isn’t FrontRoomDim1 a String Item that you needed to perform a complicated parse operation on in the main body of the rule?

[rlkoshak (Rich Koshak)]

@dastrix80

No, it’s not an item, its a variable set in the rule.

One of the ways to make code easier to read is by following conventions. In OH, the most common convention is:

• Items start with an upper case letter and use camel case or _, e.g. MyItem, My_Other_Item
• variables start with a lower case letter and use camel case, e.g. myVar
• constants are all upper case letters with underscores where necessary, e.g. MY_VAL

I admit that I don’t do the last one in my Rules.

Anyway, when users like rossko57 or I see something named “Dim_FrontRoom”, without any other information, we will assume it is an Item because it is following the Item naming convention.

[dastrix80 (Kris K)]

Yes, so I can move that check to the timer section. Ill try this

rule "Front Room motion detection turns ON Front Room Lights when Lux is less than 20, with a 2 Minute Inactivity Timer"
when
       Item FrontRoomMotion changed to ON
then
      if(FrontRoomMotion_Armed.state == ON && Motion_PartyMode.state != ON){
               if (FrontRoomLux.state < 20) {
                if (FrontRoom_Timer !== null) {
                        FrontRoomDim1.sendCommand("30")
                        logInfo("FrontRoom_Motion","Front Room Motion Timer rescheduled for " + Eye1_TimeOut + " minutes")
                        FrontRoom_Timer.reschedule(now.plusMinutes(FrontRoom_TimeOut))
                } else {
                        logInfo("FrontRoom_Motion", "Front Room Motion Detected! Turning ON Front Room Lights")
                        FrontRoomDim1.sendCommand("30")
                        logInfo("FrontRoom_Motion","Front Room Timer created with " + Eye1_TimeOut + " minutes")
                        FrontRoom_Timer = createTimer(now.plusMinutes(FrontRoom_TimeOut))
                        [ |
                            var int Dim_FrontRoom = 0
                            try {Dim_FrontRoom = Integer::parseInt(FrontRoomDim1.state.toString)} catch (Exception e) {}
                              if (FrontRoomMotion.state ==  ON) {
                                        logInfo("FrontRoom_Motion","Front Room Motion triggered, but rescheduled again for " + FrontRoom_TimeOut + " minutes")
                                        FrontRoom_Timer.reschedule(now.plusMinutes(FrontRoom_TimeOut))
                                } else {
                                        if (Dim_FrontRoom < 31)
                                        logInfo("FrontRoom_Motion", "Front Room, No Motion Detected! Turning OFF Front Room Lights")
                                        FrontRoomSw1.sendCommand("OFF")
                                        FrontRoom_Timer = null
                                }
                        ]
                }
        }
}
end

[rossko57]

@dastrix80

I can move that check to the timer section.

The way you had your original rule, it blocked the turn-light-on motion response if the current dim > 31.
Presumably that was desirable, because you didn’t want the motion action dimming the lights after “manual” setting.
You might want to reintroduce that check as well rather than instead of.