Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

sys/net/nanocoap: Implement Observe (Server-Side) #21147

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

sys/net/nanocoap: Implement Observe (Server-Side) #21147

wants to merge 2 commits into from
+523 −38

Conversation

maribu
Copy link
Member

@maribu maribu commented Jan 20, 2025

Contribution description

  • Clean up and extend the separate response feature of nanocoap
  • Add a bit of glue to use the separate response feature to implement observe

Testing procedure

Starting the Server

$ sudo ip tuntap add tap0 mode tap user $(whoami)
$ sudo ip link set tap0 up
$ make BOARD=native64 -C examples/nanocoap_server -j flash term

Running a Client

$ coap-client-notls -s 60 -m get 'coap://[fe80::9826:30ff:feb8:31f4%tap0]/time'
8326
9000
10001
11000
12001
13000
14000
15001
16000
17001
18000
19000
20001
21000
22000
[...]

Issues/PRs references

None

@maribu maribu added Type: new feature The issue requests / The PR implemements a new feature for RIOT CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Jan 20, 2025
@github-actions github-actions bot added Area: network Area: Networking Area: CoAP Area: Constrained Application Protocol implementations Area: sys Area: System Area: examples Area: Example Applications labels Jan 20, 2025
@riot-ci
Copy link

riot-ci commented Jan 21, 2025 •
edited
Loading

Murdock results

❌ FAILED

a496a1f fixup! sys/net/nanocoap: implement observe

Success Failures Total Runtime
7934 0 10029 05m:29s

Artifacts

@maribu maribu force-pushed the sys/net/nanocoap/observe branch 2 times, most recently from f0a998f to 124752f Compare January 21, 2025 08:06
@maribu maribu marked this pull request as ready for review January 21, 2025 08:06
@maribu maribu force-pushed the sys/net/nanocoap/observe branch 2 times, most recently from a3e9b1c to c4de045 Compare January 21, 2025 08:41
@MrKevinWeiss MrKevinWeiss added the Process: blocked by feature freeze Integration Process: The impact of this PR is too high for merging it during soft feature freeze. label Jan 21, 2025
mguetschow
mguetschow reviewed Jan 21, 2025
View reviewed changes
Copy link
Contributor

@mguetschow mguetschow left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just had a high-level look at it and have some nit's below. In general I don't feel confident enough in nanocoap_sock to tell whether this takes all nanocoap quirks into account, but the implementation looks good to me in general.

examples/nanocoap_server/coap_handler.c Outdated Show resolved Hide resolved
examples/nanocoap_server/coap_handler.c Show resolved Hide resolved
sys/include/net/nanocoap_sock.h Outdated Show resolved Hide resolved
sys/include/net/nanocoap_sock.h Show resolved Hide resolved
sys/include/net/nanocoap_sock.h Outdated Show resolved Hide resolved
sys/include/net/nanocoap_sock.h Outdated Show resolved Hide resolved
sys/include/net/nanocoap_sock.h Outdated Show resolved Hide resolved
sys/net/application_layer/nanocoap/nanocoap.c Outdated Show resolved Hide resolved
sys/net/application_layer/nanocoap/sock.c Show resolved Hide resolved
sys/net/application_layer/nanocoap/sock.c Show resolved Hide resolved
@MrKevinWeiss MrKevinWeiss removed the Process: blocked by feature freeze Integration Process: The impact of this PR is too high for merging it during soft feature freeze. label Jan 21, 2025
@MrKevinWeiss
Copy link
Contributor

Freeze is over so if anyone wants to pick this up...

@maribu maribu force-pushed the sys/net/nanocoap/observe branch from c4de045 to 26f4f06 Compare January 21, 2025 15:38
benpicco
benpicco reviewed Jan 22, 2025
View reviewed changes
examples/nanocoap_server/coap_handler.c Show resolved Hide resolved
examples/nanocoap_server/coap_handler.c Show resolved Hide resolved
examples/nanocoap_server/coap_handler.c Show resolved Hide resolved
examples/nanocoap_server/coap_handler.c Outdated Show resolved Hide resolved
examples/nanocoap_server/coap_handler.c Show resolved Hide resolved
sys/include/net/nanocoap_sock.h Outdated Show resolved Hide resolved
sys/include/net/nanocoap_sock.h Outdated Show resolved Hide resolved
sys/net/application_layer/nanocoap/sock.c Show resolved Hide resolved
sys/net/application_layer/nanocoap/sock.c Outdated Show resolved Hide resolved
sys/net/application_layer/nanocoap/sock.c Outdated Show resolved Hide resolved
@maribu
Copy link
Member Author

maribu commented Jan 22, 2025

Time for squash?

squash player dancing with racket in his hand

@benpicco
Copy link
Contributor

Please squash!

maribu and others added 2 commits January 23, 2025 14:25
@maribu
sys/net/nanocoap: improve separate response handling
1c7ba9e 
This allows sending a separate response with CoAP Options and adds a
helper to detect duplicate requests, so that resource handlers can
repeat their empty ACK on duplicates.
@maribu @mguetschow @benpicco
sys/net/nanocoap: implement observe
feeb684 
This adds the new `nanocoap_server_observe` module that implements the
server side of the CoAP Observe option. It does require cooperation
from the resource handler to work, though.

Co-Authored-By: mguetschow <[email protected]>
Co-authored-by: benpicco <[email protected]>
@maribu maribu force-pushed the sys/net/nanocoap/observe branch from 88ab5fd to feeb684 Compare January 23, 2025 13:27
@maribu maribu requested a review from mguetschow January 23, 2025 13:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@benpicco benpicco benpicco approved these changes

@miri64 miri64 Awaiting requested review from miri64 miri64 is a code owner

@PeterKietzmann PeterKietzmann Awaiting requested review from PeterKietzmann PeterKietzmann is a code owner

@chrysn chrysn Awaiting requested review from chrysn

@mguetschow mguetschow Awaiting requested review from mguetschow

Assignees
No one assigned
Labels
Area: CoAP Area: Constrained Application Protocol implementations Area: examples Area: Example Applications Area: network Area: Networking Area: sys Area: System CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Type: new feature The issue requests / The PR implemements a new feature for RIOT
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@maribu @riot-ci @MrKevinWeiss @benpicco @miri64 @mguetschow