-
Notifications
You must be signed in to change notification settings - Fork 88
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
Storing MAC capabilities received with DeviceAnnounce #1237
Comments
It is not uncommon for the information in the DeviceAnnounce message to be wrong. This was experienced on a number of devices in the past, and at least on the devices I was testing with, the node descriptor was found to be more reliable. Of course, if the same information is different in two different places, then it's not ultimately possible to know what is correct and this is not a good situation, but that was the experience. |
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
Hi Chris, I would like to raise this again. In order not to impact existing code, I would like to propose adding a setting to the network manager to decide if MAC capabilities should be saved to node when receiving a DeviceAnnounce. Of course, this setting will be set to false by default so that actual behavior remains unchanged. Please, let me know if you're okay with that and I will make a PR. Thanks |
Hi and sorry for the slow response. So, reading through a few things, I think actually we should use the announce message to set the Mac capabilities and not the node descriptor. The only downside I see is that this could cause different results on some devices that report differently in the announce compared to the descriptor. The main reason I think that's the best approach, is actually, that's what the docs state -:
So, I'm kind of err-ing toward changing the approach and not having the flag that you propose. What do you think? Again, this is potentially going to impact some users as a few devices do report different attributes in the node descriptor and announce message (which is IMHO just wrong), but I think there are advantages to doing this as we obviously know the capabilities as soon as the device announces itself. |
Hi Chris, Thanks for considering this again. I also think we should save MAC capabilities as soon as we receive them. I understand your consideration regarding the fact some devices report different capabilities between the device announce and node descriptor but as you said, this is wrong. I would add that the library should not behave as a workaround for such devices. Instead, a custom plugin can be implemented to fetch "correct" capabilities using a node descriptor request at a later stage to override the capabilities received via the device announce. Please let me know how can we move further with that issue. I can provide a PR if relevant. |
Sorry - I hadn't realised this issue was closed when we were discussing it a few months back, so it dropped off my radar. I'm certainly happy to accept a PR for this @mikomarrache |
When a DeviceAnnounce is received, it includes MAC capabilities.
In the library, MAC capabilities are stored in the NodeDescriptor class only, so that this information can only be accessed after node descriptor has been requested.
Is there a reason not to store the MAC capabilities in the ZigBeeNode class when a DeviceAnnounce is received? This way, the library would be able to get this information regardless if node descriptor is known.
The text was updated successfully, but these errors were encountered: