Ultimately, it’s reasonable to use setInterval in most cases. However as implemented here there are two problems that make it’s use problematic as written.
-
Previously created timers are not cleared before creating a new one. Consequently every time the rule runs a new timer is created and the reference to the old timer is lost and therefore never get cleared. Over time you can have dozens of intervals running every second.
-
Because a new timer is created every time the rule runs, after a few runs of the rule the chances that they conflict increases resulting in a Multithreaded exception.
If you store what setInterval returns into the cache, and only create a new interval if one doesn’t already exist, then both of these problems go away and the chances of a multithreaded exception goes way down.
cache.private.get('interval', () => setInterval(MySampleCollector, interval_time)); // only creates a new interval if one isn't already in the cache
...
clearInterval(cache.private.get('interval'));
cache.private.put('interval', null); // clear the cache so a new interval is created next time it's warranted
Managing the creation and clearing of the intervals should address the problem and make the multithreaded exception extremely unlikely to occur.
You would still want to do the same when using an OH Timer. However, if you did have a conflict, the locks on the OH Timer should avoid the multithreaded exception. But you’d still have dozens of orphan Timers running over time.
Using Blockly might be a good choice in that case. My fear is that @Jadblack is practicing cargo cult programming for this and will be unable to maintain this solution in the long run. With Blockly it could be more understandable for non-programmers.
Something like the following (warning, untested):
I tried to stick as closely as possible to your code above with the major differences being:
-
I use the existence of the looping timer as the flag to determine whether or not to do something when the rule is triggered instead of
windspeed_mnonitor_cycle_active -
I only create the one looping timer and only if it doesn’t already exist
-
I use a List instead of separate variables for each sample
The two places that say “MyItem” should be changed to use the Items in question.
The Looping Timer block comes from openHAB Rules Tools [4.1.0.0;4.9.9.9]