zwave-js / node-zwave-js

Z-Wave driver written entirely in JavaScript/TypeScript
https://zwave-js.github.io/node-zwave-js/
MIT License
741 stars 596 forks source link

[bug] Nano Dimmers by Aeotec does not report back to the controller when switched manually #1287

Closed pbathuk closed 3 years ago

pbathuk commented 3 years ago

latest from docker : zwavejs/zwavejs2mqtt:latest

Build/Run method

zwavejs2mqtt version: Alpha - sorry did not grab the version number (but I've updated the docker today if that helps - problem has been there since first docker version of zwavejs

Describe the bug When using Nano Dimmers by Aeotec the switch itself (normal UK toggle switch) does not report back to the controller it seems. On the older zwave2mqtt docker this worked correctly, however no matter what setting for

Notification report association group 1 (4-112-1-80)

I set on any of my switches the switch vs HA / Dashboard become out of sync as soon as the light switch itself is used. I have tracked this down to an error on the debug log - see bottom Additional Context

To Reproduce Steps to reproduce the behavior:

  1. Setup latest zwavejs
  2. Have a aeotec nano dimmer
  3. using the hail function as Send Hail CC when using the manual switch to change the load state
  4. Turn the light on via the switch
  5. notice that the dashboard / HA does not mirror the switch values

Interestingly the dimmer value IS set if you use anything other than Hail, however the switch does not appear "on"

Expected behavior Dashboard / HA to be in-sync with light regardless of source of turning on or off.

Additional context

15:46:23.553 SERIAL » [ACK] (0x06)
15:46:23.555 DRIVER « [Node 004] [REQ] [ApplicationCommand]
└─[ConfigurationCCReport]
parameter #: 80
value size: 1
value: 4
15:46:35.408 SERIAL « 0x010a00040004028201cf00bb (12 bytes)
15:46:35.409 DRIVER Dropping message because it could not be deserialized: The command class Hail
is not implemented
15:46:35.409 SERIAL » [ACK] (0x06)
15:46:38.108 SERIAL « 0x010a00040004028201d200a6 (12 bytes)
15:46:38.109 DRIVER Dropping message because it could not be deserialized: The command class Hail
is not implemented
varet80 commented 3 years ago

looking into CCs Hail command class is obsolete. This command class is not currently implemented in the driver. document source https://github.com/zwave-js/node-zwave-js/blob/master/specs/SDS13548%20List%20of%20defined%20Z-Wave%20Command%20Classes.xlsx

robertsLando commented 3 years ago

@pbathuk you should check the device associations. Does it have lifeline associations with controller?

@AlCalzone Is this a device compatibility issue?

robertsLando commented 3 years ago

Humm I think it's something fixed in https://github.com/zwave-js/node-zwave-js/pull/1278

AlCalzone commented 3 years ago
3\. using the hail function as Send Hail CC

Do you have any other option, e.g. Basic Set or Binary Switch Report?

pbathuk commented 3 years ago

I've tried to set as basic CC and get the following:

19:25:38.313 SERIAL « 0x010b0004000603200363c9007c (13 bytes)
19:25:38.314 SERIAL » [ACK] (0x06)
19:25:38.316 DRIVER « [Node 006] [REQ] [ApplicationCommand]
└─[BasicCCReport]
current value: 99
19:25:38.317 DRIVER Send queue: (empty) (0 messages)
19:25:38.318 DRIVER send thread state: idle
19:25:38.318 DRIVER Send queue: (empty) (0 messages)
19:25:38.319 CNTRLR [Node 006] [~] [Multilevel Switch] currentValue: 0 => 99 [Endpoint 0]
z2m:Zwave Node 6: value updated: 38-0-currentValue 0 => 99 +12s
19:25:38.320 DRIVER send thread state: idle
z2m:Mqtt Message received on zwave/7/37/1/0/set +26s

As you can see below it's the current value 6 that is still false. In the older version both the number and true / false that were set. (Based on hail CC) Screenshot_20210104-192611_Chrome

AlCalzone commented 3 years ago

The Multilevel switch value gets updated correctly now. Currently only the "main" mapping gets done automatically. Since this device identifies itself as a multilevel switch, this is the CC that Basic gets mapped to.

I'm unsure how to proceed with this. Maybe with a compat flag that maps the report to both CCs?

kpine commented 3 years ago

@pbathuk In zwave2mqtt were you actually able to control the switch using the binary value, or was it read-only?

blhoward2 commented 3 years ago

Obsolete or not, there are random older devices that use the Hail class. How big of a deal is it to add it? I haven't tested this yet but I think my Leviton DZPA1's send a Hail when controlled by direct association. When I use a scene controller to turn them on they don't reflect being turned on and they used to work find under ozw 1.4 and 1.6.

My Aeotec dimmers are also set to Hail and I recall Basic CC not working correctly but it was a few years ago.

kpine commented 3 years ago

The Hail class has no meaning as defined by the Z-Wave spec, it is application specific. However, I believe a few devices were using this to circumvent "instant status" patents, or something like that, so several hubs treat this as a message to poll the device, including OZW (since 2010!) and OpenHAB. SmartThings implements this on specific device handlers, and for those implementations it seems to just do a Basic Get on the node (only for certain hub versions??). Hubitat is probably the same since it is based on SmartThings. Not sure about other hubs. The consensus seems to be do some sort of Get to refresh the device, whether that's the entire device or just a Basic Get. If there is interest in supporting these old devices, it would be good to implement in this manner.

That said, you can imagine that using the Hail setting on a device like the Nano Dimmer is not ideal if the hub is polling all its values. This dimmer has several value to refresh (several power related, alarms, dimmer level), which will add extra network traffic. In OZW even using the Basic CC isn't the best because it triggers a Get of the multilevel value instead of just using the mapped value as reported. For this reason alone I'd personally configure the Nano Dimmer to send Basic Report (only if there are no Get commands by zwave-js involved) or Switch Multilevel, and just use the dimmer level values instead of the binary one.

For comparison, neither OpenHAB and SmartThings utilize the Binary Switch command class for this device, and SmartThings automatically configures it for Basic Reports.

Also why isn't the current value of the level (6-38-0-currentValue) shown in the screenshot, was it cutoff?

pbathuk commented 3 years ago

Thanks for all the comments, seems like I have a device (or 4) that does not fully meet the standards, which is always fun. @kpine I will repeat the basic CC update and copy the full logs + send before and after images as well. Unfortunately I just accidentally only took one screen shot, rather than a full image.

What I can say, and will grab the logs to share, is that on zwave2mqtt the hail cc only updated a few sections, so not a full refresh.

It will take me a while as working now, but once I have done it I will post all info. Hopefully that helps. The alternative is (and this would only help me) is that for these devices the HA template uses the dimmer value for on / off rather than the true / false value. That way no other changes would be needed.

AlCalzone commented 3 years ago

I tend to agree with @kpine. Implementing the Hail CC is no big deal per se, but the driver needs to react to it, which depends on #1163. While there are sensible workarounds that also don't cause additional network traffic (the Basic Set/Report does not trigger a query), I prefer to work on more pressing matters.

I've lately seen a few devices where the Binary Switch mirrors the Multilevel Switch and don't understand why. Maybe it is worth just hiding the CC on affected devices?

blhoward2 commented 3 years ago

I tend to agree with @kpine. Implementing the Hail CC is no big deal per se, but the driver needs to react to it, which depends on #1163. While there are sensible workarounds that also don't cause additional network traffic (the Basic Set/Report does not trigger a query), I prefer to work on more pressing matters.

I've lately seen a few devices where the Binary Switch mirrors the Multilevel Switch and don't understand why. Maybe it is worth just hiding the CC on affected devices?

@AlCalzone While I completely understand wanting to work on more pressing things first, unless you disagree I’m going to open a feature request. The fact is that we know some devices use this so it seems odd to just say those won’t work as they were intended. More specifically, though, I think not implementing this will cause many more future headaches. People transitioning will need to reconfigure their devices to use the BasicCC which is going to generate a lot of more basic tech support type issues explaining that and walking them through it. And people that are spinning up an image to evaluate whether to switch are going to be reluctant to reconfigure a bunch of devices breaking their stable snapshot. This also isn’t a minor thing...as I understand how this class is used it informs that the device is on (or rather informs the controller that the device needs to be polled)...so without it any manual control, direct association, or energy-type reports will result in the device state not updating.

In my humble opinion, the transition needs to be as out-of-the-box as possible to reduce strain on your time. If qt-openzwave has 100,000 active users and even if only 1% have an issue with the Hail class, that is still 1,000 issues being open... It would be better/easier to just duplicate this and any other basic functionality I suspect, even if it isn’t used in most newer devices or potentially causes other annoyances like network traffic (which to many users with small networks isn’t a concern).

AlCalzone commented 3 years ago

If qt-openzwave has 100,000 active users and even if only 1% have an issue with the Hail class, that is still 1,000 issues being open...

Agreed. In the meantime it would help if you (or others) could collect some examples/requirements polling support. Like what is being polled and when.

blhoward2 commented 3 years ago

If qt-openzwave has 100,000 active users and even if only 1% have an issue with the Hail class, that is still 1,000 issues being open...

Agreed. In the meantime it would help if you (or others) could collect some examples/requirements polling support. Like what is being polled and when.

Will do. I’ll see which of my devices have a Hail option (even if I’m not using it) and then capture what they’re doing.

blhoward2 commented 3 years ago

@AlCalzone I can confirm that with the #1308 cc/hail branch my Aeotec Nano dimmers now report their status when set to either Hail or Basic CC.

I’m controlling this via direct association as mine don’t have switches... @pbathuk can you confirm with a manual switch state? It should be the same.

pbathuk commented 3 years ago

@blhoward2 thanks. From reading #1308 it does seem to do what is needed. I was wondering how I build my own docker image (as run using docker-compose on Ubuntu) to test this? @AlCalzone I see you have managed to get this working, if you could let me know how I will gladly test.

blhoward2 commented 3 years ago

I was wondering how I build my own docker image (as run using docker-compose on Ubuntu) to test this?

DOCKER_BUILDKIT=1 docker build --build-arg SRC=git-clone-src --build-arg Z2M_BRANCH=fix#23 --build-arg ZWJ_BRANCH=cc/hail --no-cache -f zwavejs2mqtt/docker/Dockerfile.contrib -t zwavejs2mqtt .

Then change your image in your docker-compose to be zwavejs2mqtt:latest. That will get you a working image with hail, but logging will not work.

pbathuk commented 3 years ago

@AlCalzone @blhoward2 I managed to get a chance to run the latest zwavejs2mqtt (dev) on docker - sorry did not build a new one but used a pre-built dockerhub.com one Happy to be told if this does not include this fix, but tracking through I can see that it was merged & built into master 16 hours ago and the dev branch was last updated 2 hours ... but maybe i'm misssing something.

So I have run the latest version and tried to turn on / off a light (log below)

Log removed due to length

AlCalzone commented 3 years ago

@pbathuk That log doesn't look like it comes from the current master.

For future logs, please enable writing the zwave-js logs to a file and attach them. Inline logs make these issue threads insanely long and your paste is formatted strangely.

pbathuk commented 3 years ago

Happy to close this issue as the new zwavejs2mqtt beta (1.0.0) uses the latest version of zwave-js and it fixed my light issue :) Hail now works for me on all my switches.