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

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

Sure. What problem do you anticipate with this?

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.

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)

They are exactly the same? scratching head

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.

1 Like

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

Okay. Which part of this is your issue?

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

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

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

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

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.

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.

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

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.

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

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?

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.

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


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.

No, that’s OK. Because the motion driven dim is always 30, if its less than 31 im happy for it to go off.

I only want it to stay ON if its above that.

Note that the last version of the rule posted does this regardless of the current state of it.
If you manually set “60”, the next motion will set it to “30”.

Im not following. Feel free to suggest a rule rossko that fixes the issue, I’m no expert.

The original rule worked perfectly for motion driven lights but would switch off, even if the dim was above 30