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

[enocean] Improved device discovery and added SMACK capability (#10156) #10157

Merged
merged 2 commits into from
Feb 20, 2021

Conversation

fruggy83
Copy link
Contributor

  • Added SMACK teach in
  • Teached in devices can be teach out on a repeated teach in
  • Improved detection of RPS devices, device types can be better distinguished now
  • Bugfixes for discovery fallback to GenericThings
  • Responses to message requests are send automatically now, no need for linking SEND_COMMAND channel

Fixes #10156

Signed-off-by: Daniel Weber uni@fruggy.de

@fruggy83 fruggy83 requested a review from J-N-K February 14, 2021 23:03
@fruggy83 fruggy83 added the enhancement An enhancement or new feature for an existing add-on label Feb 14, 2021
…hab#10156)

 * Added SMACK teach in
 * Teached in devices can be teach out on a repeated teach in
 * Improved detection of RPS devices, device types can be better distinguished now
 * Bugfixes for discovery fallback to GenericThings
 * Responses to message requests are send automatically now, no need for linking SEND_COMMAND channel

Fixes openhab#10156

Signed-off-by: Daniel Weber <uni@fruggy.de>
@fruggy83 fruggy83 force-pushed the issue10156_DeviceDiscoverySMACK branch from f5f4e8d to 68a5fdf Compare February 14, 2021 23:08
@fruggy83 fruggy83 removed the request for review from J-N-K February 18, 2021 14:28
bundles/org.openhab.binding.enocean/README.md Outdated Show resolved Hide resolved
Comment on lines 416 to 418
logger.debug("Exception during activating smack teach in: {}", e.getMessage());
updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.COMMUNICATION_ERROR,
"Smack packet could not be send");
Copy link
Member

Choose a reason for hiding this comment

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

The log message could be removed, as the status update is already logged by the framework. You could include the exception's message in the status detail message.

The state change originated by updateStatus() is logged to events.log. Including the status detail message.

Same for below.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am not really sure what you mean with status detail message. I hope my change is ok now. I changed this and other occurences. Thanks for the hint 👍

…ab#10156)

 * Code improvements after CR

Fixes openhab#10156

Signed-off-by: Daniel Weber <uni@fruggy.de>
@fruggy83
Copy link
Contributor Author

Hi Fabian @fwolter,

thanks a lot for your review hints 👍 I hope you are fine with my improvements. Have a nice weekend.

Copy link
Member

@fwolter fwolter left a comment

Choose a reason for hiding this comment

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

LGTM

Actually all newly added loggings to info should be debug or trace, but I see that lowering the severity would make it inconsistent to the existing code, so I'll merge it anyway.

info should be used for example when a new OSGi component started. There are almost no use cases where info should be used in bindings.

Would be awesome if you could file a follow-up PR to adjust all the info log levels!

@fwolter fwolter merged commit fd1c966 into openhab:main Feb 20, 2021
@fwolter fwolter added this to the 3.1 milestone Feb 20, 2021
themillhousegroup pushed a commit to themillhousegroup/openhab2-addons that referenced this pull request May 10, 2021
…hab#10157)

 * Added SMACK teach in
 * Teached in devices can be teach out on a repeated teach in
 * Improved detection of RPS devices, device types can be better distinguished now
 * Bugfixes for discovery fallback to GenericThings
 * Responses to message requests are send automatically now, no need for linking SEND_COMMAND channel

Fixes openhab#10156

Signed-off-by: Daniel Weber <uni@fruggy.de>
Signed-off-by: John Marshall <john.marshall.au@gmail.com>
thinkingstone pushed a commit to thinkingstone/openhab-addons that referenced this pull request Nov 7, 2021
…hab#10157)

 * Added SMACK teach in
 * Teached in devices can be teach out on a repeated teach in
 * Improved detection of RPS devices, device types can be better distinguished now
 * Bugfixes for discovery fallback to GenericThings
 * Responses to message requests are send automatically now, no need for linking SEND_COMMAND channel

Fixes openhab#10156

Signed-off-by: Daniel Weber <uni@fruggy.de>
marcfischerboschio pushed a commit to bosch-io/openhab-addons that referenced this pull request May 5, 2022
…hab#10157)

 * Added SMACK teach in
 * Teached in devices can be teach out on a repeated teach in
 * Improved detection of RPS devices, device types can be better distinguished now
 * Bugfixes for discovery fallback to GenericThings
 * Responses to message requests are send automatically now, no need for linking SEND_COMMAND channel

Fixes openhab#10156

Signed-off-by: Daniel Weber <uni@fruggy.de>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement An enhancement or new feature for an existing add-on
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[enocean] Improve Device Discovery, add SMACK capability
2 participants