Design Pattern - Timer Management

This is a problem in the language. There is a similar problem in Python as well. Without going into a whole lot of detail, unlike in Rules DSL where each lambda function that gets passed to the timer gets a copy of the current set of variables, in JavaScript the lambda gets a pointer to the actual variable.

And because the context gets reused for every call to the script action, the event variable gets reused.

Combine those two and what you end up with is event.itemName in your Timer will show you what ever item most recently triggered the rule, not the name of the item that triggered the rule when the Timer was created.

To avoid this you need to “fix” the state of the variables before you create the function that gets passed to the timer so that it gets a copy instead of the pointing to the “real” events varaible.

Based on my research the “JavaScript” way to do this is through function generators.

this.OPENHAB_CONF = (this.OPENHAB_CONF === undefined) ? java.lang.System.getenv("OPENHAB_CONF") : this.OPENHAB_CONF;
load(OPENHAB_CONF+'/automation/lib/javascript/community/timerMgr.js');
load(OPENHAB_CONF+'/automation/lib/javascript/community/timeUtils.js'); // you don't need this import
this.ZonedDateTime = (this.ZonedDateTime === undefined) ? Java.type("java.time.ZonedDateTime") : this.ZonedDateTime;

var log = Java.type("org.openhab.core.model.script.actions.Log");

// Only create a new manager if one doesn't already exist or else it will be wiped out each time the rule runs
this.tm = (this.tm === undefined) ? new TimerMgr() : this.tm;


log.logWarn("Routinen", "Routine "+ event.itemName +" activated.  - Time: " + event.itemState );

var timerFuncGenerator = function(itemName) {
    return function() {
        log.logWarn("Test", "Timer " + event.itemName + " ended"); 
    }
}

this.tm.check(event.itemName,
              event.itemState,
              timerFuncGenerator(event.itemName)
             );

Another approach, which would require changes to timerMgr itself, might be to use createTimerWithArguments instead of createTimer but that is on my long and growing list of things to do.

Thanks, was afraid that could be the problem, I will test your suggested fix later.

But I don’t understand why thats only the problem with item event.itemName. The Timer expires at the correct time, so event.itemState should be correct, right?

I don’t think you understood my explanation.

event gets reused. All three timers are pointing at the same event variable. The sequence of events is as follows:

Event event.itemName value Action
rule is triggered Rolladen_Wohnzimmer_Morgens timer is created
rule is triggered Rolladen_Wohnzimmer_Abends another timer is created
rule is triggered Rolladen_Wohnzimmer_Mittags a third timer is created
first timer runs Rolladen_Wohnzimmer_Mittags the current value of event.itemName is Rolladen_Wohnzimmer_Mittags and that’s what is logged.

The timer sees the current and most recent value of event.itemName, not what it happened to be when the Timer was create.

It’s not only a problem with event.itemName. Any variable used by the timer’s function will have the same issue. Unless you force a copy to be made by using a function generator, all the timers will be using the exact same variable meaning that what ever value the variable had when the timer was created will be replaced by subsequent runs of the rule.

1 Like

Thanks for your patience, now I understood!
I changed the code and added the function generator, but the problems stays the same:

2021-04-05 18:31:31.036 [WARN ] [org.openhab.core.model.script.Test  ] - Timer Rolladen_Wohnzimmer_Mittags ended
2021-04-05 18:31:41.002 [WARN ] [org.openhab.core.model.script.Test  ] - Timer Rolladen_Wohnzimmer_Mittags ended
2021-04-05 18:31:51.001 [WARN ] [org.openhab.core.model.script.Test  ] - Timer Rolladen_Wohnzimmer_Mittags ended

Show the code.

I just copied your code from Design Pattern - Timer Management - #7 by rlkoshak.

Ok, there is a copy and paste error there. Log itemName, the name of the variable passed to timerFuncGenerator as an argument, not event.itemName.

1 Like

Great, thats working as expected! Now I will add the other variables and metadata the same way.

Dear Rich,
i tried to use your DP time_mgr and time_utils to create rules ising timers.
Unfortunately i am not able to get it run with your *tests.py files. I got errors that org.joda.time is not available (somewhere i read that OH3 does not support Joda any longer). I switched the output of time_utils.py to python and Java without succes.
Do you have a hint how to get your timers run in OH3?
Best regards
Pudermueller

As I’ve said a few posts up in this thread (don’t worry I don’t expect you to have read that), only the JavaScript version is written for and working in OH 3. I have no plans to update the Rules DSL version (and will probably remove it from the original post at some point when I have time) and the Python version only works in OH 2. I’ve not had time and likely will not have time to update any of the openhab-rules-tools Python libraries to OH 3. The challenging part is making them backwards compatible with Oh 2.5 so as not to cut off those who still use them there.

But PRs are always welcome for anyone willing to take a shot at it.

You have to migrate all the uses of Joda DateTime to java.time.ZonedDateTime.

1 Like

java.time.ZonedDateTime seems to work, at least when I print debug messages to the log I get valid time stamps for ZonedDateTime.now().plusMinutes(5).
But there is still something wrong with my timer - instead of delaying for the time configured, the timer function is called hundreds of times without delay until I get a RuntimeError: maximum recursion depth exceeded (Java StackOverflowError).

I think I follow the example in the helper-libraries, I use the ScriptExecution.createTimer version since @rlkoshak mentions in another post that Pythons threading.Time is unreliable for rescheduling timers, and I replaced Joda DateTime with java.time.ZonedDateTime.
So what’s wrong? Complete rule:

from core.rules import rule
from core.triggers import when
from core.log import logging, LOG_PREFIX
from core.actions import ScriptExecution
from java.time import ZonedDateTime

log = logging.getLogger("{}.colorscenes".format(LOG_PREFIX))
rotateTimer = None
timeInBetweenRotations = 10 # in minutes


@rule("Random Colors - Syncron", description="random changing colors synchronized over all color lights", tags=["color", "scene"])
@when("Item SceneColor received command 400")
def syncedColors(event):
    # 1. Check to see if the Rule has to run at all, if not exit
    if event.itemCommand != DecimalType(400): return
    log.info("Rule \"Random Colors - Syncron\" triggered")

    # 2. Calculate what needs to be done
    log.debug("if timer != null: cancel timer")
    global rotateTimer
    if rotateTimer is not None and not rotateTimer.hasTerminated():
        rotateTimer.cancel()
        log.debug("stopped timer")
    log.debug("if all color lights are off: do nothing")
    if ir.getItem("gColorLights").state == OFF:
        log.debug("all color lights are OFF -> do nothing")
        return
    
    # 3. Do it. Only do actions in like sendCommand in one place at the end of the Rule
    def timerFunc():
        global timeInBetweenRotations
        global rotateTimer
        log.debug("create random number between 1-360")
        color = 12 #TODO (Math::random * 360.0).intValue + 1
        log.debug("send commands to rooms")
        events.sendCommand("SceneColorBedroom", str(color))
        events.sendCommand("SceneColorLivingroom", str(color))
        if rotateTimer is None or rotateTimer.hasTerminated():
            log.debug("reschedule timer")
            rotateTimer = ScriptExecution.createTimer(ZonedDateTime.now().plusMinutes(timeInBetweenRotations), timerFunc())
    log.debug("call timer function")
    timerFunc()
    log.debug("Rule \"Random Colors - Syncron\" finished")

Okay - I found my mistake. Yes, I’m just getting started with Python syntax, so I was not aware about the difference of using timerFunc()or timerFunc in the createTimer call.

  • versions 1 (timerFunc()) calls the function - immediately
  • version 2 (timerFunc) threads the function as an argument, so the function will only be called when the timer is finished (Thats what we want here!)

That’s an important thing to understand in JavaScript too. In Rules DSL functions that can be passed around like any other variable are special and we call them “lambdas”. Any time you see [ | in Rules DSL you are seeing the creation of a lambda. And, as you should see by now, createTimer requires a lambda which is the function that gets called when the timer expires.

In JavaScript, regular old functions can already be passed around like any other variable. So you don’t have any special syntax to define such a function. It’s just a function. Therefore, in order to distinguish between when you want the function and when you want to call the function, the parens are used. As you accurately describe, passing the function itself is timerFunc without the parens. Calling the function though would use timerFunc() with the parens.

In Python there is also the concept of a lambda. But in this case, the lambda can only contain one line of code. Usually you want to do more than that so you would call some other function you’ve created from the lambda.

There is a further complication that comes in with JavaScript and in rare cases in Python too. Sometimes you need to create a function and explicitly pass the data it needs to it as an argument and then that function returns the function that gets called. I’ve seen this called a “functionGenerator”. the reason you might want to do this is if you just rely on the implicit existence of a variable to use it inside the function (e.g. if you use event.itemName in timerFunc), when the timer finally runs, event.itemName will be what ever it was the most recent time the rule was triggered, not the value it was when the timer was created.

In JavaScript you would do something like:

var timerFunGenerator = function(itemName) {
    return function() {
        // code that runs when the timer expires goes here
    };
};

ScriptExecution.createTimer( sometime, timerFunGenerator(event.itemName));

In Python it would look something like:

def timerFunc(itemName):
    # code that runs when the timer expires goes here

ScriptExecution.createTimer(sometime, lambda itemName=event.itemName: timerFunc(itemName))

Most of the time in Python this isn’t necessary. However, when iterating over a collection of something it’s required or else all the timerFuncs get called with what ever was the last item in the list.

3 Likes

@rlkoshak do you already have migrated your amazing library to oh3 conditions? Like the joda.time issue?
I want to use the timer management, but only find the oh2 version in your repo

The python version is OH 2.5 only. The JavaScript version is OH 3 only.

I’ve not the time right now to update the Python to 3, though prs are always welcome. The challenging part is too implement it in such a way as to not lose 2.5 support as well which makes it more than just replacing joda.

I tried to replace the relevant code by the new snippets. But it seems not as easy :slight_smile:
I’m struggling at the moment with python on my openhab3 system. Maybe it was the wrong decision to got with python, because at the moment it’s not fully supported and the acceptance in the community seems low.
Anyone has a best practice with oh3 python without timer management class?

That’s why I haven’t done it yet myself. :wink:

It’s important to remember that almost everything to do with a Timer is working with Java classes. Therefore, with some minor adjustments to account for different syntax in the various languages, creating and interacting with a Timer is going to be the same. They are all working with the same Java Timer and ZonedDateTime class.

Therefore, you should be able to use a JavaScript example for creating a Timer or a Rules DSL example for creating a Timer and, with some minor changes to account for differences in the language, it will work the same.

In Python’s case you need to import ScriptExecution and java.time.ZonedDateTime instead of Joda DateTime. Then call ScriptExecution.createTimer passing a ZonedDateTime Object as the time to run the Timer instead of a Joda DateTime.

To get by you could just simply remove the Joda DateTime stuff from time_utils.py which will break OH 2.5 support but you don’t need 2.5 support. See DateTime Conversion (openHAB 3.x). Even though that’s all Rules DSL, because it’s talking about the same Java Objects and Classes you should be able to translate that to Python relatively easily. Some of the examples there will work line for line.

I’ve been working on implementing the Timer Management. So far, I had both successes and failures. What I’m working currently on is switching on 2 lights when, at night time, the frontdoor or backdoor is opened. However, when either or both these two lights are already on, the timer should not be started for that specific light that is already on (winter case where one uses more lights). Else we’ll end up in darkness.

I first tried

var logger =
        Java.type("org.slf4j.LoggerFactory").getLogger("org.openhab.model.script.Rules.Lights");

        scriptExtension.importPreset("default");

        this.ZonedDateTime = (this.ZonedDateTime === undefined) ? Java.type("java.time.ZonedDateTime") : this.ZonedDateTime;

        // Load TimerMgr

        this.OPENHAB_CONF = (this.OPENHAB_CONF === undefined) ? java.lang.System.getenv("OPENHAB_CONF") : this.OPENHAB_CONF;

        load(OPENHAB_CONF+'/automation/lib/javascript/community/timerMgr.js');

        load(OPENHAB_CONF+'/automation/lib/javascript/community/timeUtils.js');

        // Create timer only when there isn't one, create function to xcan timer

        this.tm = (this.tm === undefined) ? new TimerMgr() : this.tm;

        // Get light states

        var wandState = items["koofSpots_gFloor_switch"]; var pilaarState = items["pilaarverlichting_gFloor_switch"];

        if(wandState == "OFF") {
          events.sendCommand("koofSpots_gFloor_switch", "ON");
          this.tm.check("wand", "30s", function() { events.sendCommand("koofSpots_gFloor_switch", "OFF");
            }
          );
        }

The 30s is just a test, or I’ll be waiting too long during the test phase. What I only accomplished in the rule above is: when the light is off (one light for simplicity), it goes on when the rule is triggered and switches off as the timer expires. So this does work.

Now for resetting or rescheduling the timer I tried

...
events.sendCommand("koofSpots_gFloor_switch", "ON");
          this.tm.check("wand", "30s", function() { events.sendCommand("koofSpots_gFloor_switch", "OFF");
            }
          );
...

And this does work as well. Each time an ON command is sent and the timer is rescheduled/reset for another, in this case, 30 seconds.
So I thought I’ll make it into this:

...
if(wandState == "OFF" || tm.hasTimer("wand")) {
          events.sendCommand("koofSpots_gFloor_switch", "ON");
          this.tm.check("wand", "30s", function() { events.sendCommand("koofSpots_gFloor_switch", "OFF");
            }
          );
        }
...

The idea is that the timer is rescheduled/reset to start again if the door is opened because that same code runs when 1) the light has already gone out or 2) when the timer is active. It also prevents from switching off lights that were on in the first place as the timer would never start. However, somehow this doesn’t work. It looks like the timer is cancelled when the door is opened for a second time, so when the timer has started counting down. The lights will then stay on indefinitely. If I do not open the door during the running timer, it works as advertised and the light goes off in 30s. This tells me that the timer is created on the first run.

Anyone who has a solution for this issue? Or maybe even better. Cleaner code/more elegant or generic way of coding?

There are a number of ways to deal with this.

1. Disable rules

Create a rule that triggers at sundown or an appropriate time.

Add a condition to only run this rule when the light is not already ON.

The Script Action is to enable your current rule.

Sometime at night trigger a third rule that disables your current rule again so it won’t run until the next day.

Neither of the two new rules require any code and can be managed through the UI alone.

This lets you simplify the code you have to write because you already know that if the code is running at all it’s late enough in the day and the light wasn’t already ON when the it became late enough. However it will violate DRY because you’ll have two of the same set of three rules for each light. but the simplicity of the code might make that worth the trade.

2. Condition Script

Add a trigger to the rule to run at the start of the time when the timer should be considered.

Add a Condition to prevent the rule from running outside of the allowed times of day.

Add a Script Condition that checks to see if the rule was triggered by an Item or not (event is undefined when it was triggered based on Time). If it wasn’t triggered based on an Item event, save the current states of the lights and return false. If it was triggered based on an Item event, check if the light in question was ON when the rule was triggered that first time and only return true if it was ON.

Again, your Script Action just has to set and manage the timer because we’ve already verified it needs to run.

The Script Condition would look something like

this.startStates = (this.startStates === undefined) ? { "koofSpots_gFloor_switch":"OFF",
                                                        "pilaarverlichting_gFloor_switch": "OFF" } : this.startStates;

// It's a time based trigger
if(this.event === undefined){
    // Record the current states of the lights
    this.startStates = { "koofSpots_gFloor_switch":items["koofSpots_gFloor_switch"],
                         "pilaarverlichting_gFloor_switch":items["pilaarverlichting_gFloor_switch"] };
    // Don't run the rule
    false;
}

// It's an Item based trigger
else {
    // Only run the rule if the light was OFF when the rule was triggered based on Time
    var lightSwitch = <some code to build the name of the light switch based on the door switch Item's name>;
    this.startStates(lightSwitch) == OFF;
}

The conditions will prevent the Script Action from running except in cases where a timer is required. It prevents it from running in all other cases.

3. Add conditions to the script action

This is basically the same as the above except instead of using a Script Condition put the variables and checks inside the Script Action.

Hi Rich,

Thanks once more for your elaborate answer. I am using your Time of Day pattern so that is an easy condition to use → Only run when it is EVENING or NIGHT. The starterState is a nice way of determining the rest of the condition required to run or not to run the timer. I’ll try and implement that in the next few days.