While loop within a dsl rule causes 100% cpu load

Hello,

I want to control a light which should be switched off, when the motion sensors state is motion = OFF after a timer of 5 minutes. I wrote a rule which is working fine as long as I leave the room within the timer period. But when I stay in the room longer than the timer period. The light will remain ON. So I tried with a while loop. When I switch the light on, the CPU load of my Openhab server is raising up to 100%. Perhaps I am doing something wrong. Please help.

import org.openhab.model.script.actions.Timer

rule "Licht Bad ausschalten, wenn keine Bewegung erkannt"

when

Item Moes10504_Switch1 changed to ON

then

while(Moes10504_Switch1.state==ON){
createTimer(now.plusMinutes(5))[
	if (Moes10504_Switch1.state==ON && Bewegungsmelder_Bad_IKEA_of_Sweden_Motion.state==OFF)
		{sendCommand(Moes10504_Switch1, OFF)}
	]
                                                   }
end

What happens here is that when the rule triggers, and for as long as Moes10504_Switch1 is ON, the rule keeps creating timers in every iteration of the while-loop. Since there are no sleep or anything else that pauses execution, it will use as much resources as possible.

I am quite sure this is not what you intend the rule to do, can you explain what that is?

1 Like

I think this is because the while loop will run all the time as long as the status = on and thus start thousands, possibly millions of timers, because you do not query whether a timer is already running… :slight_smile:

  1. would I create a logging in the console within the while loop and verify my assertion

  2. if it is so, create a query on an already running timer and only create it if no timer is running

  1. I would solve it completely differently:

No while loop, but simply create a timer when the status = on and when the timer has expired, query the status again within the timer and if it is still = on, retrigger the timer, otherwise cancel…

I also use something like this quite often, but here it is a js file rule:

rules.JSRule({
    name: "Licht Bad ausschalten, wenn keine Bewegung erkannt",
    triggers: [triggers.ItemStateChangeTrigger('Moes10504_Switch1')],
    execute: (event) => {
        
       const item = items['Moes10504_Switch1'];
       const timerName = event.itemName + '_timer';
       var timer = cache.private.get(timerName);

       function cancelTimer()
        {
            timer.cancel();
            cache.private.put(timerName, null);
            log(ruleName + 'cancel timer');
        }

       if(item.state == 'ON' && timer == null)
       {
            var timer = actions.ScriptExecution.createTimer(timerName, time.ZonedDateTime.now().plusSeconds(300), () => {
                const itemBewegung = items['Bewegungsmelder_Bad_IKEA_of_Sweden_Motion'];
                if(item.state == 'ON' && itemBewegung.state == 'OFF')
                {
                    item.sendCommand('OFF');
                    cancelTimer()
                }else{
                    timer.reschedule(time.ZonedDateTime.now().plusSeconds(300))
                }
            });
       }

       if(item.state == 'OFF' && timer !== null)
       {
            cancelTimer() 
       }

    },
});

Well, don’t use while (here it’s nonsense)

var Timer tLichtBad = null                                  // Pointer for timer

rule "Licht Bad schalten"
when
    Item Bewegungsmelder_Bad_IKEA_of_Sweden_Motion changed  // motion detection changed state
then
    tLichtBad?.cancel                                       // cancel existing timer, if there is any
    if(newState == ON) {                                    // if motion was detected
        if(Moes10504_Switch1.state != ON)                   // if light isn't ON
            Moes10504_Switch1.sendCommand(ON)               // switch light to ON
    } else {                                                // no motion was detected, so
        tLichtBad = createTimer(now.plusMinutes(5), [|      // start the timer and after 5 minutes
            Moes10504_Switch1.sendCommand(OFF)              // switch the light to OFF
        ])
    }
end

The rule is triggered if the motion detector changed its state.
Any existing timer is cancelled, as either the light should stay on or a new timer will be created soon.
If newState (this is the state of Bewegungsmelder_Bad_IKEA_of_Sweden_Motion which actually triggered the rule, not necessarily the current state of the item!) is ON, the light should be turned on.
If newState is not ON, the timer has to be started and the rule ends.

If the timer was started, after 5 Minutes the light will be switched OFF. If there is any motion detected within these 5 minutes, the rule is started again.

Hi Udo,

I do not want to switch light in the bath by the motion sensor. The rule must be triggered, when the light has been switched on by the wall switch. Then the light must stay on for minimum 5mins and should be switched off when no motion is detected anymore.

I solved it for now like this:

import org.openhab.model.script.actions.Timer

rule "Licht Bad ausschalten, wenn keine Bewegung erkannt"

when

Item Moes10504_Switch1 changed to ON

then

createTimer(now.plusMinutes(5))[
		while(Bewegungsmelder_Bad_IKEA_of_Sweden_Motion.state==ON){
		Thread::sleep(60000)}

		sendCommand(Moes10504_Switch1, OFF)
	]
end

Please, do not use while at all, please, do not use Thread::sleep if not really needed (no, it’s not) and better don’t use Thread::sleep() for more than 500 msec values. There are tons of postings here why it’s a bad idea…

You don’t need any import for timers either - at least if not using openHAB1(!)

As you are using a timer already, use it the efficient way:

var Timer tBadLicht = null

rule "Licht Bad ausschalten, wenn keine Bewegung erkannt"
when
    Item Moes10504_Switch1 changed to ON
then
    tBadLicht?.cancel
    tBadLicht = createTimer(now.plusMinutes(5), [
        if(Bewegungsmelder_Bad_IKEA_of_Sweden_Motion.state == ON)
            tBadLicht.reschedule(now.plusMinutes(1))
        else 
            Moes10504_Switch1.sendCommand(OFF)
    ])
end

But I need a loop until motion is off. :thinking:

Then include the motion sensor in the rule trigger and when it switches to off, switch off the light and delete the timer :slight_smile:

My last DSL rule is a long time ago, but it should look like this… :

var Timer tBadLicht = null

rule "Licht Bad ausschalten, wenn keine Bewegung erkannt"
when
    Item Moes10504_Switch1 changed to ON or
    Item Bewegungsmelder_Bad_IKEA_of_Sweden_Motion changed to OFF
then

    tBadLicht?.cancel

    if(Bewegungsmelder_Bad_IKEA_of_Sweden_Motion.state == OFF and Moes10504_Switch1.state == ON)
    {
        Moes10504_Switch1.sendCommand(OFF)
        return;
    }

    createTimer(now.plusMinutes(5), [
        if(Bewegungsmelder_Bad_IKEA_of_Sweden_Motion.state == ON)
            tBadLicht.reschedule(now.plusMinutes(1))
        else 
            Moes10504_Switch1.sendCommand(OFF)
    ])
end

@Udo_Hartmann Wouldn’t it be better to use the chache in the DSL rules as well? :thinking:

Why?

Back up to your original requirements:

“Light should switch OFF five minutes after last motion”

OK, so we have a motion sensor Item that changes when motion is detected. Does this Item change back to OFF automatically, in less than five minutes, in more than five minutes, or never?

If the Item is already ON, what happens if new motion is detected? Nothing or does the Item get updated to ON?

The answer to these are important because that changes how the rule should be triggered, whether by changed to ON or by update to ON.

How does the light turn ON? Should that happen in this rule?

If we assume that the Item gets updated to ON on every motion detection, the rule gets triggered by “updated to ON” and in the rule test to see if a timer already exists. If it does, reschedule it for five minutes from now. If not, create a new timer to command the light to OFF five minutes from now.

Five minutes after the last motion is detected, the timer will go off and command the light to OFF.

No loop required as shown in the replies here.

See Design Pattern: Motion Sensor Timer for a detailed discussion with examples.

But the key is understanding when and how your motion sensor Item works. You want to trigger the rule every time motion is detected.

It is probably overkill for this but Threshold Alert and Open Reminder [4.0.0.0;4.9.9.9] can be configured to handle this use case, no coding required. But the correct configuration of this too depends on the behavior of the motion sensor Item.

Before I enter the room I switch light on with a wallswitch and this sets Moes10504_Switch1 from OFF to state ON. Then I enter the bathroom and this causes the motion sensor to motion=ON. If I leave the room after 2 minutes the rule works fine, because the timer is not finished. But when I leave the room after 10 minutes the timer has already finished and light stays on. That why I need a loop checking if motion=OFF and then switch the light off. The motion sensor sends a real OFF. I want that the light remains on as long as I am inside the room.

I tried your example but it doesnt work.

A while loop is never going to work for this. Forget that. It’s an XY Problem.

To get to something that does work, you need to answer the questions about how the motion sensor Item behaves. They are not rhetorical questions.

  1. Does this Item change back to OFF automatically, in less than five minutes, in more than five minutes, or never?

  2. If the Item is already ON, what happens if new motion is detected? Nothing or does the Item get updated to ON?

The answer to both questions are important but the answer to 2 is critical.

This works as expected:

rule "Licht Bad ausschalten, wenn keine Bewegung erkannt"

when

Item Moes10504_Switch1 changed to ON

then

createTimer(now.plusMinutes(5))[
		while(Bewegungsmelder_Bad_IKEA_of_Sweden_Motion.state==ON){
		Thread::sleep(60000)}

		sendCommand(Moes10504_Switch1, OFF)
	]
end

If the motion sensor detects motion it sets its state to motion=ON. After 90 seconds it changes its state to motion=OFF if no motion is detected. The state remains ON as long as motion is detected.

Long running rules are very proplematic. While this rule is running if it’s triggered again the subsequent triggers will run the rule again. Depending on timing and behaviors you can starve out this rule making it less and less rsponsive until it won’t run at all.

It’s also less responsive and brittle.

Even if it works in this case, which it might, using long running rules with while loops and sleeps should be a last resort, never a first resort.

And does it update the Item to ON inbetween when new motion is detected?

State remains ON as long as motion is detected. In Pimatic you say “Switch the switch off if no motion is detected for 5 minutes” and you are done. Takes one minute.

Yes, I know, but does it get updated?

Most (but not all) devices will update the Item to ON even if it’s already ON. By default updates are not shown in events.log but there are lots of ways to descover how the Item is updated:

  • sometimes it’s covered in the binding docs
  • use the developer sidebar and fillter the event stream to just see those updated events
  • create a rule that triggers on “received update to ON” and log something

Updates are a different event from changed. Update will trigger the rule even if the Item is already ON.

We want that because it means the rule will trigger every time motion is detected, not just every 90 seconds.

But, since the 90 seconds < 5 minutes the following approach should work. We are not at your machine. We cannot guarantee that any rule will work exactly as typed. You will have to have to adjust as necessary.

var Timer tBadLicht = null

rule "Licht Bad ausschalten, wenn keine Bewegung erkannt"
when
    Item Bewegungsmelder_Bad_IKEA_of_Sweden_Motion updated to ON
then
    if(tBadLicht == null || tBadLicht.hasTerminated) {
        createTimer(now.plusMinutes(5), [
            Moes10504_Switch1.sendCommand(OFF)
         ])
    }
    else {
        tBadLicht.reschedule(now.plusMinutes(5))
    }
end

That rule will sendCommand OFF to Moes10504_Switch1 five minutes after the last time
Bewegungsmelder_Bad_IKEA_of_Sweden_Motion is updated to ON.

Another alternative that can work in this case is to use Expire. Navigate to Moes10504_Switch1 and click “Add metadata”. Choose Expire and configure it as follows:

value: 0h5m0s,command=OFF
config: {}

Then create a rule:

var Timer tBadLicht = null

rule "Licht Bad ausschalten, wenn keine Bewegung erkannt"
when
    Item Bewegungsmelder_Bad_IKEA_of_Sweden_Motion updated to ON
then
    Moes10504_Switch1.sendCommand(ON)
end

This approach will command the switch to ON every time the motion sensor detects motion. Five minutes after the last motion is detected Expire will command the Item to OFF.

In both cases though that it’s the motion sensor that drives things, not the light switch.

But this will switch the light also on, when I enter the room. The light must not been switched on by the motion controller. I want to switch the light only by the wall switch. The rule has the only goal to switch the light off, when I have forgotten to switch it off. My girfriend hates it, when things happen without touching a switch before.

Why not allow both motion and switch to turn on.
1 rule then using motion and expire.
Girlfriend can use switch…noone else need bother.

You also need to assign the timer to the variable:

        tBadLicht = createTimer(now.plusMinutes(5), [
            Moes10504_Switch1.sendCommand(OFF)
         ])

Otherwise there will be nothing to reschedule.