nanocoap sock: stop retransmissions after empty ACK

[fabian18]

Contribution description

The nanocoap_sock_request_cb() function does a synchronous CoAP request.
This PR fixes that further retransmissions are done, even though a matching empty ACK was received.
The behavior is mandatory for CoAP and should also be done when no response callback is set, because the ACK is not the actual response.

Testing procedure

Run examples/nanocoap_server and tests/net/nanocoap_cli and increase the timeout for the /separate resource.
Without this fix, the client keeps retransmitting even after the empty ACK.

Test when I set the server separate response delay to 10 seconds.

client get fe80::4d3:11ff:fe35:6a43 5683 /well-known/core
client get fe80::4d3:11ff:fe35:6a43 5683 /well-known/core
nanocli: sending msg ID 1, 22 bytes
nanocli: msg send failed: -6
> client get fe80::4d3:11ff:fe35:6a43 5683 /.well-known/core
client get fe80::4d3:11ff:fe35:6a43 5683 /.well-known/core
nanocli: sending msg ID 1, 23 bytes
nanocli: response Success, code 2.05, 16 bytes
</vfs>,</separat
> client get fe80::4d3:11ff:fe35:6a43 5683 /separate
client get fe80::4d3:11ff:fe35:6a43 5683 /separate
nanocli: sending msg ID 1, 15 bytes
nanocli: response Success, code 2.05, 28 bytes
00000000  54  68  69  73  20  69  73  20  61  20  64  65  6C  61  79  65
00000010  64  20  72  65  73  70  6F  6E  73  65  2E  00

bug: Look at </vfs>,</separat no bug just blockwise transfer

When I disable the ACK on the server side I can see retransmissions in wireshark:

But another bug: The serveice unavailable ACK is treated as the response. :(

Issues/PRs references

first split out of #20696

[fabian18]

need to think if the state handling is done correct here.state = STATE_RESPONSE_RCVD; for everything that is received

[fabian18]

But another bug: The serveice unavailable ACK is treated as the response.

If the separate response is pending for a different request.

RIOT/examples/nanocoap_server/coap_handler.c

Lines 207 to 210 in bf3392b

	if (event_timeout_is_pending(&event_timeout)) {
	puts("_separate_handler(): response already scheduled");
	return coap_build_reply(pkt, COAP_CODE_SERVICE_UNAVAILABLE, buf, len, 0);
	}

Can I fix it right here in this PR?

[benpicco]

Can I fix it right here in this PR?

Sure!

[riot-ci]

Murdock results

✔️ PASSED

9dfdf2b examples/nanocoap_server: 5.03 only for different requests to /separate

Success	Failures	Total	Runtime
10143	0	10143	12m:36s

Artifacts

• Documentation preview

[fabian18]

Given that the request contains a no response option to suppresses certain response classes, this must be communicated to the function somehow. Even if there is no callback, maybe the program must wait for a response.

I think before the assumption was, if the no-response option is included, the caller does not provide a callback.

[fabian18]

When you send a request in which you suppress 2.xx replies you still have to wait to maybe get a 4.xx or 5.xx reply.
No response could be handled like: When the request with a no-response option times out, you expect it timed out because the response got suppressed. But this should be known to the caller of nanocoap_sock_request_cb.

[benpicco]

So you suggest we should differentiate between

• ACK received but no separate response (because it was suppressed or lost)
• no ACK received

But I think this is a separate issue.

[fabian18]

Thanks for approval.

Luckily we only add a no-response option for NON without buffer to write the response to. This case is still catched.

- ACK received but no separate response (because it was suppressed or lost)
- no ACK received

That I did not event think of. But yes, if there is a timeout it could be due to no ACK or no response, which means different things.

But I think this is a separate issue.

Yes