Merge pull request #10892 from kb2ma/gcoap/use_opt_finish

net/gcoap: Replace use of gcoap_finish() with coap_opt_finish()

sys/include/net/gcoap.h

@@ -17,9 +17,7 @@
  * application only needs to focus on request/response handling. For a server,
  * gcoap accepts a list of resource paths with callbacks for writing the
  * response. For a client, gcoap provides a function to send a request, with a
- * callback for reading the server response. Generation of the request or
- * response requires from one to three well-defined steps, depending on
- * inclusion of a payload.
+ * callback for reading the server response.
  *
  * gcoap allocates a RIOT message processing thread, so a single instance can
  * serve multiple applications. This approach also means gcoap uses a single UDP
@@ -56,35 +54,37 @@
  * ### Creating a response ###
  *
  * An application resource includes a callback function, a coap_handler_t. After
- * reading the request, the callback must use one or two functions provided by
- * gcoap to format the response, as described below. The callback *must* read
- * the request thoroughly before calling the functions, because the response
- * buffer likely reuses the request buffer. See `examples/gcoap/gcoap_cli.c`
- * for a simple example of a callback.
+ * reading the request, the callback must use functions provided by gcoap to
+ * format the response, as described below. The callback *must* read the request
+ * thoroughly before calling the functions, because the response buffer likely
+ * reuses the request buffer. See `examples/gcoap/gcoap_cli.c` for a simple
+ * example of a callback.
  *
  * Here is the expected sequence for a callback function:
  *
  * Read request completely and parse request payload, if any. Use the
  * coap_pkt_t _payload_ and _payload_len_ attributes.
  *
- * If there is a payload, follow the three steps below.
+ * If there is a payload, follow the steps below.
  *
  * -# Call gcoap_resp_init() to initialize the response.
+ * -# Use the coap_opt_add_xxx() functions to include any Options, for example
+ *    coap_opt_add_format() for Content-Format of the payload.
+ * -# Call coap_opt_finish() to complete the PDU metadata. Retain the returned
+ *    metadata length.
  * -# Write the response payload, starting at the updated _payload_ pointer
- *    in the coap_pkt_t. If some error occurs, return a negative errno
- *    code from the handler, and gcoap will send a server error (5.00).
- * -# Call gcoap_finish() to complete the PDU after writing the payload,
- *    and return the result. gcoap will send the message.
+ *    in the coap_pkt_t, for up to _payload_len_ bytes.
+ * -# Return the sum of the metadata length and payload length. If some error
+ *    has occurred, return a negative errno code from the handler, and gcoap
+ *    will send a server error (5.00).
  *
- * If no payload, call only gcoap_response() to write the full response.
- * Alternatively, you still can use gcoap_resp_init() and gcoap_finish(), as
- * described above. In fact, the gcoap_response() function is inline, and uses
- * those two functions.
+ * If no payload, call only gcoap_response() to write the full response. If you
+ * need to add Options, follow the first three steps in the list above instead.
  *
  * ## Client Operation ##
  *
- * Client operation includes two phases:  creating and sending a request, and
- * handling the response aynchronously in a client supplied callback.  See
+ * Client operation includes two phases: creating and sending a request, and
+ * handling the response aynchronously in a client supplied callback. See
  * `examples/gcoap/gcoap_cli.c` for a simple example of sending a request and
  * reading the response.
  *
@@ -97,19 +97,21 @@
  * If there is a payload, follow the steps below.
  *
  * -# Call gcoap_req_init() to initialize the request.
- *    -# Optionally, mark the request confirmable by calling
- *       coap_hdr_set_type() with COAP_TYPE_CON.
+ * -# Optionally, mark the request confirmable by calling coap_hdr_set_type()
+ *    with COAP_TYPE_CON.
+ * -# Use the coap_opt_add_xxx() functions to include any Options beyond
+ *    Uri-Path, which was added in the first step.
+ * -# Call coap_opt_finish() to complete the PDU metadata. Retain the returned
+ *    metadata length.
  * -# Write the request payload, starting at the updated _payload_ pointer
- *    in the coap_pkt_t.
- * -# Call gcoap_finish(), which updates the packet for the payload.
+ *    in the coap_pkt_t, for up to _payload_len_ bytes.
  *
- * If no payload, call only gcoap_request() to write the full request.
- * Alternatively, you still can use gcoap_req_init() and gcoap_finish(),
- * as described above. The gcoap_request() function is inline, and uses those
- * two functions.
+ * If no payload, call only gcoap_request() to write the full request. If you
+ * need to add Options, follow the first four steps in the list above instead.
  *
- * Finally, call gcoap_req_send2() for the destination endpoint, as well as a
- * callback function for the host's response.
+ * Finally, call gcoap_req_send2() with the sum of the metadata length and
+ * payload length, the destination endpoint, and a callback function for the
+ * host's response.
  *
  * ### Handling the response ###
  *
@@ -147,11 +149,15 @@
  * -# Call gcoap_obs_init() to initialize the notification for a resource.
  *    Test the return value, which may indicate there is not an observer for
  *    the resource. If so, you are done.
+ * -# Use the coap_opt_add_xxx() functions to include any Options, for example
+ *    coap_opt_add_format() for Content-Format of the payload.
+ * -# Call coap_opt_finish() to complete the PDU metadata. Retain the returned
+ *    metadata length.
  * -# Write the notification payload, starting at the updated _payload_ pointer
- *    in the coap_pkt_t.
- * -# Call gcoap_finish(), which updates the packet for the payload.
+ *    in the coap_pkt_t, for up to _payload_len_ bytes.
  *
- * Finally, call gcoap_obs_send() for the resource.
+ * Finally, call gcoap_obs_send() for the resource, with the sum of the
+ * metadata length and payload length for the representation.
  *
  * ### Other considerations ###
  *
@@ -171,17 +177,6 @@
  *
  * ## Implementation Notes ##
  *
- * ### Building a packet ###
- *
- * The sequence and functions described above to build a request or response
- * is designed to provide a relatively simple API for the user.
- *
- * The structure of a CoAP PDU requires that options are placed between the
- * header and the payload. So, gcoap provides space in the buffer for them in
- * the request/response ...init() function, and then writes them during
- * gcoap_finish(). We trade some inefficiency/work in the buffer for
- * simplicity in the API.
- *
  * ### Waiting for a response ###
  *
  * We take advantage of RIOT's asynchronous messaging by using an xtimer to wait
@@ -582,7 +577,7 @@ static inline ssize_t gcoap_request(coap_pkt_t *pdu, uint8_t *buf, size_t len,
                                     unsigned code, char *path)
 {
     return (gcoap_req_init(pdu, buf, len, code, path) == 0)
-                ? gcoap_finish(pdu, 0, COAP_FORMAT_NONE)
+                ? coap_opt_finish(pdu, COAP_OPT_FINISH_NONE)
                 : -1;
 }
 
@@ -648,7 +643,7 @@ static inline ssize_t gcoap_response(coap_pkt_t *pdu, uint8_t *buf,
                                      size_t len, unsigned code)
 {
     return (gcoap_resp_init(pdu, buf, len, code) == 0)
-                ? gcoap_finish(pdu, 0, COAP_FORMAT_NONE)
+                ? coap_opt_finish(pdu, COAP_OPT_FINISH_NONE)
                 : -1;
 }
 

sys/include/net/nanocoap.h

@@ -639,6 +639,23 @@ ssize_t coap_opt_add_string(coap_pkt_t *pkt, uint16_t optnum, const char *string
  */
 ssize_t coap_opt_add_uint(coap_pkt_t *pkt, uint16_t optnum, uint32_t value);
 
+/**
+ * @brief   Append a Content-Format option to the pkt buffer
+ *
+ * @post pkt.payload advanced to first byte after option
+ * @post pkt.payload_len reduced by option length
+ *
+ * @param[in,out] pkt         pkt referencing target buffer
+ * @param[in]     format      COAP_FORMAT_xxx to use
+ *
+ * @return        number of bytes written to buffer
+ * @return        <0 reserved for error but not implemented yet
+ */
+static inline ssize_t coap_opt_add_format(coap_pkt_t *pkt, uint16_t format)
+{
+    return coap_opt_add_uint(pkt, COAP_OPT_CONTENT_FORMAT, format);
+}
+
 /**
  * @brief   Finalizes options as required and prepares for payload
  *

sys/net/application_layer/gcoap/gcoap.c

@@ -496,15 +496,18 @@ static void _expire_request(gcoap_request_memo_t *memo)
  * Handler for /.well-known/core. Lists registered handlers, except for
  * /.well-known/core itself.
  */
-static ssize_t _well_known_core_handler(coap_pkt_t* pdu, uint8_t *buf, size_t len, void *ctx)
+static ssize_t _well_known_core_handler(coap_pkt_t* pdu, uint8_t *buf, size_t len,
+                                        void *ctx)
 {
     (void)ctx;
-   /* write header */
+
     gcoap_resp_init(pdu, buf, len, COAP_CODE_CONTENT);
-    int plen = gcoap_get_resource_list(pdu->payload, (size_t)pdu->payload_len,
-                                      COAP_FORMAT_LINK);
-    /* response content */
-    return gcoap_finish(pdu, (size_t)plen, COAP_FORMAT_LINK);
+    coap_opt_add_format(pdu, COAP_FORMAT_LINK);
+    ssize_t plen = coap_opt_finish(pdu, COAP_OPT_FINISH_PAYLOAD);
+
+    plen += gcoap_get_resource_list(pdu->payload, (size_t)pdu->payload_len,
+                                    COAP_FORMAT_LINK);
+    return plen;
 }
 
 /*

tests/unittests/tests-gcoap/tests-gcoap.c

@@ -140,10 +140,11 @@ static void test_gcoap__client_put_req(void)
     char payload[] = "1";
 
     gcoap_req_init(&pdu, buf, GCOAP_PDU_BUF_SIZE, COAP_METHOD_PUT, path);
+    coap_opt_add_format(&pdu, COAP_FORMAT_TEXT);
+    len = coap_opt_finish(&pdu, COAP_OPT_FINISH_PAYLOAD);
     memcpy(pdu.payload, payload, 1);
-    len = gcoap_finish(&pdu, 1, COAP_FORMAT_TEXT);
 
-    coap_parse(&pdu, buf, len);
+    coap_parse(&pdu, buf, len + 1);
 
     TEST_ASSERT_EQUAL_INT(COAP_METHOD_PUT, coap_get_code(&pdu));
     TEST_ASSERT_EQUAL_INT(1, pdu.payload_len);
@@ -172,7 +173,7 @@ static void test_gcoap__client_get_query(void)
     optlen = gcoap_add_qstring(&pdu, key2, NULL);
     TEST_ASSERT_EQUAL_INT(2, optlen);
 
-    size_t len = gcoap_finish(&pdu, 0, COAP_FORMAT_NONE);
+    size_t len = coap_opt_finish(&pdu, COAP_OPT_FINISH_NONE);
 
     coap_parse(&pdu, buf, len);
 
@@ -198,7 +199,7 @@ static void test_gcoap__client_get_path_defer(void)
     coap_opt_add_string(&pdu, COAP_OPT_URI_PATH, path, '/');
     optlen = 6;
 
-    len = gcoap_finish(&pdu, 0, COAP_FORMAT_NONE);
+    len = coap_opt_finish(&pdu, COAP_OPT_FINISH_NONE);
     TEST_ASSERT_EQUAL_INT(len, sizeof(coap_hdr_t) + GCOAP_TOKENLEN + optlen);
 
     coap_parse(&pdu, buf, len);
@@ -262,9 +263,11 @@ static void test_gcoap__server_get_resp(void)
 
     /* generate response */
     gcoap_resp_init(&pdu, &buf[0], sizeof(buf), COAP_CODE_CONTENT);
+    coap_opt_add_format(&pdu, COAP_FORMAT_TEXT);
+    ssize_t res = coap_opt_finish(&pdu, COAP_OPT_FINISH_PAYLOAD);
+
     char resp_payload[]  = "2";
     memcpy(&pdu.payload[0], &resp_payload[0], strlen(resp_payload));
-    ssize_t res = gcoap_finish(&pdu, strlen(resp_payload), COAP_FORMAT_TEXT);
 
     uint8_t resp_data[] = {
         0x52, 0x45, 0x20, 0xb6, 0x35, 0x61, 0xc0, 0xff,
@@ -275,8 +278,7 @@ static void test_gcoap__server_get_resp(void)
     TEST_ASSERT_EQUAL_INT(2, coap_get_token_len(&pdu));
     TEST_ASSERT_EQUAL_INT(4 + 2, coap_get_total_hdr_len(&pdu));
     TEST_ASSERT_EQUAL_INT(COAP_TYPE_NON, coap_get_type(&pdu));
-    TEST_ASSERT_EQUAL_INT(strlen(resp_payload), pdu.payload_len);
-    TEST_ASSERT_EQUAL_INT(sizeof(resp_data), res);
+    TEST_ASSERT_EQUAL_INT(sizeof(resp_data), res + 1);
 
     for (size_t i = 0; i < strlen(resp_payload); i++) {
         TEST_ASSERT_EQUAL_INT(resp_payload[i], pdu.payload[i]);
@@ -326,9 +328,11 @@ static void test_gcoap__server_con_resp(void)
 
     /* generate response */
     gcoap_resp_init(&pdu, &buf[0], sizeof(buf), COAP_CODE_CONTENT);
+    coap_opt_add_format(&pdu, COAP_FORMAT_TEXT);
+    ssize_t res = coap_opt_finish(&pdu, COAP_OPT_FINISH_PAYLOAD);
+
     char resp_payload[]  = "2";
     memcpy(&pdu.payload[0], &resp_payload[0], strlen(resp_payload));
-    ssize_t res = gcoap_finish(&pdu, strlen(resp_payload), COAP_FORMAT_TEXT);
 
     uint8_t resp_data[] = {
         0x62, 0x45, 0x8e, 0x03, 0x35, 0x61, 0xc0, 0xff,
@@ -337,7 +341,7 @@ static void test_gcoap__server_con_resp(void)
 
     TEST_ASSERT_EQUAL_INT(COAP_CLASS_SUCCESS, coap_get_code_class(&pdu));
     TEST_ASSERT_EQUAL_INT(COAP_TYPE_ACK, coap_get_type(&pdu));
-    TEST_ASSERT_EQUAL_INT(sizeof(resp_data), res);
+    TEST_ASSERT_EQUAL_INT(sizeof(resp_data), res + 1);
 }
 
 /*