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.

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

nanocoap_sock: add nanocoap_sock_put() #18514

Merged
merged 2 commits into from
Aug 30, 2022

Conversation

benpicco
Copy link
Contributor

@benpicco benpicco commented Aug 25, 2022

Contribution description

Same as nanocoap_sock_get(), but for PUT and POST.

Testing procedure

Run two instances of tests/nanocoap_cli, on one do a server start and on the other

> url get coap://[fe80::581a:98ff:fe23:2d6c]/value
offset 000: 0
> url put coap://[fe80::581a:98ff:fe23:2d6c]/value 23
> url get coap://[fe80::581a:98ff:fe23:2d6c]/value
offset 000: 23

Issues/PRs references

@github-actions github-actions bot added Area: CoAP Area: Constrained Application Protocol implementations Area: network Area: Networking Area: sys Area: System labels Aug 25, 2022
@benpicco benpicco added State: WIP State: The PR is still work-in-progress and its code is not in its final presentable form yet and removed State: WIP State: The PR is still work-in-progress and its code is not in its final presentable form yet labels Aug 25, 2022
@benpicco benpicco added the State: WIP State: The PR is still work-in-progress and its code is not in its final presentable form yet label Aug 25, 2022
@github-actions github-actions bot added the Area: tests Area: tests and testing framework label Aug 25, 2022
@benpicco benpicco removed the State: WIP State: The PR is still work-in-progress and its code is not in its final presentable form yet label Aug 25, 2022
@benpicco benpicco requested review from chrysn and maribu August 25, 2022 15:31
@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 Aug 25, 2022
@benpicco benpicco requested a review from fabian18 August 29, 2022 16:21
Copy link
Contributor

@fabian18 fabian18 left a comment

Choose a reason for hiding this comment

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

Looks solid and works, analog to nanocoap_sock_get():

2022-08-30 11:31:13,662 # server start
2022-08-30 11:31:13,665 # starting server on port 5683
2022-08-30 11:55:35,297 # url put coap://[fe80::4c49:6f54:4f76:0%6]/value 22

2022-08-30 11:59:46,987 # url get coap://[fe80::4c49:6f54:4f76:0%6]/value
2022-08-30 11:59:47,015 # offset 000: 22

The next steps would be nanocoap_{put | post}_blockwise_url ?

@benpicco
Copy link
Contributor Author

The next steps would be nanocoap_{put | post}_blockwise_url ?

We've got nanocoap_block_request_init_url() and nanocoap_sock_block_request() for that

@fabian18
Copy link
Contributor

But blockwise PUT / POST is not intended for the simple /value resource, so it is reasonable to use nanocoap_sock_post() / _put(), over nanocoap_block_request_init_url() and nanocoap_sock_block_request()?

@benpicco
Copy link
Contributor Author

It's a more convenient API for simple PUT/POST when no block-wise is needed.

Copy link
Contributor

@fabian18 fabian18 left a comment

Choose a reason for hiding this comment

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

It's a more convenient API for simple PUT/POST when no block-wise is needed.

Ok, ACK

@benpicco
Copy link
Contributor Author

Thank you for the quick review!

@benpicco benpicco merged commit 346c733 into RIOT-OS:master Aug 30, 2022
@benpicco benpicco deleted the nanocoap_sock_put branch August 30, 2022 12:00
@maribu maribu added this to the Release 2022.10 milestone Oct 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: CoAP Area: Constrained Application Protocol implementations Area: network Area: Networking Area: sys Area: System Area: tests Area: tests and testing framework CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants