Skip to content

nanocoap_sock: implement observe (Client-Side)#21160

Merged
benpicco merged 3 commits intoRIOT-OS:masterfrom
benpicco:sys/net/nanocoap/observe-client
Apr 4, 2025
Merged

nanocoap_sock: implement observe (Client-Side)#21160
benpicco merged 3 commits intoRIOT-OS:masterfrom
benpicco:sys/net/nanocoap/observe-client

Conversation

@benpicco
Copy link
Copy Markdown
Contributor

@benpicco benpicco commented Jan 23, 2025 •
edited
Loading

Contribution description

Testing procedure

A new observe command was added to tests/net/nanocoap_cli:

> observe coap://[fe80::d07c:7cff:fe6d:9441]/time
2025-01-23 16:25:24,793 # 00000000  32  37  36  32  30  0A
2025-01-23 16:25:25,175 # 00000000  32  38  30  30  32  0A
2025-01-23 16:25:26,174 # 00000000  32  39  30  30  31  0A
2025-01-23 16:25:27,175 # 00000000  33  30  30  30  31  0A
2025-01-23 16:25:28,175 # 00000000  33  31  30  30  32  0A
2025-01-23 16:25:29,173 # 00000000  33  32  30  30  31  0A
2025-01-23 16:25:30,173 # 00000000  33  33  30  30  31  0A
2025-01-23 16:25:31,175 # 00000000  33  34  30  30  31  0A
2025-01-23 16:25:32,175 # 00000000  33  35  30  30  32  0A
2025-01-23 16:25:33,175 # 00000000  33  36  30  30  32  0A
2025-01-23 16:25:34,173 # 00000000  33  37  30  30  31  0A
2025-01-23 16:25:35,173 # 00000000  33  38  30  30  31  0A

> observe coap://[fe80::d07c:7cff:fe6d:9441]/time off
2025-01-23 16:25:36,148 # gnrc_sock: timeout != 0 within the asynchronous callback lead to unexpected delays within the asynchronous handler.
2025-01-23 16:25:36,148 # 00000000  33  38  39  37  36  0A
>

Issues/PRs references

based on #21147

@github-actions github-actions bot added Area: network Area: Networking Area: tests Area: tests and testing framework Area: CoAP Area: Constrained Application Protocol implementations Area: sys Area: System Area: examples Area: Example Applications labels Jan 23, 2025
@benpicco benpicco requested a review from maribu January 23, 2025 15:28
maribu
maribu reviewed Jan 23, 2025
View reviewed changes
Copy link
Copy Markdown
Member

@maribu maribu left a comment

Choose a reason for hiding this comment

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

Looks quite good to me :)

I have been thinking for ages whether we should go for async sockets for nanocoap (both client and server), and provide the synchronous sock API on top of that.

IMO the synchronous interface is optimized for the rather synthetic "shell app use case". Any "headless" MCU would more likely want to use an async API and the ability to run server and client on the same endpoint.

But that is orthogonal to this PR.

@benpicco benpicco force-pushed the sys/net/nanocoap/observe-client branch 2 times, most recently from 8d0a4e9 to dcded9d Compare January 23, 2025 18:29
maribu
maribu reviewed Jan 23, 2025
View reviewed changes
@benpicco benpicco force-pushed the sys/net/nanocoap/observe-client branch from 5a58280 to 73f41af Compare January 28, 2025 13:02
@benpicco benpicco marked this pull request as ready for review January 28, 2025 13:05
@benpicco benpicco added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Jan 28, 2025
@benpicco benpicco requested review from chrysn and maribu January 28, 2025 13:05
@riot-ci
Copy link
Copy Markdown

riot-ci commented Jan 28, 2025 •
edited
Loading

Murdock results

✔️ PASSED

bf012f5 examples/nanocoap_server: add output if registration fails

Success Failures Total Runtime
150404 0 150404 01h:43m:14s

Artifacts

maribu
maribu approved these changes Jan 28, 2025
View reviewed changes
Copy link
Copy Markdown
Member

@maribu maribu left a comment

Choose a reason for hiding this comment

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

lgtm, some more comments inline and the CI also has comments.

@benpicco benpicco force-pushed the sys/net/nanocoap/observe-client branch from 73f41af to 99a66d5 Compare January 28, 2025 15:03
@maribu maribu enabled auto-merge January 28, 2025 15:05
@benpicco benpicco force-pushed the sys/net/nanocoap/observe-client branch from 99a66d5 to 890df8b Compare April 3, 2025 11:23
@maribu maribu added this pull request to the merge queue Apr 3, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Apr 3, 2025
@benpicco benpicco force-pushed the sys/net/nanocoap/observe-client branch from 890df8b to d1be87a Compare April 3, 2025 15:22
@benpicco benpicco added this pull request to the merge queue Apr 3, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Apr 3, 2025
@benpicco benpicco force-pushed the sys/net/nanocoap/observe-client branch from d1be87a to f2b9540 Compare April 4, 2025 07:45
@benpicco benpicco enabled auto-merge April 4, 2025 07:46
@benpicco benpicco added this pull request to the merge queue Apr 4, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Apr 4, 2025
@mguetschow mguetschow added CI: full build disable CI build filter CI: no fast fail don't abort PR build after first error labels Apr 4, 2025
@mguetschow
Copy link
Copy Markdown
Contributor

mguetschow commented Apr 4, 2025 •
edited
Loading

Let's just use CI once to catch all too little boards (please force-push once more). Rather, let's just restart on the current state.

@mguetschow mguetschow added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR and removed CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Apr 4, 2025
@benpicco benpicco enabled auto-merge April 4, 2025 12:06
@benpicco benpicco force-pushed the sys/net/nanocoap/observe-client branch from f2b9540 to bf012f5 Compare April 4, 2025 13:30
@benpicco benpicco added this pull request to the merge queue Apr 4, 2025
@maribu
Copy link
Copy Markdown
Member

maribu commented Apr 4, 2025

🤞

Merged via the queue into RIOT-OS:master with commit 3621be5 Apr 4, 2025
25 checks passed
@benpicco benpicco deleted the sys/net/nanocoap/observe-client branch April 4, 2025 22:17
@mguetschow mguetschow added this to the Release 2025.04 milestone Apr 8, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@maribu maribu maribu approved these changes

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

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

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

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

@PeterKietzmann PeterKietzmann Awaiting requested review from PeterKietzmann

@chrysn chrysn Awaiting requested review from chrysn

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 Area: tests Area: tests and testing framework CI: full build disable CI build filter CI: no fast fail don't abort PR build after first error CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR

Projects

None yet

Milestone

Release 2025.04

Development

Successfully merging this pull request may close these issues.

4 participants

@benpicco @riot-ci @mguetschow @maribu