Skip to content

Commit

Permalink
Merge pull request #20684 from fabian18/pr/fix_gcoap_observe_response…
Browse files Browse the repository at this point in the history
…_correlation

sys/net/application_layer/gcoap: fix Observe notifications correlation
  • Loading branch information
benpicco authored Aug 26, 2024
2 parents 73581fa + 21a46dc commit 1626919
Show file tree
Hide file tree
Showing 9 changed files with 351 additions and 111 deletions.
7 changes: 5 additions & 2 deletions examples/cord_lc/Makefile.ci
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,9 @@ BOARD_INSUFFICIENT_MEMORY := \
atmega8 \
atxmega-a3bu-xplained \
bluepill-stm32f030c8 \
chronos \
derfmega128 \
i-nucleo-lrwan1 \
im880b \
microduino-corerf \
msb-430 \
msb-430h \
nucleo-c031c6 \
Expand All @@ -26,6 +26,8 @@ BOARD_INSUFFICIENT_MEMORY := \
olimex-msp430-h1611 \
olimex-msp430-h2618 \
samd10-xmini \
saml10-xpro \
saml11-xpro \
slstk3400a \
stk3200 \
stm32f030f4-demo \
Expand All @@ -37,4 +39,5 @@ BOARD_INSUFFICIENT_MEMORY := \
waspmote-pro \
weact-g030f6 \
z1 \
zigduino \
#
10 changes: 10 additions & 0 deletions examples/gcoap/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,9 @@ USEMODULE += shell_cmds_default
USEMODULE += uri_parser
USEMODULE += ps

# /rtc resource for regular observe notifications
FEATURES_OPTIONAL += periph_rtc

# Comment this out to disable code in RIOT that does safety checking
# which is not needed in a production environment but helps in the
# development process:
Expand Down Expand Up @@ -81,6 +84,13 @@ DOCKER_ENV_VARS += ZEP_PORT_BASE

include $(RIOTBASE)/Makefile.include

ifneq (,$(filter periph_rtc,$(USEMODULE)))
USEMODULE += event_periodic
USEMODULE += event_periodic_callback
USEMODULE += event_thread
USEMODULE += ztimer_msec
endif

# For now this goes after the inclusion of Makefile.include so Kconfig symbols
# are available. Only set configuration via CFLAGS if Kconfig is not being used
# for this module.
Expand Down
3 changes: 3 additions & 0 deletions examples/gcoap/Makefile.ci
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,17 @@ BOARD_INSUFFICIENT_MEMORY := \
atmega328p-xplained-mini \
atmega8 \
atxmega-a3bu-xplained \
blackpill-stm32f103c8 \
bluepill-stm32f030c8 \
bluepill-stm32f103c8 \
i-nucleo-lrwan1 \
msb-430 \
msb-430h \
nucleo-c031c6 \
nucleo-f030r8 \
nucleo-f031k6 \
nucleo-f042k6 \
nucleo-f302r8 \
nucleo-f303k8 \
nucleo-f334r8 \
nucleo-l011k4 \
Expand Down
44 changes: 15 additions & 29 deletions examples/gcoap/client.c
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
#include <string.h>

#include "net/gcoap.h"
#include "net/sock/udp.h"
#include "net/sock/util.h"
#include "od.h"
#include "uri_parser.h"
Expand All @@ -53,8 +54,8 @@ static char _proxy_uri[CONFIG_URI_MAX];
* completes or times out. */
static char _last_req_uri[CONFIG_URI_MAX];

/* whether this node is currently observing a resource as a client */
static bool observing = false;
/* Last remote endpoint where an Observe request has been sent to */
static sock_udp_ep_t obs_remote;

/* the token used for observing a remote resource */
static uint8_t obs_req_token[GCOAP_TOKENLEN_MAX];
Expand Down Expand Up @@ -190,6 +191,8 @@ static int _print_usage(char **argv)
printf(" %s proxy unset\n", argv[0]);
printf("Options\n");
printf(" -c Send confirmably (defaults to non-confirmable)\n");
printf(" -o include Observe registration option\n");
printf(" -d include Observe deregistration option\n");
return 1;
}

Expand Down Expand Up @@ -300,17 +303,10 @@ int gcoap_cli_cmd(int argc, char **argv)
if (code_pos == COAP_METHOD_GET) {
if (argc > apos) {
if (strcmp(argv[apos], "-o") == 0) {
if (observing) {
puts("Only one observe supported");
return 1;
}
observe = true;
apos++;
} else if (strcmp(argv[apos], "-d") == 0) {
if (!observing) {
puts("Not observing");
return 1;
}
}
else if (strcmp(argv[apos], "-d") == 0) {
observe = true;
apos++;
obs_value = COAP_OBS_DEREGISTER;
Expand All @@ -333,21 +329,6 @@ int gcoap_cli_cmd(int argc, char **argv)
gcoap_req_init(&pdu, buf, CONFIG_GCOAP_PDU_BUF_SIZE, code_pos, NULL);

if (observe) {
uint8_t *token = coap_get_token(&pdu);
if (obs_value == COAP_OBS_REGISTER) {
obs_req_tkl = coap_get_token_len(&pdu);
/* backup the token of the initial observe registration */
memcpy(obs_req_token, token, obs_req_tkl);
} else {
/* use the token of the registration for deregistration
* (manually replace the token set by gcoap_req_init) */
memcpy(token, obs_req_token, obs_req_tkl);
if (gcoap_obs_req_forget(&remote, obs_req_token, obs_req_tkl)) {
printf("could not remove observe request\n");
return 1;
}
}

coap_opt_add_uint(&pdu, COAP_OPT_OBSERVE, obs_value);
}

Expand Down Expand Up @@ -399,9 +380,14 @@ int gcoap_cli_cmd(int argc, char **argv)
}
else {
if (observe) {
/* on successful observe request, store that this node is
* observing / not observing anymore */
observing = obs_value == COAP_OBS_REGISTER;
/* forget last Observe token, as only one can be stored in this example */
gcoap_obs_req_forget(&obs_remote, obs_req_token, obs_req_tkl);
if (obs_value == COAP_OBS_REGISTER) {
obs_req_tkl = coap_get_token_len(&pdu);
/* backup the token of the initial observe registration */
memcpy(obs_req_token, coap_get_token(&pdu), obs_req_tkl);
obs_remote = remote;
}
}
/* send Observe notification for /cli/stats */
notify_observers();
Expand Down
72 changes: 72 additions & 0 deletions examples/gcoap/server.c
Original file line number Diff line number Diff line change
Expand Up @@ -23,11 +23,16 @@
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <time.h>

#include "event/periodic_callback.h"
#include "event/thread.h"
#include "fmt.h"
#include "net/gcoap.h"
#include "net/utils.h"
#include "od.h"
#include "periph/rtc.h"
#include "time_units.h"

#include "gcoap_example.h"

Expand Down Expand Up @@ -60,11 +65,17 @@ static ssize_t _encode_link(const coap_resource_t *resource, char *buf,
size_t maxlen, coap_link_encoder_ctx_t *context);
static ssize_t _stats_handler(coap_pkt_t* pdu, uint8_t *buf, size_t len, coap_request_ctx_t *ctx);
static ssize_t _riot_board_handler(coap_pkt_t* pdu, uint8_t *buf, size_t len, coap_request_ctx_t *ctx);

Check warning on line 67 in examples/gcoap/server.c

View workflow job for this annotation

GitHub Actions / static-tests

line is longer than 100 characters
#if IS_USED(MODULE_PERIPH_RTC)
static ssize_t _rtc_handler(coap_pkt_t* pdu, uint8_t *buf, size_t len, coap_request_ctx_t *ctx);
#endif

/* CoAP resources. Must be sorted by path (ASCII order). */
static const coap_resource_t _resources[] = {
{ "/cli/stats", COAP_GET | COAP_PUT, _stats_handler, NULL },
{ "/riot/board", COAP_GET, _riot_board_handler, NULL },
#if IS_USED(MODULE_PERIPH_RTC)
{ "/rtc", COAP_GET, _rtc_handler, NULL },
#endif
};

static const char *_link_params[] = {
Expand Down Expand Up @@ -100,6 +111,62 @@ static ssize_t _encode_link(const coap_resource_t *resource, char *buf,
return res;
}

#if IS_USED(MODULE_PERIPH_RTC)
static void _rtc_notify_observers(void *arg)
{
(void)arg;
struct tm tm_now;
if (rtc_get_time(&tm_now)) {
DEBUG_PUTS("gcoap_server: RTC error");
return;
}
size_t len;
char str_time[20] = "";
uint8_t buf[sizeof(coap_hdr_t) + COAP_TOKEN_LENGTH_MAX + 1 + sizeof(str_time)];
coap_pkt_t pdu;
const coap_resource_t *rtc_resource = NULL;
const gcoap_listener_t *listener = NULL;
while ((rtc_resource = gcoap_get_resource_by_path_iterator(&listener, rtc_resource, "/rtc"))) {
if (!strcmp(rtc_resource->path, "/rtc")) {
break; /* exact match */
}
}
if (rtc_resource) {
switch (gcoap_obs_init(&pdu, buf, sizeof(buf), rtc_resource)) {
case GCOAP_OBS_INIT_OK:
len = coap_opt_finish(&pdu, COAP_OPT_FINISH_PAYLOAD);
memcpy(pdu.payload, str_time, strftime(str_time, sizeof(str_time), "%Y-%m-%d %H:%M:%S", &tm_now));

Check warning on line 138 in examples/gcoap/server.c

View workflow job for this annotation

GitHub Actions / static-tests

line is longer than 100 characters
pdu.payload_len = strlen(str_time);
len += pdu.payload_len;
if (!gcoap_obs_send(buf, len, rtc_resource)) {
DEBUG_PUTS("gcoap_server: cannot send /rtc notification");
}
break;
case GCOAP_OBS_INIT_UNUSED:
DEBUG_PUTS("gcoap_server: no observer for /rtc");
break;
case GCOAP_OBS_INIT_ERR:
DEBUG_PUTS("gcoap_server: error initializing /rtc notification");
break;
}
}
}

static ssize_t _rtc_handler(coap_pkt_t* pdu, uint8_t *buf, size_t len, coap_request_ctx_t *ctx)
{
(void)ctx;
struct tm tm_now;
rtc_get_time(&tm_now);
gcoap_resp_init(pdu, buf, len, COAP_CODE_CONTENT);
size_t resp_len = coap_opt_finish(pdu, COAP_OPT_FINISH_PAYLOAD);
char str_time[20] = "";
memcpy(pdu->payload, str_time, strftime(str_time, sizeof(str_time), "%Y-%m-%d %H:%M:%S", &tm_now));

Check warning on line 163 in examples/gcoap/server.c

View workflow job for this annotation

GitHub Actions / static-tests

line is longer than 100 characters
pdu->payload_len = strlen(str_time);
resp_len += pdu->payload_len;
return resp_len;
}
#endif

/*
* Server callback for /cli/stats. Accepts either a GET or a PUT.
*
Expand Down Expand Up @@ -202,4 +269,9 @@ void server_init(void)
#endif

gcoap_register_listener(&_listener);
#if IS_USED(MODULE_PERIPH_RTC)
static event_periodic_callback_t _ev_pcb_rtc;
event_periodic_callback_init(&_ev_pcb_rtc, ZTIMER_MSEC, EVENT_PRIO_MEDIUM, _rtc_notify_observers, NULL);

Check warning on line 274 in examples/gcoap/server.c

View workflow job for this annotation

GitHub Actions / static-tests

line is longer than 100 characters
event_periodic_callback_start(&_ev_pcb_rtc, 10 * MS_PER_SEC);
#endif
}
41 changes: 41 additions & 0 deletions sys/include/net/gcoap.h
Original file line number Diff line number Diff line change
Expand Up @@ -552,14 +552,35 @@ extern "C" {
/**
* @ingroup net_gcoap_conf
* @brief Maximum number of Observe clients
*
* @note As documented in this file, the implementation is limited to one observer per resource.
* Therefore, every stored observer is associated with a different resource.
* If you have only one observable resource, you could set this value to 1.
*/
#ifndef CONFIG_GCOAP_OBS_CLIENTS_MAX
#define CONFIG_GCOAP_OBS_CLIENTS_MAX (2)
#endif

/**
* @ingroup net_gcoap_conf
* @brief Maximum number of local notifying endpoint addresses
*
* @note As documented in this file, the implementation is limited to one observer per resource.
* Therefore, every stored local endpoint alias is associated with an observation context
* of a different resource.
* If you have only one observable resource, you could set this value to 1.
*/
#ifndef CONFIG_GCOAP_OBS_NOTIFIERS_MAX
#define CONFIG_GCOAP_OBS_NOTIFIERS_MAX (2)
#endif

/**
* @ingroup net_gcoap_conf
* @brief Maximum number of registrations for Observable resources
*
* @note As documented in this file, the implementation is limited to one observer per resource.
* Therefore, every stored observation context is associated with a different resource.
* If you have only one observable resource, you could set this value to 1.
*/
#ifndef CONFIG_GCOAP_OBS_REGISTRATIONS_MAX
#define CONFIG_GCOAP_OBS_REGISTRATIONS_MAX (2)
Expand Down Expand Up @@ -839,6 +860,7 @@ struct gcoap_request_memo {
*/
typedef struct {
sock_udp_ep_t *observer; /**< Client endpoint; unused if null */
sock_udp_ep_t *notifier; /**< Local endpoint to send notifications */
const coap_resource_t *resource; /**< Entity being observed */
uint8_t token[GCOAP_TOKENLEN_MAX]; /**< Client token for notifications */
uint16_t last_msgid; /**< Message ID of last notification */
Expand Down Expand Up @@ -874,6 +896,22 @@ kernel_pid_t gcoap_init(void);
*/
void gcoap_register_listener(gcoap_listener_t *listener);

/**
* @brief Iterate through all registered listeners and check for a resource, matching by @p uri_path

Check warning on line 900 in sys/include/net/gcoap.h

View workflow job for this annotation

GitHub Actions / static-tests

line is longer than 100 characters
*
* This functions returns resources matching a subpath @see COAP_MATCH_SUBTREE.
* If an exact match is required, check with `strncmp()`.
*
* @param[in, out] last_listener A pointer to NULL for the first call, otherwise the last returned listener
* @param[in] last_resource NULL for the first call, otherwise the last returned resource
* @param[in] uri_path The URI path to search for
*
* @return The resource that matches the URI path
*/
const coap_resource_t *gcoap_get_resource_by_path_iterator(const gcoap_listener_t **last_listener,
const coap_resource_t *last_resource,
const char *uri_path);

/**
* @brief Initializes a CoAP request PDU on a buffer.
*
Expand Down Expand Up @@ -1055,6 +1093,9 @@ static inline ssize_t gcoap_response(coap_pkt_t *pdu, uint8_t *buf,
*
* First verifies that an observer has been registered for the resource.
*
* @post If this function returns @see GCOAP_OBS_INIT_OK you have to call
* @ref gcoap_obs_send() afterwards to release a mutex.
*
* @param[out] pdu Notification metadata
* @param[out] buf Buffer containing the PDU
* @param[in] len Length of the buffer
Expand Down
2 changes: 1 addition & 1 deletion sys/include/net/nanocoap.h
Original file line number Diff line number Diff line change
Expand Up @@ -2245,7 +2245,7 @@ extern ssize_t coap_well_known_core_default_handler(coap_pkt_t *pkt, \
* @return <0 if the resource path sorts before the URI
* @return >0 if the resource path sorts after the URI
*/
int coap_match_path(const coap_resource_t *resource, uint8_t *uri);
int coap_match_path(const coap_resource_t *resource, const uint8_t *uri);

#if defined(MODULE_GCOAP) || defined(DOXYGEN)
/**
Expand Down
Loading

0 comments on commit 1626919

Please sign in to comment.