net/gcoap: Create confirm request infrastructure and adapt existing messaging#7336
Conversation
|
Just been playing a little with this PR and I think I found an issue: I am sending a As I understood this PR, it should actually handle empty messages (or at least empty ACK messages by now), right? |
|
An empty (code 0.00) ACK indicates the server intends to send the separate response later. See Sec. 2.2 of the spec. This scenario is not included in this PR because it would require the client to retain the request memo after the initial ACK response. Handling for a separate response is the sixth item in #7192. |
|
Yes, makes sense. Seems like I got something wrong when debugging, now I can't even reproduce the behavior I've seen on Monday... So never mind I guess. |
d9ed185 to
47569a3
Compare
|
Rebased. |
|
Sorry for not giving this a proper review yet, hope to get to it soon. |
|
Thanks, @haukepetersen. This PR is a stepping stone to the main goal of merging #7337. I plan to rebase that one in the next day or so. |
|
Apologies, all. I dropped a commit in the rebase. Must redo it. |
|
@kb2ma you can use the |
Using a temporary branch is usually much faster. |
Why? As long as you don't |
| } | ||
|
|
||
| if (coap_get_token_len(memo_pdu) == cmplen) { | ||
| if (cmplen) { |
There was a problem hiding this comment.
this can be optimised, i.e., by merging the 2 if to if (cmplen && (coap_get_token_len(memo_pdu) == cmplen)) {. However, I would rather put this into a helper function _cmp_hdr_token(token A, token B) which returns -1, 0, or 1 like typical compare functions. That would make this a one line and this whole for-loop a lot more readable (IMHO).
There was a problem hiding this comment.
I just looked deeper into memcmp(), and I was wrong about the need to explicitly test when the comparison length is zero, as explained in this SO question.
So, the token comparison can be simplified to:
if (coap_get_token_len(memo_pdu) == cmplen) {
memo_pdu->token = &memo_pdu->hdr->data[0];
if (memcmp(src_pdu->token, memo_pdu->token, cmplen) == 0) {
*memo_ptr = memo;
break;
}
}
Good enough?
smlng
left a comment
There was a problem hiding this comment.
tested NONCON and CON message do work.
|
please squash |
Add support for a confirmable request and standardize token match.
1986248 to
37f272b
Compare
|
Squashed and rebased. |
|
@haukepetersen can you spare some time and have a look too, I think we should move this and related PRs by @kb2ma forward, otherwise I'm also fine to push this on my own. |
This PR implements the second step in the confirmable messaging plan in #7192. It extends the request memo struct to support sending, and resending, a message of type CON. In addition, the existing message sending code has been adapted to use the changed struct.
This PR allows sending a request of CON type, but the message will not be resent if the response times out. The gcoap example also has been updated to specify use of the CON type, by addition of a
-coption. Notice the call tocoap_hdr_set_type()after initializing the PDU ingcoap_cli_cmd().The resend implementation and documentation on sending a confirmable request are the subjects of the next PR in the series. I have started to implement it as a WIP (#7337) to verify that the present PR provides what's required.