GladysAssistant / Gladys

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

ZwaveJS-UI: Add support for multilevel switch #2038

Closed MathieuAndrade closed 4 months ago

MathieuAndrade commented 6 months ago

Pull Request check-list

To ensure your Pull Request can be accepted as fast as possible, make sure to review and check all of these items:

NOTE: these things are not required to open a PR and can be done afterwards / while the PR is open.

Description of change

Add support for multilevel switch

image

image

codecov[bot] commented 6 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 98.39%. Comparing base (9d50528) to head (13bd988). Report is 23 commits behind head on master.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #2038 +/- ## ======================================= Coverage 98.38% 98.39% ======================================= Files 844 846 +2 Lines 13743 13805 +62 ======================================= + Hits 13521 13583 +62 Misses 222 222 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

relativeci[bot] commented 6 months ago

Job #2481: Bundle Size — 10.07MiB (~+0.01%).

13bd988(current) vs e4c0f82 master#2480(baseline)

[!WARNING] Bundle contains 3 duplicate packages – View duplicate packages

Bundle metrics  Change 2 changes Regression 1 regression
                 Current
Job #2481
     Baseline
Job #2480
Regression  Initial JS 5.39MiB(~+0.01%) 5.39MiB
No change  Initial CSS 303.21KiB 303.21KiB
Change  Cache Invalidation 53.43% 0%
No change  Chunks 51 51
No change  Assets 170 170
No change  Modules 1465 1465
No change  Duplicate Modules 21 21
No change  Duplicate Code 0.82% 0.82%
No change  Packages 124 124
No change  Duplicate Packages 3 3

Bundle size by type  Change 1 change Regression 1 regression
|            |       Current
[Job #2481](https://app.relative-ci.com/projects/PUROh8FAVkDKmpUrqr4u/jobs/2481-XrgsCR1SzKTnTYwKhQOQ?utm_source=github&utm_medium=pr-report "View job report") |      Baseline
[Job #2480](https://app.relative-ci.com/projects/PUROh8FAVkDKmpUrqr4u/jobs/2480-keFmvmjhRF0g2dRGPxw9?utm_source=github&utm_medium=pr-report "View baseline job report") | |:--|--:|--:| | Regression  [JS](https://app.relative-ci.com/projects/PUROh8FAVkDKmpUrqr4u/jobs/2481-XrgsCR1SzKTnTYwKhQOQ/assets?ba=%7B%22filters%22%3A%22ft.CSS-0_ft.JS-1_ft.IMG-0_ft.MEDIA-0_ft.FONT-0_ft.HTML-0_ft.OTHER-0%22%7D "View JS assets") | `7.17MiB` (`~+0.01%`) | `7.17MiB` | | Not changed  [IMG](https://app.relative-ci.com/projects/PUROh8FAVkDKmpUrqr4u/jobs/2481-XrgsCR1SzKTnTYwKhQOQ/assets?ba=%7B%22filters%22%3A%22ft.CSS-0_ft.JS-0_ft.IMG-1_ft.MEDIA-0_ft.FONT-0_ft.HTML-0_ft.OTHER-0%22%7D "View IMG assets") | `2.46MiB` | `2.46MiB` | | Not changed  [CSS](https://app.relative-ci.com/projects/PUROh8FAVkDKmpUrqr4u/jobs/2481-XrgsCR1SzKTnTYwKhQOQ/assets?ba=%7B%22filters%22%3A%22ft.CSS-1_ft.JS-0_ft.IMG-0_ft.MEDIA-0_ft.FONT-0_ft.HTML-0_ft.OTHER-0%22%7D "View CSS assets") | `319.99KiB` | `319.99KiB` | | Not changed  [Fonts](https://app.relative-ci.com/projects/PUROh8FAVkDKmpUrqr4u/jobs/2481-XrgsCR1SzKTnTYwKhQOQ/assets?ba=%7B%22filters%22%3A%22ft.CSS-0_ft.JS-0_ft.IMG-0_ft.MEDIA-0_ft.FONT-1_ft.HTML-0_ft.OTHER-0%22%7D "View Fonts assets") | `93.55KiB` | `93.55KiB` | | Not changed  [Other](https://app.relative-ci.com/projects/PUROh8FAVkDKmpUrqr4u/jobs/2481-XrgsCR1SzKTnTYwKhQOQ/assets?ba=%7B%22filters%22%3A%22ft.CSS-0_ft.JS-0_ft.IMG-0_ft.MEDIA-0_ft.FONT-0_ft.HTML-0_ft.OTHER-1%22%7D "View Other assets") | `17.53KiB` | `17.53KiB` | | Not changed  [HTML](https://app.relative-ci.com/projects/PUROh8FAVkDKmpUrqr4u/jobs/2481-XrgsCR1SzKTnTYwKhQOQ/assets?ba=%7B%22filters%22%3A%22ft.CSS-0_ft.JS-0_ft.IMG-0_ft.MEDIA-0_ft.FONT-0_ft.HTML-1_ft.OTHER-0%22%7D "View HTML assets") | `13.58KiB` | `13.58KiB` |

View job #2481 reportView MathieuAndrade:master branch activityView project dashboard

William-De71 commented 6 months ago

I have device with the same feature multilevel switch but in my case it's not a lighting ! So are you sure this is the good category? For example my devices are used to drive electrical heater and shutter.

MathieuAndrade commented 6 months ago

Which category should we use instead? Gladys doesn't have a "generic" category for this device type that you can use in various cases, and the service doesn't seem designed to manually set the device type either. 🤔

William-De71 commented 6 months ago

I also started working on the integration and chose the switch.dimmer type. I think it's the most 'generic' type you can use? What do you think? Because I don't want to see my shutters or heaters as lights

image

Pierre-Gilles commented 5 months ago

@MathieuAndrade Thanks for the PR 🙌

Indeed, is there any way in ZwaveJS UI MQTT payloads to differentiate devices? How do you know if it's a light or a rolling shutter?

MathieuAndrade commented 5 months ago

Hello !

@William-De71 If this category works as same as LIGHT.BRIGHTNESS why not, if i have time this week a tried to check this.

@Pierre-Gilles There is no difference for ZwaveJSUI, a multilevel is a multilevel, you can plug in a light, a rolling shutter, a fan, a motor or anything that works with power variation. The must would be to let the user tell to gladys what kind of device is plugged.

PS: Sorry for the late response

William-De71 commented 5 months ago

@MathieuAndrade , Maybe my solution isn't the best solution, but in my opinion it's the most generic feature in Gladys that could work.

@Pierre-Gilles , @MathieuAndrade is right, a multilevel is a multilevel, in my case the only way to differentiate devices is to read the "productDescription" Below an example of my shutter and one of my heater: image

image

Pierre-Gilles commented 5 months ago

@MathieuAndrade @William-De71

There is no difference for ZwaveJSUI

Shit, sure there is no way? It'll pose issues for basic lights, if they are detected as power plugs, it'll be really annoying in Gladys... Same for rolling shutters...

We need to find a way, even if it's a hacky one.

MathieuAndrade commented 5 months ago

@William-De71 I have switched to SWITCH.DIMMER and it works as expected, i can push it if is good for @Pierre-Gilles

@Pierre-Gilles If you want, you can read product information (manufacturer, id, description and type) to know what type device is, but this adds a useless complexity in my opinion. And like I said above, the user can plugin anything that works with power management on this type of Zwave device so it won't work in many cases,

For exemple: A multilevel roller shutter, A multilevel RGBW controller, A multilevel dimmer

If you read the device's description:

So :

These examples are only for Fibaro devices, but all Zwave devices are not same, and all manufacturer can implement functionalities differently in his device. So, there is no real method to know how a user use his Zwave device, otherwise, ZwaveJS would have already implemented it. :information_desk_person:

In my opinion, the generic type proposed by @William-De71 (SWITCH.DIMMER) is the best solution until the user can manualy tell to Gladys what kind of device is plugged on his Zwave device.

Pierre-Gilles commented 5 months ago

@MathieuAndrade Ok let's do it then :)

sescandell commented 4 months ago

Hello,

I also worked on the Multilevel Switch integration. Unfortunatelly, I saw your PR only when I first started to push my integration on Github. PR is available here.

There is no difference for ZwaveJSUI, a multilevel is a multilevel, you can plug in a light, a rolling shutter, a fan, a motor or anything that works with power variation. The must would be to let the user tell to gladys what kind of device is plugged.

You are totally right for a multilevel switch device such as... "classical" switches. But, zWave provides some context about the device class. The property is called deviceClass. In the case of multilevel switch for curtains, usually, devices let us know they are for managing curtains.

That's one thing handled in my proposition. Thanks to that, we can nativelly map to COVER devices or DIMMER ones. There is no way for dimmers to know if there is a light or anything else at the end. But, for curtains, we can.

Another thing managed in my PR proposition is that one zWave property could be mapped to multiple "Gladys feature". This provide the capacity for dimmers to have two kind of actionners: dimmer (you can set any value from 0 to 99) or binary state ; and for multilevel curtains switch: position (you can set any value from 0 to 99) or managing OPEN|STOP|CLOSE. That way we can provide different ways to interact with the device. Example here: image

Let me know what you think about this proposition

Pierre-Gilles commented 4 months ago

Thanks @sescandell for the PR 🙏

I agree with your PR, if there are no feedback from @William-De71 and @MathieuAndrade before Monday 6, I'll deploy your PR :)

Pierre-Gilles commented 4 months ago

Merged in https://github.com/GladysAssistant/Gladys/pull/2061