Does CoAP require it to be an (ASCII) string or could we also add binary data here?

The function works for every option, and some options take binary data (typically used with coap_opt_add_opaque)

In the concrete place where it's being introduced for (path components and query parameters of a CoAP URI), it's arbitrary UTF-8 that may contain any code points. (Using a function like coap_opt_add_chars rules out one of them unless the delimiter is 0xff or some other byte invalid in UTF-8).

Let me summarize the conversation to this point. To be clear, when I mention a String option or Opaque option, I mean them as described in sec. 3.2 of 7252 on option value formats.

Originally, this PR intended to extend coap_opt_add_string() for multiple String options to allow passing in a length-specified char array (without the terminating null byte). @chrysn has suggested that the function also could be used to write multiple Opaque options (like If-Match or ETag) as well as String options. However, the use of a delimiter character parameter to separate the embedded String/Opaque options means that an Opaque option cannot include that character in its contents.

So, the question then is do we want to extend/publish this function to allow writing multiple Opaque options as well as multiple String options? Or perhaps it would be more useful to create a separate function to add multiple Opaque options more tailored to that use case. For example, it might be better to use a size_t array to identify the starting offset for each embedded Opaque value within the buffer so we don't eliminate use of a particular character. At the same time, someone could still use coap_opt_add_chars() for Opaque options given the limitation with delimiters.

@kb2ma thanks for the feedback! I added two new fixup commits to address your comments. @benpicco I think we should keep this PR small and clean. We could add coap_opt_add_uri_path_chars() in a follow-up?

So, the question then is do we want to extend/publish this function to allow writing multiple Opaque options as well as multiple String options? Or perhaps it would be more useful to create a separate function to add multiple Opaque options more tailored to that use case.

Hm, I am a little bit indifferent about the naming here. I do not see a use case for a generic function that adds opaque values and at the same time includes the feature to split the buffer at a specific character (e.g., '/'). I had a quick look at the available CoAP options and I could not find a scenario, where such a function would be beneficial (although, this doesn't mean that such an option wouldn't be defined in future specs).

Fixups look good. I think this PR is ready to merge, but I'll wait a couple of days for any other feedback before approving.

If someone wants to use coap_opt_add_chars() to generate Opaque options, they certainly can do that. I think it's OK to leave the function doc as is, without commenting on its use for a particular option value format.

@cgundogan, I just noticed Travis found an unused variable.

@kb2ma I fixed the unused variable warning, squashed all commits and triggered murdock. Let's wait for the outcome.

Murdock and Travis agree (:

Excellent, I'll take one more look to be sure we're good.

thanks for the review @kb2ma ! (: