zwave-js / zwave-js-ui

Full featured Z-Wave Control Panel UI and MQTT gateway. Built using Nodejs, and Vue/Vuetify
https://zwave-js.github.io/zwave-js-ui
MIT License
984 stars 201 forks source link

[Bug]: node_notification event parameter 2 is always null #2382

Closed j9brown closed 2 years ago

j9brown commented 2 years ago

Checklist

Deploy method

Docker

Zwavejs2Mqtt version

6.7.2

ZwaveJS version

9.0.2

Describe the bug

I'm trying to handle MultilevelSwitchChange notifications from ZwaveJS by subscribing to the node_notification MQTT topic.

I receive an object like this (pruned for clarity):

[
  { /* tons of information about the node */ },
  {
    id: "57-38-0-5"
    nodeId: 57
    commandClass: 38
    commandClassName: "Multilevel Switch"
    property: 5
    propertyName: 5
   },
  null
]

That null is the problem. It should be an object containing an object with eventType and direction fields as can be seen in the last parameter of the call to emit() in ZwaveJS.

Without having read too deeply into ZwaveJS2MQTT, I suspect this parameter is not being serialized properly when the MQTT message is generated, resulting in a null instead of an object.

To Reproduce

Interact with a Zwave device that generates mutlilevel switch change messages. In my case, press the up or down arrow buttons on a VRCS4 or VRCZ4.

Expected behavior

The second parameter should contain an object with the event data that was supplied in the call to emit(), not null.

Additional context

Here's the pull request that added this feature to ZwaveJS in case it's of use to you: https://github.com/zwave-js/node-zwave-js/pull/4282

robertsLando commented 2 years ago

Notifications are catched here: https://github.com/zwave-js/zwavejs2mqtt/blob/master/lib/ZwaveClient.ts#L3543

The third parameter in my case is args: https://github.com/zwave-js/zwavejs2mqtt/blob/master/lib/ZwaveClient.ts#L3355

In case of amultilevel switch notification data is parsed using args.eventData so if that is null it means that field is empty: https://github.com/zwave-js/zwavejs2mqtt/blob/master/lib/ZwaveClient.ts#L3381

cc @AlCalzone

AlCalzone commented 2 years ago

Not sure where you get the eventData from as this is specific to the Entry Control CC. Not enough TypeScript enabled, heh? ;)

The third parameter of the "notification" event depends on the CC that emits it (2nd parameter): https://github.com/zwave-js/node-zwave-js/blob/e7897140cde5b8fac1064dabe9ec92ddc47b23a9/packages/zwave-js/src/lib/node/Types.ts#L87-L94 and is currently one of these: Notification CC https://github.com/zwave-js/node-zwave-js/blob/76c76fb90161bb1d68f0559bf9156261e15f0c23/packages/zwave-js/src/lib/commandclass/NotificationCC.ts#L71-L83

Entry Control CC https://github.com/zwave-js/node-zwave-js/blob/76c76fb90161bb1d68f0559bf9156261e15f0c23/packages/zwave-js/src/lib/commandclass/EntryControlCC.ts#L94-L99

Powerlevel CC (handled internally) https://github.com/zwave-js/node-zwave-js/blob/76c76fb90161bb1d68f0559bf9156261e15f0c23/packages/zwave-js/src/lib/commandclass/PowerlevelCC.ts#L63-L68

Multilevel Switch CC https://github.com/zwave-js/node-zwave-js/blob/3ef1368bd1f2fd1e41ceac43bc9f89e0d1ffe550/packages/zwave-js/src/lib/commandclass/MultilevelSwitchCC.ts#L127-L135


IMO when passing these through MQTT, you shouldn't be parsing/converting anything. Well... maybe not the entire node object, but the entire payload (3rd argument) is relevant.

You can do some conversion when writing to special topics, but this handling must be CC specific.