Skip to content

nanocoap: introduce coap_get_method()#20191

Merged
benpicco merged 2 commits intoRIOT-OS:masterfrom
benpicco:coap_get_method
Mar 20, 2024
Merged

nanocoap: introduce coap_get_method()#20191
benpicco merged 2 commits intoRIOT-OS:masterfrom
benpicco:coap_get_method

Conversation

@benpicco
Copy link
Copy Markdown
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
Copy Markdown

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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
@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
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 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