search

rkoshak / openhab-rules-tools

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

Allow passing names to created timers #71

Closed florian-h05 closed 1 year ago

florian-h05 commented 1 year ago

Reference #70.

@rkoshak I have finished the utility function to create timers.

I also added a first proposal how to implement the timer naming in all libs that use it.

florian-h05 commented 1 year ago

All tests except the timerMgrTests are passing. I checked the timerMgrTests with with the current version of the repo (so without my changes), they also fail.

I think that I should have a look at the timerMgrTests.

florian-h05 commented 1 year ago

@rkoshak I currently have no idea what's the problem with the timerMgrTests failing and I am not able to find it out currently, as I am having problems with https://github.com/openhab/openhab-addons/issues/13022 while running the tests.

All I can say, is that I am not expecting any problems with timerMgr due to the minimal changes and that it is running in my production setup.

rkoshak commented 1 year ago

I'll take it as tested then. Writing those timerMgr tests was a huge pain in the butt because of that multithreaded problem. The only way to fix it is to massage the times to avoid timers from trying to run at the same time.

This is ready to merge then, right?

florian-h05 commented 1 year ago

Writing those timerMgr tests was a huge pain in the butt because of that multithreaded problem.

I can imagine. I even have that problem in my production code, so would be great if somebody would fix it. Unluckily, I don't have the knowledge to solve that problem.

This is ready to merge then, right?

Yes.

One thing to mention about the timerMgr tests: Some time, I was able to run them. With this PR, there were less tests to fail than with the current codebase in main.

rkoshak commented 1 year ago

I can imagine. I even have that problem in my production code, so would be great if somebody would fix it.

Me too. My Open Door rule template can hit that problem if a bunch of events happen all at once.

Speaking of that, a little bit that might help some and why I wanted to centralize the code that actually creates the timers in this PR is because I've an idea to keep track of the scheduled timers and add half a second to one if it's scheduled on top of a timer that is already scheduled. I've done this in the Open Door rule template and it cuts down on the number of times the error occurs.

But a permanent fix would be greatly appreciated.

OK, I'll get this merged and cut a new version so it gets picked up by npm.

Thanks!