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
935 stars 198 forks source link

[bug] Node rename is not always respected in MQTT #217

Closed lazyoldbear closed 3 years ago

lazyoldbear commented 3 years ago

Official container zwavejs/zwavejs2mqtt:1.0.0-alpha.2. Mosquitto 2.0.4. Gateway is set for ValueID topics with enforced using of node names. Devices are created without names. Now, when I add a name for them, it is saved in ZWave, all good. For some devices (smart plugs) the topics in MQTT get updated, and once I reload HomeAssistant (after killing the devices there), I get pretty names for devices and entities. Unfortunately, for Fibaro buttons (FGPB101) they stay as nodeXX, no matter what. Tried removing and healing in several ways. The name in ZWave is updated, MQTT topics are always numbered.

Please advise me what information to provide to help understand the trouble.

larstobi commented 3 years ago

I get this as well.

AlCalzone commented 3 years ago

Is there a difference in support for the node naming and location cc between the devices?

robertsLando commented 3 years ago

@AlCalzone No worries this is not something related to zwavejs. I never use node naming an location cc as I use my custom db for them as them were badly managed on ozw

robertsLando commented 3 years ago

@lazyoldbear Could you try to use dev tag? @larstobi Are you using latest version from master? Could someone show me an export of the node with this problem?

larstobi commented 3 years ago

node_93.json.zip

larstobi commented 3 years ago

I'm currently running: zwavejs/zwavejs2mqtt:dev (9ad34d1bf555) 4 days ago

larstobi commented 3 years ago

You'll see:

  "_name": "temperatur_hovedsoverom"

but still:

      "discoveryTopic": "binary_sensor/nodeID_93/battery_islow/config",
larstobi commented 3 years ago

My entity name template is just "%n"

robertsLando commented 3 years ago

@larstobi Is this only happening on discovery topic? Or also in mqtt publish?

larstobi commented 3 years ago
Screenshot 2021-01-12 at 12 29 37
larstobi commented 3 years ago
Screenshot 2021-01-12 at 12 31 45
robertsLando commented 3 years ago

So to answer my question the problem seems to happen only on discovery topic. The mqtt side is ok

lazyoldbear commented 3 years ago

Worked on dev (7e519cef465bf7eee4b3db54b8a6c85c2956f212c682823bb3732a1d07f1bd4e). Had to alter the name template to %n_%o, so that it does not put a dash (I feel better to control location in HASS) in front, but otherwise all seems good.

larstobi commented 3 years ago

@robertsLando Yes, I think you're right.

I'll try to update to the latest dev and see if something else happens.

robertsLando commented 3 years ago

@larstobi ALso If possible I need to see zwavejs2mqtt logs, it's strange

larstobi commented 3 years ago

After updating to yesterdays dev image, I'm not seeing this anymore. Not sure what was fixed in the last three days, but it seems to have worked. :-D

robertsLando commented 3 years ago

Sincerly I have no idea as I never edited nothing in code that could break that :man_shrugging: Glad it's working BTW

lazyoldbear commented 3 years ago

I have seen it again. Sorry. Interestingly, this time "Rediscover" helped to fix the topics.

larstobi commented 3 years ago

@lazyoldbear What is the "Rediscover" feature? Where can I find it?

robertsLando commented 3 years ago

Rediscover node is a button you find on the top of hass devices table

lazyoldbear commented 3 years ago

I don't know all this smart javascript stuff, but from a brief glance over the source code, here are some silly questions:

I don't know how it is done these days, but strict consistency is a nice thing when it comes to object sets...

robertsLando commented 3 years ago

Is there some locking magically in place?

I cannot think about a case that could cause data loss. I always write the entire object that is always updated, so two consecutive calls even if them are done in the same moment will update it based on the order they comes to js stack, nodejs it's not multi-thread process so there is not this problem

Same file, similar question. Is it possible that initNode gets called after _setNodeName was triggered, but before it was actually executed?..

I'm looking at this too, I saw some users reported me they see unknown manufacturer and product id/type in control panel, I think there could be a case where initNode overrides the node props for some reason. I'm investigating, if it happens to you provide me a log outupt so I can debug this

lazyoldbear commented 3 years ago

I also often saw unknown for Fibaro button. Some back and forth requests of info usually eventually helped.

lazyoldbear commented 3 years ago

Should this be reopened, for there might be a floating trouble?

robertsLando commented 3 years ago

@lazyoldbear I have an idea, let me check

robertsLando commented 3 years ago

233 could be related also to the same problem

lazyoldbear commented 3 years ago

I am just observing the same behaviour as described there. A lot of empty fields. One plug constantly turns dead. Everything seems fine from HA, though. This all pretty much tempts to think about loose changing sequences. Async execution has its disadvantages. :-)

In particular, one trouble is that you always store the whole node. It might be beneficial to decouple UI from the thinking core, and talk to it through an API. So that you don't _setNodeName, but POST/GET that. And process all sequentially on the other end. Easier to summon consistency. Also it is convenient to run pods separately. One has hardware, persistence and runs the net, another one bells, whistles and UI (and completely stateless).

robertsLando commented 3 years ago

Try with #240

lazyoldbear commented 3 years ago

Recent dev image, same behaviour.

robertsLando commented 3 years ago

@lazyoldbear Yep because you have to try the branch fix-init-node not the dev tag (master branch)

lazyoldbear commented 3 years ago

That might be a bit tricky to test: I don't even have Docker or something similar at home. But if I get time, I will try to fire it up manually in a blank pod, or push retrieval before the entrypoint.

lazyoldbear commented 3 years ago

Tried. Two fibaro buttons got back with full information. A smart plug is "unknown", but it was dead and I had to revive it.

robertsLando commented 3 years ago

@lazyoldbear Can I close this so?

larstobi commented 3 years ago

@robertsLando I'm seeing this again with the zwavejs/zwavejs2mqtt:dev (2753bf04be41 - 24 hours ago) image. node_60.json.gz

robertsLando commented 3 years ago

@larstobi Does this only happens on notifications topics? If so that is fixed now

larstobi commented 3 years ago

Thanks, I will try.

larstobi commented 3 years ago

@robertsLando Sorry, still seeing this on docker image zwavejs/zwavejs2mqtt:sha-0767844. :-(

larstobi commented 3 years ago
Screenshot 2021-01-26 at 09 18 23
robertsLando commented 3 years ago

@larstobi This is happening only for discovery right?

larstobi commented 3 years ago

Correct

robertsLando commented 3 years ago

@larstobi Hehehe I was looking at the code without finding any issues. The problem is that your hass devices have "persistent": true,, when persistent is true I don't recreate them but I discover them as they are, it's like a cache system that prevents to override any manual changes on the discovery payload. So please disable the persistent on those and rediscover the node

larstobi commented 3 years ago

What setting am I supposed to disable? I set the "Store" setting under the MQTT settings in zj2m to false. It's description is: "Enable persistent storage of packets (QoS > 0) while client is offline. If disabled the in memory store will be used." Is this the correct one?

Also, I don't seem to have a redisvover button in my Home Assistant under devices, nor under each device.

I did Re-Interview Node 60, and deleted the nodeid_60 topics manually, then set the name of the device in sj2m again. And nodeid_60 showed up in MQTT Explorer right away.

robertsLando commented 3 years ago

@larstobi In hass devices table:

Schermata da 2021-01-26 13-36-22

Check the column Persistent, here it says no and it's ok but yours was Yes, it means that you previously pressed on Store button. When it is persistent it means that it's loaded from a cache file (the file nodes.json), for devices loaded from cache the topics are not updated them are sent as them are stored to prevent override the user edits

larstobi commented 3 years ago
Screenshot 2021-01-26 at 13 42 34

I don't have that in my HA. What's the URL? I haven't found a way to navigate there either... I feel like a total n00b...

robertsLando commented 3 years ago

I don't have that in my HA. What's the URL? I haven't found a way to navigate there either... I feel like a total n00b...

LOL that's not on hass, it's on zwavejs2mqtt UI control panel table :)

Schermata da 2021-01-26 13-45-54

larstobi commented 3 years ago

LOL, total n00b affiirmed... 😂 😂 😂

Yes, that totally fixed it for me. Thanks for spoon feeding me. 💯 All good now.

robertsLando commented 3 years ago

No worries :)