From 220a57b0b1cf31e1c5061d81e1fabd277bdde60b Mon Sep 17 00:00:00 2001 From: Kuno Heltborg Date: Wed, 9 Aug 2023 08:15:22 +0200 Subject: [PATCH] mgmt: mcumgr: improve custom payload types Use a flag in `c:struct:mgmt_handler` to skip the cbor start and end byte, and instead use pure custom user defined payload. Signed-off-by: Kuno Heltborg --- include/zephyr/mgmt/mcumgr/mgmt/mgmt.h | 21 ++++++++++- subsys/mgmt/mcumgr/mgmt/Kconfig | 7 ++++ subsys/mgmt/mcumgr/mgmt/src/mgmt.c | 23 +++++++++--- subsys/mgmt/mcumgr/smp/src/smp.c | 51 +++++++++++++++++--------- 4 files changed, 77 insertions(+), 25 deletions(-) diff --git a/include/zephyr/mgmt/mcumgr/mgmt/mgmt.h b/include/zephyr/mgmt/mcumgr/mgmt/mgmt.h index ed84b03763..7699dd57cc 100644 --- a/include/zephyr/mgmt/mcumgr/mgmt/mgmt.h +++ b/include/zephyr/mgmt/mcumgr/mgmt/mgmt.h @@ -67,6 +67,7 @@ typedef int (*mgmt_handler_fn)(struct smp_streamer *ctxt); /** * @brief Read handler and write handler for a single command ID. + * Set use_custom_payload to true when using a user defined payload type */ struct mgmt_handler { mgmt_handler_fn mh_read; @@ -96,6 +97,11 @@ struct mgmt_group { */ smp_translate_error_fn mg_translate_error; #endif + +#if defined(CONFIG_MCUMGR_MGMT_CUSTOM_PAYLOAD) + /** Should be true when using user defined payload */ + bool custom_payload; +#endif }; /** @@ -126,13 +132,24 @@ const struct mgmt_handler *mgmt_find_handler(uint16_t group_id, uint16_t command /** * @brief Finds a registered command group. * - * @param group_id The command group id to find. + * @param group_id The group id of the command group to find. * - * @return The requested command group on success; + * @return The requested group on success; * NULL on failure. */ const struct mgmt_group *mgmt_find_group(uint16_t group_id); +/** + * @brief Finds a registered command handler. + * + * @param group The group of the command to find. + * @param command_id The ID of the command to find. + * + * @return The requested command handler on success; + * NULL on failure. + */ +const struct mgmt_handler *mgmt_get_handler(const struct mgmt_group *group, uint16_t command_id); + #if IS_ENABLED(CONFIG_MCUMGR_SMP_SUPPORT_ORIGINAL_PROTOCOL) /** * @brief Finds a registered error translation function for converting from SMP diff --git a/subsys/mgmt/mcumgr/mgmt/Kconfig b/subsys/mgmt/mcumgr/mgmt/Kconfig index a0c889f0c5..32fc05fed2 100644 --- a/subsys/mgmt/mcumgr/mgmt/Kconfig +++ b/subsys/mgmt/mcumgr/mgmt/Kconfig @@ -31,3 +31,10 @@ config MCUMGR_MGMT_HANDLER_USER_DATA help This will add an extra field to the struct mgmt_handler that will allow a user to pass user_data when the defined handler is called. + +config MCUMGR_MGMT_CUSTOM_PAYLOAD + bool "MCUmgr custom payload" + help + When this config is enabled, a user can use the field `custom_payload` in `mgmt_handler` to + skip the generation of the cbor start- and end byte in `smp_handle_single_payload` and + instead use a user defined payload in SMP messages. diff --git a/subsys/mgmt/mcumgr/mgmt/src/mgmt.c b/subsys/mgmt/mcumgr/mgmt/src/mgmt.c index 56dcbb0254..adb2c8e4bc 100644 --- a/subsys/mgmt/mcumgr/mgmt/src/mgmt.c +++ b/subsys/mgmt/mcumgr/mgmt/src/mgmt.c @@ -72,24 +72,35 @@ mgmt_find_handler(uint16_t group_id, uint16_t command_id) const struct mgmt_group * mgmt_find_group(uint16_t group_id) { - struct mgmt_group *group = NULL; sys_snode_t *snp, *sns; /* * Find the group with the specified group id - * from the registered group list, if one exists - * return the matching mgmt group pointer, otherwise return NULL */ SYS_SLIST_FOR_EACH_NODE_SAFE(&mgmt_group_list, snp, sns) { struct mgmt_group *loop_group = CONTAINER_OF(snp, struct mgmt_group, node); if (loop_group->mg_group_id == group_id) { - group = loop_group; - break; + return loop_group; } } - return group; + return NULL; +} + +const struct mgmt_handler * +mgmt_get_handler(const struct mgmt_group *group, uint16_t command_id) +{ + if (command_id >= group->mg_handlers_count) { + return NULL; + } + + if (!group->mg_handlers[command_id].mh_read && + !group->mg_handlers[command_id].mh_write) { + return NULL; + } + + return &group->mg_handlers[command_id]; } #if IS_ENABLED(CONFIG_MCUMGR_SMP_SUPPORT_ORIGINAL_PROTOCOL) diff --git a/subsys/mgmt/mcumgr/smp/src/smp.c b/subsys/mgmt/mcumgr/smp/src/smp.c index a97beebdda..a0004f6aa6 100644 --- a/subsys/mgmt/mcumgr/smp/src/smp.c +++ b/subsys/mgmt/mcumgr/smp/src/smp.c @@ -156,9 +156,9 @@ static int smp_build_err_rsp(struct smp_streamer *streamer, const struct smp_hdr * * @return A MGMT_ERR_[...] error code. */ -static int smp_handle_single_payload(struct smp_streamer *cbuf, const struct smp_hdr *req_hdr, - bool *handler_found) +static int smp_handle_single_payload(struct smp_streamer *cbuf, const struct smp_hdr *req_hdr) { + const struct mgmt_group *group; const struct mgmt_handler *handler; mgmt_handler_fn handler_fn; int rc; @@ -169,7 +169,12 @@ static int smp_handle_single_payload(struct smp_streamer *cbuf, const struct smp uint16_t err_group; #endif - handler = mgmt_find_handler(req_hdr->nh_group, req_hdr->nh_id); + group = mgmt_find_group(req_hdr->nh_group); + if (group == NULL) { + return MGMT_ERR_ENOTSUP; + } + + handler = mgmt_get_handler(group, req_hdr->nh_id); if (handler == NULL) { return MGMT_ERR_ENOTSUP; } @@ -190,15 +195,20 @@ static int smp_handle_single_payload(struct smp_streamer *cbuf, const struct smp if (handler_fn) { bool ok; - *handler_found = true; - ok = zcbor_map_start_encode(cbuf->writer->zs, - CONFIG_MCUMGR_SMP_CBOR_MAX_MAIN_MAP_ENTRIES); +#if defined(CONFIG_MCUMGR_MGMT_CUSTOM_PAYLOAD) + if (!group->custom_payload) { +#endif + ok = zcbor_map_start_encode(cbuf->writer->zs, + CONFIG_MCUMGR_SMP_CBOR_MAX_MAIN_MAP_ENTRIES); - MGMT_CTXT_SET_RC_RSN(cbuf, NULL); + MGMT_CTXT_SET_RC_RSN(cbuf, NULL); - if (!ok) { - return MGMT_ERR_EMSGSIZE; + if (!ok) { + return MGMT_ERR_EMSGSIZE; + } +#if defined(CONFIG_MCUMGR_MGMT_CUSTOM_PAYLOAD) } +#endif #if defined(CONFIG_MCUMGR_SMP_COMMAND_STATUS_HOOKS) cmd_recv.group = req_hdr->nh_group; @@ -231,12 +241,18 @@ static int smp_handle_single_payload(struct smp_streamer *cbuf, const struct smp #if defined(CONFIG_MCUMGR_SMP_COMMAND_STATUS_HOOKS) end: #endif - /* End response payload. */ - if (!zcbor_map_end_encode(cbuf->writer->zs, - CONFIG_MCUMGR_SMP_CBOR_MAX_MAIN_MAP_ENTRIES) && - rc == 0) { - rc = MGMT_ERR_EMSGSIZE; +#if defined(CONFIG_MCUMGR_MGMT_CUSTOM_PAYLOAD) + if (!group->custom_payload) { +#endif + /* End response payload. */ + if (!zcbor_map_end_encode(cbuf->writer->zs, + CONFIG_MCUMGR_SMP_CBOR_MAX_MAIN_MAP_ENTRIES) && + rc == 0) { + rc = MGMT_ERR_EMSGSIZE; + } +#if defined(CONFIG_MCUMGR_MGMT_CUSTOM_PAYLOAD) } +#endif } else { rc = MGMT_ERR_ENOTSUP; } @@ -256,7 +272,7 @@ end: * @return A MGMT_ERR_[...] error code. */ static int smp_handle_single_req(struct smp_streamer *streamer, const struct smp_hdr *req_hdr, - bool *handler_found, const char **rsn) + const char **rsn) { struct smp_hdr rsp_hdr; struct cbor_nb_writer *nbw = streamer->writer; @@ -279,7 +295,7 @@ static int smp_handle_single_req(struct smp_streamer *streamer, const struct smp } /* Process the request and write the response payload. */ - rc = smp_handle_single_payload(streamer, req_hdr, handler_found); + rc = smp_handle_single_payload(streamer, req_hdr); if (rc != 0) { *rsn = MGMT_CTXT_RC_RSN(streamer); return rc; @@ -411,7 +427,8 @@ int smp_process_request_packet(struct smp_streamer *streamer, void *vreq) cbor_nb_writer_init(streamer->writer, rsp); /* Process the request payload and build the response. */ - rc = smp_handle_single_req(streamer, &req_hdr, &handler_found, &rsn); + rc = smp_handle_single_req(streamer, &req_hdr, &rsn); + handler_found = (rc != MGMT_ERR_ENOTSUP); if (rc != 0) { break; }