search

zsmartsystems / com.zsmartsystems.zigbee

ZigBee Cluster Library Java framework supporting multiple dongles
Eclipse Public License 1.0
142 stars 88 forks source link

Storing MAC capabilities received with DeviceAnnounce #1237

Open mikomarrache opened 3 years ago

mikomarrache commented 3 years ago

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 commented 3 years ago

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[bot] commented 3 years ago

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.

mikomarrache commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 10 months ago

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