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_link_format: add helper function to parse Link Format #18134

Merged
merged 2 commits into from
Jan 10, 2023

Conversation

benpicco
Copy link
Contributor

@benpicco benpicco commented May 24, 2022

Contribution description

This moves the code to parse the link format from the ncget shell command to a common location and makes it more general.

A callback is called for each resource path in the directory.

Testing procedure

ncget still works as before for listing directories:

> ncget coap://[fe80::2454:cfff:fea7:4598]/
/fw/
/suit_manifest.signed
/suit_manifest
/payload.bin

> ncget coap://[fe80::2454:cfff:fea7:4598]/fw/
/fw/suit_update/

> ncget coap://[fe80::2454:cfff:fea7:4598]/fw/suit_update/
/fw/suit_update/same54-xpro/
/fw/suit_update/saml21-xpro/

Issues/PRs references

@benpicco benpicco requested a review from chrysn May 24, 2022 14:30
@github-actions github-actions bot added Area: CoAP Area: Constrained Application Protocol implementations Area: network Area: Networking Area: sys Area: System labels May 24, 2022
@benpicco benpicco requested review from fabian18 and maribu May 26, 2022 20:43
@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 May 30, 2022
@maribu
Copy link
Member

maribu commented Jun 17, 2022

@fengelhardt may be interested in this

@maribu
Copy link
Member

maribu commented Jun 17, 2022

Hmm, from the text it more looks like this is an utility to obtain a CoAP Resource that is in the CoRE Link Format, rather than anything specific to Resource Directories. RDs do use the CoRE Link Format (due to the lack of decent CBOR based alternatives not yet being standardized), but it is also notably used by the default resource discovery.

Maybe it could be named better to indicate that it is useful for interaction with any server that provides a list of links in the CoRE Link Format?

Out of scope I guess, but it may be useful to also be able to add URI Query Options. E.g. querying coap://some.iot.device/.well-known/core?rt=oic.d.light would only list resources with a resource type of "oic.d.light", which (hopefully) matches smart light bulbs and similar. I guess if nanocoap_sock_url_connect() would be extended to just parse a query component from the URI given, this should just work, shouldn't it?

@benpicco
Copy link
Contributor Author

Thank you, I was searching for what was the proper name for this.
I renamed the functions and the module accordingly.

@benpicco benpicco changed the title nanocoap_rd: add helper function to parse Resource Directory nanocoap_link_format: add helper function to parse Link Format Aug 24, 2022
@benpicco benpicco requested review from maribu and removed request for maribu September 4, 2022 16:14
@riot-ci
Copy link

riot-ci commented Oct 4, 2022

Murdock results

✔️ PASSED

f06c763 sys/shell: ncget: make use of nanocoap_link_format_get()

Artifacts

This only reflects a subset of all builds from https://ci-prod.riot-os.org. Please refer to https://ci.riot-os.org for a complete build for now.

@benpicco benpicco requested a review from bergzand October 11, 2022 08:18
@benpicco benpicco added the State: waiting for maintainer State: Action by a maintainer is required label Oct 11, 2022
@benpicco benpicco requested a review from kfessel November 15, 2022 09:46
@benpicco benpicco removed CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR State: waiting for maintainer State: Action by a maintainer is required labels Nov 15, 2022
@benpicco benpicco 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 Nov 22, 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 Dec 11, 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 looks good to me. I'd say we wait the three days to hard feature freeze before merging this, but it never hurts to have an ACK :)

@benpicco
Copy link
Contributor Author

bors merge

bors bot added a commit that referenced this pull request Jan 10, 2023
18134: nanocoap_link_format: add helper function to parse Link Format r=benpicco a=benpicco



Co-authored-by: Benjamin Valentin <benpicco@beuth-hochschule.de>
Co-authored-by: Benjamin Valentin <benjamin.valentin@ml-pa.com>
@kaspar030
Copy link
Contributor

bors cancel
bors merge

@bors
Copy link
Contributor

bors bot commented Jan 10, 2023

Canceled.

@bors
Copy link
Contributor

bors bot commented Jan 10, 2023

Build succeeded:

@bors bors bot merged commit 53176f7 into RIOT-OS:master Jan 10, 2023
@benpicco benpicco deleted the nanocoap_rd branch January 10, 2023 15:25
@jia200x jia200x added this to the Release 2023.04 milestone Apr 25, 2023
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 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.

6 participants