-
Notifications
You must be signed in to change notification settings - Fork 2.1k
nanocoap: add payload helper functions #13726
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
Conversation
7ab647d
to
743134e
Compare
Conceptually, these additions are a great idea. I'd like to think a little about how these functions relate to our mechanism for blockwise payload writing. See the At the same time, the use of Block options could be a semi-automatic mechanism to handle a big payload. It could be a kind of contract between (g|nano)coap and the application. The application's responsibility is to write the bytes, and (g|nano)coap handles slicing as needed. We don't need to get there in a single step, but it would useful to have a vision. Of course it also should be the user's choice whether to take on this overhead. |
743134e
to
f444f81
Compare
I would prefer I do see some use in unifying the functions for blockwise and regular transfers and update the block option if it is already present in the packet. While it would be nice to automatically start a blockwise transfer when required, I think it is an edge case that we should not try to cover. I expect that most of the time, the rough size (a few bytes or a large amount of bytes) is known at development time and that automatically switching between the two is never used. For now I think this PR is a small step in the right direction and covers a currently used pattern. |
Alright - I also replaces |
👍 in general. I'm a bit confused on when this can be used, though. |
Whenever a user wants to add payload to a CoAP request - or am I getting the CoAP API wrong here? |
An example of the value of this ability is for
+1 I have an outline for an implementation that I'll put in an RFC issue. |
@kb2ma should we wait for your RFC issue before merging here? |
Yes, please. I should have something later today. |
Please also add the function below to provide equivalent functionality to blockwise.
|
#13942 proposes reuse of the functions declared here as-is for blockwise. If that approach looks reasonable, there is no impact on this PR. |
I've added |
Thanks for the additional function. It's part of the diabolical plan hatched in #13942 to make it easy to write a blockwise payload in gcoap. ;-) I'll be back with a formal review in the next day or so. |
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.
See inline comments for mostly mechanical and consistency issues.
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.
Fixups look good. Please squash!
This adds two functions `coap_payload_add()` and `coap_payload_advance()`. - `coap_payload_add()` will add n bytes to the payload buffer and advance payload pointer accordingly. const char hello[] = "Hello CoAP!"; coap_payload_add(pkt, hello, sizeof(hello)); - `coap_payload_advance()` will advance the payload buffer after data has been added to it. int len = snprintf(pkt->payload, pkt->payload_len, "%s %s!", "Hello", "CoAP"); coap_payload_advance(pkt, len); I considered adding an additional parameter to keep track of the total request size (returned size from coap_opt_finish() incremented by each added payload fragment), but decided against it to keep consistency with the existing API.
ac3e8e2
to
e5c20b1
Compare
Contribution description
This adds two functions
coap_payload_add()
andcoap_payload_advance()
.coap_payload_add()
will add n bytes to the payload buffer and advancepayload pointer accordingly.
coap_payload_advance()
will advance the payload buffer after datahas been added to it.
I considered adding an additional parameter to keep track of the total request size
(returned size from coap_opt_finish() incremented by each added payload fragment),
but decided against it to keep consistency with the existing API.
Testing procedure
Issues/PRs references