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

Set the default link key #257

Merged
merged 2 commits into from Sep 17, 2018
Merged

Set the default link key #257

merged 2 commits into from Sep 17, 2018

Conversation

cdjackson
Copy link
Contributor

Recent Ember dongle driver does not set the default link key, so at the moment, the link key won't be set during initialisation. Other dongles should also be updated in future so this ensures the standard link key is set.
Signed-off-by: Chris Jackson chris@cd-jackson.com

@openhab-bot
Copy link
Collaborator

This pull request has been mentioned on openHAB Community. There might be relevant details there:

https://community.openhab.org/t/zigbee-binding/15763/1327

Copy link
Contributor

@hsudbrock hsudbrock left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In general, this looks good to me.

I added two small comments, you could check whether you want to add those small improvements to the PR, then I can approve again.


The key is defined as 16 hexadecimal values. If not defined, this will default to the well known ZigBee HA link key.

If defined with the work ```INSTALLCODE:``` before the key, then this will create a link key from an install code which may be shorter than 16 bytes.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could these be typos?

  • with the work -> with the word
  • before the key, then this will -> before the key, this will

try {
logger.debug("Link Key String {}", linkKeyString);
linkKey = new ZigBeeKey(linkKeyString);
} catch (IllegalArgumentException e) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor suggestion: Add a log statement within the catch block, so that it gets visible in the log that (and why) the system falls back to the well-known key.

That the system fell back to the well-known key is also visible (though somewhat implicit) from the log statement with the final link key array, the additional log statement would make this more explicit.

linkKey = new ZigBeeKey(linkKeyString);
} catch (IllegalArgumentException e) {
linkKey = KEY_ZIGBEE_ALLIANCE_O9;
logger.debug("Link Key String has invalid format. Revert to default key.", linkKeyString);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe the {} placeholder for the linkKeyString parameter is missing in the log string.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks - cut and paste error ;)

Signed-off-by: Chris Jackson <chris@cd-jackson.com>
@hsudbrock
Copy link
Contributor

Thanks for the update, looks good to me.

@cdjackson cdjackson merged commit c858309 into openhab:master Sep 17, 2018
@cdjackson cdjackson deleted the link_key branch September 17, 2018 20:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants