Keep and replace Switch state in Rule

Setup, Configuration and Use Beginners
1

Openhab 4.0.4

Hi All,

I’m sure this is a simple one but I’m really struggling.
I want to write a rule that:-

  1. When the doorbell is rung:-
  2. The rule remembers / holds the state of a Switch (whether it is on or off)
  3. Turns the switch on for a period of time
  4. returns the Switch to whichever state it was in before 3)

I’ve no issue with the timer logic, but I’m struggling with what seems to be the simplest part - setting the switch back to it’s initial state.

This is the rule:-

// Doorbell related rules
rule "Someone has rung the doorbell"
when 
	Item RingDoorbell changed from OFF to ON
then   
    sendBroadcastNotification("Someone rang the doorbell...")
    logInfo("org.openhab","Someone rang the doorbell...")
    
    RingDoorbell.postUpdate(OFF)

    if ( doorbellTimer === null || doorbellTimer.hasTerminated() ) {
    	    
        logInfo("org.openhab","... and it's dark")
		
        // Get the initial state of the items we're going to change
        var SwitchItem initialState_SP_Lights_Garage

        initialState_SP_Lights_Garage = SP_Lights_Garage //.state as Switch

        logInfo("org.openhab","initialState_SP_Lights_Garage: " + initialState_SP_Lights_Garage)

        SP_Lights_Garage.sendCommand(ON)

		if (doorbellTimer !== null) {
			doorbellTimer.cancel
			doorbellTimer = null
		}

		// Wait for 5 minutes
		doorbellTimer = createTimer(now.plusMinutes(1), [|
			// RETURN STUFF TO PREVIOUS STATE
            logInfo("org.openhab","Returning items to initial state")
            if (initialState_SP_Lights_Garage.state == ON) {
                logInfo("org.openhab","SP_Lights_Garage.sendCommand(ON)")
                SP_Lights_Garage.sendCommand(ON)
            } else {
                logInfo("org.openhab","SP_Lights_Garage.sendCommand(OFF)")
                SP_Lights_Garage.sendCommand(OFF)
            }
                
		    doorbellTimer = null   // reset the timer
		])

	} else {
		doorbellTimer.reschedule(now.plusMinutes(10))
	}
end

and this is the log output:-

2023-12-20 08:51:03.019 [INFO ] [penhab.core.model.script.org.openhab] - Someone rang the doorbell...

2023-12-20 08:51:03.022 [INFO ] [penhab.core.model.script.org.openhab] - ... and it's dark

2023-12-20 08:51:03.024 [INFO ] [penhab.core.model.script.org.openhab] - initialState_SP_Lights_Garage: SP_Lights_Garage (Type=SwitchItem, State=OFF, Label=Outside Garage Lights, Category=light)

2023-12-20 08:51:03.028 [INFO ] [penhab.core.model.script.org.openhab] - TP_Link_KP105_1.sendCommand('ON')

2023-12-20 08:52:03.027 [INFO ] [penhab.core.model.script.org.openhab] - Returning items to initial state

2023-12-20 08:52:03.032 [INFO ] [penhab.core.model.script.org.openhab] - SP_Lights_Garage.sendCommand(ON)

So whist the initialState_SP_Lights_Garage variable appears to capture the initial state as being OFF, it fails to return the SP_Lights_Garage Switch to being in a state of OFF at the end (and the lights physically stay on). There seems to be something wrong here:-

if (**initialState_SP_Lights_Garage.state == ON**) {
    logInfo("org.openhab","SP_Lights_Garage.sendCommand(ON)")
    SP_Lights_Garage.sendCommand(ON)
} else {
    logInfo("org.openhab","SP_Lights_Garage.sendCommand(OFF)")
    SP_Lights_Garage.sendCommand(OFF)
}

Would really appreciate any help / pointers as this one is driving me mad!
Many thanks
Steve

2

OK, fixed it - so posting in case helps others (although still think code could be a lot more elegant).

Working code:-

rule "Someone has rung the doorbell"
when 
	Item RingDoorbell changed from OFF to ON
then   
    sendBroadcastNotification("Someone rang the doorbell...")
    logInfo("org.openhab","Someone rang the doorbell...")
    
    RingDoorbell.postUpdate(OFF)

    if ( doorbellTimer === null || doorbellTimer.hasTerminated() ) {
    	    
        // Get the initial state of the items we're going to change
        var SwitchItem initialState_SP_Lights_Garage

        logInfo("org.openhab","SP_Lights_Garage.getState: " + SP_Lights_Garage.getState)
        initialState_SP_Lights_Garage = SP_Lights_Garage.getState

        logInfo("org.openhab","initialState_SP_Lights_Garage: " + initialState_SP_Lights_Garage)

        SP_Lights_Garage.sendCommand(ON)

		if (doorbellTimer !== null) {
			doorbellTimer.cancel
			doorbellTimer = null
		}

		// Wait for 5 minutes
		doorbellTimer = createTimer(now.plusMinutes(1), [|
			// RETURN STUFF TO PREVIOUS STATE
            logInfo("org.openhab","Returning items to initial state")
            logInfo("org.openhab","SP_Lights_Garage.getState: " + SP_Lights_Garage.getState)
            logInfo("org.openhab","initialState_SP_Lights_Garage: " + initialState_SP_Lights_Garage)
            //SP_Lights_Garage.sendCommand(initialState_SP_Lights_Garage as state)
            
            if (initialState_SP_Lights_Garage == ON) {
                logInfo("org.openhab","SP_Lights_Garage.sendCommand(ON)")
                SP_Lights_Garage.sendCommand(ON)
            } else {
                logInfo("org.openhab","SP_Lights_Garage.sendCommand(OFF)")
                SP_Lights_Garage.sendCommand(OFF)
            }
                
		    doorbellTimer = null   // reset the timer
		])

	} else {
		doorbellTimer.reschedule(now.plusMinutes(10))
	}
end

Note in particular:-
initialState_SP_Lights_Garage = SP_Lights_Garage.getState
So the initial state does not equal the whole object (as this means changes to SP_Lights_Garage are reflected always in initialState_SP_Lights_Garage which somewhat defeats the purpose!)
I still think this bit could / should be cleaner:-

            if (initialState_SP_Lights_Garage == ON) {
                logInfo("org.openhab","SP_Lights_Garage.sendCommand(ON)")
                SP_Lights_Garage.sendCommand(ON)
            } else {
                logInfo("org.openhab","SP_Lights_Garage.sendCommand(OFF)")
                SP_Lights_Garage.sendCommand(OFF)
            }

Thought this would be better but doesn’t work:-
SP_Lights_Garage.sendCommand(initialState_SP_Lights_Garage as state)
But code does at least now work…

3

Where did you get this syntax? It’s not incorrect but it’s an unusual way to do it in Rules DSL. Normally you’d just use SP_Lights_Garage.state.

As of OH 4, (and a little before) with the introduction of the cache it’s better to use privateCache to store variables rather than a global variable. It’s particularly useful for Timers as any stored in the cache when a rule is unloaded gets cancelled instead of becoming orphaned and throwing an error later on.

This is unnecessary. You’ve already established the Timer is null or terminated. If it’s terminated there’s nothing to cancel and you will be reassigning doorbellTimer later so there’s no need to set it to null here.

Just sendCommand initialState_SP_Lights_Garage. There is no need for the if else here.

That doesn’t work because you cannot send a State as a Command. If you just used SP_Lights_Garage.sendCommand(initialState_SP_Lights_Garage) it would have worked.

Finally, if you have more than one Item to store and restore, there are Rule actions built for that. Actions | openHAB. This sidesteps this whole block of code in the first place.

Putting it all together:

rule "Someone has run the doorbell"
when
    Item RingDoorbell changed from OFF to ON
then
    logInfo("doorbell","Someone rang the doorbell...") // Always put log statements first so they don't get suppressed when something goes wrong
    // normally you'd want to use something specific to this rule for the log name, I've changed it to "doorbell"
    sendBroadcastNotification("Someone rang the doorbell...")

    RingDoorbell.postUpdate(OFF)

    var doorbellTimer = privateCache.get('timer')
    if( doorbellTimer === null || doorbellTimer.hasTerminated) {
        logInfo("doorbell","SP_Lights_Garage.getState: " + SP_Lights_Garage.state)
        privateCache.put('initialStates', storeStates(SP_Lights_garage)) // add additional Items as additional arguments to storeStates

        SP_Lights_Garage.sendCommand(ON)

        privateCache.put('timer', createTimer(now.plusMinutes(1), [ |
            // RETURN STUFF TO PREVIOUS STATE
            logInfo("doorbell","Returning items to initial state")
            logInfo("doorbell","SP_Lights_Garage.state: " + SP_Lights_Garage.state)
            val initialStates = privateCache.get('initialStates')
            logInfo("doorbell","initialStates: " + initialStates.toString())

            restoreStates(initialStates)
            privateCache.put('timer', null)
        ]))
    }
    else {
        privateCache.get('timer').reschedule(now.plusMinutes(10))
    }
end

By using storeStates and restoreStates above, if you have more than one Item to store and restore all you need to do is add them to the one call to storeStates and leave the rest of the rule unchanged.

By using the cache, you don’t need to maintain global variables making the rule more self contained and easier to migrate to a managed rule in the future if you desire. It also avoids errors from orphaned Timers as the Timer is automatically cancelled for you when the rule gets unloaded.

By giving your log statements a unique logger name (first argument to logInfo you can more easily identity and isolate the logs from this specific rule from all other logs.

1 Like
4

Hi Rich

I see you responding an awful lot on these forums. Your responses are always very detailed, clear and helpful. Thank you! (for this one and all the others too!).

I will review all of your points and let you know how I get on

Thanks
Steve

5

Hi Rich,

Thanks for your excellent help. All works very nicely now and clearly much better / cleaner code (and you predicted / guessed I’d want to add other states too - which was correct and very helpful).

One thing that I haven’t got working is the privateCache though? I swapped privateCache for myCache with this import at the top of the rules file and it worked fine:-

import java.util.Map

val Map<String, Object> myCache = newHashMap

rule "Someone has run the doorbell"

But trying to use privateCache threw an error:-

2023-12-21 11:57:07.221 [ERROR] [internal.handler.ScriptActionHandler] - Script execution of rule with UID 'ring2-1' failed: The name 'privateCache' cannot be resolved to an item or type

Guess I’m missing an import or similar?

Thanks
Steve

6

I am pretty sure that the caches were added to Rules DSL in 4.0 but perhaps it wasn’t until 4.1.

The 4.0 docs show them as being supported though: