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

[Tradfri] Force session resume after communication errors #4764

Merged
merged 1 commit into from
Feb 1, 2019

Conversation

hreichert
Copy link

As discussed in eclipse-archived/smarthome#6065 the tradfri gateway does not support a session resumption after a gateway reboot or other communcation errors.

@boaks implemented changes in californium scandium:
Californium 1.0.7 is able to force a full handshake instead.

This change calls "forceResumeSessionFor" after a communication error, so that a full handshake is done.

Related discussion: eclipse-archived/smarthome#6065 (comment)
Californium 1.0.7 version bump: openhab/openhab-core#475

Copy link
Contributor

@cweitkamp cweitkamp left a comment

Choose a reason for hiding this comment

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

Code looks fine. Thanks. I did not test it. I left one minor comment inside.

@@ -340,6 +340,15 @@ private synchronized void requestDeviceDetails(String instanceId) {

@Override
public void setStatus(ThingStatus status, ThingStatusDetail statusDetail) {

Copy link
Contributor

Choose a reason for hiding this comment

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

Please remove this empty line.

As discussed in eclipse-archived/smarthome#6065 the tradfri gateway does not support a session resumption after a gateway reboot or other communcation errors.
Californium 1.0.7 is able to force a full handshake instead.
This change calls "forceResumeSessionFor" after a communication error, so that a full handshake is done.

Signed-off-by: Holger Reichert <[email protected]>
@hreichert
Copy link
Author

Empty line removed.

Offtopic: Is "squashing after review" still the correct procedure? The openhab-distro/CONTRIBUTING.md is not clear about this topic.

@cweitkamp
Copy link
Contributor

cweitkamp commented Jan 31, 2019

Thanks.

Is "squashing after review" still the correct procedure?

No, not for small changes like this. We are able to and will do that while merging.

Copy link
Contributor

@cweitkamp cweitkamp left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks.

@cweitkamp cweitkamp merged commit 539cb6f into openhab:master Feb 1, 2019
@wborn wborn added this to the 2.5 milestone Feb 28, 2019
@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/tradfri-binding-loses-connection-after-power-failure-gateway-reboot/47833/90

@hreichert hreichert deleted the tradfri-forceResumeSession branch April 22, 2019 18:09
Pshatsillo pushed a commit to Pshatsillo/openhab-addons that referenced this pull request Jun 19, 2019
As discussed in eclipse-archived/smarthome#6065 the tradfri gateway does not support a session resumption after a gateway reboot or other communcation errors.
Californium 1.0.7 is able to force a full handshake instead.
This change calls "forceResumeSessionFor" after a communication error, so that a full handshake is done.

Signed-off-by: Holger Reichert <[email protected]>
Signed-off-by: Pshatsillo <[email protected]>
ne0h pushed a commit to ne0h/openhab-addons that referenced this pull request Sep 15, 2019
As discussed in eclipse-archived/smarthome#6065 the tradfri gateway does not support a session resumption after a gateway reboot or other communcation errors.
Californium 1.0.7 is able to force a full handshake instead.
This change calls "forceResumeSessionFor" after a communication error, so that a full handshake is done.

Signed-off-by: Holger Reichert <[email protected]>
Signed-off-by: Maximilian Hess <[email protected]>
@Tobster77
Copy link

The reported issue persists. Is there any recommendation?

@9037568
Copy link
Contributor

9037568 commented Mar 12, 2020

Is there any recommendation?

Open a new ticket with the details of the problem you're seeing.

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.

6 participants