Skip to content

nanocoap_sock: add nanocoap_sock_url_connect()#17960

Merged
fjmolinas merged 3 commits intoRIOT-OS:masterfrom
benpicco:sock_urlpath
Apr 22, 2022
Merged

nanocoap_sock: add nanocoap_sock_url_connect()#17960
fjmolinas merged 3 commits intoRIOT-OS:masterfrom
benpicco:sock_urlpath

Conversation

@benpicco
Copy link
Copy Markdown
Contributor

@benpicco benpicco commented Apr 17, 2022 •
edited
Loading

Contribution description

This introduces a sock_urlpath() function that just returns a pointer to the path component of a URL without having to copy it.

This moves the URL parsing code out of nanocoap_get_blockwise_url() into nanocoap_sock_url_connect() and save some stack by not having to allocate a CONFIG_SOCK_URLPATH_MAXLEN sized buffer there.

Testing procedure

The url command in tests/nanocoap_cli still works

main(): This is RIOT! (Version: 2022.07-devel-91-g135d3-sock_urlpath)
nanocoap test app
All up, running the shell now
> url get coap://[fe80::90a7:a6ff:fe4b:2e32%5]/.well-known/core
url get coap://[fe80::90a7:a6ff:fe4b:2e32%5]/.well-known/core
offset 000: </>;ct=40;rt="tag:chrysn@fsfe.or
offset 032: g,2022:fileserver"

Issues/PRs references

useful for #17962, #17937

@benpicco benpicco requested a review from chrysn April 17, 2022 23:47
@github-actions github-actions bot added Area: CoAP Area: Constrained Application Protocol implementations Area: network Area: Networking Area: sys Area: System Area: tests Area: tests and testing framework labels Apr 17, 2022
@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 Apr 17, 2022
@benpicco benpicco mentioned this pull request Apr 18, 2022
Merged
@benpicco benpicco requested a review from fjmolinas April 21, 2022 14:48
maribu
maribu approved these changes Apr 22, 2022
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.

ACK. Code changes are unscary and mostly reorganising already tested code. Since there are unit tests that Murdock will execute (at least on native) for the newly exposed features, IMO no manual testing is required.

@fjmolinas fjmolinas merged commit 7307923 into RIOT-OS:master Apr 22, 2022
@benpicco benpicco deleted the sock_urlpath branch April 22, 2022 06:46
@chrysn chrysn added this to the Release 2022.07 milestone Aug 25, 2022
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

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

@haukepetersen haukepetersen Awaiting requested review from haukepetersen

@PeterKietzmann PeterKietzmann Awaiting requested review from PeterKietzmann

@cgundogan cgundogan Awaiting requested review from cgundogan

@chrysn chrysn Awaiting requested review from chrysn

@fjmolinas fjmolinas Awaiting requested review from fjmolinas

Assignees

No one assigned

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

Milestone

Release 2022.07

Development

Successfully merging this pull request may close these issues.

4 participants

@benpicco @maribu @chrysn @fjmolinas