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

[cli] Add support for CoAP URI queries in CLI requests #10003

Open
wants to merge 15 commits into
base: main
Choose a base branch
from

Conversation

feherdave
Copy link
Contributor

@feherdave feherdave commented Apr 7, 2024

New feature:
Added CoAP URI query processing functionality to CLI.

Solution:
URI string is checked for a '?' character. If found, the URI query related part is moved to another string. Afterwards, the modified coapUri string will contain only the path-related parts, and a second string (coapUriQuery) will hold only the URI query related parts. Then coapUri is passed to the original otCoapMessageAppendUriPathOptions() method and coapUriQuery is passed to a new method called otCoapMessageAppendUriQueryOptions(), which - similarly to the other - splits the URI query part into more, smaller pieces by the delimiter '&' and adds them as separate options to the CoAP message making the request generated by this CLI function RFC compliant.
If '?' is not found, everything is processed the old way.

Copy link

google-cla bot commented Apr 7, 2024

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

Copy link

size-report bot commented Apr 7, 2024

Size Report of OpenThread

Merging #10003 into main(d0f6d17).

name branch text data bss total
ot-cli-ftd main 467176 856 66364 534396
#10003 467288 856 66364 534508
+/- +112 0 0 +112
ot-ncp-ftd main 436076 760 61576 498412
#10003 436076 760 61576 498412
+/- 0 0 0 0
libopenthread-ftd.a main 236220 95 40310 276625
#10003 236278 95 40310 276683
+/- +58 0 0 +58
libopenthread-cli-ftd.a main 57549 0 8075 65624
#10003 57605 0 8075 65680
+/- +56 0 0 +56
libopenthread-ncp-ftd.a main 31857 0 5916 37773
#10003 31857 0 5916 37773
+/- 0 0 0 0
ot-cli-mtd main 364712 760 51220 416692
#10003 364824 760 51220 416804
+/- +112 0 0 +112
ot-ncp-mtd main 347244 760 46448 394452
#10003 347244 760 46448 394452
+/- 0 0 0 0
libopenthread-mtd.a main 158331 0 25182 183513
#10003 158389 0 25182 183571
+/- +58 0 0 +58
libopenthread-cli-mtd.a main 39787 0 8059 47846
#10003 39843 0 8059 47902
+/- +56 0 0 +56
libopenthread-ncp-mtd.a main 24737 0 5916 30653
#10003 24737 0 5916 30653
+/- 0 0 0 0
ot-cli-ftd-br main 549792 864 131196 681852
#10003 549904 864 131196 681964
+/- +112 0 0 +112
libopenthread-ftd-br.a main 322951 100 105118 428169
#10003 323009 100 105118 428227
+/- +58 0 0 +58
libopenthread-cli-ftd-br.a main 71212 0 8099 79311
#10003 71268 0 8099 79367
+/- +56 0 0 +56
ot-rcp main 62248 564 20604 83416
#10003 62248 564 20604 83416
+/- 0 0 0 0
libopenthread-rcp.a main 9542 0 5052 14594
#10003 9542 0 5052 14594
+/- 0 0 0 0
libopenthread-radio.a main 18870 0 214 19084
#10003 18870 0 214 19084
+/- 0 0 0 0

@feherdave feherdave changed the title [cli] Add support for URI queries in CLI requests [cli] Add support for CoAP URI queries in CLI requests Apr 9, 2024
@feherdave
Copy link
Contributor Author

Any further steps required from my side?

Copy link
Member

@abtink abtink left a comment

Choose a reason for hiding this comment

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

Looks good overall. Thanks @feherdave.
Some suggestions below:

Edit: Also may need to check the CLA -> #10003 (comment) (please ignore this - I see it is good).

src/cli/cli_coap.cpp Outdated Show resolved Hide resolved
src/cli/cli_coap.cpp Outdated Show resolved Hide resolved
Copy link
Member

@abtink abtink left a comment

Choose a reason for hiding this comment

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

Looks great. Thank you. 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants