zephyr: coap: add cancel observations #468
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Visit the preview URL for this PR (updated for commit 724cfae): https://golioth-firmware-sdk-doxygen-dev--pr468-szczys-cancel-87so7hvd.web.app (expires Mon, 27 May 2024 19:07:21 GMT) 🔥 via Firebase Hosting GitHub Action 🌎 Sign: a9993e61697a3983f3479e468bcb0b616f9a0578 |
szczys
force-pushed
the
szczys/cancel_observations
branch
from
93ccc11
to
5ad8222
Compare
Code Coverage
|
szczys
force-pushed
the
szczys/cancel_observations
branch
from
5ad8222
to
db9d4c4
Compare
szczys
force-pushed
the
szczys/cancel_observations
branch
from
db9d4c4
to
91ec330
Compare
szczys
force-pushed
the
szczys/cancel_observations
branch
3 times, most recently
from
f938d13
to
cb62433
Compare
szczys
force-pushed
the
szczys/cancel_observations
branch
from
cb62433
to
161664d
Compare
szczys
force-pushed
the
szczys/fix_add_observation
branch
from
6ad8ec9
to
4b4040e
Compare
szczys
force-pushed
the
szczys/cancel_observations
branch
from
161664d
to
26ecaa6
Compare
sam-golioth
reviewed
May 8, 2024
There was a problem hiding this comment.
Great work cutting through the cruft here @szczys!
The way zephyr_coap_req is interacting with coap_client_zephyr feels a little weird - can we see if there's a way to simplify that?
mniestroj
reviewed
May 9, 2024
szczys
force-pushed
the
szczys/cancel_observations
branch
from
26ecaa6
to
5752223
Compare
szczys
force-pushed
the
szczys/cancel_observations
branch
from
5752223
to
19334fc
Compare
sam-golioth
requested changes
May 20, 2024
Check that path length will fit into the path member of a golioth_coap_request_msg_t before trying to copy the string. Signed-off-by: Mike Szczys <[email protected]>
The golioth_coap_client_observe_async() command is renamed to golioth_coap_client_observe(). This doesn't change functionality, but merely makes the function name shorted as async is assumed for all observations. Signed-off-by: Mike Szczys <[email protected]>
Introduce function that adds a coap "eager release" for telling the server we want to cancel an observation. Signed-off-by: Mike Szczys <[email protected]>
Notify the server that we are cancelling an observation. Signed-off-by: Mike Szczys <[email protected]>
Observe requests are stored in the client coap_reqs linked list. Walk the coap_reqs list, comparing the user_data pointer to each client observe slot address. Observations are cancelled by setting the client observations slot as !in_use, removing the request from the coap_reqs list, then sending an "eager release" request to the server using the original token, path, etc. Finally, the request memory is freed. Signed-off-by: Mike Szczys <[email protected]>
Handle requests of the GOLIOTH_COAP_REQUEST_OBSERVE_CANCEL type by sending an "eager release" to the server indicating an observation is being cancelled. Signed-off-by: Mike Szczys <[email protected]>
Observation updates are received from the server as unsolicited requests. The token will always match the token used when the observation was registered. The client observations list stores a copy of the original request message that contains the token to compare against, as well as the callback to notify. Observations are cancelled by settings the client observations slot as !in_use, then sending an "eager release" request to the server using the original token, path, etc. Signed-off-by: Mike Szczys <[email protected]>
Add wrapper function around the port-specific cancel functions. Signed-off-by: Mike Szczys <[email protected]>
This tests that observations can be cancelled and reestablished. This is currently an internal API which will be extended to service APIs in future work. Signed-off-by: Mike Szczys <[email protected]>
Correct the name of the board used in the build step for running tests locally. Signed-off-by: Mike Szczys <[email protected]>
szczys
force-pushed
the
szczys/cancel_observations
branch
from
19334fc
to
724cfae
Compare
sam-golioth
approved these changes
May 20, 2024
There was a problem hiding this comment.
Great work on this one @szczys! Quite the beast to wrangle.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Add ability to remove observations from the client coap_reqs lists, and mark the observation slots as not in_use. The cancellation process sends an "eager release" coap packet to the server indicating the observation has been cancelled.
This searches the coap_reqs linked list by matching the request message pointer which is why the
coap_client.hheader was included. I think a future improvement to how observations are handled should be storing crucial observation values instead of the the request message. If we store the coap token as one of those values, it can be used to search in the coap_reqs.