openhab / openhabian

openHABian - empowering the smart home, for Raspberry Pi and Debian systems
https://community.openhab.org/t/13379
ISC License
820 stars 251 forks source link

Add openhab_rules_tools #1651

Closed mstormi closed 1 year ago

mstormi commented 2 years ago

https://community.openhab.org/t/announcing-the-initial-release-of-openhab-rules-tools/132032/3

florian-h05 commented 2 years ago

@mstormi When you add an option to install those, you could also add an option to install the openhab-js library from npm.

mstormi commented 2 years ago

what about contributing yourself ?

florian-h05 commented 2 years ago

When I have some time, I will take a look into this. Then I can also add a option to add the openhab_rules_tools.

florian-h05 commented 1 year ago

@mstormi Which menu section would you choose for adding openhab-js and openhab_rules_tools?

„openHAB Related“ or „Optional Comments“? I don‘t have a strong opinion about that, but I would slightly prefer „openHAB Related“.

mstormi commented 1 year ago

yes 40, OH-related

florian-h05 commented 1 year ago

@mstormi I opened the PR just a few seconds ago.

rkoshak commented 1 year ago

This has long been on my to-do. Thanks for forging ahead.

The only thing I have is once it's installed, it also needs to be a part of the upgrade process somehow.

If this is already taken care of, excellent! I'm not really in a place to read and understand the code right now though to see.

florian-h05 commented 1 year ago

The only thing I have is once it's installed, it also needs to be a part of the upgrade process somehow.

Good point, that is something I did not think about. I'll fix that.

mstormi commented 1 year ago

do we need an install option after all or can we install it by default ?

florian-h05 commented 1 year ago

I am not sure about auto-installing.

openhab-js is included in the JS Scripting Add-On by default, so the only reason to manually install it is to upgrade to a newer version that is not included in the Add-On yet. If we auto-install, we override the included version and have to update it manually and the functionality of the library might be out of sync with the add-on docs, which is a problem. Regarding openhab-js, I am against auto-installing.

If the user decides to install it manually, we should update it when openHAB is updated.

With openhab_rules_tools it's something different, as the add-on docs don't mention it and it can't be out of sync. I am not sure where we could mention that it is auto-installed, if we don't, it would not make much sense because when you don't mention something is there you can't use it.

rkoshak commented 1 year ago

With openhab_rules_tools it's something different, as the add-on docs don't mention it and it can't be out of sync.

It will be out of sync. I've not considered trying to keep it in sync with openHAB, but perhaps it makes sense to consider some sort of sync. But I'm not sure I have the insight and bandwidth to do that. I do have a bunch of upgrades planned in the near future to take advantage of the ability to apply a name to timers so it's easier to track errors that occur in timers.

While it won't hurt anything to have it installed by default, it's only useful when the JS Scripting add-on is installed so I'm not sure if it makes sense to auto install it from that perspective alone. Does it make sense to install something that isn't used? There might be those who do not want to use openhab_rules_tools too and I wonder if we should force them to get it.

florian-h05 commented 1 year ago

It will be out of sync. I've not considered trying to keep it in sync with openHAB, but perhaps it makes sense to consider some sort of sync.

When I was talking about „in-sync“ I primarily thinking of the JS Scripting Add-On docs, which is only a problem for openhab-js, but we also have to consider the compatibility side.

Does it make sense to install something that isn't used? There might be those who do not want to use openhab_rules_tools too and I wonder if we should force them to get it.

It doesn‘t make sense to install it if JS Scripting is not installed.

I vote against auto-installing in both cases, but if one decides to install it using openhabian-config, we should provide automatic updates of the libraries. Can we agree on that? @rkoshak @mstormi

mstormi commented 1 year ago

There might be those who do not want to use openhab_rules_tools too and I wonder if we should force them to get it

I wouldn't overpathologize. The number of people to notice it's installed at all will be low and they aren't forced to use it, are they? The number of people to find it in some menu will be even lower. My vote is for auto-install. Then again I agree it's more important than this that updates are automated.

openhabian-config tool check its github repo on start, tells user about news and if user agrees syncs them down to his box. Feel free to reuse that mechanism.

florian-h05 commented 1 year ago

My vote is for auto-install. Then again I agree it's more important than this that updates are automated.

Auto-install for openhab_rules_tools. Auto-install for openhab-js doesn‘t make much sense, as it is included in the Add-On by default and should be up-to-date for the moment a openHAB version is released (I am taking care of this as one of the openhab-js maintainers).

openhabian-config tool check its github repo on start, tells user about news and if user agrees syncs them down to his box. Feel free to reuse that mechanism.

Sounds interesting, but then somebody (@rkoshak as the maintainer of openhab_rules_tools) needs to update something in the repo. Maybe I can implement something to check for updates directly on npm, should be the better option. I‘ll try this when I have some time.

mstormi commented 1 year ago

but then somebody (@rkoshak as the maintainer of openhab_rules_tools) needs to update something in the repo

We can also retrieve the latest version directly from his repo [that's the openHABian philosophy anyway] and add the check to the startup checker routine.

florian-h05 commented 1 year ago

This sounds good.

We can get the current installed version with npm list openhab_rules_tools --prefix /etc/openhab/automation/js/ and check for the recent version with two different methods:

I would prefer to use npm, as the return is easier to handle and this doesn‘t depend on release names.

rkoshak commented 1 year ago

Sounds interesting, but then somebody (@rkoshak as the maintainer of openhab_rules_tools) needs to update something in the repo.

Don't worry about that. To support npm I have to do this anyway. Any time I have a new version I have to publish it to npm. And since it's installed and updated through npm, all you have to worry about here is to npm update and it'll pull down the latest published version.

And in the unlikely event that the need arises to keep openHABian on an older version, we can fix the version in the package-lock.json.

We can also retrieve the latest version directly from his repo [that's the openHABian philosophy anyway] and add the check to the startup checker routine.

Please get and install it via npm. My repo is mostly just me so I'm not really great at working in branches and such since I don't really need to. If I'm actively working on something, main will probably be broken while I'm working it. But I test fairly thoroughly before I cut a new release version and publish it to npm. Also, in the event that I depend on some other node library, npm will handle installing the dependencies automatically.

I make a pretty big effort to keep the library backwards compatible so auto upgrading with the openHABian scripts should not be too much of a problem.

I don't have any major problems with it being installed by default either. The security engineer in me doesn't like it but in the scheme of things having it there when it's not needed is relatively benign.

I would prefer to use npm, as the return is easier to handle and this doesn‘t depend on release names.

As you can see, I prefer npm too. When I create a release on github, it's because I'm publishing a new version to npm so npm will always have the latest version I want people installing.

florian-h05 commented 1 year ago

@rkoshak Thanks for your detailed answer, I don't want to explain my current way of handling that at two places.

Would be cool if you'd have a look at the PR and leave your feedback.

rkoshak commented 1 year ago

I hate to ask here since this is closed but not sure where else to ask. Does this mean that when existing users of openHABian upgrade that they will automatically get openhab_rules_tools or will those users need to manually install them? I tried to look at the code but I couldn't trace to see what happens in that case. I'd like to update my docs as appropriate.

florian-h05 commented 1 year ago

AFAIK as I know, existing openHABian installations need to manually install it.

@mstormi Please correct if I’m wrong.

mstormi commented 1 year ago

correct