search

zsmartsystems / com.zsmartsystems.zigbee

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

Move static EZSP version to non-static property in Zigbee Dongle EZSP #1359

Open mikomarrache opened 1 year ago

mikomarrache commented 1 year ago

This PR is very important when working with multiple NCPs and therefore multiple network managers. The fact that the EZSP version is stored in a static property makes it shared by all network managers and can cause various issues (like not being able to manage NCPs with pre-v8 and v8 versions).

The changes have been tested.

lgtm-com[bot] commented 1 year ago

This pull request fixes 1 alert when merging abf454aa9f08f046c6e67d1746383dc3a5bf975b into b15130c021219cbfe138dc4ec60423a0b8b3f679 - view on LGTM.com

fixed alerts:

Heads-up: LGTM.com's PR analysis will be disabled on the 5th of December, and LGTM.com will be shut down ⏻ completely on the 16th of December 2022. It looks like GitHub code scanning with CodeQL is already set up for this repo, so no further action is needed :rocket:. For more information, please check out our post on the GitHub blog.

mikomarrache commented 1 year ago

I would also add that these changes have been heavily tested in a production environnement with dozens of Ember NCPs and thousands of ZigBee devices. In the future, we will probably have to deal with Ember NCPs of different versions and the actual library code will not allow that since we use the library in one application that manages all these NCPs.

cdjackson commented 1 year ago

In the future, we will probably have to deal with Ember NCPs of different versions and the actual library code will not allow that since we use the library in one application that manages all these NCPs.

I really wish I'd added a version tag to each method in the XML definition so that we could manage different versions better. However Silabs don't really provide this information in an easy format - we'd have to sift through the different versions of UG100... Still it could be done where the API has changed.

Hedda commented 1 year ago

@mikomarrache May I suggest that you also check out the somewhat indirectly related discussion/request here -> https://github.com/zsmartsystems/com.zsmartsystems.zigbee/issues/1332