net/gcoap: add Uri-Query strings for requests

[haukepetersen]

Needs kaspar030/sock#16 merged to work...

This PR adds means to gcoap for adding query strings (Uri-Query options) to any given CoAP request. The functionality is split and partly placed in nanocoap.

The usage is very simple, e.g.:

gcoap_req_init(&pkt, buf, sizeof(buf), COAP_METHOD_POST, "/some/url");
gcoap_add_qstring(&pkt, "key1", "some_value");
gcoap_add_qstring(&pkt, "bier", NULL);
gcoap_add_qstring(&pkt, "foo", "bar");
gcoap_finish(&pkt, 0, COAP_FORMAT_TEXT);

will result in:

POST: HOST.../some/url?key1=some_value&bier&foo=bar

[haukepetersen]

added missing qs buffer initialization

[kb2ma]

Here is an idea for a somewhat lower level approach to this implementation. We assume there is no intrinsic value to gcoap in storage of the URL-encoded query string in coap_pkt_t.qs. Instead, we can save some memcpys and write the coded CoAP options to coap_pkt_t.qs.

With this approach, gcoap_add_qstring() uses coap_put_option() to append each encoded option to qs. This approach means we need to add a qs_len attribute to coap_pkt_t to know where to append the next option.

Then in _write_options(), one more adaptation required -- fixing the option number delta for the first option in qs. To solve this, we need to expose a low level API in nanocoap to read and write an option header:

1. Convert the existing _put_odelta() to coap_put_opt_hdr()
2. Convert the existing _decode_value() to coap_get_opt_hdr()

So, in _write_options() first call coap_get_opt_hdr() on qs to get the length of the first option. Then call coap_put_opt_hdr() on the PDU buf, using this length. Finally memcpy qs to the PDU buf, skipping the first byte in qs.

On the receiving side, use the new coap_find_opt() in nanocoap #18 to read out the query values directly from the packet buf.

So then, with this approach the rework to coap_put_option_url() in nanocoap #16 becomes unnecessary.

We could even take this implementation another step and eliminate coap_pkt_t.qs. The insight here is that the order of Uri-Query options is not significant. So, we can take the technique described above and write to coap_pkt_t.url, but write from the back of the buffer toward the front. It doesn't matter because the option delta always is zero. In this case we would need to add a url_qopt_head pointer parameter to coap_pkt_t to know where the Uri-Query options begin in the url buffer.

With this extension we can eliminate nanocoap #16 altogether. The real beauty here is that the use of the url buffer for its original purpose it not affected because the url and the Uri-Query options are separated by \0 bytes. We just need to be sure they don't overlap.

[haukepetersen]

I agree that we can quite significantly optimize this PR (and some other parts with it). I would actually go so far as to remove the 'special' handling of the Uri-Query and the Uri-Path option all together and unify everything down to one set of generic option handling functions (probably as port of nanocaop), and maybe only extend this with some optional, user friendly access function(s) in gcaop. And idea we should think about here is to simply add pointers to options into a list of points, which is then compiled into the actual coap header. With this approach, there is no need for separate buffers for any type of option anymore.

My problem is only, that this type of refactoring will take a (little) while, stalling the merger of all the RD related code. So I would like to request if we could merge my current proposal like this (with all its drawbacks), and optimize the whole option handling as the next step.

[kb2ma]

I look forward to the proposal for generic improvements to option handling. At the same time, I am disappointed to see the addition of a 64-byte array to coap_pkt_t, and hope its life is short. I will review this PR ASAP from that perspective.

[kb2ma]

I suggest limiting GCOAP_REQ_OPTIONS_BUF to 40 bytes. My concern is with a PUT or a POST over 802.15.4. gcoap_req_init() allows a payload length reduced by the size of the header, the path, and this options buf value. Let's assume 128 - 6 (header+token) - 10 (path option) = 112. Then 112 - 48 (options) = 64 bytes for the payload.

For the payload, I think 64 bytes is the minimum acceptable, to support a power-of-2 block transfer. Ideally I would want to support 80 bytes for the payload without a block option, assuming we have header compression.

So, I think 48 bytes is too generous on the options side, and would cut that back a bit. The other option is to extend GCOAP_PDU_BUF_SIZE larger than 128. I don't feel really strongly about this change though -- just putting it out there for feedback.

[kb2ma]

Tested on native. Code is clear, and nice comment for the function. Also, consider referencing the new function in the 'Creating a request' section of the module documentation.

[kb2ma]

I suggest "Adds a Uri-Query option..." I have used the 3rd person verb rather than the 2nd person imperative form for other gcoap commands.

[haukepetersen]

I agree, fixed.

[kb2ma]

I suggest naming the variable 'pdu', to be consistent with other gcoap functions.

[haukepetersen]

fixed.

[kb2ma]

Please remove the comment above.

[haukepetersen]

fixed.

[haukepetersen]

rebased, fixed comments, and switched nanocoap pkg to newest commit.

[kb2ma]

Thanks for those updates. The only open question for me then is your reaction to my comment above on the size of GCOAP_REQ_OPTIONS_BUF relative to the remaining payload size.

[haukepetersen]

Oh sorry, slipped through yesterday. Changed GCOAP_REQ_OPTIONS_BUF to 40 byte.

[kb2ma]

I'll ACK to that!

[miri64]

Proxy-ACK, but please rebase.

[haukepetersen]

done

[haukepetersen]

all green -> go