From ab34a9df48004cd4049ee0b5d74520960988ea68 Mon Sep 17 00:00:00 2001 From: Jordan Yates Date: Tue, 21 Jan 2025 08:49:40 +1000 Subject: [PATCH] zbus: remove k_malloc dependency for `ZBUS_RUNTIME_OBSERVERS` Remove the dependency on the system heap existing when enabling `ZBUS_RUNTIME_OBSERVERS`. Instead the previously allocated memory is required to be provided to `zbus_chan_add_obs` (which can still be allocated through malloc). Signed-off-by: Jordan Yates --- include/zephyr/zbus/zbus.h | 21 +++++----- .../zbus/runtime_obs_registration/src/main.c | 5 ++- subsys/zbus/zbus_runtime_observers.c | 19 ++------- .../runtime_observers_registration/src/main.c | 39 ++++++++----------- tests/subsys/zbus/unittests/src/main.c | 17 +++++--- 5 files changed, 45 insertions(+), 56 deletions(-) diff --git a/include/zephyr/zbus/zbus.h b/include/zephyr/zbus/zbus.h index 478268d85b4..c13e77082e7 100644 --- a/include/zephyr/zbus/zbus.h +++ b/include/zephyr/zbus/zbus.h @@ -861,6 +861,14 @@ static inline void zbus_chan_pub_stats_update(const struct zbus_channel *chan) #if defined(CONFIG_ZBUS_RUNTIME_OBSERVERS) || defined(__DOXYGEN__) +/** + * @brief Structure for linking observers to chanels + */ +struct zbus_observer_node { + sys_snode_t node; + const struct zbus_observer *obs; +}; + /** * @brief Add an observer to a channel. * @@ -868,17 +876,17 @@ static inline void zbus_chan_pub_stats_update(const struct zbus_channel *chan) * * @param chan The channel's reference. * @param obs The observer's reference to be added. + * @param node Persistent structure to link the channel to the observer * @param timeout Waiting period to add an observer, * or one of the special values K_NO_WAIT and K_FOREVER. * * @retval 0 Observer added to the channel. * @retval -EALREADY The observer is already present in the channel's runtime observers list. - * @retval -ENOMEM Returned without waiting. * @retval -EAGAIN Waiting period timed out. * @retval -EINVAL Some parameter is invalid. */ int zbus_chan_add_obs(const struct zbus_channel *chan, const struct zbus_observer *obs, - k_timeout_t timeout); + struct zbus_observer_node *node, k_timeout_t timeout); /** * @brief Remove an observer from a channel. @@ -900,15 +908,6 @@ int zbus_chan_add_obs(const struct zbus_channel *chan, const struct zbus_observe int zbus_chan_rm_obs(const struct zbus_channel *chan, const struct zbus_observer *obs, k_timeout_t timeout); -/** @cond INTERNAL_HIDDEN */ - -struct zbus_observer_node { - sys_snode_t node; - const struct zbus_observer *obs; -}; - -/** @endcond */ - #endif /* CONFIG_ZBUS_RUNTIME_OBSERVERS */ /** diff --git a/samples/subsys/zbus/runtime_obs_registration/src/main.c b/samples/subsys/zbus/runtime_obs_registration/src/main.c index f389d61c706..8f66f3644e1 100644 --- a/samples/subsys/zbus/runtime_obs_registration/src/main.c +++ b/samples/subsys/zbus/runtime_obs_registration/src/main.c @@ -44,10 +44,11 @@ int main(void) LOG_INF("System started"); const struct zbus_channel *chan; + struct zbus_observer_node obs_node; while (1) { LOG_INF("Activating filter"); - zbus_chan_add_obs(&raw_data_chan, &filter_lis, K_MSEC(200)); + zbus_chan_add_obs(&raw_data_chan, &filter_lis, &obs_node, K_MSEC(200)); zbus_sub_wait(&state_change_sub, &chan, K_FOREVER); @@ -55,7 +56,7 @@ int main(void) zbus_chan_rm_obs(&raw_data_chan, &filter_lis, K_MSEC(200)); LOG_INF("Bypass filter"); - zbus_chan_add_obs(&raw_data_chan, &consumer_sub, K_MSEC(200)); + zbus_chan_add_obs(&raw_data_chan, &consumer_sub, &obs_node, K_MSEC(200)); zbus_sub_wait(&state_change_sub, &chan, K_FOREVER); diff --git a/subsys/zbus/zbus_runtime_observers.c b/subsys/zbus/zbus_runtime_observers.c index 56c7cbb4530..41c00c9aae7 100644 --- a/subsys/zbus/zbus_runtime_observers.c +++ b/subsys/zbus/zbus_runtime_observers.c @@ -9,7 +9,7 @@ LOG_MODULE_DECLARE(zbus, CONFIG_ZBUS_LOG_LEVEL); int zbus_chan_add_obs(const struct zbus_channel *chan, const struct zbus_observer *obs, - k_timeout_t timeout) + struct zbus_observer_node *node, k_timeout_t timeout) { int err; struct zbus_observer_node *obs_nd, *tmp; @@ -18,6 +18,7 @@ int zbus_chan_add_obs(const struct zbus_channel *chan, const struct zbus_observe _ZBUS_ASSERT(!k_is_in_isr(), "ISR blocked"); _ZBUS_ASSERT(chan != NULL, "chan is required"); _ZBUS_ASSERT(obs != NULL, "obs is required"); + _ZBUS_ASSERT(node != NULL, "node is required"); err = k_sem_take(&chan->data->sem, timeout); if (err) { @@ -46,19 +47,9 @@ int zbus_chan_add_obs(const struct zbus_channel *chan, const struct zbus_observe } } - struct zbus_observer_node *new_obs_nd = k_malloc(sizeof(struct zbus_observer_node)); + node->obs = obs; - if (new_obs_nd == NULL) { - LOG_ERR("Could not allocate observer node the heap is full!"); - - k_sem_give(&chan->data->sem); - - return -ENOMEM; - } - - new_obs_nd->obs = obs; - - sys_slist_append(&chan->data->observers, &new_obs_nd->node); + sys_slist_append(&chan->data->observers, &node->node); k_sem_give(&chan->data->sem); @@ -85,8 +76,6 @@ int zbus_chan_rm_obs(const struct zbus_channel *chan, const struct zbus_observer if (obs_nd->obs == obs) { sys_slist_remove(&chan->data->observers, &prev_obs_nd->node, &obs_nd->node); - k_free(obs_nd); - k_sem_give(&chan->data->sem); return 0; diff --git a/tests/subsys/zbus/runtime_observers_registration/src/main.c b/tests/subsys/zbus/runtime_observers_registration/src/main.c index e5aee7c4c00..b72586aff27 100644 --- a/tests/subsys/zbus/runtime_observers_registration/src/main.c +++ b/tests/subsys/zbus/runtime_observers_registration/src/main.c @@ -69,15 +69,16 @@ ZTEST(basic, test_specification_based__zbus_obs_add_rm_obs) { count_callback1 = 0; struct sensor_data_msg sd = {.a = 10, .b = 100}; + static struct zbus_observer_node n1, n2, n3, n4, n5, n6; /* Tyring to add same static observer as one dynamic */ - zassert_equal(-EEXIST, zbus_chan_add_obs(&chan2, &lis2, K_MSEC(200)), NULL); + zassert_equal(-EEXIST, zbus_chan_add_obs(&chan2, &lis2, &n2, K_MSEC(200)), NULL); zassert_equal(0, zbus_chan_pub(&chan1, &sd, K_MSEC(500)), NULL); zassert_equal(count_callback1, 0, "The counter could not be more than zero, no obs"); - zassert_equal(0, zbus_chan_add_obs(&chan1, &lis1, K_MSEC(200)), NULL); - zassert_equal(-EALREADY, zbus_chan_add_obs(&chan1, &lis1, K_MSEC(200)), + zassert_equal(0, zbus_chan_add_obs(&chan1, &lis1, &n1, K_MSEC(200)), NULL); + zassert_equal(-EALREADY, zbus_chan_add_obs(&chan1, &lis1, &n1, K_MSEC(200)), "It cannot be added twice"); zassert_equal(0, zbus_chan_pub(&chan1, &sd, K_MSEC(500)), NULL); @@ -98,30 +99,21 @@ ZTEST(basic, test_specification_based__zbus_obs_add_rm_obs) zassert_equal(0, zbus_chan_pub(&chan2, &sd, K_MSEC(500)), NULL); zassert_equal(count_callback2, 1, "The counter could not be more than zero, no obs"); - zassert_equal(0, zbus_chan_add_obs(&chan2, &lis3, K_MSEC(200)), NULL); + zassert_equal(0, zbus_chan_add_obs(&chan2, &lis3, &n3, K_MSEC(200)), NULL); - zassert_equal(-EALREADY, zbus_chan_add_obs(&chan2, &lis3, K_MSEC(200)), + zassert_equal(-EALREADY, zbus_chan_add_obs(&chan2, &lis3, &n3, K_MSEC(200)), "It cannot be added twice"); zassert_equal(0, zbus_chan_pub(&chan2, &sd, K_MSEC(500)), NULL); zassert_equal(count_callback2, 3, "The counter could not be more than zero, no obs, %d", count_callback2); count_callback2 = 0; - zassert_equal(0, zbus_chan_add_obs(&chan2, &sub1, K_MSEC(200)), NULL); - zassert_equal(0, zbus_chan_add_obs(&chan2, &sub2, K_MSEC(200)), NULL); - zassert_equal(0, zbus_chan_add_obs(&chan2, &lis4, K_MSEC(200)), "It must add the obs"); - zassert_equal(0, zbus_chan_add_obs(&chan2, &lis5, K_MSEC(200)), "It must add the obs"); - zassert_equal(0, zbus_chan_add_obs(&chan2, &lis6, K_MSEC(200)), "It must add the obs"); + zassert_equal(0, zbus_chan_add_obs(&chan2, &sub1, &n1, K_MSEC(200)), NULL); + zassert_equal(0, zbus_chan_add_obs(&chan2, &sub2, &n2, K_MSEC(200)), NULL); + zassert_equal(0, zbus_chan_add_obs(&chan2, &lis4, &n4, K_MSEC(200)), "It must add the obs"); + zassert_equal(0, zbus_chan_add_obs(&chan2, &lis5, &n5, K_MSEC(200)), "It must add the obs"); + zassert_equal(0, zbus_chan_add_obs(&chan2, &lis6, &n6, K_MSEC(200)), "It must add the obs"); - /* Make the heap full */ - void *mem; - - do { - mem = k_malloc(1); - } while (mem != NULL); - - /* With the heap full it will not be possible to add another obs */ - zassert_equal(-ENOMEM, zbus_chan_add_obs(&chan2, &lis7, K_MSEC(200)), NULL); zassert_equal(0, zbus_chan_pub(&chan2, &sd, K_MSEC(500)), NULL); zassert_equal(count_callback2, 5, NULL); @@ -142,7 +134,9 @@ static struct aux2_wq_data wq_handler; static void wq_dh_cb(struct k_work *item) { - zassert_equal(-EAGAIN, zbus_chan_add_obs(&chan2, &sub1, K_MSEC(200)), NULL); + static struct zbus_observer_node node; + + zassert_equal(-EAGAIN, zbus_chan_add_obs(&chan2, &sub1, &node, K_MSEC(200)), NULL); zassert_equal(-EAGAIN, zbus_chan_rm_obs(&chan2, &sub2, K_MSEC(200)), NULL); } @@ -192,11 +186,12 @@ ZBUS_CHAN_ADD_OBS(chan4, prio_lis4, 2); ZTEST(basic, test_specification_based__zbus_obs_priority) { struct sensor_data_msg sd = {.a = 70, .b = 116}; + static struct zbus_observer_node n1, n2; execution_sequence_idx = 0; - zassert_equal(0, zbus_chan_add_obs(&chan4, &prio_lis2, K_MSEC(200)), NULL); - zassert_equal(0, zbus_chan_add_obs(&chan4, &prio_lis1, K_MSEC(200)), NULL); + zassert_equal(0, zbus_chan_add_obs(&chan4, &prio_lis2, &n1, K_MSEC(200)), NULL); + zassert_equal(0, zbus_chan_add_obs(&chan4, &prio_lis1, &n2, K_MSEC(200)), NULL); zassert_equal(0, zbus_chan_pub(&chan4, &sd, K_MSEC(500)), NULL); diff --git a/tests/subsys/zbus/unittests/src/main.c b/tests/subsys/zbus/unittests/src/main.c index c94d0c28b0e..a40a70082f1 100644 --- a/tests/subsys/zbus/unittests/src/main.c +++ b/tests/subsys/zbus/unittests/src/main.c @@ -137,6 +137,7 @@ static struct action_msg ga; static void isr_handler(const void *operation) { + static struct zbus_observer_node fast_node, aux_node; enum operation *op = (enum operation *)operation; switch (*op) { @@ -156,7 +157,7 @@ static void isr_handler(const void *operation) isr_return = zbus_chan_finish(NULL); break; case ADD_OBS_ISR_INVAL: - isr_return = zbus_chan_add_obs(&aux2_chan, &fast_lis, K_MSEC(200)); + isr_return = zbus_chan_add_obs(&aux2_chan, &fast_lis, &fast_node, K_MSEC(200)); break; case RM_OBS_ISR_INVAL: isr_return = zbus_chan_rm_obs(&aux2_chan, &fast_lis, K_MSEC(200)); @@ -177,7 +178,7 @@ static void isr_handler(const void *operation) isr_return = zbus_chan_finish(&aux2_chan); break; case ADD_OBS_ISR: - isr_return = zbus_chan_add_obs(&aux2_chan, NULL, K_MSEC(200)); + isr_return = zbus_chan_add_obs(&aux2_chan, NULL, &aux_node, K_MSEC(200)); break; case RM_OBS_ISR: isr_return = zbus_chan_rm_obs(&aux2_chan, NULL, K_MSEC(200)); @@ -244,6 +245,7 @@ const STRUCT_SECTION_ITERABLE(zbus_observer, invalid_obs) = { ZTEST(basic, test_specification_based__zbus_chan) { + static struct zbus_observer_node node; struct action_msg a = {0}; /* Trying invalid parameters */ @@ -269,9 +271,11 @@ ZTEST(basic, test_specification_based__zbus_chan) zassert_equal(-EFAULT, zbus_chan_finish(NULL), "It must be -EFAULT"); - zassert_equal(-EFAULT, zbus_chan_add_obs(NULL, &sub1, K_MSEC(200)), NULL); + zassert_equal(-EFAULT, zbus_chan_add_obs(NULL, &sub1, &node, K_MSEC(200)), NULL); - zassert_equal(-EFAULT, zbus_chan_add_obs(&aux2_chan, NULL, K_MSEC(200)), NULL); + zassert_equal(-EFAULT, zbus_chan_add_obs(&aux2_chan, NULL, &node, K_MSEC(200)), NULL); + + zassert_equal(-EFAULT, zbus_chan_add_obs(&aux2_chan, &sub1, NULL, K_MSEC(200)), NULL); zassert_equal(-EFAULT, zbus_chan_rm_obs(NULL, &sub1, K_MSEC(200)), NULL); @@ -326,7 +330,7 @@ ZTEST(basic, test_specification_based__zbus_chan) k_msleep(100); - zassert_equal(0, zbus_chan_add_obs(&stuck_chan, &sub1, K_MSEC(200)), NULL); + zassert_equal(0, zbus_chan_add_obs(&stuck_chan, &sub1, &node, K_MSEC(200)), NULL); zassert_equal(0, zbus_chan_notify(&stuck_chan, K_MSEC(200)), "It must finish correctly"); @@ -599,6 +603,7 @@ ZTEST(basic, test_hard_channel) ZTEST(basic, test_specification_based__zbus_obs_set_enable) { + struct zbus_observer_node node; bool enable; count_fast = 0; @@ -617,7 +622,7 @@ ZTEST(basic, test_specification_based__zbus_obs_set_enable) zbus_obs_is_enabled(&rt_fast_lis, &enable); zassert_equal(false, enable); - zassert_equal(0, zbus_chan_add_obs(&aux1_chan, &rt_fast_lis, K_MSEC(200)), NULL); + zassert_equal(0, zbus_chan_add_obs(&aux1_chan, &rt_fast_lis, &node, K_MSEC(200)), NULL); zassert_equal(0, zbus_obs_set_enable(&fast_lis, false), "Must be zero. The observer must be disabled");