createTimer into while loop

I intend to create a light signal (with dimmer) when a water pump is running.
But I found that a timer could not be created inside of a loop. Is that correct or my script is incorrect?

The rule bellow do not stop for 3 seconds, as I’ve asking in the code. Instead is running over and over very fast without any pause.

var Timer timer = null

rule "light signal"
when Item pump_2 changed to ON
then 
while (pump_2.state == ON)
      {
      light_terrace.sendCommand(15)
      timer = createTimer(now.plusSeconds(3), [|light_terrace.sendCommand(100)] )
      timer = null
      }
light_terrace.sendCommand(0)
end

Thanks for you time!

The createTimer is non-blocking, you should use “Thread::sleep(3000)”.

Try:

while (pump_2.state == ON)
      {
      light_terrace.sendCommand(15)
      Thread::sleep(3000)
      light_terrace.sendCommand(100)
      }

Thanks a lot, mate!

There are 2 issues here.
Using a while loop monopolises the thread for the duration of the loop
You don’t need the loop. You can use rule triggers.

Again, for the same reason, Thread::sleep is not advised if you can use a timer instead.

As I read the rule you want the light on when the pump is on and off when the pump is off
Why are you using a timer at all?

var Timer timer = null

rule "light signal"
when Item pump_2 changed
then 
    if (pump_2.state == ON) {
        light_terrace.sendCommand(15)
        timer = createTimer(now.plusSeconds(3), [ |
            light_terrace.sendCommand(100)
            timer = null
        ])
    } else if (pump_2.state == OFF) {
        light_terrace.sendCommand(0)
    }
end

My assumption was the intention of the while loop is to make the light blink back and forth continously. In your solution that routine happens only once.
But I do agree that it may not be an optimal solution…

The assumption is correct.

This is working resonable (blinking the light):

while (pump_2.state == ON)
      {
      light_terrace.sendCommand(10)
      Thread::sleep(3000)
      light_terrace.sendCommand(100)
      Thread::sleep(3000)
      }
light_terrace.sendCommand(0)

You have the same problem with thread locks
Openhab only have 5 thread to play with

This should work without locking up a thread

var Timer timer = null

rule "light signal"
when Item pump_2 changed
then 
    if (pump_2.state == ON) {
        light_terrace.sendCommand(15)
        timer = createTimer(now.plusSeconds(3), [ |
            if (light_terrace.state != 100) light_terrace.sendCommand(100)
            else light_terrace.sendCommand(0)
            timer.reschedule(now.plusSeconds(3))
        ])
    } else if (pump_2.state == OFF) {
        light_terrace.sendCommand(0)
    }
end

And when is the timer stopped?

Good point,
I meant to put it in the else condition, just forgot…

var Timer timer = null

rule "light signal"
when Item pump_2 changed
then 
    if (pump_2.state == ON) {
        light_terrace.sendCommand(15)
        timer = createTimer(now.plusSeconds(3), [ |
            if (light_terrace.state != 100) light_terrace.sendCommand(100)
            else light_terrace.sendCommand(0)
            timer.reschedule(now.plusSeconds(3))
        ])
    } else if (pump_2.state == OFF) {
        timer = null
        light_terrace.sendCommand(0)
    }
end
1 Like

You need to cancel the timer, not just set it to null.

timer?.cancel
timer = null

If you just set it to null, the Timer still exists out there running in the background but you’ve deleted your reference to it to control it.

A better approach would be to put an if around the timer.reschedule inside the timer.

if(pump_2.state != ON) timer.rescedule(now.plusSeconds(3)) // != ON used to catch NULL cases
else light_terrace.sendCommand(OFF)                        // probably should be updated to restore the light to its previous state
var Timer timer = null

rule "light signal"
when
    Item pump_2 changed to ON
then
    val currLightState = light_terrace.state
    light_terrace.sendCommand(15)

    timer = createTimer(now.plusSeconds(3), [ |
                // blink the lights
                if(light_terrace.state != 100) light_terrace.sendCommand(100)
                else light_terrace.sendCommand(0)
                
                // reschedule if necessary
                if(pump_2.state == ON) timer.reschedule(now.plusSeconds(3))
                else {
                    light_terrace.sendCommand(currLightState) // restore the light to prior state
                    timer = null
                }
            ])
end

Good.
2 questions…
What does the ? do?
Why no cancel in your rule?

It is a short hand way of doing

if(timer !== null) timer.cancel

There is no need to cancel it. When pump_2 is no longer ON the Timer will exit on its own since it is no longer rescheduled.

There is one little edge to this though. If you have a situation where the light needs to turn off exactly when the pump turns off then a cancel needs to be implemented along the lines of your original else clause. In this case the Timer will exit at most three seconds after the pump turns off. But given this was the case in the original posting with Thread::sleeps I figure this is acceptable behavior.

At a high level, to implement a while loop with a sleep using Timers:

while(condition1) {
    // do work
    Thread::sleep(iterationTime)
}

becomes

timer = createTimer(now.plusSeconds(iterationTime), [ |
    // do work
    if(condition1) timer.reschedule(now.plusSeconds(iterationTime))
    else timer = null
])
2 Likes