[SOLVED] Advice needed with a rule

Hi,
Need help in modifying this rule.

So the rule starts a timer every time kodi screensaver is ON, and switches off TV if still ON. myKodi_screensaver gets updated every minute by cron job script.

This rule works perfectly:

rule "KODI Screen Saver"
when 
    Item myKodi_screensaver received update
then
if(myKodi_screensaver.state == ON && Television.state == ON && timerKodi === null)
{logInfo("kodi.rules", "LGTV_ON - timer started")
timerKodi = createTimer(now.plusMinutes(8))
 [ if(myKodi_screensaver.state == ON && Television.state == ON)
 {Television.sendCommand(OFF)
 logInfo("kodi.rules", "LGTV_OFF - TV has powered off")}
 timerKodi = null]}
else if(myKodi_screensaver.state == OFF && timerKodi !== null)
{logInfo("kodi.rules", "LGTV_ON - timer canceled")
 timerKodi.cancel
 timerKodi = null}
end

However, since the item is updated every minute, if you start using kodi during the last minute of the timer, it will never get cancelled and TV will be switched off.
For that purpose I want to manually trigger kodi screensaver update status script (via switch KODIScreen).
I would like to run that switch AFTER the timer expired (to update the item and verify screensaver still ON), and then if screensaver state changed to OFF I would like the rule to go to cancel the timer, but that does not happen, it still goes through and fires…

any advice how to make it happen?

rule "KODI Screen Saver"
when 
    Item myKodi_screensaver received update
then
if(myKodi_screensaver.state == ON && Television.state == ON && timerKodi === null)
{logInfo("kodi.rules", "LGTV_ON - timer started")
timerKodi = createTimer(now.plusMinutes(8))
 [KODIScreen.sendCommand(ON)
 if(myKodi_screensaver.state == ON && Television.state == ON)
 {Television.sendCommand(OFF)
 logInfo("kodi.rules", "LGTV_OFF - TV has powered off")}
 timerKodi = null]}
else if(myKodi_screensaver.state == OFF && timerKodi !== null)
{logInfo("kodi.rules", "LGTV_ON - timer canceled")
 timerKodi.cancel
 timerKodi = null}
end

EDIT: got it working it seems by just adding a bracket around the item {KODIScreen}
working rule:

rule "KODI Screen Saver"
when 
    Item myKodi_screensaver received update
then
if(myKodi_screensaver.state == ON && Television.state == ON && timerKodi === null)
{logInfo("kodi.rules", "LGTV_ON - timer started")
timerKodi = createTimer(now.plusMinutes(2))
 [{KODIScreen.sendCommand(ON)}
 if(myKodi_screensaver.state == ON && Television.state == ON)
 {Television.sendCommand(OFF)
 logInfo("kodi.rules", "LGTV_OFF - TV has powered off")}
 timerKodi = null]}
else if(myKodi_screensaver.state == OFF && timerKodi !== null)
{logInfo("kodi.rules", "LGTV_ON - timer canceled")
 timerKodi.cancel
 timerKodi = null}
end

Once again proven, morning is smarter than the night :slight_smile:

There is no reason why that should have changed anything. Putting the curly brackets there is basically a noop (doesn’t actually do anything).

I don’t want you or others thinking that using curly brackets in this way is a tool to be used to solve problems.

Why that fixed it I can’t say. This code is really hard to read. See Coding Conventions in Rules for details but here using consistent indentation would go a long way towards making the code clearer.

rule "KODI Screen Saver"
when 
    Item myKodi_screensaver received update
then
    if(myKodi_screensaver.state == ON
       && Television.state == ON 
       && timerKodi === null) {
        logInfo("kodi.rules", "LGTV_ON - timer started")
        timerKodi = createTimer(now.plusMinutes(8)) [
            KODIScreen.sendCommand(ON)
            if(myKodi_screensaver.state == ON && Television.state == ON) {
                Television.sendCommand(OFF)
                logInfo("kodi.rules", "LGTV_OFF - TV has powered off")
            }
            timerKodi = null
        ]
    }
    else if(myKodi_screensaver.state == OFF && timerKodi !== null) {
        logInfo("kodi.rules", "LGTV_ON - timer canceled")
        timerKodi.cancel
        timerKodi = null
    }
end

Now it is much more apparent which blocks of code go with what context. And now that we can see, there is nothing apparently wrong about this rule. So one has to wonder if the Items were in the expected states when you were testing before. It’s a good idea to log that stuff out. It’s also a good idea to add a catch-all else clause so you can see that the rule ran but none of the conditions in the if/else if matched.

really don’t know why it makes a difference then…
i was thinking, since that item is exec item that runs a script that runs ssh to another machine, it takes him too much time to finish, so the item in question does not get updated in time and the rule just proceeds with execution…but then it started working fine with added brackets…
maybe brackets waits for .sh script to finish and echo the result?
or maybe brackets slow him down just enough so the mykodi_screensaver gets updated :joy:

No, sending a command works exactly as you first describe it. It’s asynchronous. Once the ON command event is put on the event bus that line returns and the rest of the code in that timer will run immediately. It does not wait around for KODIScreen to process the ON command before continuing. It is almost guaranteed that the if(myKodi_screensaver... lines execute a long time before anything triggered to happen by the ON command to KODIScreen even has a chance to start, let alone finish.

I suspect what happened is the Items are already in the expected state this morning and yesterday they were not. But adding the { } does not force the rule to wait for KODIScreen to process that ON command before continuing. The addition of those { } would slightly increase the amount of time required to parse the rule but it’s not going to add enough of a delay in processing to account for an exec and ssh.

so, in case it will end up not working as expected after few more tests, should I add Thread sleep 1000 for example, before if part…so it gives him the chance to complete the script execution and update the screensaver item…?

That’s one potential option, though are you sure a second is long enough? ssh can take some time to establish sometimes and who knows how long the script being called will take?

It would be better to structure this such that it’s event based. Can you set up a separate rule that triggers when KODIScreen changes from ON to OFF and put your if code in there? That would be the ideal way to handle this since it doesn’t matter how long it takes.

sorry it’s not ssh, its curl. script connects to kodi api, then to rest api. so its executed very fast, but when i put thread sleep 1000, rule crashes…

2022-08-11 18:20:00.633 [WARN ] [ore.internal.scheduler.SchedulerImpl] - Scheduled job failed and stopped
java.lang.reflect.UndeclaredThrowableException: null
        at com.sun.proxy.$Proxy294.apply(Unknown Source) ~[?:?]
        at org.openhab.core.model.script.actions.ScriptExecution.lambda$0(ScriptExecution.java:82) ~[?:?]
        at org.openhab.core.internal.scheduler.SchedulerImpl.lambda$12(SchedulerImpl.java:184) ~[?:?]
        at org.openhab.core.internal.scheduler.SchedulerImpl.lambda$1(SchedulerImpl.java:87) ~[?:?]
        at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:515) [?:?]
        at java.util.concurrent.FutureTask.run(FutureTask.java:264) [?:?]
        at java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.run(ScheduledThreadPoolExecutor.java:304) [?:?]
        at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1128) [?:?]
        at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:628) [?:?]
        at java.lang.Thread.run(Thread.java:829) [?:?]
Caused by: org.openhab.core.model.script.engine.ScriptExecutionException: The name '<unkown>' cannot be resolved to an item or type; line 0, column 0
        at org.openhab.core.model.script.interpreter.ScriptInterpreter.invokeFeature(ScriptInterpreter.java:141) ~[?:?]
        at org.eclipse.xtext.xbase.interpreter.impl.XbaseInterpreter._doEvaluate(XbaseInterpreter.java:1008) ~[?:?]
        at org.eclipse.xtext.xbase.interpreter.impl.XbaseInterpreter._doEvaluate(XbaseInterpreter.java:971) ~[?:?]
        at org.eclipse.xtext.xbase.interpreter.impl.XbaseInterpreter.doEvaluate(XbaseInterpreter.java:247) ~[?:?]
        at org.openhab.core.model.script.interpreter.ScriptInterpreter.doEvaluate(ScriptInterpreter.java:226) ~[?:?]
        at org.eclipse.xtext.xbase.interpreter.impl.XbaseInterpreter.internalEvaluate(XbaseInterpreter.java:227) ~[?:?]
        at org.eclipse.xtext.xbase.interpreter.impl.XbaseInterpreter.evaluateArgumentExpressions(XbaseInterpreter.java:1222) ~[?:?]
        at org.eclipse.xtext.xbase.interpreter.impl.XbaseInterpreter._invokeFeature(XbaseInterpreter.java:1152) ~[?:?]
        at org.eclipse.xtext.xbase.interpreter.impl.XbaseInterpreter.invokeFeature(XbaseInterpreter.java:1098) ~[?:?]
        at org.openhab.core.model.script.interpreter.ScriptInterpreter.invokeFeature(ScriptInterpreter.java:151) ~[?:?]
        at org.eclipse.xtext.xbase.interpreter.impl.XbaseInterpreter._doEvaluate(XbaseInterpreter.java:878) ~[?:?]
        at org.eclipse.xtext.xbase.interpreter.impl.XbaseInterpreter.doEvaluate(XbaseInterpreter.java:243) ~[?:?]
        at org.openhab.core.model.script.interpreter.ScriptInterpreter.doEvaluate(ScriptInterpreter.java:226) ~[?:?]
        at org.eclipse.xtext.xbase.interpreter.impl.XbaseInterpreter.internalEvaluate(XbaseInterpreter.java:227) ~[?:?]
        at org.eclipse.xtext.xbase.interpreter.impl.XbaseInterpreter._doEvaluate(XbaseInterpreter.java:475) ~[?:?]
        at org.eclipse.xtext.xbase.interpreter.impl.XbaseInterpreter.doEvaluate(XbaseInterpreter.java:251) ~[?:?]
        at org.openhab.core.model.script.interpreter.ScriptInterpreter.doEvaluate(ScriptInterpreter.java:226) ~[?:?]
        at org.eclipse.xtext.xbase.interpreter.impl.XbaseInterpreter.internalEvaluate(XbaseInterpreter.java:227) ~[?:?]
        at org.eclipse.xtext.xbase.interpreter.impl.XbaseInterpreter._doEvaluate(XbaseInterpreter.java:475) ~[?:?]
        at org.eclipse.xtext.xbase.interpreter.impl.XbaseInterpreter.doEvaluate(XbaseInterpreter.java:251) ~[?:?]
        at org.openhab.core.model.script.interpreter.ScriptInterpreter.doEvaluate(ScriptInterpreter.java:226) ~[?:?]
        at org.eclipse.xtext.xbase.interpreter.impl.XbaseInterpreter.internalEvaluate(XbaseInterpreter.java:227) ~[?:?]
        at org.eclipse.xtext.xbase.interpreter.impl.XbaseInterpreter.evaluate(XbaseInterpreter.java:213) ~[?:?]
        at org.eclipse.xtext.xbase.interpreter.impl.ClosureInvocationHandler.doInvoke(ClosureInvocationHandler.java:47) ~[?:?]
        at org.eclipse.xtext.xbase.interpreter.impl.AbstractClosureInvocationHandler.invoke(AbstractClosureInvocationHandler.java:30) ~[?:?]
        ... 10 more

You’ve referenced something unknown inside the body of the Timer. Maybe you used the wrong syntax for Thread::sleep()

I found a nice solution, i changed the timer to 482 seconds instead 8 minutes. since item mykodi_screensaver gets updated exactly every minute by cron…the timer will end 2 seconds after last update of the item, so he will always have the latest state of it.
it’s a very small chance that you decide to use kodi in the last 2 seconds of the timer :slight_smile: