Skip to content
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

[RFC] gcoap adaptation to #8772 #8831

Closed
kb2ma opened this issue Mar 25, 2018 · 9 comments
Closed

[RFC] gcoap adaptation to #8772 #8831

kb2ma opened this issue Mar 25, 2018 · 9 comments

Comments

@kb2ma
Copy link
Member

kb2ma commented Mar 25, 2018

[ Edited to add gcoap_opt_add_string() ]

I have an outline now for adaptation of gcoap for #8772. Overall, I don't see any issues. The existing gcoap API can remain, and I'll add a couple of functions for options. For implementation, I'll add a stack-based buffer for compiling options and an additional memcpy(). The overall gcoap approach remains -- a higher-level API to CoAP.

The gcoap functions for adding options will be like:

gcoap_opt_add_buf(pdu, optnum, buf_ptr, buf_len)
gcoap_opt_add_string(pdu, optnum, string_ptr, separator)
gcoap_opt_add_value(pdu, optnum, uint32)

The implementation appends option values to the packet buffer as they are provided by the user. The user is not required to add Options in order by option number. These values will be in the format of the raw data, like a query string. gcoap will use the new coap_optpos_t options[] array to store the position of these options in the buffer. gcoap will move the packet payload pointer as options are added, so it will be ready for the user when they decide to write the payload. You can see an example of this approach below.

   +-------+---------+---------+---------+----- ... --+
      hdr  |  path   | uint32  |  query  |    payload
           |         |         |         |
        opt_pos   opt_pos   opt_pos    payload*
        Uri-Path  Observe   Uri-Query

Finally, gcoap_finish() will sort the Options into order by option number. It will create a stack buffer, and use the nanocoap functions like coap_put_option() to write the contents in order to the stack buffer. Finally, gcoap will copy the stack buffer to the packet buffer and do the final memmove to reposition the payload, as it does currently.

@kb2ma kb2ma changed the title [RFC] gcoap Adaptation to #8772 [RFC] gcoap adaptation to #8772 Mar 25, 2018
@bergzand
Copy link
Member

@kb2ma overall I like the idea. Do I understand correctly that the options are only added to the coap_optpos_t array and are only written to the buffer when the packet is finalised, and thus that the order of adding options doesn't matter?

@kb2ma
Copy link
Member Author

kb2ma commented Apr 13, 2018

Cool, thanks for the confirmation. Yes, option positions are stored in the coap_optpos_t array. The option values are actually stored in the packet buffer as a scratchpad as the user adds them. gcoap also maintains the payload pointer while building the options, so the user can copy that after specifying the last option. Then when the packet is finalized in the gcoap_finish() call, gcoap uses the coap_optpos_t array and the raw options in the packet buffer to write the real CoAP options in the proper order to a stack-allocated buffer. Finally the stack buffer contents are copied back to the packet buffer, ready to send.

gcoap makes the trade-off of the stack buffer and extra memcpy for a simpler interface to the user.

I have started on the implementation for a request, and plan to have a WIP PR as soon as possible.

@kb2ma
Copy link
Member Author

kb2ma commented Apr 14, 2018

@bergzand, I just reread my comment, and I see I didn't clearly answer your question. Yes, you're right, the user is not required to add options in any particular order.

@bergzand
Copy link
Member

Mostly to confirm since it's been a while since I've used the gcoap API. Is it possible to get request headers from within the handler? For example the Accept header to determine the content type that should be returned.

@bergzand
Copy link
Member

From a quick look at the current code, the relevant headers should be fetched before doing a coap_resp_init. That answers my question from the comment above :)

@kb2ma
Copy link
Member Author

kb2ma commented Apr 14, 2018

Yes, that's right, a server should query/collect header information from the request before writing the response. The response will overwrite the request. I tried to say that in the "Creating a response" portion of "Server Operation" in the documentation.

@kb2ma
Copy link
Member Author

kb2ma commented Apr 14, 2018

[ Edited to use 'gcoap_opt_add_X rather than 'gcoap_opt_X_add'. I find subject-verb-predicate reads better. ]

I am making an attempt to standardize function names. I like the convention proposed by @kaspar030. It fits well with gcoap's use of 'gcoap_req_X', 'gcoap_resp_X'.

So, I just updated the function names in my original comment above to use the format 'gcoap_opt_add_X'. I also updated my proposal for nanocoap option functions also to use this convention.

Overall, I think this standardization will help users find the function they need. I also think the synchronization between gcoap and nanocoap is helpful. @oleg made this comment at the T2TRG meeting just before the RIOT summit last yer.

@kb2ma
Copy link
Member Author

kb2ma commented May 16, 2018

Do I understand correctly that the options are only added to the coap_optpos_t array and are only written to the buffer when the packet is finalised, and thus that the order of adding options doesn't matter?

@bergzand, I see that I still didn't answer your question clearly. :-( Yes, you can enter options out of order, but they are written twice -- once out of order, and then sorted in order. See #9144. Given the definition of coap_optpos_t, I don't see how to record the location for the option data until coap_opt_finish().

@kb2ma kb2ma closed this as completed May 20, 2018
@kb2ma
Copy link
Member Author

kb2ma commented May 20, 2018

#9048, specifically CoAP API Options Update, describes how this work actually was implemented. Relative to the discussion here, the implementation does not require sorting the options. However, #9144 implements sorting if it's needed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants