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

tests/gcoap_fileserver: add test for PUT #18725

Merged
merged 2 commits into from
Oct 12, 2022

Conversation

benpicco
Copy link
Contributor

Contribution description

This adds a test for blockwise PUT to tests/gcoap_fileserver.

Doing so uncovered a bug: When a block would be transmitted, but the ACK got lost, the sender would re-transmit the block.

This triggered the block1.offset != (unsigned)ret check in the GCoAP fileserver which considered this an error and aborted the transmission.

To fix this, simply ignore old blocks and send a COAP_CODE_CONTINUE anyway.

Testing procedure

CI will run tests/gcoap_fileserver.

Issues/PRs references

@benpicco benpicco requested review from fabian18 and chrysn October 11, 2022 22:49
@github-actions github-actions bot added Area: CoAP Area: Constrained Application Protocol implementations Area: network Area: Networking Area: sys Area: System Area: tests Area: tests and testing framework labels Oct 11, 2022
@benpicco benpicco added the Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors) label Oct 11, 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 Oct 11, 2022
@benpicco benpicco force-pushed the tests/gcoap_fileserver-put branch from 3b344b0 to 245aef3 Compare October 11, 2022 22:52
@riot-ci
Copy link

riot-ci commented Oct 11, 2022

Murdock results

✔️ PASSED

7e0aeb3 tests/gcoap_fileserver: add test for PUT

Success Failures Total Runtime
1979 0 1980 06m:16s

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 force-pushed the tests/gcoap_fileserver-put branch from 245aef3 to 5c7e8d6 Compare October 12, 2022 08:39
@benpicco benpicco force-pushed the tests/gcoap_fileserver-put branch from 5c7e8d6 to 7e0aeb3 Compare October 12, 2022 09:37
Copy link
Contributor

@fabian18 fabian18 left a comment

Choose a reason for hiding this comment

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

ACK

@benpicco benpicco merged commit b11af08 into RIOT-OS:master Oct 12, 2022
@benpicco
Copy link
Contributor Author

Thank you for the quick review!

@benpicco benpicco deleted the tests/gcoap_fileserver-put branch October 12, 2022 14:52
@maribu maribu added this to the Release 2022.10 milestone Oct 14, 2022
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 Area: tests Area: tests and testing framework CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants