GladysAssistant / Gladys

A privacy-first, open-source home assistant
https://gladysassistant.com
Apache License 2.0
2.54k stars 278 forks source link

zWave Version discussion #614

Closed sescandell closed 3 years ago

sescandell commented 4 years ago

Hi,

I'm opening this issue in order to firstly open the discussion before going on any modification. There is an issue with how "openzwave lib is installed". I'm taking here Docker as a sample. But, I imagine, the same goes for the raspberian version (even if for this one I didn't yet dive into the workflow to generate the image).

There is 2 issues from my POV with the openzwave installation in Docker:

  1. the version installed is the 1.4.164: that is... more than outdated (released around... April 2016 if I'm not wrong... based on this package and the same Changelog that can be found on this Commit but not that one made just after )
  2. the OZW database is so really outdated and most of the devices are unknown (this is the most important issue in my opinion)

I see two ways to fix this:

  1. use a manual install through a git clone && make && make install into the build script based on a known version from Github
  2. stay with the apk add openzwave installation but manually update the device database (said simply by cloning the repo then copy/pasting the config folder)

Issue with first solution: if I'm not wrong, version 1.6.X is not fully supported by node-openzwave-share. That being said, bases on some simple tests on a custom installation, it seems to be working so far... But I didn't (yet) tested all Alarm/Notification/ControlScene features (features that might need changes...)

With the second solution, I have no idea if there might be any "new feature" that could be referenced in the device database that would not be supported by the version 1.4...

I can work on these changes. But before working on a PR that is not aligned with your vision, I prefer to open this issue and discuss with you about this.

Waiting for your feedbacks

Pierre-Gilles commented 4 years ago

Hey! Thanks for your message :)

There are 2 topics here:

Unknown device problem

This is not related to the version of openzwave, there was a bug in the Docker image that I fixed yesterday in this commit => https://github.com/GladysAssistant/Gladys/commit/5685b452d01aed1e4b2abeda62980f624509a773 (haven't fully tested yet so i didn't communicate on this yet)

OpenZwave device db was missing + the path node-openzwave-shared was looking for was not the right one for Alpine Linux. This commit fixes this, and users should now see there devices.

Open-Zwave version

It's not as bad as you describe, Open-Zwave announced that 1.6 was out in March 2019, and it seems that dependent library had some work to do in the meantime to migrate from 1.4 to 1.6.

On node-openzwave-shared side, there were still working on it this summer!

I'm 100% for using 1.6 if there are no bug with node-openzwave-shared, but let's test this extensively before doing the change.

To install OZW 1.6 in Alpine Linux, I don't think we want to build from source (I won't recommended that, OZW often publish bugged things on master and fix that in tagged releases)

Instead, Alpine Package provide a "Edge" release tag which provide the 1.6 version:

https://pkgs.alpinelinux.org/packages?name=openzwave&branch=edge

On Z-Wave in Gladys in general

I want Gladys Z-Wave integration to be one of our best integration, so I work hard to make this service really clean!

Here is my todos of improvements that I have in mind: https://github.com/GladysAssistant/Gladys/issues/611

sescandell commented 4 years ago

Unknown device problem

This is not related to the version of openzwave, there was a bug in the Docker image that I fixed yesterday in this commit => 5685b45 (haven't fully tested yet so i didn't communicate on this yet)

OpenZwave device db was missing + the path node-openzwave-shared was looking for was not the right one for Alpine Linux. This commit fixes this, and users should now see there devices.

That was only a part of the problem. The DB in the 1.4 version is from April, 2016. Since then, many devices have been added. It might still remains some unknown devices. As an example, the latest Wall Plug from Fibaro is not recognized by 1.4 version. But, a simple update of the DB can fix this issue.

To install OZW 1.6 in Alpine Linux, I don't think we want to build from source (I won't recommended that, OZW often publish bugged things on master and fix that in tagged releases)

I was not thinking about compiling from master, but from a given tag or commit. That being said, edge repo might be a better solution as you suggested 👍

I probably mis-expressed myself. The big issue (from my POV) is not about 1.4 vs 1.6 but more about the devices DB (well... until node openzwave-share release a 1.6 compliant version). What I can suggest here is simply to change the Build process to clone this latest DB and push it in place of the one from the apk add command. I'll made a PR, you'll do what you want with it, code is sometimes simpler to understand 😄

Thanks for your time!

Pierre-Gilles commented 4 years ago

Sure! But maybe just upgrading to 1.6 is ok, can you try first just upgrading to 1.6? If it works, let's just migrate to this new version ;)

Pierre-Gilles commented 4 years ago

Hi @sescandell ! Any news on that ? :)

sescandell commented 4 years ago

Hi @Pierre-Gilles ,

Sorry, I was personnaly very busy for the latest weeks. I should be able to be back on Gladys in the coming days and try something about zWave and its version,

Pierre-Gilles commented 4 years ago

Nice! Don't hesitate to post everything you try here :)

What I see:

sescandell commented 4 years ago

Hi,

I was going to test this but, since Dec. 19 (2 weeks after the opening of this issue finally), the node12-alpine image is referencing the alpine 3.11 that natively includes the OZW1.6. So Gladys is already using the OZW 1.6 (starting from Dec. 19). If you didn't encounter any issue... it would means this is working as expected :)

That means one important thing: Gladys project should be very precise about the alpine version to use in its Docker file:

FROM $target/node:12-alpine3.11

Or if you want to go back to the 1.4 version:

FROM $target/node:12-alpine3.10

That being said, if I'm not wrong, the test pipeline is not based on the 1.6 version, but on the 1.5 (that is 1.4 if i'm not wrong). If you can confirm this, I can try to update your CI pipeline to run tests on 1.6

Let me know what you think,

Pierre-Gilles commented 4 years ago

@sescandell Thanks for your message, that's good news !

If you can confirm this, I can try to update your CI pipeline to run tests on 1.6

Yes, could you propose a PR based on 1.6 in tests? :)

Thanks 🙏

VonOx commented 4 years ago

If I understand, we must use OZW 1.5 in Gladys image right?

Pierre-Gilles commented 4 years ago

No! We want to use the "latest stable release" available.

The issue is that OZW team is not really consistent in their official package release.. they tend to release only "once in a while".

So @sescandell is trying to build Gladys Docker image using a fixed commit on GitHub, fixed commit that we'll update frequently.