[riot-notifications] [RIOT-OS/RIOT] net/nanocoap: fix confirmable retry countdown (#10671)

Sebastian Meiling notifications at github.com
Mon Jan 7 10:34:54 CET 2019


smlng commented on this pull request.



> @@ -46,19 +46,14 @@ ssize_t nanocoap_request(coap_pkt_t *pkt, sock_udp_ep_t *local, sock_udp_ep_t *r
 
     /* TODO: timeout random between between ACK_TIMEOUT and (ACK_TIMEOUT *
      * ACK_RANDOM_FACTOR) */
-    uint32_t timeout = COAP_ACK_TIMEOUT * (1000000U);
-    int tries = 0;
-    while (tries++ < COAP_MAX_RETRANSMIT) {
-        if (!tries) {
-            DEBUG("nanocoap: maximum retries reached.\n");
-            res = -ETIMEDOUT;
-            goto out;
-        }
+    uint32_t timeout = COAP_ACK_TIMEOUT * (1 * US_PER_SEC);

I guess you can skip the `1 *` and simplify to `COAP_ACK_TIMEOUT * US_PER_SEC`, or not?

> @@ -46,19 +46,14 @@ ssize_t nanocoap_request(coap_pkt_t *pkt, sock_udp_ep_t *local, sock_udp_ep_t *r
 
     /* TODO: timeout random between between ACK_TIMEOUT and (ACK_TIMEOUT *
      * ACK_RANDOM_FACTOR) */
-    uint32_t timeout = COAP_ACK_TIMEOUT * (1000000U);
-    int tries = 0;
-    while (tries++ < COAP_MAX_RETRANSMIT) {
-        if (!tries) {
-            DEBUG("nanocoap: maximum retries reached.\n");
-            res = -ETIMEDOUT;
-            goto out;
-        }
+    uint32_t timeout = COAP_ACK_TIMEOUT * (1 * US_PER_SEC);
+    unsigned tries_left = COAP_MAX_RETRANSMIT + 1;  /* add 1 for initial transmit */
+    while (tries_left) {

I guess the *correct* (more C'ish) (?) way would be to use `do { ... } while ( ... );`, like:

```
unsigned tries_left = COAP_MAX_RETRANSMIT;
do {
[...]
} while (tries_left--);
```

This should also work for `COAP_MAX_RETRANSMIT = 0`, that is: send once and no retries.
though I also guess this will not impact code size, as the compiler will optimise both the same way.

-- 
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/10671#pullrequestreview-189728374
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.riot-os.org/pipermail/notifications/attachments/20190107/95a69e67/attachment.html>


More information about the notifications mailing list