This repository was archived by the owner on May 17, 2021. It is now read-only.
[Plugwise] Reliability improvements #4797
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Improves the binding reliability by: * making sure CR is followed by LF when reading incoming packets, which should result in less protocol message errors * polling max 1 second for AcknowledgeMessages so the SendThread does not wait indefinitely when ACKs get lost Signed-off-by: Wouter Born <github@maindrain.net>
watou
reviewed
Nov 19, 2016
| private final static char LF = '\n'; | ||
| /** Plugwise protocol trailer code (hex) */ | ||
| private final static String PROTOCOL_TRAILER = "\r\n"; | ||
| private final static String PROTOCOL_TRAILER = new String(new char[] { CR, LF }); |
Contributor
There was a problem hiding this comment.
I know you didn't introduce this, and it's very minor, but there is a recommended order of modifiers in Java which improves readability. Here is one place that lists the common order:
- Access modifier (public / private / protected)
- abstract
- static
- final
- transient
- volatile
- default
- synchronized
- native
- strictfp
watou
approved these changes
Nov 19, 2016
Contributor
watou
left a comment
There was a problem hiding this comment.
Aside from the small comment, lgtm.
Signed-off-by: Wouter Born <github@maindrain.net>
Member
Author
|
Thanks for reviewing @watou! I think it should be fixed now. |
Contributor
|
Thanks @wborn! |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Improves the binding reliability by:
should result in less protocol message errors
AcknowledgeMessages so theSendThreaddoesnot wait indefinitely when ACKs get lost
The most common scenario for the binding to stop working should now be solved. I.e. a received
AcknowledgeMessageresults in a protocol message error (e.g. because it is preceded by the LF of a previous message). Especially on Windows this seems to be an issue due to non-blocking reads.