Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

nanocoap: introduce coap_get_method() #20191

Merged
merged 2 commits into from
Mar 20, 2024
Merged

Conversation

benpicco
Copy link
Contributor

Contribution description

This is just an alias for coap_get_code_raw().
When writing a CoAP handler function, it's very unintuitive that the 'raw Code' is needed to obtain the request method.

This also gives us the opportunity to use the coap_method_t type to avoid ambiguity.

Testing procedure

No functional changes.

Issues/PRs references

@github-actions github-actions bot added Area: network Area: Networking Area: CoAP Area: Constrained Application Protocol implementations Area: sys Area: System labels Dec 18, 2023
@benpicco benpicco added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Type: cleanup The issue proposes a clean-up / The PR cleans-up parts of the codebase / documentation labels Dec 18, 2023
@riot-ci
Copy link

riot-ci commented Dec 18, 2023

Murdock results

✔️ PASSED

446509c gcoap_fileserver: use coap_get_method()

Success Failures Total Runtime
10009 0 10009 07m:48s

Artifacts

*/
static inline coap_method_t coap_get_method(const coap_pkt_t *pkt)
{
return pkt->hdr->code;
Copy link
Contributor

Choose a reason for hiding this comment

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

Would it make sense to check whether the packet is actually a request and not a response? Something along the lines of (pkt->hdr->code & 0b11100000) == 0. Otherwise this could return values with are actually not part of the enum coap_method_t.

Copy link
Contributor Author

@benpicco benpicco Dec 19, 2023

Choose a reason for hiding this comment

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

True, I'm not sure how to handle this though - slapping an assert() on there is a bad idea because the source of pkt is a network packet and we don't want to crash on a failed assertion when badly crafted packet arrives that would otherwise gracefully be ignored by not being part of a e.g. switch case.

Copy link
Contributor

Choose a reason for hiding this comment

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

I see, I don't have any suggestions for this either.

Copy link
Contributor

Choose a reason for hiding this comment

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

How about another enum for coap_method_t? COAP_METHOD_INVALID?

@benpicco benpicco added this pull request to the merge queue Mar 19, 2024
Merged via the queue into RIOT-OS:master with commit e3ce765 Mar 20, 2024
26 checks passed
@benpicco benpicco deleted the coap_get_method branch March 20, 2024 09:38
@MrKevinWeiss MrKevinWeiss added this to the Release 2024.04 milestone Apr 30, 2024
# for free to join this conversation on GitHub. Already have an account? # 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 Type: cleanup The issue proposes a clean-up / The PR cleans-up parts of the codebase / documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants