Skip to content

net/unicoap: basic CoAP group communication support #22484

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.

Sign up for GitHub

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

Draft
elenaf9 wants to merge 2 commits into
base: pr/unicoap-client
Choose a base branch
from
+96 −37
Draft

net/unicoap: basic CoAP group communication support #22484

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d53226e
unicoap: basic group communication support
elenaf9 Sep 4, 2026
48cdf27
fixup! unicoap: basic group communication support
elenaf9 Sep 5, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 17 additions & 8 deletions sys/include/net/unicoap/client.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,15 @@ typedef enum {
* **Default**: disabled
*/
UNICOAP_CLIENT_FLAG_RELIABLE = 0x0001,

/**
* @brief Sets the type of the message to multicast.
*
* This flag will be set if the destination address for a request is an
* IP multicast address.
* It is not compatible with @ref UNICOAP_CLIENT_FLAG_RELIABLE.
*/
UNICOAP_CLIENT_FLAG_MULTICAST = 0x8000,
} unicoap_request_flags_t;

/**
Expand All @@ -64,18 +73,18 @@ typedef enum {
void unicoap_print_client_flags(unicoap_request_flags_t flags);


/**
/**
* @brief Additional parameters to customize request behavior
*/
typedef struct {
/**
* @brief Opaque argument passed to callback both in success and failure cases
* @brief Opaque argument passed to callback both in success and failure cases
*/
void* callback_arg;

/**
* @brief Response timeout in milliseconds
*
*
* Leave set to zero to fall back to @ref CONFIG_UNICOAP_TIMEOUT_CLIENT_RESPONSE_MS
*/
uint32_t timeout_ms;
Expand Down Expand Up @@ -140,13 +149,13 @@ typedef int (*unicoap_response_callback_t)(const unicoap_message_t* response,
* @param parameters Optional parameters (nullable)
* @param flags Client flags
*
* @returns Zero on success or positive refno if [cancellable requests](@ref unicoap_cancel_request)
* @returns Zero on success or positive refno if [cancellable requests](@ref unicoap_cancel_request)
* are enabled
* @returns Negative integer on failure
*/
int unicoap_send_request_async(unicoap_message_t* request,
unicoap_destination_t* destination,
unicoap_response_callback_t callback,
unicoap_response_callback_t callback,
unicoap_request_parameters_t* parameters,
unicoap_request_flags_t flags);

Expand All @@ -165,7 +174,7 @@ int unicoap_send_request_async(unicoap_message_t* request,
* sent rather than a non-confirmable message.
*
* @param[in,out] request Initialized request message to send
* @param destination URI or endpoint. Use @ref unicoap_destination_uri_string or
* @param destination URI or endpoint. Use @ref unicoap_destination_uri_string or
* @ref unicoap_destination_endpoint
* @param callback Function executed when the entire response is available or if an error occurred
* @param parameters Optional parameters (nullable)
Expand All @@ -176,7 +185,7 @@ int unicoap_send_request_async(unicoap_message_t* request,
*/
int unicoap_send_request_sync(unicoap_message_t* request,
unicoap_destination_t* destination,
unicoap_response_callback_t callback,
unicoap_response_callback_t callback,
unicoap_request_parameters_t* parameters,
unicoap_request_flags_t flags);

Expand Down Expand Up @@ -221,7 +230,7 @@ int unicoap_send_request_sync(unicoap_message_t* request,
*/
int unicoap_send_request_sync_copy(unicoap_message_t* request,
unicoap_destination_t* destination,
unicoap_message_t* response,
unicoap_message_t* response,
unicoap_request_parameters_t* parameters,
unicoap_request_flags_t flags,
unicoap_aux_t* aux);
Expand Down
12 changes: 12 additions & 0 deletions sys/include/net/unicoap/config.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -279,6 +279,18 @@ static_assert(CONFIG_UNICOAP_GENERATED_TOKEN_LENGTH > 0,
#if !defined(CONFIG_UNICOAP_TIMEOUT_CLIENT_RESPONSE_MS) || defined(DOXYGEN)
# define CONFIG_UNICOAP_TIMEOUT_CLIENT_RESPONSE_MS (7000)
#endif

/**
* @brief Default time to wait for a multicast response.
*
* Setting the timeout to zero will disable it.
*
* **Unit**: milliseconds
* **Default**: 10000
*/
#if !defined(CONFIG_UNICOAP_TIMEOUT_CLIENT_MULTICAST_RESPONSE_MS) || defined(DOXYGEN)
# define CONFIG_UNICOAP_TIMEOUT_CLIENT_MULTICAST_RESPONSE_MS (10000)
#endif
Comment thread
elenaf9 marked this conversation as resolved.
/** @} */

/* MARK: - Resource observation */
Expand Down
67 changes: 46 additions & 21 deletions sys/net/application_layer/unicoap/client/client.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
#include <string.h>
#include <errno.h>

#include "net/unicoap/transport.h"
#include "ztimer.h"
#include "mutex.h"
#include "compiler_hints.h"
Expand Down Expand Up @@ -71,7 +72,10 @@ int unicoap_client_process_response(unicoap_packet_t* packet, unicoap_client_mem
/* TODO: Block-wise */
res = unicoap_client_callback_success(memo, packet, UNICOAP_BLOCK_OPTION_NONE);

unicoap_client_memo_free(memo);
if ((memo->flags & UNICOAP_CLIENT_FLAG_MULTICAST) == 0 ) {
unicoap_client_memo_free(memo);
}

return res;
}

Expand Down Expand Up @@ -104,7 +108,7 @@ int unicoap_client_send_request_part(unicoap_packet_t* packet, unicoap_client_me

int unicoap_client_send_request_body(unicoap_message_t* request,
unicoap_endpoint_t* endpoint,
unicoap_callback_t callback,
unicoap_callback_t callback,
unicoap_request_parameters_t* parameters,
unicoap_request_flags_t flags)
{
Expand All @@ -123,6 +127,16 @@ int unicoap_client_send_request_body(unicoap_message_t* request,
.token_length = sizeof(token),
} };

bool multicast = unicoap_endpoint_is_multicast(endpoint);

if (multicast) {
if (flags & UNICOAP_CLIENT_FLAG_RELIABLE) {
_CLIENT_DEBUG("error trying to send reliable datagram via multicast\n");
return -EINVAL;
}
flags |= UNICOAP_CLIENT_FLAG_MULTICAST;
}

if (unicoap_callback_is_present(callback)) {
_CLIENT_DEBUG("need a memo\n");
if (!(memo = unicoap_client_memo_create(endpoint))) {
Expand All @@ -132,11 +146,22 @@ int unicoap_client_send_request_body(unicoap_message_t* request,
memo->callback_arg = parameters ? parameters->callback_arg : NULL;
memo->flags = flags;

unicoap_event_schedule(&memo->super.exchange.timeout, _on_response_timeout,
(parameters && parameters->timeout_ms > 0) ?
parameters->timeout_ms : CONFIG_UNICOAP_TIMEOUT_CLIENT_RESPONSE_MS,
"client.resp-timeout");
if (!multicast) {
unicoap_event_schedule(&memo->super.exchange.timeout, _on_response_timeout,
(parameters && parameters->timeout_ms > 0) ?
parameters->timeout_ms :
CONFIG_UNICOAP_TIMEOUT_CLIENT_RESPONSE_MS,
"client.resp-timeout");
}
else if (CONFIG_UNICOAP_TIMEOUT_CLIENT_MULTICAST_RESPONSE_MS > 0) {
unicoap_event_schedule(&memo->super.exchange.timeout, _on_response_timeout,
(parameters && parameters->timeout_ms > 0) ?
parameters->timeout_ms :
CONFIG_UNICOAP_TIMEOUT_CLIENT_MULTICAST_RESPONSE_MS,
"client.resp-timeout");
}
}

Comment on lines 148 to +164

@mguetschow mguetschow Sep 5, 2026

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.

Suggested change
unicoap_event_schedule(&memo->super.exchange.timeout, _on_response_timeout,
(parameters && parameters->timeout_ms > 0) ?
parameters->timeout_ms : CONFIG_UNICOAP_TIMEOUT_CLIENT_RESPONSE_MS,
"client.resp-timeout");
if (!multicast) {
unicoap_event_schedule(&memo->super.exchange.timeout, _on_response_timeout,
(parameters && parameters->timeout_ms > 0) ?
parameters->timeout_ms :
CONFIG_UNICOAP_TIMEOUT_CLIENT_RESPONSE_MS,
"client.resp-timeout");
}
else if (CONFIG_UNICOAP_TIMEOUT_CLIENT_MULTICAST_RESPONSE_MS > 0) {
unicoap_event_schedule(&memo->super.exchange.timeout, _on_response_timeout,
(parameters && parameters->timeout_ms > 0) ?
parameters->timeout_ms :
CONFIG_UNICOAP_TIMEOUT_CLIENT_MULTICAST_RESPONSE_MS,
"client.resp-timeout");
}
}
uint32_t timeout_ms =
(parameters && parameters->timeout_ms > 0) ? parameters->timeout_ms :
(multicast) ? CONFIG_UNICOAP_TIMEOUT_CLIENT_MULTICAST_RESPONSE_MS :
CONFIG_UNICOAP_TIMEOUT_CLIENT_RESPONSE_MS;
if (timeout_ms > 0) {
unicoap_event_schedule(&memo->super.exchange.timeout, _on_response_timeout,
timeout_ms, "client.resp-timeout");
}
}

is timeout_ms == 0 even a sensible value?

@elenaf9 elenaf9 Sep 5, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

timeout == 0 is used to disable the timeout, see #22484 (comment).

@mguetschow mguetschow Sep 5, 2026

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.

Oh I see. That's missing in the documentation of the config value right now. Also what does it mean to be disabled? Wouldn't the client then be unable to free up any resources it may have used for the transmission forever? Or does it not need transmissions for multicast anyways?

I could think of DTLS sessions e.g.. But anyways, how about interplay with DTLS for multicast? Probably just not supported? Is is caught somewhere right now?

@elenaf9 elenaf9 Sep 5, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

That's missing in the documentation of the config value right now

https://github.com/elenaf9/RIOT/blob/48cdf27bbacef19b4cd22323af5f04e29a9b9954/sys/include/net/unicoap/config.h#L286

Wouldn't the client then be unable to free up any resources it may have used for the transmission forever?

The user can still free up the resource by manually calling unicoap_cancel_request. This was discussed with @carl-tud out-of-band before.

But anyways, how about interplay with DTLS for multicast? Probably just not supported? Is is caught somewhere right now?

Per RFC 7390, the DTLS- based approach for CoAP is only for unicast and does not support group security features. Draft draft-ietf-core-groupcomm-bis specifies Group OSCORE as the default.
For this PR I think it's out of scope, but I'll add a check to catch the DTLS case.

/* TODO: OSCORE */
if ((res = unicoap_client_send_request_part(&packet, memo, flags)) < 0) {
goto error;
Expand Down Expand Up @@ -178,7 +203,7 @@ int unicoap_cancel_request(int refno) {

static int _open_request(unicoap_message_t* request,
unicoap_destination_t* destination,
unicoap_callback_t callback,
unicoap_callback_t callback,
unicoap_request_parameters_t* parameters,
unicoap_request_flags_t flags)
{
Expand All @@ -193,7 +218,7 @@ static int _open_request(unicoap_message_t* request,
request->options = options;
}
unicoap_endpoint_t endpoint = { 0 };
assert(uri_parser_is_absolute(destination->remote.uri,
assert(uri_parser_is_absolute(destination->remote.uri,
strlen(destination->remote.uri)));

uri_parser_result_t parsed = { 0 };
Expand Down Expand Up @@ -273,7 +298,7 @@ static int _copy_callback(const unicoap_message_t* response, const unicoap_aux_t
}

if (unicoap_options_size(response->options) > response->options->storage_capacity) {
_CLIENT_DEBUG("not enough buffer space to copy options, " _UNICOAP_NEED_HAVE "\n",
_CLIENT_DEBUG("not enough buffer space to copy options, " _UNICOAP_NEED_HAVE "\n",
unicoap_options_size(response->options), dest_options->storage_capacity);
error = -ENOBUFS;
goto out;
Expand All @@ -286,14 +311,14 @@ static int _copy_callback(const unicoap_message_t* response, const unicoap_aux_t
}

if (response->payload) {
if (!dest_payload) {
if (!dest_payload) {
_CLIENT_DEBUG("no payload buffer provided\n");
error = -ENOBUFS;
goto out;
}

if (dest_payload_capacity < response->payload_size) {
_CLIENT_DEBUG("not enough buffer space to copy payload, " _UNICOAP_NEED_HAVE "\n",
if (dest_payload_capacity < response->payload_size) {
_CLIENT_DEBUG("not enough buffer space to copy payload, " _UNICOAP_NEED_HAVE "\n",
response->payload_size, dest_payload_capacity);
}
memcpy(dest_payload, response->payload, response->payload_size);
Expand All @@ -318,7 +343,7 @@ static int _copy_callback(const unicoap_message_t* response, const unicoap_aux_t

int unicoap_send_request_sync_copy(unicoap_message_t* request,
unicoap_destination_t* destination,
unicoap_message_t* response,
unicoap_message_t* response,
unicoap_request_parameters_t* parameters,
unicoap_request_flags_t flags,
unicoap_aux_t* aux)
Expand All @@ -343,7 +368,7 @@ int unicoap_send_request_sync_copy(unicoap_message_t* request,
assert(false);
return -1;
}

_sync_copy_args_t args = (_sync_copy_args_t) {
.response = response,
.aux = aux,
Expand Down Expand Up @@ -385,7 +410,7 @@ static int _sync_callback(const unicoap_message_t *response, const unicoap_aux_t

int unicoap_send_request_sync(unicoap_message_t* request,
unicoap_destination_t* destination,
unicoap_response_callback_t callback,
unicoap_response_callback_t callback,
unicoap_request_parameters_t* parameters,
unicoap_request_flags_t flags)
{
Expand All @@ -407,9 +432,9 @@ int unicoap_send_request_sync(unicoap_message_t* request,
return -1;
}

_sync_args_t args = {
.callback = callback,
.roadblock = MUTEX_INIT_LOCKED
_sync_args_t args = {
.callback = callback,
.roadblock = MUTEX_INIT_LOCKED
};

unicoap_request_parameters_t sync_parameters = {};
Expand All @@ -419,7 +444,7 @@ int unicoap_send_request_sync(unicoap_message_t* request,
}
sync_parameters.callback_arg = &args;

int res = _open_request(request, destination,
int res = _open_request(request, destination,
(unicoap_callback_t) { .response = _sync_callback }, &sync_parameters, flags);

if (res < 0) {
Expand All @@ -432,9 +457,9 @@ int unicoap_send_request_sync(unicoap_message_t* request,

int unicoap_send_request_async(unicoap_message_t* request,
unicoap_destination_t* destination,
unicoap_response_callback_t callback,
unicoap_response_callback_t callback,
unicoap_request_parameters_t* parameters,
unicoap_request_flags_t flags) {
return _open_request(request, destination,
return _open_request(request, destination,
(unicoap_callback_t) { .response = callback }, parameters, flags);
}
29 changes: 21 additions & 8 deletions sys/net/application_layer/unicoap/state.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,11 @@ static unicoap_client_memo_t* _alloc_client(void) {
return NULL;
}

static inline bool _is_multicast(unicoap_client_memo_t *memo)
{
return (memo->flags & UNICOAP_CLIENT_FLAG_MULTICAST) != 0;
}

static inline bool _is_client(const unicoap_memo_t* memo) {
(void)memo;
#if UNICOAP_HAVE_CLIENT_STATE
Expand Down Expand Up @@ -162,7 +167,7 @@ void unicoap_client_memo_free(unicoap_client_memo_t* memo) {
* the state object is released as usual. Should the messaging layer rely on this
* information, the exchange-messaging abstraction has a design flaw. */
if (unicoap_memo_messaging_state(&memo->super)) {
unicoap_messaging_notify(unicoap_memo_messaging_state(&memo->super),
unicoap_messaging_notify(unicoap_memo_messaging_state(&memo->super),
UNICOAP_LAYER_NOTIFICATION_STATE_RELEASE, NULL, proto);
}
#endif
Expand All @@ -180,9 +185,13 @@ unicoap_client_memo_t* unicoap_client_memo_find_token(const unicoap_endpoint_t*
for (size_t i = 0; i < (size_t)ARRAY_SIZE(_state.client_memos); i += 1) {
unicoap_client_memo_t* memo = &_state.client_memos[i];

if ((!endpoint || unicoap_endpoint_is_equal(&memo->super.endpoint, endpoint)) &&
token_length == sizeof(memo->token) &&
memcmp(memo->token, token, token_length) == 0) {

if (endpoint && !unicoap_endpoint_is_equal(&memo->super.endpoint,
endpoint) && !_is_multicast(memo)) {
continue;
}

if (token_length == sizeof(memo->token) && memcmp(memo->token, token, token_length) == 0) {
return memo;
}
Comment on lines +188 to 196

@mguetschow mguetschow Sep 5, 2026 •
edited
Loading

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.

Not sure I understand that reasoning on the first glance. Could you elaborate?

I would expect: if multicast, just match on token. otherwise check endpoint and token. or does endpoint_is_equal does some special handling of multicast adresses?

@elenaf9 elenaf9 Sep 5, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I would expect: if multicast, just match on token. otherwise check endpoint. or does endpoint_is_equal does some special handling of multicast adresses?

Yes, that's what this implements: if the endpoint doesn't match and it's not multicast =>continue. But will move the is_multicast check to the beginning so that we check that before comparing the endpoint.

@mguetschow mguetschow Sep 5, 2026

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.

then i would propose:

if ((multicast || (endpoint && is_equal)) && token-comparison)

in a single if-clause to avoid the continue?

@elenaf9 elenaf9 Sep 5, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

That's how it was before. I personally find these very long multi-line if-clauses harder to read and understand. But I don't feel strongly about it. Will change it back.

}
Expand Down Expand Up @@ -221,7 +230,7 @@ int unicoap_client_memo_assign_refno(unicoap_client_memo_t* memo) {
* exchange. Should the memo struct in the memo array get reused for another exchange,
* the refno can be detected to be obsolete. */
#if IS_USED(MODULE_UNICOAP_CLIENT_CANCELLATION)
/* Memos require a unique reference ID and array index for fast lookup. Because functions return
/* Memos require a unique reference ID and array index for fast lookup. Because functions return
* the refno as an int where negative values represent errors, we assume a minimum 16-bit int
* and use the 15 positive bits.
* 15 available bits = 12 bits (reference ID) + 3 bits (minimum index)
Expand All @@ -230,7 +239,7 @@ int unicoap_client_memo_assign_refno(unicoap_client_memo_t* memo) {
* arrays. */
memo->reference_id = random_uint32_range(1, 0xfff); /* 12 bits for reference ID */
int refno = memo->reference_id | (MIN(_client_index(memo), 0x7) << 12); /* 3 bits min index */
_STATE_DEBUG("refno=%i (min_client_ix=#%" PRIuSIZE ", refid=%u)\n",
_STATE_DEBUG("refno=%i (min_client_ix=#%" PRIuSIZE ", refid=%u)\n",
refno, MIN(_client_index(memo), 0x7), memo->reference_id);
return refno;
#else
Expand Down Expand Up @@ -340,7 +349,7 @@ void unicoap_event_schedule(unicoap_scheduled_event_t* event, unicoap_event_call

void unicoap_event_reschedule(unicoap_scheduled_event_t* event, uint32_t duration) {
if (IS_ACTIVE(DEVELHELP)) {
_STATE_EVENT_DEBUG("%s in %"PRIu32"ms (rescheduled)\n",
_STATE_EVENT_DEBUG("%s in %"PRIu32"ms (rescheduled)\n",
unicoap_scheduled_event_name(event), duration);
}
ztimer_set(UNICOAP_CLOCK, &event->ztimer, duration);
Expand Down Expand Up @@ -648,7 +657,11 @@ unicoap_preprocessing_result_t unicoap_exchange_preprocess(unicoap_packet_t* pac
unicoap_client_memo_free(memo);
return UNICOAP_PREPROCESSING_ERROR_TRUNCATED;
}
unicoap_event_cancel(&memo->super.exchange.timeout);

if (!_is_multicast(memo)) {
unicoap_event_cancel(&memo->super.exchange.timeout);
Comment on lines +661 to +662

@mguetschow mguetschow Sep 5, 2026

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.

maybe add a comment why we don't cancel for multicast.

}

arg->client = memo;
*flags = _messaging_flags_client(memo->flags);
return UNICOAP_PREPROCESSING_SUCCESS_RESPONSE;
Expand Down