From c08bec3efaeb67ec954c3770095b61b4d4a78dd0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20Ga=C5=82at?= Date: Mon, 20 May 2024 14:30:25 +0000 Subject: [PATCH] sample: fix issue with sending malformed payload on autoconnect MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit KRKNWK-18805 The issue has been investigated, and it was caused by the `sid_put_msg` performing shallow copy of the data to send. This was not intended, and will be fixed in next release. Temporary fix for this issue, is to track the payload passed to `sid_put_msg` and free it when message has been send, or when there will be no further attempts with sending. So this commit is temporary and should be reverted for the next release. Signed-off-by: Robert Gałat --- samples/sid_end_device/include/sidewalk.h | 5 +++ samples/sid_end_device/src/cli/app.c | 14 ++++++++ samples/sid_end_device/src/hello/app.c | 15 ++++++++ .../src/sensor_monitoring/app.c | 14 ++++++++ samples/sid_end_device/src/sidewalk.c | 36 ++++++++++++++++--- 5 files changed, 80 insertions(+), 4 deletions(-) diff --git a/samples/sid_end_device/include/sidewalk.h b/samples/sid_end_device/include/sidewalk.h index 35e13073b7..60d0ea1c19 100644 --- a/samples/sid_end_device/include/sidewalk.h +++ b/samples/sid_end_device/include/sidewalk.h @@ -6,7 +6,9 @@ #ifndef SIDEWALK_APP_H #define SIDEWALK_APP_H +#include #include +#include typedef enum { SID_EVENT_SIDEWALK, @@ -32,6 +34,7 @@ typedef struct { } sidewalk_ctx_t; typedef struct { + sys_snode_t node; struct sid_msg msg; struct sid_msg_desc desc; } sidewalk_msg_t; @@ -46,4 +49,6 @@ void sidewalk_start(sidewalk_ctx_t *context); int sidewalk_event_send(sidewalk_event_t event, void *ctx); +sidewalk_msg_t *get_message_buffer(uint16_t message_id); + #endif /* SIDEWALK_APP_H */ diff --git a/samples/sid_end_device/src/cli/app.c b/samples/sid_end_device/src/cli/app.c index 0dad5c53d7..3799a48d89 100644 --- a/samples/sid_end_device/src/cli/app.c +++ b/samples/sid_end_device/src/cli/app.c @@ -40,6 +40,13 @@ static void on_sidewalk_msg_sent(const struct sid_msg_desc *msg_desc, void *cont LOG_INF("Message send success"); printk(JSON_NEW_LINE(JSON_OBJ(JSON_NAME( "on_msg_sent", JSON_OBJ(JSON_VAL_sid_msg_desc("sid_msg_desc", msg_desc, 0)))))); + sidewalk_msg_t *message = get_message_buffer(msg_desc->id); + if (message == NULL) { + LOG_ERR("failed to find message buffer to clean"); + return; + } + sid_hal_free(message->msg.data); + sid_hal_free(message); } static void on_sidewalk_send_error(sid_error_t error, const struct sid_msg_desc *msg_desc, @@ -50,6 +57,13 @@ static void on_sidewalk_send_error(sid_error_t error, const struct sid_msg_desc "on_send_error", JSON_OBJ(JSON_LIST_2(JSON_VAL_sid_error_t("error", error), JSON_VAL_sid_msg_desc("sid_msg_desc", msg_desc, 0))))))); + sidewalk_msg_t *message = get_message_buffer(msg_desc->id); + if (message == NULL) { + LOG_ERR("failed to find message buffer to clean"); + return; + } + sid_hal_free(message->msg.data); + sid_hal_free(message); } static void on_sidewalk_factory_reset(void *context) diff --git a/samples/sid_end_device/src/hello/app.c b/samples/sid_end_device/src/hello/app.c index b9a6594224..78557abec9 100644 --- a/samples/sid_end_device/src/hello/app.c +++ b/samples/sid_end_device/src/hello/app.c @@ -90,6 +90,13 @@ static void on_sidewalk_msg_sent(const struct sid_msg_desc *msg_desc, void *cont "on_msg_sent", JSON_OBJ(JSON_VAL_sid_msg_desc("sid_msg_desc", msg_desc, 0)))))); application_state_sending(&global_state_notifier, false); + sidewalk_msg_t *message = get_message_buffer(msg_desc->id); + if (message == NULL) { + LOG_ERR("failed to find message buffer to clean"); + return; + } + sid_hal_free(message->msg.data); + sid_hal_free(message); } static void on_sidewalk_send_error(sid_error_t error, const struct sid_msg_desc *msg_desc, @@ -102,6 +109,14 @@ static void on_sidewalk_send_error(sid_error_t error, const struct sid_msg_desc JSON_VAL_sid_msg_desc("sid_msg_desc", msg_desc, 0))))))); application_state_sending(&global_state_notifier, false); + + sidewalk_msg_t *message = get_message_buffer(msg_desc->id); + if (message == NULL) { + LOG_ERR("failed to find message buffer to clean"); + return; + } + sid_hal_free(message->msg.data); + sid_hal_free(message); } static void on_sidewalk_factory_reset(void *context) diff --git a/samples/sid_end_device/src/sensor_monitoring/app.c b/samples/sid_end_device/src/sensor_monitoring/app.c index fd67048601..6a43a1d721 100644 --- a/samples/sid_end_device/src/sensor_monitoring/app.c +++ b/samples/sid_end_device/src/sensor_monitoring/app.c @@ -71,6 +71,13 @@ static void on_sidewalk_msg_received(const struct sid_msg_desc *msg_desc, const static void on_sidewalk_msg_sent(const struct sid_msg_desc *msg_desc, void *context) { LOG_DBG("sent message(type: %d, id: %u)", (int)msg_desc->type, msg_desc->id); + sidewalk_msg_t *message = get_message_buffer(msg_desc->id); + if (message == NULL) { + LOG_ERR("failed to find message buffer to clean"); + return; + } + sid_hal_free(message->msg.data); + sid_hal_free(message); } static void on_sidewalk_send_error(sid_error_t error, const struct sid_msg_desc *msg_desc, @@ -78,6 +85,13 @@ static void on_sidewalk_send_error(sid_error_t error, const struct sid_msg_desc { LOG_ERR("Send message err %d", (int)error); LOG_DBG("Failed to send message(type: %d, id: %u)", (int)msg_desc->type, msg_desc->id); + sidewalk_msg_t *message = get_message_buffer(msg_desc->id); + if (message == NULL) { + LOG_ERR("failed to find message buffer to clean"); + return; + } + sid_hal_free(message->msg.data); + sid_hal_free(message); } static void on_sidewalk_factory_reset(void *context) diff --git a/samples/sid_end_device/src/sidewalk.c b/samples/sid_end_device/src/sidewalk.c index 020f2cc7e1..373b33edda 100644 --- a/samples/sid_end_device/src/sidewalk.c +++ b/samples/sid_end_device/src/sidewalk.c @@ -3,7 +3,6 @@ * * SPDX-License-Identifier: LicenseRef-Nordic-5-Clause */ - #include #include @@ -43,6 +42,24 @@ LOG_MODULE_REGISTER(sidewalk_app, CONFIG_SIDEWALK_LOG_LEVEL); static struct k_thread sid_thread; K_THREAD_STACK_DEFINE(sid_thread_stack, CONFIG_SIDEWALK_THREAD_STACK_SIZE); +sys_slist_t pending_message_list = SYS_SLIST_STATIC_INIT(&pending_message_list); + +sidewalk_msg_t *get_message_buffer(uint16_t message_id) +{ + sidewalk_msg_t *pending_message; + sidewalk_msg_t *iterator; + SYS_SLIST_FOR_EACH_CONTAINER_SAFE (&pending_message_list, pending_message, iterator, node) { + if (pending_message->desc.id == message_id) { + if (sys_slist_find_and_remove(&pending_message_list, + &pending_message->node) == false) { + LOG_ERR("Failed to remove pending message from list"); + }; + return pending_message; + } + } + return NULL; +} + typedef struct sm_s { struct smf_ctx ctx; struct k_msgq msgq; @@ -57,11 +74,13 @@ enum state { static void state_sidewalk_run(void *o); static void state_sidewalk_entry(void *o); +static void state_sidewalk_exit(void *o); static void state_dfu_entry(void *o); static void state_dfu_run(void *o); static const struct smf_state sid_states[] = { - [STATE_SIDEWALK] = SMF_CREATE_STATE(state_sidewalk_entry, state_sidewalk_run, NULL), + [STATE_SIDEWALK] = + SMF_CREATE_STATE(state_sidewalk_entry, state_sidewalk_run, state_sidewalk_exit), [STATE_DFU] = SMF_CREATE_STATE(state_dfu_entry, state_dfu_run, NULL), }; @@ -283,8 +302,7 @@ static void state_sidewalk_run(void *o) LOG_ERR("sid send err %d", (int)e); } LOG_DBG("sid send (type: %d, id: %u)", (int)p_msg->desc.type, p_msg->desc.id); - sid_hal_free(p_msg->msg.data); - sid_hal_free(p_msg); + sys_slist_append(&pending_message_list, &p_msg->node); } break; case SID_EVENT_CONNECT: { if (!(sm->sid->config.link_mask & SID_LINK_TYPE_1)) { @@ -345,6 +363,16 @@ static void state_sidewalk_run(void *o) #endif /* CONFIG_SID_END_DEVICE_CLI */ } +static void state_sidewalk_exit(void *o) +{ + while (sys_slist_is_empty(&pending_message_list) == false) { + sidewalk_msg_t *message; + message = SYS_SLIST_CONTAINER(sys_slist_get(&pending_message_list), message, node); + sid_hal_free(message->msg.data); + sid_hal_free(message); + } +} + static void state_dfu_entry(void *o) { sm_t *sm = (sm_t *)o;