[Solved] Postponed rule execution? Do rules get put on stack?

Edited:
Reading this edit is definitely easier and faster than reading the accepted solution, so for a quick answers:
- Yes, rules are put in queue;
- The “no more than 5 rules” limit means no more than 5 rules at a time, the others are queued and executed afterwards;
- The solution is don’t use thread:sleep, but use Timers instead.

Original post:

My setup is all zwave on Raspberry Pi4 with UZB Stick, Openhab 2.5

I have a rule to turn on the light on opening the door.
It was working well (even though there is a thread::sleep in it) and I’ve never had any issues. Until today, when I was playing around with the door (I had to adjust it a little bit and that involved multiple opening and closing). When I have finished the light continued to go on and off again for quite a while. I checked in the log and the door was closed for 20 minutes or so, but the rule kept getting triggered.

Here is the rule (I put a lot of logging in it after editing due to some hardware changes and upgrade to OH2.5):


rule "door light" 
when
    Item door_contact changed to OPEN
then
    logInfo("Test", "door light triggered!")
    logInfo("Test","door state(start): " + door_contact.state.toString)
    logInfo("Test","motion(start): " + door_motion.state.toString)
    sendCommand(door_light,ON)
    Thread::sleep(60000)
    while (door_light.state != OFF) {
        if  ((door_contact.state != CLOSED) || ( door_motion.state != 0 )) {
            logInfo("Test","door state(if): " + door_contact.state.toString)
            logInfo("Test","motion state(if): " + door_motion.state.toString)
            logInfo("Test", "waiting")
            }
        else {
            sendCommand(door_light,OFF)
            logInfo("Test","door state(else): " + door_contact.state.toString)
            logInfo("Test","motion state(else): " + door_motion.state.toString)
            logInfo("Test", "turning off")
        }
        Thread::sleep(30000)
    }    
    logInfo("Test", "off, nothing to do")
end

Like I said I opened and closed the door plenty of times, but closed it definitely at about 1:21 and the light kept going on (with “door light triggered” in the openhab.log) until 1:57.

It seems to me like if the rule triggers were put on stack and taken one after another until the stack was empty. The rules kept being executed for each opening of the door, even if the door was already closed. I thought there is a limit of 5 (or less than 10) rules that can be run, but never thought the rules can get stacked and postponed, can they? Am I right or is there another explanation?
The most important question is how to avoid that? Is it possible to check - from within the rule - if the same rule has already been started and stop executing new rules then? (advices welcome)
I would even prefer to abandon the rule at all if the execution is not possible within 5 or 10 seconds, because I don’t need it after that time.
Also, is there a way to stop executing all the stacked rules? I didn’t want to reboot openhab, so I waited until it stopped blinking (somewhat annoying :wink: )

Here is the log:

2020-01-31 01:56:00.380 [INFO ] [.eclipse.smarthome.model.script.Test] - turning off
2020-01-31 01:56:06.245 [INFO ] [.eclipse.smarthome.model.script.Test] - off, nothing to do
2020-01-31 01:56:06.252 [INFO ] [.eclipse.smarthome.model.script.Test] - off, nothing to do
2020-01-31 01:56:06.257 [INFO ] [.eclipse.smarthome.model.script.Test] - door light triggered!
2020-01-31 01:56:06.260 [INFO ] [.eclipse.smarthome.model.script.Test] - door light triggered!
2020-01-31 01:56:06.261 [INFO ] [.eclipse.smarthome.model.script.Test] - door state(start): CLOSED
2020-01-31 01:56:06.263 [INFO ] [.eclipse.smarthome.model.script.Test] - door state(start): CLOSED
2020-01-31 01:56:06.264 [INFO ] [.eclipse.smarthome.model.script.Test] - motion state(start): 0
2020-01-31 01:56:06.267 [INFO ] [.eclipse.smarthome.model.script.Test] - motion state(start): 0
2020-01-31 01:56:30.386 [INFO ] [.eclipse.smarthome.model.script.Test] - door state(else): CLOSED
2020-01-31 01:56:30.390 [INFO ] [.eclipse.smarthome.model.script.Test] - motion state(else): 0
2020-01-31 01:56:30.394 [INFO ] [.eclipse.smarthome.model.script.Test] - turning off
2020-01-31 01:56:30.395 [INFO ] [.eclipse.smarthome.model.script.Test] - door state(else): CLOSED
2020-01-31 01:56:30.399 [INFO ] [.eclipse.smarthome.model.script.Test] - motion state(else): 0
2020-01-31 01:56:30.401 [INFO ] [.eclipse.smarthome.model.script.Test] - turning off
2020-01-31 01:56:51.549 [INFO ] [.eclipse.smarthome.model.script.Test] - off, nothing to do
2020-01-31 01:56:51.557 [INFO ] [.eclipse.smarthome.model.script.Test] - door light triggered!
2020-01-31 01:56:51.561 [INFO ] [.eclipse.smarthome.model.script.Test] - door state(start): CLOSED
2020-01-31 01:56:51.564 [INFO ] [.eclipse.smarthome.model.script.Test] - motion state(start): 0
2020-01-31 01:57:00.407 [INFO ] [.eclipse.smarthome.model.script.Test] - door state(else): CLOSED
2020-01-31 01:57:00.411 [INFO ] [.eclipse.smarthome.model.script.Test] - motion state(else): 0
2020-01-31 01:57:00.414 [INFO ] [.eclipse.smarthome.model.script.Test] - door state(else): CLOSED
2020-01-31 01:57:00.415 [INFO ] [.eclipse.smarthome.model.script.Test] - turning off
2020-01-31 01:57:00.418 [INFO ] [.eclipse.smarthome.model.script.Test] - motion state(else): 0
2020-01-31 01:57:00.421 [INFO ] [.eclipse.smarthome.model.script.Test] - turning off
2020-01-31 01:57:06.271 [INFO ] [.eclipse.smarthome.model.script.Test] - off, nothing to do
2020-01-31 01:57:06.272 [INFO ] [.eclipse.smarthome.model.script.Test] - off, nothing to do
2020-01-31 01:57:06.277 [INFO ] [.eclipse.smarthome.model.script.Test] - door light triggered!
2020-01-31 01:57:06.281 [INFO ] [.eclipse.smarthome.model.script.Test] - door state(start): CLOSED
2020-01-31 01:57:06.286 [INFO ] [.eclipse.smarthome.model.script.Test] - motion state(start): 0
2020-01-31 01:57:07.421 [INFO ] [.eclipse.smarthome.model.script.Test] - door light triggered!
2020-01-31 01:57:07.425 [INFO ] [.eclipse.smarthome.model.script.Test] - door state(start): CLOSED
2020-01-31 01:57:07.428 [INFO ] [.eclipse.smarthome.model.script.Test] - motion state(start): 0
2020-01-31 01:57:30.424 [INFO ] [.eclipse.smarthome.model.script.Test] - door state(else): CLOSED
2020-01-31 01:57:30.428 [INFO ] [.eclipse.smarthome.model.script.Test] - motion state(else): 0
2020-01-31 01:57:30.430 [INFO ] [.eclipse.smarthome.model.script.Test] - turning off
2020-01-31 01:57:30.430 [INFO ] [.eclipse.smarthome.model.script.Test] - door state(else): CLOSED
2020-01-31 01:57:30.433 [INFO ] [.eclipse.smarthome.model.script.Test] - motion state(else): 0
2020-01-31 01:57:30.436 [INFO ] [.eclipse.smarthome.model.script.Test] - turning off
2020-01-31 01:57:51.570 [INFO ] [.eclipse.smarthome.model.script.Test] - off, nothing to do
2020-01-31 01:58:00.434 [INFO ] [.eclipse.smarthome.model.script.Test] - off, nothing to do
2020-01-31 01:58:00.440 [INFO ] [.eclipse.smarthome.model.script.Test] - off, nothing to do
2020-01-31 01:58:06.293 [INFO ] [.eclipse.smarthome.model.script.Test] - off, nothing to do
2020-01-31 01:58:07.435 [INFO ] [.eclipse.smarthome.model.script.Test] - off, nothing to do

Your rule will trigger multiple versions every time you twiddle the door. Each version will sit and wait at that point. If you trigger enough rules to use up all the rule threads, the triggering events will queue up instead. Once one of the sleeping rules finally moves on and exits,another will be allowed to trigger. It will impact anything else that is going on as well.

It’s a long way to say that line is really bad practice.

What on earth is the second sleep for?

This rule as written is probably the absolute worst thing you can do in OH. The only thing that would make it worse is to add in a ReentrantLock. You have really long sleeps (anything longer than 100 msec is considered too long) and a while loop that could cause the Rule to remain running for minutes to indefinitely. See Why have my Rules stopped running? Why Thread::sleep is a bad idea for a full explanation and alternatives (i.e. use Timers).

Well, your while loop only checks to see if the door is CLOSED every 30 seconds. That would account for most of the delay. The rest could be accounted for by the fact that it had to wait to get a Thread before it could start running in the first place.

What do you think happens to them? If only five can be running at a given time and you have five actively running, there are two choices:

  1. drop the event and don’t run the Rule
  2. queue up the event and wait for a thread to free up

2 is a much more appropriate response.

See the link above. Tl;dr, use Timers.

Reload the .rules files.

It was meant to not check hundreds times per second if anything has changed. without the second “sleep” the while loop would be much faster than me going through the door. At least that’s what I thought.

That might be even the worse rule on earth :blush: , still it has worked quite well, I’d say :wink:
I remember having read your opinion on thread::sleep at some point in time (although not that specific post) - that’s why I wrote this:

Unfortunately I remember also not having enough programmatic skills (not to mention the programmer’s set of mind) to implement timers etc. Maybe I should give it a second try today.
OT I have no idea what ReentrantLock is, but from some of the posts (forums?) I remembered one should definitely avoid “locks”. “Sleep” seemed the lesser of two evils :blush:

Well, I don’t find it appropriate at all actually. I mean in the “programmer’s world” it may be, but as a user of interactive house I’d rather not have this behaviour.
If something does not react, I can accept it. But triggering the garage door opening in the middle of the day and leaving it open just because the morning queue was too long and overloaded seems a security risk to me. It was only light in my case, but would any of you take responsibility for leaving the garage door open because of some unforeseen mistake in another part of the house?

I was actually convinced the 5 (or more) limit of the running rules will cause the next instances of the same rule to be discarded. In my (strictly personal) opinion they should be dropped and forgotten.

That’s what I did, and the light kept blinking. It seemed the queue was not cleared on reload, so I asked the question.

Anyway thank you guys, I will have a second look into timers. On the other hand though I don’t plan to play with the door for some time now :wink:

When you write rules in openHAB, that’s exactly what you accept.

Someone else would want them queued up and actioned - just running a simple open/close counter, for example.
Taking away that choice would not be a good system design.

You can have your “discard extra events” behaviour - but you have to design for it.

It’s not that hard. First event,check -no timer running yet, so start a timer. Subsequent events, check - ooh, a timer is already running, we’ll just exit then.

I was talking from the user’s perspective. I don’t want OH to silently throw away events. I want to have the option for how to deal with it, which means OH need trigger the Rule for every event. That was you, as the user can support ALL use cases (i.e. ignore them, queue them for processing, etc.) If they are just ignored only the “just ignore it” case is possible to support.

To throw a use case out there that shows the opposite, let’s say I get an alarm event from a sensor that says my basement is flooding. I definitely don’t want that event to just be ignored just because I have a Rule that is sitting on all the execution threads for minutes at a time doing nothing.

It’s definitely not acceptable to have the garage door go up minutes or hours after triggering it. But that is easily avoidable without eliminating the ability to receive all events.

That’s a side effect of not writing your rules properly.

It is worth noting that this is not necessarily a problem in Scripted Automation as there is no thread limit to run up against. But it’s still not a good idea because then you then have multiple copies of the same Rule running at the same time which can have strange interactions between them if you don’t code it right.

I have never seen a programmer taking responsibility for some material loss due to application functioning (or dysfunction). There is always some limitation in the EULA (or whatever you call it).
I have seen “dysfunctions” in software so many times… and there always can be viruses etc.

Home automation - and explicitly “action on motion” or “action on door opening” are really “real time” actions. I cannot imagine when it is preferable to postpone such an action. Can you point me to some real life situation? I find queuing is accepting unnecessary risks. I’m open on real life examples to prove me wrong.

Can you please show me how you do that?

BTW what does [ | in


createTimer(now.plusSeconds(1), [ | 
    // do some  stuff 
])

actually mean?

Excellent point, I haven’t thought about that. Partly because I have much more confidence in the hydraulic pipes, than in software loops :wink:

Exactly. That’s why I thought that the very same rule started 5 times or more, should automatically be discarded. Obviously you should get a log info about it, or even an e-mail if configured. I just misunderstood the concept of instances limitation.

In this case you are the programmer.

OH is not a “real time” system. If that is what you expect, you are looking in the wrong place. But in a “real time” system, it’s simply unacceptable to just throw away events too. In a “real time” system what that means is you must process the event within a certain amount of time or else it’s an error. So even in that sort of system, you would still be responsible for making sure your processing of the events (i.e. your Rules) complete within a given amount of time which would be in the milliseconds range, not minutes like the current Rule runs.

I’m sorry, but this is not a problem with OH. It might be a problem with the docs. It is definitely a problem with your expectations which are not based on anything in the docs.

When you are creating Rules and your Home Automation, you are the programmer. So you are the one responsible for building a home automation system that performs as you need it to within the constraints of the tool presented which is OH. And that constraint is you must write Rules that run to completion in milliseconds, not minutes.

But sometimes you want that to happen. For example, you can do something different in the subsequent Rules (e.g. just immediately exit, reschedule a Timer, set a flag so the Rule that is still running exits earlier, etc.) We don’t know enough about all the ways someone may need OH to work to achieve their use case. We cannot just forbid all sorts of possible use cases just so you don’t have to learn how to use Timers.

That’s the little block of code that the Timer will execute when it goes off. In one second, all the lines of code between the [ | ] will execute.

I think you’re approaching the point of demanding your money back. Let’s not go there.

Here’s how it works;it’s an event-driven system. No events are discarded; you opt whether to do anything about them or not.
You may have been surprised to find that you did that unknowingly, that’s fair enough, but it’s no-one else’s responsibility to have validated your system.

You’d like help implementing something better, that’s great,let’s do it.
The principles in this tutorial for the timer based solution would be a good start.

It’s got all the building blocks. How to make a timer that survives between runs of rules. How to detect if the timer is running within a rule. How to reschedule an existing timer.

1 Like

If I understand it correctly the real “real time” applications cannot (easily if at all) be managed software wise. So no, I don’t expect “real time” operation. Nevertheless, I expect it to work in a reasonable manner. That’s why I’m asking questions here. I have tried many HA systems and liked the modularity and flexibility of OH. Despite the incomplete documentation I am eager to learn, so I am not in the wrong place. Unless you guys will throw me out. Just to be clear (I don’t know how many times I wrote that I am expressing my personal opinions) I am not here to cry for the moon. I ask questions to learn from answers.
OT: At no place I said “I want my money back” (not even close), but honestly if pointing out a potential security risk (private opinion) shall ever be a reason to void the contract (instead of explaining why I’m wrong), so be it. I wouldn’t like such a contract anyway.

I feel quite stupid now. I’m not sure what you exactly mean… Putting my incompetence to write the rules aside for a moment, I mean I will try to rewrite my rule anyway, so let’s suppose I have “good rules”: can I expect that pushing the button/or opening the door will almost immediately cause the light to turn on? Or is the HomeAutomation not meant for that? I have quite a working stuff here and for instance when I start watching a movie the lights go off and my amplifier powers on the right source etc. I thought that’s the idea of home automation, isn’t it?

Thank you. I can accept this as an answer. As I wrote above I misunderstood the “5 rules limit”.
But… I maintain my opinion, that repeating something more than 5 times, won’t make it any more true (that’s a :joke)

Thanks, but I wanted actually to know what happens between [ and |
all stuff to do seems to be between | and ]

createTimer(now.plusSeconds(1), [ *what is happening here?* | 
    // because here is some stuff to happen, 
])

Here is my new rule, could you please be so kind and see if it would work? What to change if not?

var Timer doorlighttimer = null
rule "door light" 
when
    Item door_contact changed to OPEN
then
    if (door_light.state != ON) {
        sendCommand(door_light,ON)
        if(doorlighttimer === null || doorlighttimer.hasTerminated()) {
            doorlighttimer = createTimer(now.plusMinutes(1), [|
                if (door_light.state != OFF) {
                    if ((door_contact.state != CLOSED) || ( door_motion.state != 0 )) {
                    doorlighttimer.reschedule(now.plusMinutes(1))
                    }
                    else {
                        sendCommand(door_light,OFF)
                    }
                }
                else {
                    doorlighttimer = null
                }
            ])
        }
    }
end

Rich may be able to give you complicated stuff about lambdas. A similar structure is used in “functions” and then meaningful stuff goes in there.
So far as I’m concerned it’s never used with createTimer(), you just leave it empty, that’s the way it is.
It’s certainly irrelevant here, I have never seen an example of it populated for timer.

Depends what you want to happen!

What i see is…

Door is opened, if light is already ON (perhaps manually), nothing changes.

Otherwise, door is opened,light is turned ON, timer is started.

Should you shut the door within time, nothing changes.

Should you open the door again within the original time, nothing changes.

If time expires AND light is on …
then if door is still open (or been closed and reopened) we will extend the timer.
Or if a motion detector is active at that moment, we will extend the timer.
(if motion is detected before or after this exact moment,nothing changes)

Otherwise we turn the light OFF at expiry time, and keep the finished timer hanging around but terminated.
Personally I’d set timer handle null here,which would allow to simplify the if() in the rule by deleting hasTerminated part.

If time expires and light is already OFF (perhaps manually) ,we set the timer handle to null.

After the timer has expired without rescheduling, the next door open will start the whole process again.

I really don’t understand the grizzle here.
You had a badly designed rule. You’d previously read warnings about the bad practice used in it, but didn’t really appreciate the problem… It had unexpected consequences. You may have chosen to link your rule to a safety or security conscious device. I’m not sure what you expect to happen next.

Thanks, that seems to be what I wanted.

Is there any better solution for that? I can imagine that using “persistence” I could set some time after motion detection?

So you say this rule would be better?:

var Timer doorlighttimer = null
rule "door light" 
when
    Item door_contact changed to OPEN
then
    if (door_light.state != ON) {
        sendCommand(door_light,ON)
        if(doorlighttimer === null) {
            doorlighttimer = createTimer(now.plusMinutes(1), [|
                if (door_light.state != OFF) {
                    if ((door_contact.state != CLOSED) || ( door_motion.state != 0 )) {
                    doorlighttimer.reschedule(now.plusMinutes(1))
                    }
                    else {
                        sendCommand(door_light,OFF)
                        doorlighttimer = null
                    }
                }
                else {
                    doorlighttimer = null
                }
            ])
        }
    }
end

No idea. I don’t know what you want to happen.

Persistence in openHAB is about keeping historical records, Can’t see the application here?

I wanted exactly what you described. I just thought you were suggesting there is something that can be done better or differently, like checking when the motion was triggered or something… never mind. I’ll be happy with that rule if it won’t cause problems

With all respect, there is no grizzle here. I cannot program, even as simple things. I can only copy and paste and adjust what exists. That’s how I wrote the timer rule. I don’t know for sure, but I can imagine there are other people like me, that can be clumsy in writing rules. I would not use an uncertain rule for security devices. But I can imagine things, and I can possibly imagine there is someone who would do that - write a poorly designed rule and use it inadequately. It is a security risk for me. Using thread::sleep I had one problem in 3 years with my light and try to do things better. You tell me I use the worst possible solution, but I don’t even know why the Thread::sleep exists. You can be sure I didn’t invent it. I found it on this forum. So people do use it for some reasons.
I tried another system before, where there was a manual lock for maximum number of rule instances that can be run simultaneously. Even more, you could check how many instances are already running and do a simple “if there are already 3, then don’t execute the rule at all”.
Now that’s something I call giving user the choice. For other reasons I don’t want to use the other system, I want to use OH. I am very sorry my invalid assumption disturbs you, but in that other system they did it for a reason. So I don’t think I’m the only one thinking the decisions on what to do with incorrectly invoked rules may be more granular.
From what I understood of your answer, this system is not designed to ignore clumsy rules automatically and is not allowed to silently exit with errors. I don’t know the mechanisms behind that. I accept that. I need (want) to have an insightful opinion on that.

This is correct. There is no way round it, the vast flexibility of openHAB does require some technical skills and the learning curve is steep.
Part of that flexibility is that if you dislike DSL rules you can use other rules setups. Look into NodeRed.

The distinction is that queuing events and activities is a design feature, not an error. By design, this system is intended to run on underpowered cheap hardware and cope as well as it can. That necessitates this kind of action.

If you want to write rules to ignore events under some circumstances, that’s your business, it’s no problem. You’ve just done one. There are other ways to do it too.

If someone else wants to write a rule that counts how many times you can push a button in a minute, they can do that without fear of missing one.

If you want to reduce the number of rule threads to 3, you can.

So far as I know there is no simple way to limit the number of times you can trigger a given rule “simultaneously”.
But here comes openHABs flexibility …

It would be possible to construct some complicated mechanism involving reentrant locks and a counter if you really wanted it.
Note, that there is nothing inherently wrong with using reentrant locks - BUT it’s a complete arse to develop, because
(a) you have to reeaally think about multithreading and
(b) when you make a tiny error in your rule you’ve “lost” the lock (or rather,the unlock) … making thorough development of anything non-trivial very difficult.
I would not recommend it and see no value in it here.

@hoomee I would look at the expire binding personally for this task.

I use it for my motion detector. If motion gets triggered on then the light switch gets updated. This intern updates the expire timer.

This in my mind is an alternate way to handle the problem and I believe it takes some complexity out of the rule.

2 Likes

Yes. But only if you write your rules in a way that supports that. Long running rules do not allow that.

To avoid long running rules, you must use timers instead of sleeps.

I don’t know how to answer that. It runs one second from now (based on your example). Exactly what runs depends on what code you put between the []. If you want to log out “hello world” one second from now:…

createTimer(now.plusSeconds(1), [|
    logInfo("test", "hello world")
])

The stuff between [ and | is how you world pass arguments to the that little block of code, which you don’t have to do for timers so you will usually either see the | omitted or nothing between the [ and |. To see an example when full explanation where stuffy is put been the [ and| see Reusable Functions: A simple lambda example with copious notes.

tl:dr, because the underlying programming language offers it. There is no way to hide that it exists not any way to prevent it’s use. And in certain circumstances, it can be useful, so long as the sleeps are short (fractions of a second).

You can find lots of stuff on this forum that either were never right, never a good idea, or is no longer the best way. It’s hard enough keeping up with the docs let alone going around and cleaning up everyone’s forum postings. Pay attention to the age of the post. If it’s more than a year old, look for more recent examples of you can. There might be a better way.

Thank you, I will check that, just to understand it better.

Thanks for the idea, but if I got you right, anytime you turn the light on, it will turn off after given time.
I want to have the possibility to turn it on manually without expiring. I use “expire” on few other lights though.

So it’s there for passing the arguments. It seems to be exactly the answer to my question. Thanks.

Sorry for bothering you, but I have some cron invoked rules (midnight in that case) and I’ve read the advice somewhere to make the thread sleep for second or two just to let Astro binding to recalculate times of day.
Also some system stuff may be happening at midnight.
Would you consider this a valid point?
Or is there a better way of how to write a rule to be run at midnight? (beside the obvious one to make it run at 0:01)

Thinking about it I have another question. Do timers - invoked from withing a rule - live outside the rule? In other words: the rule is executed and finished but the timer continues to count down in a separate thread?
If not, how does it work?
If yes: wouldn’t my new rule with timers, the one I have written above, also call many timers and wouldn’t they end after some time with blinking light? If not, can you explain why?

And one last (hopefully) question. In a rare case there are 10 different, good written, rules called simultaneously - will execution of the 6th and next be blocked until the 1st is closed? If there are timers in each of the hypothetical rules, the execution will be almost immediate? (in seconds?)