openhab-scripters / openhab-helper-libraries

Scripts and modules for use with openHAB
Eclipse Public License 1.0
88 stars 69 forks source link

Initial checking of deferred actions #283

Closed rkoshak closed 3 years ago

rkoshak commented 4 years ago

A set of library functions that lets one schedule a command to be sent to an Item at a defined point in the future. This library makes this use case simpler to perform instead of requiring users to create and manage the timers themselves.

There is some significant amount of code shared between the this and Expire Binding replacement for parsing the times. I'd like to extract that code and put it in the core.utils if that makes sense. I've found two places where having it is useful, I'm certain I'll find more and others will have their own use cases as well.

Signed-off-by: Rich Koshak rlkoshak@gmail.com

CrazyIvan359 commented 4 years ago

I like the idea of pulling this out of the rule and making it a library. I'm only using something like this in a couple of rules but this makes it much more portable.

rkoshak commented 4 years ago

I don't use this at all in my Rules but someone wrote a DP on it and I figured there must be some demand for something like this. I agree, it made more sense to make it a module than a script, similar to the gatekeeper.

What is nagging me in the recesses of my brain is whether this is actually all that different from gatekeeper or if this capability should be merged with it. The only difference is gatekeeper runs and then sleeps while this sleeps and then runs. Maybe that's enough of a difference to keep them separated.

CrazyIvan359 commented 4 years ago

Indeed, in my case I need the sleep then command style. Essentially what I'm doing is a variable expire for nightstand lights depending on what triggered them. If it was someone getting up the timeout is 5m and if it's a pee light the timeout is 3m.

The sleep then run behaviour is actually what I expected the gatekeeper to do when I read the overview the first time.

rkoshak commented 4 years ago

Some of the changes you made to Expire probably need to be made in this one too around the parsing of the time string. I'll need to take a look at that. Though, like I said above, I'd really like to pull that stuff out into it's own library or into core.utils. But I'll wait for Scott to start reviewing these PRs before I start doing that.

There are some open questions I have that I want answered before I go too far:

CrazyIvan359 commented 4 years ago

The changes I made to the time parser were just the way it handles exceptions, but I agree that it would be useful as a utility function of its own.

5iver commented 4 years ago

But I'll wait for Scott to start reviewing these PRs before I start doing that.

I actually started reviewing several of your PRs, but have had to restart several times because you've added more commits. I've wasted many hours now, so I will stop until you specifically tell me you are ready for review (i.e. done committing code). Typically, you only commit once you are code complete and ready for a review. You then only make changes that are asked for by a maintainer, and give them a courtesy heads up after each round of review/fixes. If you are not ready for a review or want others to provide input, then have people look at the code in your fork, or create a draft PR, or mark the PR as a WIP, so that a maintainer knows that it is not ready to start reviewing.

I had thought you were more familiar with this process or I would have mentioned something sooner.

rkoshak commented 4 years ago

I figured since the PRs were left without comment or indication that there was a review that they were just hanging out in limbo. I will submit no more commits or PRs until told to do so. When I submitted the original PRs the code was ready for review and I had no intention on changing anything. But over the weeks and months stuff was discovered so I fixed them. I'll refrain from doing so from now on, though bug and other stuff will be discovered when the time between submission and review extends. This isn't a dig at you, I know you've been busy. But some of my first submissions were made in August and have been in use by me and others. Problems will be discovered over that time.

I believe I have provided a heads up after making the changes for hysteresis which is the only one of my PRs that has been reviewed by you.

There has only been the one commit made to this PR. I made some commits to other PRs based on a review from CrazyIvan. Should I not respond to those until you review? I thought (perhaps mistakenly) that CrazyIvan was a maintainer here too.

rkoshak commented 4 years ago

This PR is ready for review

CrazyIvan359 commented 4 years ago

No, I'm not a maintainer, just a contributor. I'm still converting my rules to Python and your libraries interest me and are useful for my rules most of the time, so I read them and offer my 2 cents.

rkoshak commented 4 years ago

Does it make sense for me to withdraw these and let them "burn in" for a few months before submitting them? Or maybe just focus on role templates. Everyone's time is precious and I have that I've waisted yours.

rkoshak commented 3 years ago

I've basically completely rewritten this so am closing this PR. When reviewing submissions to the helper library becomes a priority I'll resubmit. In the mean time I'll keep in in my own repo where I can continue development.