[riot-notifications] [RIOT-OS/RIOT] nanocoap: Allow accessing opaque and other written options (#11437)

Ken Bannister notifications at github.com
Tue Apr 30 11:49:34 CEST 2019


kb2ma requested changes on this pull request.

@chrysn, thanks for the PR! It's so helpful to learn what you need from the library.

To summarize, I read that the use cases below are the main concern. 

* Read opaque option
* Update value after added
* Iterate over options

For the discussion below, it helps to see nanocoap's function documentation grouped by subject. See #10876, which hopefully will be merged soon.

### Reading options
I agree we need a way to read an opaque option. Currently nanocoap has a string reader (coap_opt_get_string()). We also need coap_opt_get_uint(), which can easily be built from coap_get_content_type().

### Update value after added
I agree on the usefulness for the ETag example, although it would only work with the gcoap API when writing a message because it requires a coap_pkt_t.

### Iterate over options
If OSCORE needs an option iterator, we would want to support that. Also, nanocoap generally looks up options by option number, so it would be nice to support both approaches.

## Implementation
The current structure of coap_opt_by_index() accesses an option by index in the list of options. However, coap_parse() only adds a coap_opt_pos_t option for each *unique* option number. For example, it would only add a single entry for Uri-Query. You can see this effect in your aiocoap-client example. Try adding a second query option to the request, and the output still will show a single 15 key in the output.

How about this idea:

ssize_t coap_opt_get_next(const coap_pkt_t *pkt, coap_opt_pos_t *opt, uint8_t *value)

Let the `opt` param be inout. coap_opt_pos_t includes an option number and a pointer to the option. So on the input, it is a qualifier so we can find by option number and allow for iteration. If the option number is zero, start from the first option. Othewise start from the first option for the given option number. If the pointer is not null, start from that as the last option read so we can iterate over multiple options with the same option number. 

For the response, the function sets the option number and pointer to the start of the option. If the requested option is not found, return 0. This means the user doesn't need to query pkt.options_len when iterating. I'd like to avoid exposing that to the user if we can.

The pkt and value options would retain the same meaning as in your coap_opt_by_index(), as would the return value as the length of value. This gives us the ability to read an opaque option and enough information to write the value. I also would allow negative values for the return just in case we want to do that in the future. 

## Other issues with PR

Example error handling -- Is it useful to reply on error? Why not just print an error message?



-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/RIOT-OS/RIOT/pull/11437#pullrequestreview-232080748
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.riot-os.org/pipermail/notifications/attachments/20190430/988b1fb4/attachment-0001.html>


More information about the notifications mailing list