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

Add realtime expiring handshakes. #1165

Merged
merged 1 commit into from
Jan 2, 2020

Conversation

boaks
Copy link
Contributor

@boaks boaks commented Dec 12, 2019

Support android/os deep sleep during handshake.

Signed-off-by: Achim Kraus achim.kraus@bosch-si.com

@boaks boaks force-pushed the expire_handshakes branch 4 times, most recently from fb67b6b to e3aa3d3 Compare December 16, 2019 17:01
@boaks boaks added this to the 2.1.0 milestone Dec 17, 2019
* @see #handshakeFailed(Throwable)
* @see #isRemovingConnection()
*/
public final void handshakeAborted(Throwable cause) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I do not understand the benefits of this final ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The api-changes.json contains:

"justification": "keep API consistent with other similar functions"

FMPOV, similar functions are:
handshakeCompleted or handshakeFailed(Throwable cause), both are already final.

Copy link
Contributor

Choose a reason for hiding this comment

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

Without API break in mind :
I don't understand the benefits of all those final keyword. Most of the time I really don't like the use of final for methods or classes.

With API break in mind :
Even if I agree that finalMethodAddedToNonFinalClass is a questionable break, I would not break just for this, mainly because I find final keyword for method a bad idea most of the time.

@boaks boaks force-pushed the expire_handshakes branch from e3aa3d3 to ccbc2fc Compare December 20, 2019 15:30
Copy link
Contributor

@sbernard31 sbernard31 left a comment

Choose a reason for hiding this comment

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

No objection

@@ -176,6 +177,12 @@
/** Realtime nanoseconds of last sending a flight */
private long flightSendNanos;

/** Realtime nanoseconds when handshakes get's expired. */
private long nonosExpireTime;
Copy link
Contributor

Choose a reason for hiding this comment

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

typo : nonos => nanos

/**
* Test, if handshake is expired according nano realtime.
*
* Used to mitigate deep sleep during handhsakes.
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe ? (personally I didn't know deep sleep before those PRs)

Used to mitigate **Android** deep sleep during handhsakes

* @see #handshakeFailed(Throwable)
* @see #isRemovingConnection()
*/
public final void handshakeAborted(Throwable cause) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Without API break in mind :
I don't understand the benefits of all those final keyword. Most of the time I really don't like the use of final for methods or classes.

With API break in mind :
Even if I agree that finalMethodAddedToNonFinalClass is a questionable break, I would not break just for this, mainly because I find final keyword for method a bad idea most of the time.

Support android/os deep sleep during handshake.

Signed-off-by: Achim Kraus <achim.kraus@bosch-si.com>
@boaks boaks force-pushed the expire_handshakes branch from ccbc2fc to 2c4e777 Compare January 2, 2020 16:53
@boaks boaks merged commit fac47a9 into eclipse-californium:master Jan 2, 2020
@boaks boaks deleted the expire_handshakes branch February 12, 2020 16:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants