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

sys/net/nanocoap: add and use coap_get_response_hdr_len() #20950

Merged

Conversation

maribu
Copy link
Member

@maribu maribu commented Nov 5, 2024

Contribution description

Before, handlers writing blockwise transfer assumed that the response header length will match the request header length. This is true for UDP, but not for TCP: The CoAP over TCP header contains a Len field, that gets extended for larger messages. Since the reply often is indeed larger than the request, this is indeed often the case for CoAP over TCP.

Note

Right now, no CoAP over TCP implementation is upstream. However, getting rid of incorrect assumptions now will make life easier later on.

Testing procedure

This will not change generated binaries, as right now the implementation just calls the same function that was used before. After all, for CoAP over UDP the assumption that request and response header match in size is valid.

Issues/PRs references

Relevant for #20900

Before, handlers writing blockwise transfer assumed that the response
header length will match the request header length. This is true for
UDP, but not for TCP: The CoAP over TCP header contains a Len field,
that gets extended for larger messages. Since the reply often is indeed
larger than the request, this is indeed often the case for CoAP over
TCP.

Note: Right now, no CoAP over TCP implementation is upstream. However,
      getting rid of incorrect assumptions now will make life easier
      later on.
@maribu maribu added Process: API change Integration Process: PR contains or issue proposes an API change. Should be handled with care. CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Nov 5, 2024
@github-actions github-actions bot added Area: network Area: Networking Area: CoAP Area: Constrained Application Protocol implementations Area: sys Area: System Area: examples Area: Example Applications labels Nov 5, 2024
@maribu maribu requested a review from benpicco November 5, 2024 09:55
@riot-ci
Copy link

riot-ci commented Nov 5, 2024

Murdock results

✔️ PASSED

68beb52 sys/net/nanocoap: add and use coap_get_response_hdr_len()

Success Failures Total Runtime
10214 0 10215 17m:47s

Artifacts

@maribu maribu added this pull request to the merge queue Nov 5, 2024
Merged via the queue into RIOT-OS:master with commit c70075b Nov 5, 2024
28 checks passed
@maribu maribu deleted the sys/net/nanocoap/coap_get_response_hdr_len branch November 5, 2024 17:04
@maribu
Copy link
Member Author

maribu commented Nov 5, 2024

Thx :)

@MrKevinWeiss MrKevinWeiss added this to the Release 2025.01 milestone Jan 20, 2025
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: examples Area: Example Applications 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 Process: API change Integration Process: PR contains or issue proposes an API change. Should be handled with care.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants