Creating multiple timers switching same light instead of different ones (JSS)

[arjan-io (Arjan)]

I am trying the ‘new’ Rules engine so JSScripting and take a step by step approach to filter out any issues. So far so good but I’ve run into a problem.

Goal:
Generate a random list of switches. For each switch, create a timer and then send an ON command to the corresponding switch when the timer is up. The length of the timer is also randomly generated.

Problem:
The list of swicthes is indeed generated in random order. This is confirmed by the logs. Further, timers are created at random intervals, also confirmed by the logs. However, when the timer is up, the ON command is send to the same item over and over again and not to the next item. In other words, it looks like the timer is created for the same item while the log part that states “Winner is…” is moving through the list of items instead of sticking to the same item.

Code:

        function startTimer(){
          for (var i = 0; i < maxLights; i++) {
            var rand = makeUniqueRandom();
            var randomSwitch = switches[rand];
            var timeoutID = setTimeout(function(){  // If timer is up
              randomSwitch.sendCommand("ON"),  // Switch light on
              console.log("Timer is up for", randomSwitch.name)  // Log entry to debug code
            }, getRandomInt(0, startDiff) * 60000);  //Set timeout by generating a random minute number and convert to milli seconds
            var temp = timeoutID.toString();
            console.log("Winner is", randomSwitch.name, ". TimerID is", temp.split('.')[7]);
          }
        }

        var test = startTimer();

Logs:

2022-01-12 19:32:01.341 [INFO ] [org.openhab.automation.script       ] - Generated random time is 4 mins.
2022-01-12 19:32:01.351 [INFO ] [org.openhab.automation.script       ] - Winner is wandverlichting_gFloor_switch . TimerID is TimerImpl@77639552
2022-01-12 19:32:01.352 [INFO ] [org.openhab.automation.script       ] - Generated random time is 26 mins.
2022-01-12 19:32:01.354 [INFO ] [org.openhab.automation.script       ] - Winner is plafondverlichtingAchter_fFloor_switch . TimerID is TimerImpl@5f57d4f9
2022-01-12 19:32:01.356 [INFO ] [org.openhab.automation.script       ] - Generated random time is 28 mins.
2022-01-12 19:32:01.358 [INFO ] [org.openhab.automation.script       ] - Winner is plafondlampenVoor_fFloor_switch . TimerID is TimerImpl@564c02c4
2022-01-12 19:32:01.359 [INFO ] [org.openhab.automation.script       ] - Generated random time is 20 mins.
2022-01-12 19:32:01.361 [INFO ] [org.openhab.automation.script       ] - Winner is hoeklamp_fFloor_switch . TimerID is TimerImpl@43233cfb
2022-01-12 19:32:01.362 [INFO ] [org.openhab.automation.script       ] -
2022-01-12 19:36:01.365 [DEBUG] [org.openhab.automation.script.items ] - Sending command ON to hoeklamp_fFloor_switch
2022-01-12 19:36:01.368 [INFO ] [org.openhab.automation.script       ] - Timer is up for hoeklamp_fFloor_switch
2022-01-12 19:52:01.363 [DEBUG] [org.openhab.automation.script.items ] - Sending command ON to hoeklamp_fFloor_switch
2022-01-12 19:52:01.365 [INFO ] [org.openhab.automation.script       ] - Timer is up for hoeklamp_fFloor_switch
2022-01-12 19:58:01.356 [DEBUG] [org.openhab.automation.script.items ] - Sending command ON to hoeklamp_fFloor_switch
2022-01-12 19:58:01.357 [INFO ] [org.openhab.automation.script       ] - Timer is up for hoeklamp_fFloor_switch
2022-01-12 20:00:01.358 [DEBUG] [org.openhab.automation.script.items ] - Sending command ON to hoeklamp_fFloor_switch
2022-01-12 20:00:01.361 [INFO ] [org.openhab.automation.script       ] - Timer is up for hoeklamp_fFloor_switch

Any help greatly appreciated.

[higgers (Steve Higton)]

I see that the “Timer is up…” message refers to the last “Winner is…” timer item that was created. Could it be that although multiple timers are being created the variables passed to the timers are being overwritten each time? I.e., multiple timers are created but all will process the item most recently processed.

[arjan-io (Arjan)]

That indeed seems to be the case. Good point! Did not notice it but indeed, going back through the logs you are right.

But how do I get it in a way that this is not the case anymore? Should I use a different variable for timeoudID maybe? Ie timeoutID1, timeoutID2 etc?

I was hoping that different timeoutIDs would already solve this issue…

[higgers (Steve Higton)]

Could the contents of the timeoutID variable be moved into the array? If the array is declared at the top of a js rules file then it persists between invocations of the rule. Instead of a 1D array you’d have a 2D array, the first dimension would be the random index, the second dimension would contain details about each timer such as the setTimeout function.

Alternatively you could use the cache functions as described here.

[rossko57]

Comment - this feels very like this discussion on scoping functions, which we never really got to the bottom of

Help with context of 'this' in javascript rules

I’ve setup a rule that notifies me when I left the bathroom window open cause I often forget to close it. The rule should remind me after half an hour and then every 15 minutes if the window still has not been closed. The following code is mostly working but apparently there is one flaw that must have something to do with the context of the keyword ‘this’ in the code. What happens is: I open the window After 30 minutes I get a notification that the window is open for 30 minutes I close the wi…

[higgers (Steve Higton)]

It does seem related, yes. Timers seem to be a bit tricky to handle.

I recently wrote some JS code to turn lights on if movement was detected and turn them off after a period of time. I started using the cache functions but changed to managing the timers in a 2d array so I could list the contents of the array and store several pieces of data about each timer such as the room name, time it started, etc.

[rossko57]

I’ll bet it works fine if you do it all inline, and don’t spawn the timers from a function.

[pacive (Anders Alfredsson)]

I think this might be related:

block - JavaScript | MDN

Tl;Dr: JavaScript only have global scope and function scope with the var keyword, to have block scope you need to use let instead.

Not sure which ecmascript version jsscripting uses, but if let is not available you should be able to move the declarations to the timer body.

[higgers (Steve Higton)]

@rossko57

I’ll bet it works fine if you do it all inline, and don’t spawn the timers from a function.

Forgive my pedantry but do you mean any function or the function that was called by the timer (a timer rescheduling itself for example)?

[rossko57]

@arjan-io

setTimeout(function(){...}

I’m thinking of that as “the timer”. It would be interesting to call/create that from the base rule, instead of from an intermediate function. There’s only one of ‘something’, so it gets shared and overwritten, but not too clear what to me.

[arjan-io (Arjan)]

First of all, thanks for your replies and thinking with me on this issue.

@rossko57

I’ll bet it works fine if you do it all inline, and don’t spawn the timers from a function.

I have dropped the function startTimer(){..} bit. This is what you refer to as inline right? Unfortunately it still persists, the problem does. Please let me know if you mean something differen with ‘inline’
Log:

2022-01-13 16:40:30.135 [INFO ] [org.openhab.automation.script       ] - Generated random time is 16 mins.
2022-01-13 16:40:30.143 [INFO ] [org.openhab.automation.script       ] - Winner is hoeklamp_fFloor_switch . TimerID is TimerImpl@7e7dc062
2022-01-13 16:40:30.146 [INFO ] [org.openhab.automation.script       ] - Generated random time is 30 mins.
2022-01-13 16:40:30.148 [INFO ] [org.openhab.automation.script       ] - Winner is plafondverlichtingAchter_fFloor_switch . TimerID is TimerImpl@1cde7c0e
2022-01-13 16:40:30.150 [INFO ] [org.openhab.automation.script       ] - Generated random time is 4 mins.
2022-01-13 16:40:30.152 [INFO ] [org.openhab.automation.script       ] - Winner is wandverlichting_gFloor_switch . TimerID is TimerImpl@300a6423
2022-01-13 16:40:30.154 [INFO ] [org.openhab.automation.script       ] - Generated random time is 10 mins.
2022-01-13 16:40:30.156 [INFO ] [org.openhab.automation.script       ] - Winner is plafondlampenVoor_fFloor_switch . TimerID is TimerImpl@1a573464
2022-01-13 16:40:30.157 [INFO ] [org.openhab.automation.script       ] -
2022-01-13 16:44:30.171 [DEBUG] [org.openhab.automation.script.items ] - Sending command ON to plafondlampenVoor_fFloor_switch
2022-01-13 16:44:30.174 [INFO ] [org.openhab.automation.script       ] - Timer is up for plafondlampenVoor_fFloor_switch

@pacive

I think this might be related:

How and where would you implement let in my code in this case? At the part where it says var timeoutID = setTimeout(function(){...} and change that to let timeoutID = setTimeout(function(){...}? I’ve read the MDN-link you provided and I do get the concept but unsure as where to put it…

@higgers

Could the contents of the timeoutID variable be moved into the array?

I don’t see why not but I am not that good in JS. This would be an array with key->value I guess? Or am I thinking to complicated here? Maybe you give a little hint

[pacive (Anders Alfredsson)]

@arjan-io

How and where would you implement let in my code in this case?

randomswitch should be defined using let, to prevent it being overwritten in the next iteration of the loop.

Edit: since randomSwitch is defined using var it’s scope is the startTimer function, so it gets overwritten on every iteration of the loop. When the timers execute, all will reference this variable, which now holds the last item in the loop. If defined using let it has block scope, so it will be a different variable for each loop iteration and each timer will have a different item.

[arjan-io (Arjan)]

I give it a go but it sort of should be overwritten right? We want to create timers for all switches, not just the one…

[Edit] I see now, referring to the docs, this is sort of the var x = 2;-bit. Testing as we speak

[higgers (Steve Higton)]

@pacive

randomswitch should be defined using let, to prevent it being overwritten in the next iteration of the loop.

Edit: since randomSwitch is defined using var it’s scope is the startTimer function, so it gets overwritten on every iteration of the loop. When the timers execute, all will reference this variable, which now holds the last item in the loop. If defined using let it has block scope, so it will be a different variable for each loop iteration and each timer will have a different item.

Won’t the variables then be destroyed when execution of the block (in this case the for loop) ends? If they’re not destroyed then don’t we have memory leak? How will the JS engine know that these variable contain active timers and the memory for them should/shouldn’t be reclaimed/garbage collected?

I barely know any JS so don’t know enough about the technicalities to comment.

[higgers (Steve Higton)]

@arjan-io

I don’t see why not but I am not that good in JS. This would be an array with key->value I guess? Or am I thinking to complicated here? Maybe you give a little hint

I have code that creates a timer:

        timer = actions.ScriptExecution.createTimerWithArgument(now.plusSeconds(duration), timerName, timerOverOff);
        // Add timer object to array
        lightingTimerAddToArray(timerName, timer, roomGroup);

timerOverOff is the function that is called when the timer finishes.

The lightingTimerAddToArray function is:

function lightingTimerAddToArray(timerName, timer, roomGroup) {
    var logger = log(this.ruleUID);
    logger.info('LIGHTING: Adding timer to array timerName = ({}), timer = ({}), roomGroup = ({}).', timerName, timer, roomGroup.name);

    lightingTimers.push([timerName, timer, roomGroup]);
}

It uses the push array method to push an array that is three elements long into the lightingTimers array.

There are other functions to check for the existence of a timer in the array and remove elements from the array.

[pacive (Anders Alfredsson)]

@higgers

Won’t the variables then be destroyed when execution of the block (in this case the for loop) ends? If they’re not destroyed then don’t we have memory leak?

Not sure how it’s implemented, but the variable should be kept in memory as long as there’s a reference to it, which it is in the timer body. When the timer object is disposed the variable will be garbage collected. But I could be wrong, haven’t used jsscripting personally. Also don’t know how/if setTimeout relates to the openhab timer scheduler.

[rlkoshak (Rich Koshak)]

@arjan-io

However, when the timer is up, the ON command is send to the same item over and over again and not to the next item.

It’s sending the command to the last value that randomSwitch had.

One way to deal with this is to use function generators.

function timerFuncGenerator(item) {
    return () => {
        item.sendCommand('ON');
        console.log('Timer is up for ', item.name);
    }
}

...
    var timeoutID = setTimeout(timerFuncGenerator(randomSwitch));

@pacive

to have block scope you need to use let instead.

There are some problems with let and const when writing in UI rules for now. The problem is the Script is only created once and then reused. That’s why setting something to this persists between runs of the rule. But that means the second time the rule runs it will see that variable or constant defined with let or const will generate an error because a variable with that name already exists.

However, it might work in this case since it’s not in the global to the script scope but inside another block.

@pacive

Not sure which ecmascript version jsscripting uses

11

@higgers

I started using the cache functions but changed to managing the timers in a 2d array so I could list the contents of the array and store several pieces of data about each timer such as the room name, time it started, etc.

Store your 2d array in the cache.

Because of the limitations with let and const I describe above, the way that UI scripts are parsed and reused is almost certainly going to change in the nearish future. When that happens, the this trick to save variables between runs of the script will no longer work.

@higgers

How will the JS engine know that these variable contain active timers and the memory for them should/shouldn’t be reclaimed/garbage collected?

I’m not certain if GraalVM is garbage collected like that or not. But assuming it is, the timer will have pointers to those variables which will keep them from being garbage collected.

[higgers (Steve Higton)]

@rlkoshak

Store your 2d array in the cache.

For context and in case anyone else reading this thread finds it useful and in case you have any more advice :)…

At the moment I have the array defined outside of any rule at the very top of my lightingrules.js file like this:

let lightingTimers = [];

I also have a lightingfunctions.js file which processes the array. The lightingfunctions.js file is loaded into any rules in the lightingrules.js file that need it like this:

        var OPENHAB_CONF = java.lang.System.getenv("OPENHAB_CONF");
        load(OPENHAB_CONF+'/automation/lib/js/personal/commonfunctions.js');
        load(OPENHAB_CONF+'/automation/lib/js/personal/lightingfunctions.js');

No rules directly access the array, there are functions to interact with the array (add elements, check for existence, delete elements) so all I should need to do is load the array from the cache in any of these array functions and then write it back into the cache at the end of each function.

@rlkoshak

Because of the limitations with let and const I describe above, the way that UI scripts are parsed and reused is almost certainly going to change in the nearish future. When that happens, the this trick to save variables between runs of the script will no longer work.

Thanks for the info, I’ll keep any eye on future release notes and take backups before I upgrade.

[rlkoshak (Rich Koshak)]

@higgers

Thanks for the info, I’ll keep any eye on future release notes and take backups before I upgrade.

In that case never mind. The concern is with UI rules. the behavior is different in UI files and you can use let or const to your heart’s content.

[arjan-io (Arjan)]

@rlkoshak

One way to deal with this is to use function generators.

Would this be future proof for now? I need to read up on these generators but effectively, is ...FuncGenerator a special type of function?

However, it might work in this case since it’s not in the global to the script scope but inside another block.

The first signs are promising. The first run it did work as required. I’ll test it some more the upcoming days…

The array approach from @higgers also sounds very solid but I do need a bit more reading and test-coding to fill, check and empty the arrays as he described.