Bugfix tcp #120
Closed
Bugfix tcp #120
Conversation
This file contains 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
added 4 commits
November 3, 2016 16:55
The order of numberOfThreads and idleTimeout are exchange in TcpServerConnector. So moving numberOfThreads before idleTimeout will harmonice the API and fix the "parameter exchange", when TlsServerConnector calls super. Signed-off-by: Achim Kraus <achim.kraus@bosch-si.com>
coap+tcp doesn't use the message type, but currently some other code rely on type being set (e.g. some Message Tracer). Signed-off-by: Achim Kraus <achim.kraus@bosch-si.com>
If stop is called twice, a NullPointerException is caused by workerGroup being set to null at the first call. Add synchronize as in UDPConnector. Signed-off-by: Achim Kraus <achim.kraus@bosch-si.com>
If stop is called twice, a NullPointerException is caused by workerGroup and bossGroup being set to null at the first call. Add synchronize as in UDPConnector. Signed-off-by: Achim Kraus <achim.kraus@bosch-si.com>
|
Kai, this looks good to me. |
sophokles73
requested changes
Nov 4, 2016
| @@ -106,13 +107,15 @@ public CoapTcpStack(NetworkConfig config, Outbox outbox) { | |||
|
|
|||
| // delegate to top | |||
| @Override public void sendRequest(Request request) { | |||
| if (null == request.getType()) request.setType(Type.CON); | |||
There was a problem hiding this comment.
please put the statement on a new line and in between curly braces
| top.sendRequest(request); | ||
| // CoAP over TCP does not have acknowledgements. Everything is automatically acknowledged. | ||
| request.setAcknowledged(true); | ||
| } | ||
|
|
||
| // delegate to top | ||
| @Override public void sendResponse(Exchange exchange, Response response) { | ||
| if (null == response.getType()) response.setType(Type.CON); |
There was a problem hiding this comment.
please put the statement on a new line and in between curly braces
| @@ -26,6 +26,9 @@ | |||
| @RunWith(Parameterized.class) | |||
| public class TcpConnectorTest { | |||
|
|
|||
| private static final int NUMBER_OF_THREADS = 1; | |||
There was a problem hiding this comment.
Hell, so californium uses tabs, but connector spaces?
| @@ -35,6 +35,8 @@ | |||
|
|
|||
| public class TlsConnectorTest { | |||
|
|
|||
| private static final int NUMBER_OF_THREADS = 1; | |||
There was a problem hiding this comment.
About the formatting:
sorry I'm a little lost:
@Override public void destroy() {
for (Layer layer : layers)
layer.destroy();
}
@Override public void receiveRequest(Exchange exchange, Request request) {
// if there is no BlockwiseLayer we still have to set it
if (exchange.getRequest() == null)
exchange.setRequest(request);
if (hasDeliverer()) {
deliverer.deliverRequest(exchange);
} else {
LOGGER.severe("Top of CoAP stack has no deliverer to deliver request");
}
}
So, is it OK to correct only my changes, or is more required?
Signed-off-by: Achim Kraus <achim.kraus@bosch-si.com>
|
I would like to squash all of these commits into one since they are all fixing small issues with the TCP support. This will also make it easier to cherry pick it on master. Any objections? |
|
OK, from my side. |
sophokles73
approved these changes
Nov 4, 2016
|
merged |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Correct parameter order in TcpServerConnector.
Correct "stop()" to be executable multiple times.
#118