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

net/gcoap: add CoAP ping request #13894

Merged
merged 4 commits into from
Apr 21, 2020
Merged

Conversation

kb2ma
Copy link
Member

@kb2ma kb2ma commented Apr 17, 2020

Contribution description

Presently gcoap supports use of a piggybacked ACK response to a confirmable request. However, #13637 illustrated the need to support separate confirmable responses, as also identified in the tracking issue #7192. A prerequisite for this support is the ability to handle empty messages, which is the format for the immediate ACK response. This PR takes the first step toward empty message support by addition of handling for CoAP ping requests.

Specifically, this PR allows creation of a ping request (empty CON with code 0.00) and generation of a reset response (empty RST also with code 0.00). This PR does not include handling for the reset response because that requires empty message support for request/response correlation, which will follow in a separate PR.

Commit Description
2d3eb1a Validate empty message length in nanocoap, including unit test
f5a85e2 Add handling for a ping request in gcoap. Update debug msg if receive an empty ACK or RST.
6eae4de Extend gcoap_req_init() to generate ping request if code is 0.00, including unit test
c267633 Add 'coap ping' sub-command to gcoap example

Testing procedure

  • Run nanocoap and gcoap unit tests
  • Set up two native terminals or boards with gcoap example. Turn on debug in gcoap.c or use Wireshark to watch exchange. Use CLI 'coap ping' command to send a ping request. Notice receiver responds with reset message, but sender doesn't recognize it. Sender times out after the expected retries.
  • Also verify regular request/response handling still works with this setup.
  • Review doc for gcoap_req_init()

Issues/PRs references

See above.

@kb2ma kb2ma added Area: CoAP Area: Constrained Application Protocol implementations Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation labels Apr 17, 2020
@kb2ma kb2ma added this to the Release 2020.07 milestone Apr 17, 2020
@leandrolanzieri leandrolanzieri added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Apr 21, 2020
@leandrolanzieri
Copy link
Contributor

leandrolanzieri commented Apr 21, 2020

Tests still pass and the ping packet is generated correctly. Also, normal operations work as expected, including the proxy:
image

  • Ping:
    image
  • Ping response:
    image
  • Proxy still works:
    image

@leandrolanzieri leandrolanzieri added Reviewed: 1-fundamentals The fundamentals of the PR were reviewed according to the maintainer guidelines Reviewed: 3-testing The PR was tested according to the maintainer guidelines labels Apr 21, 2020
Copy link
Contributor

@leandrolanzieri leandrolanzieri left a comment

Choose a reason for hiding this comment

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

Changes look good and functionality works as expected. ACK.
Nice feature!

@leandrolanzieri leandrolanzieri added Reviewed: 2-code-design The code design of the PR was reviewed according to the maintainer guidelines Reviewed: 4-code-style The adherence to coding conventions by the PR were reviewed according to the maintainer guidelines Reviewed: 5-documentation The documentation details of the PR were reviewed according to the maintainer guidelines labels Apr 21, 2020
@leandrolanzieri leandrolanzieri merged commit 46507ff into RIOT-OS:master Apr 21, 2020
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 CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Reviewed: 1-fundamentals The fundamentals of the PR were reviewed according to the maintainer guidelines Reviewed: 2-code-design The code design of the PR was reviewed according to the maintainer guidelines Reviewed: 3-testing The PR was tested according to the maintainer guidelines Reviewed: 4-code-style The adherence to coding conventions by the PR were reviewed according to the maintainer guidelines Reviewed: 5-documentation The documentation details of the PR were reviewed according to the maintainer guidelines Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants