rkoshak / openhab-rules-tools

Library functions, classes, and examples to reuse in the development of new Rules.
Other
64 stars 23 forks source link

Added option to prevent timer self cleanup incl. tests #64

Closed Urmel closed 2 years ago

Urmel commented 2 years ago

Hi @rkoshak , I encountered a usage scenario, where I want to reschedule my timer within its own execution. Finding out that the timer always was cleaned up after its expiration, I thus added the possibility that it is not cleaned up to support this use case. Furthermore, I seem to have a rather slow system, so I had to extend the "second" delay time (waiting for the exec to be over) to 0.55secs. And, as a lazy person, I did not want to change the log level I get printed out just for testing... so I propose to set level for the success message to warn.

Looking forward to your review. BR Alex

rkoshak commented 2 years ago

Is this use case not supportable by looping timer? I think I need to understand this use case more because I'd prefer to handle this all consistently and really design how it should work instead of heading down the path of adding more and more flags and options to the check method.

Note that there are potential problems when keeping a timer body around as well. In practice I've found cases where it works for one or two iterations and then behind the scenes something gets garbage collected and the timer body stops working. There is a way to avoid that using function generators but I've not yet found a way to implement that generically for the users which is one reason I've not added this already.

The code looks fine though but I want to discuss the approach a bit more.

I'm less concerned about the changes in the test but I'm not a big fan of changing the log statement to a WARN. It's not a warning and given that the default logging level for rules was INFO (now it's TRACE in 3.2) I would prefer to keep it at the info level.

Urmel commented 2 years ago

Thanks for the feedback. Indeed - it did look like a nail, but using a looping timer might be the better option. As I understand, I can just return a date time (like "13.04.2022 12:34:00") and the looping function is then called again. Exactly what I need to implement my alarm clock.

I will this close this PR. Thanks.