Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[bug] Basic set events not sent to mqtt anymore? #1044

Closed
3 tasks
blhoward2 opened this issue Apr 12, 2021 · 25 comments · Fixed by #1055 or #1056
Closed
3 tasks

[bug] Basic set events not sent to mqtt anymore? #1044

blhoward2 opened this issue Apr 12, 2021 · 25 comments · Fixed by #1055 or #1056
Assignees
Labels
bug Something isn't working

Comments

@blhoward2
Copy link
Collaborator

Version

Checklist:

  • [X ] I am not using HomeAssistant. Or: a developer has told me to come here.
  • [X ] I have checked the troubleshooting section and my problem is not described there.
  • [ X] I have read the changelog problem was not mentioned there.

Build/Run method

  • [x ] Docker
  • PKG
  • Snap package
  • Manually built (git clone - npm install - npm run build )

zwavejs2mqtt: 3.2.0
zwave-js: 7.1.1

Describe the bug

Until some unknown period in the past, basic set events from devices with a compat flag to undo the mapping and treat them as an event appeared under command class 32 under 32/0/event. After a complete rebuild of my network I no longer see 32 under any devices regardless of whether I use:

  1. ValueID Topics
  2. Named Topics (for this one basic isn't listed)

Double tapping my GE switches does send through the event properly (I can see it in HA) and I see it in the zwavejs1mqtt log. But I don't see any messages anywhere in mqtt.

With named topics:
Screen Shot 2021-04-12 at 12 18 13 AM

With ValueID topics:

Screen Shot 2021-04-12 at 12 42 51 AM

Expected behavior

Double tap up/down should generate an event notification with a value of 255/0, and this should appear in mqtt.

Additional context

zwave-js:

2021-04-12T04:38:31.385Z CNTRLR   [Node 002] treating BasicCC::Set as a value event
2021-04-12T04:38:31.385Z CNTRLR   [Node 002] [!] [Basic] event: 255                                 [Endpoint 0]

zwavejs2mqtt:

node2.log

@blhoward2 blhoward2 added the bug Something isn't working label Apr 12, 2021
@blhoward2
Copy link
Collaborator Author

Ah, should have read my own log file. I see this now under homeassistant/_EVENTS/ZWAVE_GATEWAY-MQTT/node/node_value_updated

Is it not possible to publish these under the device itself anymore?

@robertsLando
Copy link
Member

homeassistant/_EVENTS/ZWAVE_GATEWAY-MQTT/node/node_value_updated

Humm that's the event, you should get it under node too 🤔 Let me check...

@robertsLando
Copy link
Member

You should get a message under: .../32/0/event topic, I don't understand where it stops the propagation here. I have created a patch that could help understand where it blocks. Could you try to use master tag and resend me a full log?

@blhoward2
Copy link
Collaborator Author

blhoward2 commented Apr 13, 2021

I've confirmed this breaks in zwavejs2mqtt version 3.0.0, apparently from the jump to node-zwave-js 7.0.0. I speculate that this is because of a 7.0.0 breaking change separating value-notification from notifications.

https://github.com/zwave-js/node-zwave-js/pull/2029/files

From version 3.2.0:

node_2 (3.2.0 with basic topic).json.zip

node 2(3.2.0).log

Note that under 3.2.0 the device setup a basic topic the one time. Running the master that @robertsLando modified (zwavejs2mqtt: 3.2.1+b3ad1a3.b3ad1a3, zwave-js: 7.2.2-fd11d22)it doesn't setup a Basic tab in the UI.

node_2 (modified master).json.zip

node 2 (modified master).log

/cc @AlCalzone @robertsLando

@robertsLando
Copy link
Member

robertsLando commented Apr 13, 2021

There are both nodeValueNotification and nodeNotification events, them are handled differently on my side:

In v7 breaking changes @AlCalzone mentions the nodeNotification one but in your case what is raised is a nodeValueNotification that for some reason I get but is not published to MQTT

it doesn't setup a Basic tab in the UI.

This is correct as it is not returned from getDefinedValueIds method. BTW I could change this if needed and manually create an entry for that value once it get triggered the first time.

@robertsLando
Copy link
Member

robertsLando commented Apr 13, 2021

https://github.com/zwave-js/zwavejs2mqtt/blob/master/lib/ZwaveClient.js#L617

https://github.com/zwave-js/zwavejs2mqtt/blob/b3ad1a3625b35f3cad963a0aa3ca50f1a342ffe6/lib/ZwaveClient.js#L632

https://github.com/zwave-js/zwavejs2mqtt/blob/b3ad1a3625b35f3cad963a0aa3ca50f1a342ffe6/lib/ZwaveClient.js#L1175

https://github.com/zwave-js/zwavejs2mqtt/blob/master/lib/Gateway.js#L210

This are the functions called when that event is triggered, essetially I get it as a nodeValueNotificaiton event then I set the stateless flag and trigger the valueUpdate, that emits the event that is then catched by the gateway who translates the valueId to its topic.

My first thought was that maybe for some reason your node is not marked as ready, in that case: https://github.com/zwave-js/zwavejs2mqtt/blob/master/lib/Gateway.js#L223

@robertsLando
Copy link
Member

Could someone give a try to #1055?

robertsLando added a commit that referenced this issue Apr 13, 2021
* fix: notification event not publiished to mqtt

Fixes #1044

* fix: reset timeout
@robertsLando robertsLando reopened this Apr 13, 2021
@robertsLando
Copy link
Member

I have merged #1055 so you can use master tag in docker to see how it goes. Let me know!

@towerhand
Copy link
Contributor

I'll report back in a few, I was just building it from the PR.

@robertsLando
Copy link
Member

robertsLando commented Apr 13, 2021

@towerhand
Copy link
Contributor

towerhand commented Apr 13, 2021

Weird, it didn't create the entities in zjs2m or the retained mqtt topic for HA but it did send the topic only one time and triggered the automation I have in HA, but broken otherwise.
zwavejs2mqtt-store.zip

@blhoward2
Copy link
Collaborator Author

This is correct as it is not returned from getDefinedValueIds method. BTW I could change this if needed and manually create an entry for that value once it get triggered the first time.

Wouldn't you need this so it sets up the entity for mqtt discovery?

@robertsLando
Copy link
Member

robertsLando commented Apr 13, 2021

So relevant logs are:

2021-04-13 08:09:41.090 INFO ZWAVE: Node 16: value added 16-32-0-event => 0
2021-04-13 08:09:41.095 DEBUG GATEWAY: Publishing discovery: {
  type: 'sensor',
  object_id: 'notification_event',
  discovery_payload: {
    value_template: '{{ {}[value_json.value] | default(value_json.value) }}',
    icon: 'mdi:alarm-light',
    state_topic: 'zwave2jsmqtt/master_bedroom_lights/32/0/event',
    json_attributes_topic: 'zwave2jsmqtt/master_bedroom_lights/32/0/event',
    device: {
      identifiers: [ 'zwavejs2mqtt_0xead969cf_node16', [length]: 1 ],
      manufacturer: 'GE/Jasco',
      model: 'In-Wall Dimmer Switch (14294 / ZW3005)',
      name: 'master_bedroom_lights',
      sw_version: '5.29'
    },
    name: 'master_bedroom_lights_notification_event',
    unique_id: 'zwavejs2mqtt_0xead969cf_16-32-0-event'
  },
  discoveryTopic: 'sensor/master_bedroom_lights/notification_event/config',
  values: [ '32-0-event', [length]: 1 ],
  persistent: false,
  ignoreDiscovery: false
}
.......
2021-04-13 08:09:41.107 DEBUG MQTT: Publishing to zwave2jsmqtt/master_bedroom_lights/32/0/event: {
  time: 1618319381106,
  value: 0,
  nodeName: 'master_bedroom_lights',
  nodeLocation: ''
} with options { qos: 1, retain: true }
........
2021-04-13 08:09:58.324 DEBUG MQTT: Publishing to zwave2jsmqtt/master_bedroom_lights/32/0/event: {
  time: 1618319398317,
  value: 0,
  nodeName: 'master_bedroom_lights',
  nodeLocation: ''
} with options { qos: 1, retain: true }

Seems the entity is created now right? Alsoo the message is published to MQTT. The strange thing here is that the retain flag is set to true

@robertsLando
Copy link
Member

Wait maybe I have found something

@towerhand
Copy link
Contributor

towerhand commented Apr 13, 2021

zwavejs2mqtt-store.zip
Tried again and it's crashing zjs2mqtt or restarting it.

@robertsLando
Copy link
Member

Try with #1056

@robertsLando
Copy link
Member

BTW I don't see the crash there

@towerhand
Copy link
Contributor

towerhand commented Apr 13, 2021

Not sure if crashing was the right word, maybe triggering a refresh is more accurate, like when we save the settings.

@robertsLando
Copy link
Member

BTW please remember to delete logs between restarts so I only get the logs needed and not the old ones

@towerhand
Copy link
Contributor

Yes I did, if you see multiple restarts then it happened when I was double clicking the switches.

@robertsLando
Copy link
Member

FYI seems the problem is fixes in #1056, just waiting for last feedback and then I will merge and release a new version

@matejdro
Copy link

Has this been changed lately? Until the recent release, basic events were toggling the state of the switch, but now they only change the "basic" value, which does not appear to be accessible through MQTT (only as event, not as state).

@robertsLando
Copy link
Member

I didn't change anything on this side. @AlCalzone ?

@AlCalzone
Copy link
Member

Depends on the device 🤷🏻‍♂️
Please open an issue in node-zwave-js and attach a driver log of a re-interview and reports which show the behavior.

@matejdro
Copy link

matejdro commented Oct 12, 2021

Thanks for prompt answer. I've just switched to zwavejs integration (from MQTT discovery) and it appears that home assistant events are properly triggered for basic changes. So this is no longer an issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
5 participants