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

Storing MAC capabilities received with DeviceAnnounce #1237

Open
mikomarrache opened this issue May 10, 2021 · 6 comments
Open

Storing MAC capabilities received with DeviceAnnounce #1237

mikomarrache opened this issue May 10, 2021 · 6 comments
Labels

Comments

@mikomarrache
Copy link
Contributor

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.

@cdjackson
Copy link
Member

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.

@stale
Copy link

stale bot commented Jul 11, 2021

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.

@stale stale bot added the wontfix label Jul 11, 2021
@stale stale bot closed this as completed Jul 26, 2021
@mikomarrache
Copy link
Contributor Author

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

@cdjackson
Copy link
Member

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 -:

     * The MAC capability flags field is eight bits in length and specifies the node capabilities, as required by the
     * IEEE 802.15.4-2003 MAC sub-layer.
     * Note that this property corresponds to the MAC capabilities reported by the
     * {@link com.zsmartsystems.zigbee.zdo.command.DeviceAnnounce} command.

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.

@mikomarrache
Copy link
Contributor Author

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.

@cdjackson cdjackson reopened this Oct 31, 2023
@stale stale bot removed the wontfix label Oct 31, 2023
@cdjackson
Copy link
Member

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants