net/unicoap: Unified and Modular CoAP Stack: Block-Wise Transfers - #22589
net/unicoap: Unified and Modular CoAP Stack: Block-Wise Transfers#22589carl-tud wants to merge 12 commits into
Conversation
|
I have yet to make the iterator support noncontiguous payloads... Only contiguous buffers at the moment... |
mguetschow
left a comment
There was a problem hiding this comment.
First round of review, no testing yet. Great work as always!
| " This is a demo of block-wise transfers in unicoap." \ | ||
| " This very long response payload will be split into several chunks aka. blocks," \ | ||
| " each sent in a separate Block2 response. The client needs to support block-wise transfer," \ | ||
| " and sent subsequent Block2 requests to retrieve the remaining response blocks." |
There was a problem hiding this comment.
| " and sent subsequent Block2 requests to retrieve the remaining response blocks." | |
| " and send subsequent Block2 requests to retrieve the remaining response blocks." |
| * In this case, we don't want to loose our precious greeting along the way. | ||
| * To send confirmable messages (CON) over UDP or DTLS, we pass the | ||
| * @ref UNICOAP_RESOURCE_FLAG_RELIABLE flag. */ | ||
| .flags = UNICOAP_RESOURCE_FLAG_RELIABLE, | ||
| #if IS_USED(MODULE_UNICOAP_BLOCKWISE) | ||
| .flags = UNICOAP_RESOURCE_FLAG_RELIABLE | ||
| | UNICOAP_RESOURCE_FLAG_REASSEMBLE | ||
| | UNICOAP_RESOURCE_FLAG_DURABLE_MESSAGE | ||
| | UNICOAP_RESOURCE_FLAG_SLICE, |
There was a problem hiding this comment.
missing some comment on blockwise support. I would move the reliable discussion below into the else and add blockwise into the if
| PSEUDOMODULES += unicoap_server_resource_declarations | ||
|
|
||
| # Automatic block-wise transfers in server API | ||
| PSEUDOMODULES += unicoap_server_blockwise | ||
|
|
||
| # URI support in unicoap client API | ||
| PSEUDOMODULES += unicoap_client_uri |
There was a problem hiding this comment.
Uh oh, those should be alphabetically sorted, but apparently already violated on the base PR...
There was a problem hiding this comment.
... why are we alphabetically sorting them? as opposed to sensibly sorting them?
There was a problem hiding this comment.
because "sensibly" is hard to tell across modules. Arguably easier among submodules with a common prefix, but I'd favor consistency here (and I'm sure @crasbe as well)
| * @brief Instructs the stack to send a given message with block-wise fragmented | ||
| * payload. | ||
| * | ||
| * @warning The body you want to slice must not exceed `UINT32_MAX` bytes. |
There was a problem hiding this comment.
where does this constraint come from?
There was a problem hiding this comment.
no clue... I'll investigate what I did there
| * If a response times out, the @p error parameter will be set to `-ETIMEDOUT`. | ||
| * Other failures are also communicated via the error parameter. | ||
| * | ||
| * Return a negative integer to abort the block-wise transfer. |
There was a problem hiding this comment.
| * Return a negative integer to abort the block-wise transfer. | |
| * Return zero to continue or a negative integer to abort the block-wise transfer. |
| for (int i = 0; i < (int)ARRAY_SIZE(_state.server_memos); i += 1) { | ||
| memo = &_state.server_memos[i]; |
There was a problem hiding this comment.
doesn't this need a _lock as well?
| printf("TEST: %s\n", __func__); | ||
|
|
There was a problem hiding this comment.
| printf("TEST: %s\n", __func__); | |
unittests are usually not printing anything in RIOT. same in the other functions
|
|
||
| unicoap_blockwise_iterator_init(&slicer, UNICOAP_BLOCK_SZX_32, (uint8_t*)body, strlen(body)); | ||
| TEST_ASSERT_EQUAL_INT(slicer.offset, 0); | ||
| _TEST_ASSERT_EQUAL_BLOCK(slicer.block_option, 0, 32, true); |
There was a problem hiding this comment.
| _TEST_ASSERT_EQUAL_BLOCK(slicer.block_option, 0, 32, true); | |
| _TEST_ASSERT_EQUAL_BLOCK(slicer._block_option, 0, 32, true); |
otherwise the tests do not compile
| res = unicoap_blockwise_collect_block1(&collector, block, chunk, chunk_size); | ||
| TEST_ASSERT_EQUAL_INT(res, 1); | ||
| _TEST_ASSERT_EQUAL_BYTES(body_buffer + 0, body + 0, chunk_size); | ||
| _TEST_ASSERT_EQUAL_BLOCK(collector.block_option, 0, 16, true); |
There was a problem hiding this comment.
| _TEST_ASSERT_EQUAL_BLOCK(collector.block_option, 0, 16, true); | |
| _TEST_ASSERT_EQUAL_BLOCK(collector.block_option, 2, 16, true); |
Would have expected this to be updated by unicoap_blockwise_collect_block1 - after all that's what you need to send to the client in control usage?
There was a problem hiding this comment.
also this would match what you expect below for collect_block2.
| /* server */ | ||
| /* test retransmissions */ | ||
| for (unsigned int i = 0; i < 3; i += 1) { | ||
| chunk_size = unicoap_blockwise_slice_block2(&slicer, block, &chunk); |
There was a problem hiding this comment.
why don't we test retransmissions for block1?
|
server: requesting |

This PR implements block-wise transfers (RFC 7959) in
unicoap, a unified and modular CoAP implementation for RIOT. An overview of all PRs related tounicoapis presented in #21389, including reasons whyunicoapis needed and a performance analysis.What does this PR include?
Block1(chunked request) andBlock2(chunked responses) transfersunicoap_clientand/orunicoap_server.UNICOAP_[CLIENT|RESOURCE]_FLAG_DURABLE_MESSAGEin addition to the slice flag to avoid the message from being copied into an internal buffer if you can guarantee the pointer will stay alive throughout the transfer (e.g., forstaticallocations in your app). Internal buffers are only needed when this is not case, or when reassembling.net/unicoap/blockwise.hheader andunicoap_blockwise_kitsubmodule, so users can easily manually implement block-wise transfer or tinker with specific APIs.coapshell command: new--sliceand--gluecommand line argumentsunicoap_serverexample: Slicing and reassembling supportedclient.pyandserver.pyscripts powered byaiocoapBlock-wise features at your fingertips
Compared to prior fiddling with block-wise APIs, this is how easy supporting block-wise transfer is now. No need to change response handlers or request handlers!
Add
USEMODULE += unicoap_blockwiseto your Makefile. Then:I'll organise the monolithic commit into multiple structured commits once this has passed review, and rebase this PR onto #22266 once merged (PR stacks do not work).
Declaration of AI-Tools / LLMs usage:
AI-Tools / LLMs that were used are:
none