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_url_connect() #17960

Merged
merged 3 commits into from
Apr 22, 2022

Conversation

benpicco
Copy link
Contributor

@benpicco benpicco commented Apr 17, 2022

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
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.

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
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.

4 participants