From d7557102c08c679999e0a67d3ee8edae601d839b Mon Sep 17 00:00:00 2001 From: Jamie McCrae Date: Fri, 9 Dec 2022 09:42:17 +0000 Subject: [PATCH] mgmt: mcumgr: Add iterable section to register MCUmgr handlers This replaces the requirement for applications to manually register MCUmgr handlers by having an iterable section which then automatically registers the handlers at boot time. Signed-off-by: Jamie McCrae --- cmake/linker_script/common/common-rom.cmake | 4 ++ .../linker/common-rom/common-rom-misc.ld | 4 ++ include/zephyr/mgmt/mcumgr/mgmt/handlers.h | 55 +++++++++++++++++++ .../src/smp_svr.c | 23 -------- samples/subsys/mgmt/mcumgr/smp_svr/src/main.c | 24 -------- subsys/mgmt/mcumgr/grp/fs_mgmt/src/fs_mgmt.c | 3 + .../mgmt/mcumgr/grp/img_mgmt/src/img_mgmt.c | 9 +-- subsys/mgmt/mcumgr/grp/os_mgmt/src/os_mgmt.c | 6 +- .../mcumgr/grp/shell_mgmt/src/shell_mgmt.c | 6 +- .../mgmt/mcumgr/grp/stat_mgmt/src/stat_mgmt.c | 6 +- .../mcumgr/grp/zephyr_basic/src/basic_mgmt.c | 9 +-- subsys/mgmt/mcumgr/mgmt/src/mgmt.c | 18 ++++++ .../mcumgr/fs_mgmt_hash_supported/src/main.c | 3 - .../mgmt/mcumgr/os_mgmt_echo/src/main.c | 3 - .../mgmt/mcumgr/os_mgmt_info/src/build_date.c | 10 +--- .../mgmt/mcumgr/os_mgmt_info/src/limited.c | 10 +--- .../mgmt/mcumgr/os_mgmt_info/src/main.c | 3 - 17 files changed, 104 insertions(+), 92 deletions(-) create mode 100644 include/zephyr/mgmt/mcumgr/mgmt/handlers.h diff --git a/cmake/linker_script/common/common-rom.cmake b/cmake/linker_script/common/common-rom.cmake index b9c7149c0df..f4c8a899870 100644 --- a/cmake/linker_script/common/common-rom.cmake +++ b/cmake/linker_script/common/common-rom.cmake @@ -146,6 +146,10 @@ if(CONFIG_SENSOR_INFO) zephyr_iterable_section(NAME sensor_info KVMA RAM_REGION GROUP RODATA_REGION SUBALIGN 4) endif() +if(CONFIG_MCUMGR) + zephyr_iterable_section(NAME mcumgr_handler KVMA RAM_REGION GROUP RODATA_REGION SUBALIGN 4) +endif() + zephyr_iterable_section(NAME k_p4wq_initparam KVMA RAM_REGION GROUP RODATA_REGION SUBALIGN 4) if(CONFIG_EMUL) diff --git a/include/zephyr/linker/common-rom/common-rom-misc.ld b/include/zephyr/linker/common-rom/common-rom-misc.ld index c629437f11c..310c1ec763a 100644 --- a/include/zephyr/linker/common-rom/common-rom-misc.ld +++ b/include/zephyr/linker/common-rom/common-rom-misc.ld @@ -12,6 +12,10 @@ ITERABLE_SECTION_ROM(sensor_info, 4) #endif +#if defined(CONFIG_MCUMGR) + ITERABLE_SECTION_ROM(mcumgr_handler, 4) +#endif + #if defined(CONFIG_EMUL) SECTION_DATA_PROLOGUE(emulators_section,,) { diff --git a/include/zephyr/mgmt/mcumgr/mgmt/handlers.h b/include/zephyr/mgmt/mcumgr/mgmt/handlers.h new file mode 100644 index 00000000000..f646bb6c4c7 --- /dev/null +++ b/include/zephyr/mgmt/mcumgr/mgmt/handlers.h @@ -0,0 +1,55 @@ +/* + * Copyright (c) 2022 Nordic Semiconductor ASA + * + * SPDX-License-Identifier: Apache-2.0 + */ + +#ifndef H_MCUMGR_MGMT_HANDLERS_ +#define H_MCUMGR_MGMT_HANDLERS_ + +#include +#include + +#ifdef __cplusplus +extern "C" { +#endif /* __cplusplus */ + +/** + * @brief MCUmgr handler registration API + * @defgroup mcumgr_handler_api MCUmgr handler API + * @ingroup mcumgr + * @{ + */ + +/** Type definition for a MCUmgr handler initialisation function */ +typedef void (*mcumgr_handler_init_t)(void); + +/** @cond INTERNAL_HIDDEN */ +struct mcumgr_handler { + /** Initialisation function to be called */ + const mcumgr_handler_init_t init; +}; +/** @endcond */ + +/** + * @brief Define a MCUmgr handler to register. + * + * This adds a new entry to the iterable section linker list of MCUmgr handers. + * + * @param name Name of the MCUmgr handler to registger. + * @param _init Init function to be called (mcumgr_handler_init_t). + */ +#define MCUMGR_HANDLER_DEFINE(name, _init) \ + STRUCT_SECTION_ITERABLE(mcumgr_handler, name) = { \ + .init = _init, \ + } + +#ifdef __cplusplus +} +#endif /* __cplusplus */ + +/** + * @} + */ + +#endif /* H_MCUMGR_MGMT_HANDLERS_ */ diff --git a/samples/boards/nrf/mesh/onoff_level_lighting_vnd_app/src/smp_svr.c b/samples/boards/nrf/mesh/onoff_level_lighting_vnd_app/src/smp_svr.c index 248a284d104..79235952ac4 100644 --- a/samples/boards/nrf/mesh/onoff_level_lighting_vnd_app/src/smp_svr.c +++ b/samples/boards/nrf/mesh/onoff_level_lighting_vnd_app/src/smp_svr.c @@ -11,15 +11,6 @@ #include #include -#ifdef CONFIG_MCUMGR_CMD_FS_MGMT -#include -#endif -#ifdef CONFIG_MCUMGR_CMD_IMG_MGMT -#include -#endif -#ifdef CONFIG_MCUMGR_CMD_OS_MGMT -#include -#endif #ifdef CONFIG_MCUMGR_CMD_STAT_MGMT #include #endif @@ -43,20 +34,6 @@ void smp_svr_init(void) rc = STATS_INIT_AND_REG(smp_svr_stats, STATS_SIZE_32, "smp_svr_stats"); __ASSERT_NO_MSG(rc == 0); - - /* Register the built-in mcumgr command handlers. */ -#ifdef CONFIG_MCUMGR_CMD_FS_MGMT - fs_mgmt_register_group(); -#endif -#ifdef CONFIG_MCUMGR_CMD_OS_MGMT - os_mgmt_register_group(); -#endif -#ifdef CONFIG_MCUMGR_CMD_IMG_MGMT - img_mgmt_register_group(); -#endif -#ifdef CONFIG_MCUMGR_CMD_STAT_MGMT - stat_mgmt_register_group(); -#endif } static void smp_svr_timer_handler(struct k_timer *dummy) diff --git a/samples/subsys/mgmt/mcumgr/smp_svr/src/main.c b/samples/subsys/mgmt/mcumgr/smp_svr/src/main.c index 1e7ab586036..23ea1fb7f49 100644 --- a/samples/subsys/mgmt/mcumgr/smp_svr/src/main.c +++ b/samples/subsys/mgmt/mcumgr/smp_svr/src/main.c @@ -13,20 +13,10 @@ #include #include #include -#include -#endif -#ifdef CONFIG_MCUMGR_CMD_OS_MGMT -#include -#endif -#ifdef CONFIG_MCUMGR_CMD_IMG_MGMT -#include #endif #ifdef CONFIG_MCUMGR_CMD_STAT_MGMT #include #endif -#ifdef CONFIG_MCUMGR_CMD_SHELL_MGMT -#include -#endif #define LOG_LEVEL LOG_LEVEL_DBG #include @@ -75,20 +65,6 @@ void main(void) if (rc < 0) { LOG_ERR("Error mounting littlefs [%d]", rc); } - - fs_mgmt_register_group(); -#endif -#ifdef CONFIG_MCUMGR_CMD_OS_MGMT - os_mgmt_register_group(); -#endif -#ifdef CONFIG_MCUMGR_CMD_IMG_MGMT - img_mgmt_register_group(); -#endif -#ifdef CONFIG_MCUMGR_CMD_STAT_MGMT - stat_mgmt_register_group(); -#endif -#ifdef CONFIG_MCUMGR_CMD_SHELL_MGMT - shell_mgmt_register_group(); #endif #ifdef CONFIG_MCUMGR_SMP_BT start_smp_bluetooth(); diff --git a/subsys/mgmt/mcumgr/grp/fs_mgmt/src/fs_mgmt.c b/subsys/mgmt/mcumgr/grp/fs_mgmt/src/fs_mgmt.c index 187c168c965..48e4fb6409f 100644 --- a/subsys/mgmt/mcumgr/grp/fs_mgmt/src/fs_mgmt.c +++ b/subsys/mgmt/mcumgr/grp/fs_mgmt/src/fs_mgmt.c @@ -11,6 +11,7 @@ #include #include #include +#include #include #include #include @@ -688,3 +689,5 @@ void fs_mgmt_register_group(void) #endif #endif } + +MCUMGR_HANDLER_DEFINE(fs_mgmt, fs_mgmt_register_group); diff --git a/subsys/mgmt/mcumgr/grp/img_mgmt/src/img_mgmt.c b/subsys/mgmt/mcumgr/grp/img_mgmt/src/img_mgmt.c index fff46b085dd..680cfbe1ce6 100644 --- a/subsys/mgmt/mcumgr/grp/img_mgmt/src/img_mgmt.c +++ b/subsys/mgmt/mcumgr/grp/img_mgmt/src/img_mgmt.c @@ -17,6 +17,7 @@ #include #include +#include #include #include @@ -611,14 +612,14 @@ static struct mgmt_group img_mgmt_group = { }; -void -img_mgmt_register_group(void) +void img_mgmt_register_group(void) { mgmt_register_group(&img_mgmt_group); } -void -img_mgmt_unregister_group(void) +void img_mgmt_unregister_group(void) { mgmt_unregister_group(&img_mgmt_group); } + +MCUMGR_HANDLER_DEFINE(img_mgmt, img_mgmt_register_group); diff --git a/subsys/mgmt/mcumgr/grp/os_mgmt/src/os_mgmt.c b/subsys/mgmt/mcumgr/grp/os_mgmt/src/os_mgmt.c index 39c75291a03..e0f05094e85 100644 --- a/subsys/mgmt/mcumgr/grp/os_mgmt/src/os_mgmt.c +++ b/subsys/mgmt/mcumgr/grp/os_mgmt/src/os_mgmt.c @@ -12,6 +12,7 @@ #include #include #include +#include #include #include #include @@ -680,7 +681,4 @@ void os_mgmt_register_group(void) mgmt_register_group(&os_mgmt_group); } -void os_mgmt_module_init(void) -{ - os_mgmt_register_group(); -} +MCUMGR_HANDLER_DEFINE(os_mgmt, os_mgmt_register_group); diff --git a/subsys/mgmt/mcumgr/grp/shell_mgmt/src/shell_mgmt.c b/subsys/mgmt/mcumgr/grp/shell_mgmt/src/shell_mgmt.c index f18537e12b1..a6d428ae3b2 100644 --- a/subsys/mgmt/mcumgr/grp/shell_mgmt/src/shell_mgmt.c +++ b/subsys/mgmt/mcumgr/grp/shell_mgmt/src/shell_mgmt.c @@ -7,6 +7,7 @@ #include #include #include +#include #include #include #include @@ -135,8 +136,9 @@ static struct mgmt_group shell_mgmt_group = { }; -void -shell_mgmt_register_group(void) +void shell_mgmt_register_group(void) { mgmt_register_group(&shell_mgmt_group); } + +MCUMGR_HANDLER_DEFINE(shell_mgmt, shell_mgmt_register_group); diff --git a/subsys/mgmt/mcumgr/grp/stat_mgmt/src/stat_mgmt.c b/subsys/mgmt/mcumgr/grp/stat_mgmt/src/stat_mgmt.c index 917ec47b648..8cd37d17d90 100644 --- a/subsys/mgmt/mcumgr/grp/stat_mgmt/src/stat_mgmt.c +++ b/subsys/mgmt/mcumgr/grp/stat_mgmt/src/stat_mgmt.c @@ -14,6 +14,7 @@ #include #include +#include #include #include @@ -237,8 +238,9 @@ static struct mgmt_group stat_mgmt_group = { .mg_group_id = MGMT_GROUP_ID_STAT, }; -void -stat_mgmt_register_group(void) +void stat_mgmt_register_group(void) { mgmt_register_group(&stat_mgmt_group); } + +MCUMGR_HANDLER_DEFINE(stat_mgmt, stat_mgmt_register_group); diff --git a/subsys/mgmt/mcumgr/grp/zephyr_basic/src/basic_mgmt.c b/subsys/mgmt/mcumgr/grp/zephyr_basic/src/basic_mgmt.c index bfd64c89249..c893f818f66 100644 --- a/subsys/mgmt/mcumgr/grp/zephyr_basic/src/basic_mgmt.c +++ b/subsys/mgmt/mcumgr/grp/zephyr_basic/src/basic_mgmt.c @@ -10,6 +10,7 @@ #include #include +#include #include LOG_MODULE_REGISTER(mcumgr_zephyr_grp); @@ -59,13 +60,9 @@ static struct mgmt_group zephyr_basic_mgmt_group = { .mg_group_id = (ZEPHYR_MGMT_GRP_BASIC), }; -static int zephyr_basic_mgmt_init(const struct device *dev) +void zephyr_basic_mgmt_init(void) { - ARG_UNUSED(dev); - - LOG_INF("Registering Zephyr basic mgmt group"); mgmt_register_group(&zephyr_basic_mgmt_group); - return 0; } -SYS_INIT(zephyr_basic_mgmt_init, APPLICATION, CONFIG_APPLICATION_INIT_PRIORITY); +MCUMGR_HANDLER_DEFINE(zephyr_basic_mgmt, zephyr_basic_mgmt_init); diff --git a/subsys/mgmt/mcumgr/mgmt/src/mgmt.c b/subsys/mgmt/mcumgr/mgmt/src/mgmt.c index fcdaf634f48..d20e9f3060d 100644 --- a/subsys/mgmt/mcumgr/mgmt/src/mgmt.c +++ b/subsys/mgmt/mcumgr/mgmt/src/mgmt.c @@ -7,7 +7,9 @@ #include #include +#include #include +#include #include #ifdef CONFIG_MCUMGR_MGMT_NOTIFICATION_HOOKS @@ -123,3 +125,19 @@ int32_t mgmt_callback_notify(uint32_t event, void *data, size_t data_size) return return_rc; } #endif + +/* Processes all registered MCUmgr handlers at start up and registers them */ +static int mcumgr_handlers_init(const struct device *dev) +{ + ARG_UNUSED(dev); + + STRUCT_SECTION_FOREACH(mcumgr_handler, handler) { + if (handler->init) { + handler->init(); + } + } + + return 0; +} + +SYS_INIT(mcumgr_handlers_init, APPLICATION, CONFIG_APPLICATION_INIT_PRIORITY); diff --git a/tests/subsys/mgmt/mcumgr/fs_mgmt_hash_supported/src/main.c b/tests/subsys/mgmt/mcumgr/fs_mgmt_hash_supported/src/main.c index c9225071a5f..b3358ed625e 100644 --- a/tests/subsys/mgmt/mcumgr/fs_mgmt_hash_supported/src/main.c +++ b/tests/subsys/mgmt/mcumgr/fs_mgmt_hash_supported/src/main.c @@ -56,9 +56,6 @@ ZTEST(fs_mgmt_hash_supported, test_supported) #endif }; - /* Register os_mgmt mcumgr group */ - fs_mgmt_register_group(); - /* Enable dummy SMP backend and ready for usage */ smp_dummy_enable(); smp_dummy_clear_state(); diff --git a/tests/subsys/mgmt/mcumgr/os_mgmt_echo/src/main.c b/tests/subsys/mgmt/mcumgr/os_mgmt_echo/src/main.c index a6275720a99..2f48fd1d215 100644 --- a/tests/subsys/mgmt/mcumgr/os_mgmt_echo/src/main.c +++ b/tests/subsys/mgmt/mcumgr/os_mgmt_echo/src/main.c @@ -38,9 +38,6 @@ ZTEST(os_mgmt_echo, test_echo) { struct net_buf *nb; - /* Register os_mgmt mcumgr group */ - os_mgmt_register_group(); - /* Enable dummy SMP backend and ready for usage */ smp_dummy_enable(); smp_dummy_clear_state(); diff --git a/tests/subsys/mgmt/mcumgr/os_mgmt_info/src/build_date.c b/tests/subsys/mgmt/mcumgr/os_mgmt_info/src/build_date.c index 54f53ac370b..4af977895c3 100644 --- a/tests/subsys/mgmt/mcumgr/os_mgmt_info/src/build_date.c +++ b/tests/subsys/mgmt/mcumgr/os_mgmt_info/src/build_date.c @@ -246,14 +246,6 @@ ZTEST(os_mgmt_info_build_date, test_info_build_date_2_all) abs(expected_time_seconds - received_time_seconds)); } -static void *setup_tests(void) -{ - /* Register os_mgmt mcumgr group */ - os_mgmt_register_group(); - - return NULL; -} - static void cleanup_test(void *p) { if (nb != NULL) { @@ -263,6 +255,6 @@ static void cleanup_test(void *p) } /* Build date/time test set */ -ZTEST_SUITE(os_mgmt_info_build_date, NULL, setup_tests, NULL, cleanup_test, NULL); +ZTEST_SUITE(os_mgmt_info_build_date, NULL, NULL, NULL, cleanup_test, NULL); #endif diff --git a/tests/subsys/mgmt/mcumgr/os_mgmt_info/src/limited.c b/tests/subsys/mgmt/mcumgr/os_mgmt_info/src/limited.c index de0230f00a4..f8899734351 100644 --- a/tests/subsys/mgmt/mcumgr/os_mgmt_info/src/limited.c +++ b/tests/subsys/mgmt/mcumgr/os_mgmt_info/src/limited.c @@ -165,14 +165,6 @@ ZTEST(os_mgmt_info_limited, test_info_2_all) rc); } -static void *setup_tests(void) -{ - /* Register os_mgmt mcumgr group */ - os_mgmt_register_group(); - - return NULL; -} - static void cleanup_test(void *p) { if (nb != NULL) { @@ -182,6 +174,6 @@ static void cleanup_test(void *p) } /* Limited size buffer test set */ -ZTEST_SUITE(os_mgmt_info_limited, NULL, setup_tests, NULL, cleanup_test, NULL); +ZTEST_SUITE(os_mgmt_info_limited, NULL, NULL, NULL, cleanup_test, NULL); #endif diff --git a/tests/subsys/mgmt/mcumgr/os_mgmt_info/src/main.c b/tests/subsys/mgmt/mcumgr/os_mgmt_info/src/main.c index 8b2f85a51a3..4d358481b88 100644 --- a/tests/subsys/mgmt/mcumgr/os_mgmt_info/src/main.c +++ b/tests/subsys/mgmt/mcumgr/os_mgmt_info/src/main.c @@ -1497,9 +1497,6 @@ static void cleanup_test(void *p) void test_main(void) { - /* Register os_mgmt mcumgr group */ - os_mgmt_register_group(); - while (test_state.test_set < OS_MGMT_TEST_SET_COUNT) { ztest_run_all(&test_state); ++test_state.test_set;