Skip to content

nanocoap_sock: implement observe (Client-Side) #21160

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
Merged
merged 3 commits into from
Apr 4, 2025
Merged

nanocoap_sock: implement observe (Client-Side) #21160

merged 3 commits into from
Apr 4, 2025

Conversation

benpicco
Copy link
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
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 maribu and chrysn January 28, 2025 13:05
@riot-ci
Copy link

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
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
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
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 PeterKietzmann is a code owner

@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