-
Notifications
You must be signed in to change notification settings - Fork 422
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
Client example show payload on block1 completion #383
Conversation
examples/client.c
Outdated
@@ -500,6 +500,10 @@ message_handler(struct coap_context_t *ctx, | |||
if (payload.length <= (block.num+1) * (1 << (block.szx + 4))) { | |||
coap_log(LOG_DEBUG, "upload ready\n"); | |||
ready = 1; | |||
|
|||
if (coap_get_data(received, &len, &databuf)) | |||
append_to_output(databuf, len); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not sure if this needs to be after the following check for duplicate block1? Otherwise, we might end up adding the payload multiple times in case of duplicates.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We only want the message to print on receipt of the last block1 response. Even if the last block is duplicated, the output routine needs to be within this if (payload.length...
conditional, right? In that case, I can verify ready
has not been set, like below. Please confirm and I'll push the change.
if (payload.length <= (block.num+1) * (1 << (block.szx + 4))) {
coap_log(LOG_DEBUG, "upload ready\n");
/* check 'ready' to ensure not duplicate ACK */
if (!ready && coap_get_data(received, &len, &databuf))
append_to_output(databuf, len);
ready = 1;
return;
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think there is too much reliance on ready
not being used elsewhere with your updated solution. I would prefer
diff --git a/examples/client.c b/examples/client.c
index edfe798..23cdd2f 100644
--- a/examples/client.c
+++ b/examples/client.c
@@ -497,11 +497,6 @@ message_handler(struct coap_context_t *ctx,
}
}
- if (payload.length <= (block.num+1) * (1 << (block.szx + 4))) {
- coap_log(LOG_DEBUG, "upload ready\n");
- ready = 1;
- return;
- }
if (last_block1_tid == received->tid) {
/*
* Duplicate BLOCK1 ACK
@@ -516,6 +511,14 @@ message_handler(struct coap_context_t *ctx,
}
last_block1_tid = received->tid;
+ if (payload.length <= (block.num+1) * (1 << (block.szx + 4))) {
+ coap_log(LOG_DEBUG, "upload ready\n");
+ if (coap_get_data(received, &len, &databuf))
+ append_to_output(databuf, len);
+ ready = 1;
+ return;
+ }
+
/* create pdu with request for next block */
pdu = coap_new_request(ctx, session, method, NULL, NULL, 0); /* first, create bare PDU w/o any option */
if (pdu) {
If the final BLOCK1 response is large enough to need BLOCK2, this is handled earlier in the message_handler() code with the appropriate data being output.
@kb2ma Can you make the updated suggested change and push the change (rebasing if needed to make it just one patch). |
Will do. Apologies for delay. |
Rebased and pushed the single change as described. I see the final payload for block1 and block2, as well as the payload without a blockwise exchange. |
Thanks - appreciated |
In a fix to RIOT's block implementation, we found that the libcoap client example does not show the payload on the final response of a block1 exchange. However, the client does show the payload when there is not a block option present.
This PR simply outputs the response payload in the block1 case.