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]: Inputing name during inclusion does not work #2210

Closed
2 of 3 tasks
Matt-PMCT opened this issue Jan 27, 2022 · 20 comments · Fixed by #2247
Closed
2 of 3 tasks

[Bug]: Inputing name during inclusion does not work #2210

Matt-PMCT opened this issue Jan 27, 2022 · 20 comments · Fixed by #2247
Labels
bug Something isn't working

Comments

@Matt-PMCT
Copy link
Contributor

Checklist

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

Deploy method

Docker

Zwavejs2Mqtt version

6.4.1

ZwaveJS version

8.11.1

Describe the bug

The new interface from pull #2194 is a very nice feature. However, I have included 3 devices on two different instances of zwave-js2mqtt and the name never gets applied to the device. Instead, the device has no name/location one inclusion is completed.

To Reproduce

Include a device, write in a name, once inclusion is complete the device will still have no name

Expected behavior

The name you input should be applied :)

Additional context

No response

@Matt-PMCT Matt-PMCT added the bug Something isn't working label Jan 27, 2022
@robertsLando
Copy link
Member

Hi @Matt-PMCT I cannot reproduce your bug, could you attach some application logs please? (debug level)

Logs should include an inclusion attemp

@joostvkempen
Copy link

I've just started with ZwaveJS2MQTT, only just added one device (Fibaro Dimmer 2) and experienced the same. So I found this issue. Still figuring out things, I'll try some logging when adding more devices.

@jmgiaever
Copy link
Contributor

Hi,
Still and issue

zwavejs2mqtt: 6.5.0.5844e6c
zwave-js: 8.11.2

zwavejs2mqtt_2022-02-06 (1).log

I'm adding the «name» and «location» myself at 2022-02-06 22:12:12.885

@kpine
Copy link
Contributor

kpine commented Feb 7, 2022

I'm adding the «name» and «location» myself at 2022-02-06 22:12:12.885

That's not what this issue is referring to. When you initiate a classic inclusion, it now prompts for a name and location. If provided, these properties will be set as soon as the node is added.

image

FWIW, I tried it once and it worked.

@kpine
Copy link
Contributor

kpine commented Feb 7, 2022

Tried it again with 6.5.0 and was successful, Here's what the inclusion logs look like when it's working:

2022-02-06 17:47:05.841 INFO ZWAVE: Calling api startInclusion with args: [
  0,
  { forceSecurity: false, name: 'Motion Sensor', location: 'Office' },
  [length]: 2
]
2022-02-06 17:47:05.865 INFO ZWAVE: Controller status: Secure inclusion started
2022-02-06 17:47:05.866 INFO ZWAVE: Success zwave api call startInclusion true
2022-02-06 17:47:10.387 INFO ZWAVE: Controller status: Inclusion stopped
2022-02-06 17:47:10.813 DEBUG ZWAVE: Updating store nodes.json
2022-02-06 17:47:10.814 DEBUG ZWAVE: Setting node name 'Motion Sensor' to node 3
2022-02-06 17:47:10.815 DEBUG ZWAVE: Setting node location 'Office' to node 3
2022-02-06 17:47:10.816 DEBUG ZWAVE: Binding to node 3 events
2022-02-06 17:47:10.816 DEBUG ZWAVE: Node 3 has been added to nodes array
2022-02-06 17:47:10.816 INFO ZWAVE: Node 3: added with security HIGH SECURITY

I provided the node name and location in the initial inclusion dialog, and they are set as soon as it's possible (inclusion stopped event).

image

inclusion.log

@Matt-PMCT
Copy link
Contributor Author

Matt-PMCT commented Feb 7, 2022

Finally got around to it on my end today.

I used the prompts to add a new node, filled out the info, choose non-encrypted for security. No names were passed on. I am still on 6.4.1. If it matters I am using Firefox, in case this is browser related. Logs at the very end.
firefox_ZuZgf3F90y
firefox_0iRRzCBeP6
firefox_XWCqP6XwRD

firefox_jUx4AeMwVU

zwavejs2mqtt-store.zip

@kpine
Copy link
Contributor

kpine commented Feb 7, 2022

I am still on 6.4.1.

I would suggest upgrading. There were some naming related fixes included in 6.5.0 (pretty sure this commit is what fixes it).

I also have MQTT disabled, if that happens to make a difference.

@robertsLando
Copy link
Member

robertsLando commented Feb 7, 2022

Like @kpine said I fixed that in latest release. Please upgrade to that one. I noticed there was a race condition causing node naming to not work sometimes due to the event inclusion stopped called before/after node added event. That timeout has fixed this

@jmgiaever
Copy link
Contributor

I'm on 6.5.0. Issue is there.

@kpine
Copy link
Contributor

kpine commented Feb 7, 2022

@jmgiaever Do you have any new logs then?

@jmgiaever
Copy link
Contributor

New logs? I have this: #2210 (comment)

@kpine
Copy link
Contributor

kpine commented Feb 7, 2022

Can you describe your problem then? According to your logs and statement, you did not set the name and location during inclusion, which is what this issue is about.

EDIT: N/M, I see it now, I completely missed the inclusion w/name & location in the logs.

@jmgiaever
Copy link
Contributor

That's not what this issue is referring to. When you initiate a classic inclusion, it now prompts for a name and location. If provided, these properties will be set as soon as the node is added.

It's what this issue is about. Check the attached log. I'm using that form to add details (line 12 - 20), but they are lost during the inclusion.

And then I add it manually at 2022-02-06 22:12:12.895 (name) and 2022-02-06 22:12:18.885 (location)

@kpine
Copy link
Contributor

kpine commented Feb 7, 2022

Sorry, I already realized that and corrected myself. I previously just focused on your statement about setting them manually...

Does it work if you don't force security and just include normally? I'm guessing the security bootstrapping takes longer than 2 seconds. Z2m throws away the node name and location if it takes more than 2 seconds to go from "inclusion stopped" to "node added".

@jmgiaever
Copy link
Contributor

jmgiaever commented Feb 7, 2022

No worries. I actually didn't get it until now that most posts after mine yesterday was addressed to me. So suddenly had a some posts to read through.

I just added a node that doesn't have any security (HS1MS-Z) and it worked.

Got to admit that I have barely experienced the bootstrapping taking less than 2 sec 😁 As of the 700-issue I've been waiting to add all devices. Now having 61 one in and still counting. ~90%+ of my devices are included S2, the rest is S0 and «none». Over 50% are main powered, but not spamming the network.

I guess the 2 seconds-rule is the cause here yes. Is there a reason why Z2M throws any of this away as long as inclusion doesn't fail?

@kpine
Copy link
Contributor

kpine commented Feb 7, 2022

Yeah, issue comments aren't always the best for conversations, I don't like @ mentioning users, but I guess it's the best approach.

We can see from your logs that it took ~18 seconds to perform the security bootstrapping, setup the lifeline association and setup the wake up destination.

2022-02-06 22:09:16.511 INFO ZWAVE: Controller status: Inclusion stopped
2022-02-06 22:09:38.921 DEBUG ZWAVE: Binding to node 201 events

The first line corresponds to driver event "inclusion stopped" and the second line corresponds to the driver event "node added". In-between those two events is the activity I mention above. Z2m sets a timer after the first event for 2 seconds and clears the inclusion name and location, which would be set in the second event. I'll have to let @robertsLando speak for the timer approach, but it sounds like it had to do with replacing nodes. I don't think that design is very feasible because you won't be able to predict how long the node bootstrapping is going to take. S2 bootstrapping could take even longer, depending on if the user is inputting a DSK manually.

@jmgiaever
Copy link
Contributor

jmgiaever commented Feb 7, 2022

I guess the driver emits inclusion-stopped in both cases where inclusion fails or succeeds.

There was an issue where node data would be reused (from nodes.json) when you've reached the 232 limit and starts reusing old id's.

Daniel wanted to preserve them in the case of «replace failed nodes», which I absolutely agree with 👍 He talked about - if my memory is correct - using a boolean that would prevent writing to the file in the situation of replacing a node, but I'm not sure what he ended up with. Haven't checked the code.

Edit: I can see it two different events, inclusion stopped and inclusion failed.

@Matt-PMCT
Copy link
Contributor Author

I did the same test in 6.5.0 and it worked for me this time. I'm ready for the issue to be closed. But maybe there is still more to be looked at?

@robertsLando
Copy link
Member

robertsLando commented Feb 8, 2022

Z2m sets a timer after the first event for 2 seconds and clears the inclusion name and location, which would be set in the second event.

I sincerly have no idea how much time it's ok to wait, I firstly didn't set a timeout at all at it was working, then @jmgiaever told me it wasn't so I added that to delay the reset. What I could do eventually is to don't reset them there and just do the reset when the inclusion is started or the node is added successfully

@robertsLando
Copy link
Member

Anyone that could give a try to #2247 ?

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
Development

Successfully merging a pull request may close this issue.

5 participants