HestiaPi / hestia-touch-openhab

OpenHAB2 files for HestiaPi Touch model
GNU General Public License v3.0
60 stars 17 forks source link

Use global lambdas and the Associated Items DP to avoid duplicated code #26

Closed rkoshak closed 4 years ago

rkoshak commented 4 years ago

While Rules DSL doesn't support proper functions, it does support lambdas and it supports global (to a single .rules file) variables. So you can use a globally defined lambda as a function that can be reused by multiple Rules in the same .rules file.

There are some significant limitations. For one lambdas are not thread safe so if two Rules call it at the same time they will stomp on each other. Also, errors that happen inside them tend to be suppressed making debugging challenging. Also, a lambda can only have up to seven arguments. finally, a global lambda has no context at all so that means you have to pass everything the function needs to work on as an argument.

In general I recommend avoiding their use. However, there are several places in the .rules files I've seen where I think that use of a global lambda is warranted. In particular the second stage heating would really benefit from a lambda.

The syntax is pretty simple. See https://community.openhab.org/t/reusable-functions-a-simple-lambda-example-with-copious-notes/15888.

All the submissions I've made so far have been pretty limited and simple. This one would be a significant restructuring. I don't want to just spring such a big change on you.

On a similar note, there are other techniques that I think would further consolidate the .rules code. For example, the difference between all the boost mode rules if pretty much just the names of the Items and Timers that are updated. These could all be consolidated into one Rule and use the triggeringItem implicit variable and https://community.openhab.org/t/design-pattern-associated-items/15790 to make the rule work for all the boost modes.

Overall, shorter Rules will parse faster so this should have benefits to both the reboot time but also make the Rules easier to understand and maintain.

I do this sort of stuff for OH users all the time and am happy to work on it but I don't want to make such drastic changes when other major changes are in the plans. Maybe this is more appropriately part of #10? Or maybe it would better to do it before #10?

gulliverrr commented 4 years ago

I can certainly see the benefits of this as I do believe a 20-30% of code in rules can reuse a few lambdas. It will make #10 easier to do and more feasible as the reason to split the rules in 2 separate files was the load they were adding when they were combined. I would vote to be done before #10 tbh but I can see it being a lot of work all over the place with a lot of testing.

rkoshak commented 4 years ago

Is there a reason everything is in one .rules file? It can be useful, particularly when developing and for those users who want to maintain their own changes to the Rules if they were in separate files. But there might be some reason I don't know to keep them separated.

I've submitted the restoreOnStartup changes and am now going to start work on this. Expect a PR in a few days.

gulliverrr commented 4 years ago

rules were split per model (US/EU) for performance purposes but for each model the rules remain in each separate single file (stored in /home/pi/scripts/hvac.rules and /home/pi/scripts/generic.rules until boot has finished). I'm happy to split them if that will make life easier. One thing to take care when this will be done is the hiding/unhiding of the rules file in the following script on boot (around line 21 and 76) but after your recent changes this technique may be obsolete: https://github.com/HestiaPi/hestia-touch-openhab/blob/ONE/home/pi/scripts/openhabloader.sh

gulliverrr commented 4 years ago

Solved by #28