Skip to content

Conversation

@fabian18
Copy link
Contributor

@fabian18 fabian18 commented May 3, 2021

Contribution description

This PR adds a const coap_ep_t *remote and const coap_ep_t *local to the CoAP response and request handlers.
A coap_ep_t * is actually a void * because there is no full support for other sockets than UDP yet and maybe CoAP over TCP will be added some day.
The handlers in the gcoap example are caested to (coap_handler_t) to gain compatibility and still deal with sock_udp_ep_t *.

Testing procedure

examples/gcoap

2021-05-03 09:57:26,415 # gcoap: remote [fe80::4c49:6fff:fe54:5c]:5683
2021-05-03 09:57:26,419 # gcoap: local [fe80::4c49:6fff:fe54:66]:5683
coap get fe80::4c49:6fff:fe54:66%6 5683 /.well-known/core
2021-05-03 09:57:17,508 # coap get fe80::4c49:6fff:fe54:66%6 5683 /.well-known/core
2021-05-03 09:57:17,512 # gcoap_cli: sending msg ID 51354, 23 bytes
2021-05-03 09:57:17,516 # gcoapgcoap: remote [fe80::4c49:6fff:fe54:66]:5683
2021-05-03 09:57:17,520 # gcoap: local [fe80::4c49:6fff:fe54:5c]:5683
2021-05-03 09:57:17,524 # gcoap: response Success, code 2.05, 46 bytes
2021-05-03 09:57:17,528 # </cli/stats>;ct=0;rt="count";obs,</riot/board>
coap get fe80::4c49:6fff:fe54:66%6 5683 /riot/board
2021-05-03 09:57:26,407 # _stats>;ct=0;rt="count";obs,</rio> coap get fe80::4c49:6fff:fe54:66%6 5683 /riot/board
2021-05-03 09:57:26,411 # gcoap_cli: sending msg ID 51355, 17 bytes
2021-05-03 09:57:26,415 # gcoap_cli: no observer for /cli/stats
> 2021-05-03 09:57:26,423 # gcoap: remote [fe80::4c49:6fff:fe54:66]:5683
2021-05-03 09:57:26,427 # gcoap: local [fe80::4c49:6fff:fe54:5c]:5683
2021-05-03 09:57:26,431 # gcoap: response Success, code 2.05, 18 bytes
2021-05-03 09:57:26,433 # nucleo-f767zi

Issues/PRs references

Trying to solve #15686

@benpicco benpicco added Area: CoAP Area: Constrained Application Protocol implementations Type: new feature The issue requests / The PR implemements a new feature for RIOT labels May 3, 2021
@fabian18 fabian18 changed the title CoAP: add remote annd local endpoint information to request and response handlers CoAP: add remote and local endpoint information to request and response handlers May 3, 2021
@maribu maribu added the Process: API change Integration Process: PR contains or issue proposes an API change. Should be handled with care. label May 4, 2021
@maribu
Copy link
Member

maribu commented May 4, 2021

I'm wondering if it would be possible to extend coap_pkt_t instead of changing the API, so that existing applications don't need to be changed.

@chrysn: You raised the issue with supporting to access the local endpoint in a way that doesn't block adding other transports such as TCP or OSCORE. Maybe you're interested in joining the discussion?

@chrysn
Copy link
Member

chrysn commented May 8, 2021

Having it in coap_pkt_t would IMO be the preferable way. As @maribu said: this would become important for TCP and gcoap-integrated OSCORE support, and both of these need different mechanisms to access the message itself. (And then they would would eventually do the typing of the remote).

Thing is, the handler interface is really a nanocoap thing (not Gcoap), so either we do it all the way through nanocoap (which then does become more complex even though it is supposed to be minimal), or we introduce a shim at pkt level (where a pkt is now always a nanocoap one with a socket (which nanocoap doesn't know of) attached, but could later become a tcpcoap one).

I'm open to both, but all do come with severe API changes. Might call for a round table on further CoAP support in RIOT to set a direction here.

@chrysn
Copy link
Member

chrysn commented May 8, 2021

Starting the wider discussion on https://forum.riot-os.org/t/coap-road-map-thread/3230

@stale
Copy link

stale bot commented Mar 2, 2022

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. If you want me to ignore this issue, please mark it with the "State: don't stale" label. Thank you for your contributions.

@stale stale bot added the State: stale State: The issue / PR has no activity for >185 days label Mar 2, 2022
@stale stale bot closed this Apr 16, 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 Process: API change Integration Process: PR contains or issue proposes an API change. Should be handled with care. State: stale State: The issue / PR has no activity for >185 days Type: new feature The issue requests / The PR implemements a new feature for RIOT

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants