Client example show payload on block1 completion#383
Conversation
examples/client.c
Outdated
| ready = 1; | ||
|
|
||
| if (coap_get_data(received, &len, &databuf)) | ||
| append_to_output(databuf, len); |
There was a problem hiding this comment.
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.
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.
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 |
3 participants
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.