mgmt: mcumgr: Rework event callback system

Reworks the event callback system to use a linked list to allow for
chained handlers and support passing a status back to the handler to
indicate if the request should be rejected or allowed. This also
removes the old base callback functionality.

Signed-off-by: Jamie McCrae <jamie.mccrae@nordicsemi.no>

Author: nordicjm

include/zephyr/mgmt/mcumgr/mgmt/callbacks.h

@@ -0,0 +1,165 @@
+/*
+ * Copyright (c) 2022 Nordic Semiconductor ASA
+ *
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+#ifndef H_MCUMGR_CALLBACKS_
+#define H_MCUMGR_CALLBACKS_
+
+#include <inttypes.h>
+#include <zephyr/sys/slist.h>
+#include <mgmt/mgmt.h>
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+/**
+ * @brief MCUmgr callback API
+ * @defgroup mcumgr_callback_api MCUmgr callback API
+ * @ingroup mcumgr
+ * @{
+ */
+
+/** @cond INTERNAL_HIDDEN */
+/** Event which signfies that all event IDs for a particular group should be enabled. */
+#define MGMT_EVT_OP_ID_ALL 0xffff
+
+/** Get event for a particular group and event ID. */
+#define MGMT_DEF_EVT_OP_ID(group, event_id) ((group << 16) | BIT(event_id))
+
+/** Get event used for enabling all event IDs of a particular group. */
+#define MGMT_DEF_EVT_OP_ALL(group) ((group << 16) | MGMT_EVT_OP_ID_ALL)
+/** @endcond  */
+
+/** Get group from event. */
+#define MGMT_EVT_GET_GROUP(event) ((event >> 16) & MGMT_EVT_OP_ID_ALL)
+
+/** Get event ID from event. */
+#define MGMT_EVT_GET_ID(event) (event & MGMT_EVT_OP_ID_ALL)
+
+/**
+ * @typedef mgmt_cb
+ * @brief Function to be called on MGMT notification/event.
+ *
+ * This callback function is used to notify an application or system about a mcumgr mgmt event.
+ *
+ * @param event		MGMT_EVT_OP_[...].
+ * @param rc		MGMT_ERR_[...] of the previous handler calls, if it is an error then it
+ *			will be the first error that was returned by a handler (i.e. this handler
+ *			is being called for a notification only, the return code will be ignored).
+ * @param abort_more	Set to true to abort further processing by additional handlers.
+ * @param data		Optional event argument.
+ * @param data_size	Size of optional event argument (0 if no data is provided).
+ *
+ * @return		MGMT_ERR_[...] of the status to return to the calling code (only checked
+ *			when failed is false).
+ */
+typedef int32_t (*mgmt_cb)(uint32_t event, int32_t rc, bool *abort_more, void *data,
+			   size_t data_size);
+
+/**
+ * MGMT event callback group IDs. Note that this is not a 1:1 mapping with MGMT_GROUP_ID_[...]
+ * values.
+ */
+enum mgmt_cb_groups {
+	MGMT_EVT_GRP_ALL			= 0,
+	MGMT_EVT_GRP_SMP,
+
+	MGMT_EVT_GRP_USER_CUSTOM_START		= MGMT_GROUP_ID_PERUSER,
+};
+
+/**
+ * MGMT event opcodes for all command processing.
+ */
+enum smp_all_events {
+	/** Used to enable all events. */
+	MGMT_EVT_OP_ALL				= MGMT_DEF_EVT_OP_ALL(MGMT_EVT_GRP_ALL),
+};
+
+/**
+ * MGMT event opcodes for base SMP command processing.
+ */
+enum smp_group_events {
+	/** Callback when a command is received, data is mgmt_evt_op_cmd_arg. */
+	MGMT_EVT_OP_CMD_RECV			= MGMT_DEF_EVT_OP_ID(MGMT_EVT_GRP_SMP, 0),
+
+	/** Callback when a a status is updated, data is mgmt_evt_op_cmd_arg. */
+	MGMT_EVT_OP_CMD_STATUS			= MGMT_DEF_EVT_OP_ID(MGMT_EVT_GRP_SMP, 1),
+
+	/** Callback when a command has been processed, data is mgmt_evt_op_cmd_arg. */
+	MGMT_EVT_OP_CMD_DONE			= MGMT_DEF_EVT_OP_ID(MGMT_EVT_GRP_SMP, 2),
+
+	/** Used to enable all smp_group events. */
+	MGMT_EVT_OP_CMD_ALL			= MGMT_DEF_EVT_OP_ALL(MGMT_EVT_GRP_SMP),
+};
+
+/**
+ * MGMT callback struct
+ */
+struct mgmt_callback {
+	/** Entry list node. */
+	sys_snode_t node;
+
+	/** Callback that will be called. */
+	mgmt_cb callback;
+
+	/** MGMT_EVT_[...] Event ID for handler to be called on. */
+	uint32_t event_id;
+};
+
+/**
+ * MGMT_EVT_OP_CMD_RECV, MGMT_EVT_OP_CMD_STATUS, MGMT_EVT_OP_CMD_DONE arguments
+ */
+struct mgmt_evt_op_cmd_arg {
+	/** MGMT_GROUP_ID_[...] */
+	uint16_t group;
+
+	/** Message ID within group */
+	uint8_t id;
+
+	union {
+		/** MGMT_ERR_[...], used in MGMT_EVT_OP_CMD_DONE */
+		int err;
+
+		/** IMG_MGMT_ID_UPLOAD_STATUS_[...], used in MGMT_EVT_OP_CMD_STATUS */
+		int status;
+	};
+};
+
+/**
+ * @brief This function is called to notify registered callbacks about mcumgr notifications/events.
+ *
+ * @param event		MGMT_EVT_OP_[...].
+ * @param data		Optional event argument.
+ * @param data_size	Size of optional event argument (0 if none).
+ *
+ * @return		MGMT_ERR_[...] either MGMT_ERR_EOK if all handlers returned it, or the
+ *			error code of the first handler that returned an error.
+ */
+int32_t mgmt_callback_notify(uint32_t event, void *data, size_t data_size);
+
+/**
+ * @brief Register event callback function.
+ *
+ * @param callback Callback struct.
+ */
+void mgmt_callback_register(struct mgmt_callback *callback);
+
+/**
+ * @brief Unregister event callback function.
+ *
+ * @param callback Callback struct.
+ */
+void mgmt_callback_unregister(struct mgmt_callback *callback);
+
+/**
+ * @}
+ */
+
+#ifdef __cplusplus
+}
+#endif
+
+#endif /* H_MCUMGR_CALLBACKS_ */

subsys/mgmt/mcumgr/Kconfig

@@ -101,6 +101,30 @@ config MGMT_MAX_DECODING_LEVELS
 	  size of cbor_nb_reader structure by zcbor_state_t size per
 	  one unit selected here.
 
+config MCUMGR_MGMT_NOTIFICATION_HOOKS
+	bool "MCUmgr notification hook support"
+	help
+	  With this enabled, applications and parts of code can register for MCUmgr event
+	  notifications which will result in callbacks when a registered event occurs. Note that
+	  this enables the base notification functionality but itself does not enable any
+	  notifications, which must be enabled by selecting other Kconfig options.
+
+	  To enable notifications in code, mgmt_callback_register() must be called with the
+	  callback function and events that want to be received. Multiple handlers can be
+	  registered and will all be called when registered events occur.
+
+	  Some callbacks support notifying the calling function of a status, in which to accept
+	  or decline the current operation, by returning false this will signal to the calling
+	  function that the request should be denied, for informal-only notifications or
+	  acceptable, true must be returned by all the registered notification handlers.
+
+config MCUMGR_SMP_COMMAND_STATUS_HOOKS
+	bool "SMP command status hooks"
+	depends on MCUMGR_MGMT_NOTIFICATION_HOOKS
+	help
+	  This will enable SMP command status notification hooks for when an SMP message is
+	  received or processed.
+
 menu "Command Handlers"
 
 rsource "lib/cmd/Kconfig"

subsys/mgmt/mcumgr/lib/cmd/img_mgmt/src/img_mgmt.c

@@ -1,5 +1,6 @@
 /*
  * Copyright (c) 2018-2021 mcumgr authors
+ * Copyright (c) 2022 Nordic Semiconductor ASA
  *
  * SPDX-License-Identifier: Apache-2.0
  */
@@ -506,8 +507,11 @@ img_mgmt_upload(struct smp_streamer *ctxt)
 end:
 
 	img_mgmt_upload_log(req.off == 0, g_img_mgmt_state.off == g_img_mgmt_state.size, rc);
-	mgmt_evt(MGMT_EVT_OP_CMD_STATUS, MGMT_GROUP_ID_IMAGE, IMG_MGMT_ID_UPLOAD,
-			 &cmd_status_arg);
+
+#if defined(CONFIG_MCUMGR_SMP_COMMAND_STATUS_HOOKS)
+	(void)mgmt_callback_notify(MGMT_EVT_OP_CMD_STATUS, MGMT_GROUP_ID_IMAGE,
+				   IMG_MGMT_ID_UPLOAD, &cmd_status_arg, false);
+#endif
 
 	if (rc != 0) {
 		img_mgmt_dfu_stopped();

subsys/mgmt/mcumgr/lib/mgmt/include/mgmt/mgmt.h

@@ -1,5 +1,6 @@
 /*
  * Copyright (c) 2018-2021 mcumgr authors
+ * Copyright (c) 2022 Nordic Semiconductor ASA
  *
  * SPDX-License-Identifier: Apache-2.0
  */
@@ -55,39 +56,6 @@ extern "C" {
 
 #define MGMT_HDR_SIZE		8
 
-/*
- * MGMT event opcodes.
- */
-#define MGMT_EVT_OP_CMD_RECV	0x01
-#define MGMT_EVT_OP_CMD_STATUS	0x02
-#define MGMT_EVT_OP_CMD_DONE	0x03
-
-/*
- * MGMT_EVT_OP_CMD_STATUS argument
- */
-struct mgmt_evt_op_cmd_status_arg {
-	int status;
-};
-
-/*
- * MGMT_EVT_OP_CMD_DONE argument
- */
-struct mgmt_evt_op_cmd_done_arg {
-	int err;			/* MGMT_ERR_[...] */
-};
-
-/** @typedef mgmt_on_evt_cb
- * @brief Function to be called on MGMT event.
- *
- * This callback function is used to notify application about mgmt event.
- *
- * @param opcode	MGMT_EVT_OP_[...].
- * @param group		MGMT_GROUP_ID_[...].
- * @param id		Message ID within group.
- * @param arg		Optional event argument.
- */
-typedef void (*mgmt_on_evt_cb)(uint8_t opcode, uint16_t group, uint8_t id, void *arg);
-
 /** @typedef mgmt_alloc_rsp_fn
  * @brief Allocates a buffer suitable for holding a response.
  *
@@ -177,23 +145,6 @@ void mgmt_unregister_group(struct mgmt_group *group);
  */
 const struct mgmt_handler *mgmt_find_handler(uint16_t group_id, uint16_t command_id);
 
-/**
- * @brief Register event callback function.
- *
- * @param cb Callback function.
- */
-void mgmt_register_evt_cb(mgmt_on_evt_cb cb);
-
-/**
- * @brief This function is called to notify about mgmt event.
- *
- * @param opcode	MGMT_EVT_OP_[...].
- * @param group		MGMT_GROUP_ID_[...].
- * @param id		Message ID within group.
- * @param arg		Optional event argument.
- */
-void mgmt_evt(uint8_t opcode, uint16_t group, uint8_t id, void *arg);
-
 #ifdef __cplusplus
 }
 #endif

subsys/mgmt/mcumgr/lib/mgmt/src/mgmt.c

@@ -9,10 +9,18 @@
 #include "mgmt/mgmt.h"
 #include "smp/smp.h"
 
-static mgmt_on_evt_cb evt_cb;
+#ifdef CONFIG_MCUMGR_MGMT_NOTIFICATION_HOOKS
+#include <zephyr/mgmt/mcumgr/mgmt/callbacks.h>
+#endif
+
 static sys_slist_t mgmt_group_list =
 	SYS_SLIST_STATIC_INIT(&mgmt_group_list);
 
+#if defined(CONFIG_MCUMGR_MGMT_NOTIFICATION_HOOKS)
+static sys_slist_t mgmt_callback_list =
+	SYS_SLIST_STATIC_INIT(&mgmt_callback_list);
+#endif
+
 void
 mgmt_unregister_group(struct mgmt_group *group)
 {
@@ -62,16 +70,55 @@ mgmt_register_group(struct mgmt_group *group)
 	sys_slist_append(&mgmt_group_list, &group->node);
 }
 
-void
-mgmt_register_evt_cb(mgmt_on_evt_cb cb)
+#if defined(CONFIG_MCUMGR_MGMT_NOTIFICATION_HOOKS)
+void mgmt_callback_register(struct mgmt_callback *callback)
 {
-	evt_cb = cb;
+	sys_slist_append(&mgmt_callback_list, &callback->node);
 }
 
-void
-mgmt_evt(uint8_t opcode, uint16_t group, uint8_t id, void *arg)
+void mgmt_callback_unregister(struct mgmt_callback *callback)
+{
+	(void)sys_slist_find_and_remove(&mgmt_callback_list, &callback->node);
+}
+
+int32_t mgmt_callback_notify(uint32_t event, void *data, size_t data_size)
 {
-	if (evt_cb) {
-		evt_cb(opcode, group, id, arg);
+	sys_snode_t *snp, *sns;
+	bool failed = false;
+	bool abort_more = false;
+	int32_t rc;
+	int32_t return_rc = MGMT_ERR_EOK;
+	uint16_t group = MGMT_EVT_GET_GROUP(event);
+
+	/*
+	 * Search through the linked list for entries that have registered for this event and
+	 * notify them, the first handler to return an error code will have this error returned
+	 * to the calling function, errors returned by additional handlers will be ignored. If
+	 * all notification handlers return MGMT_ERR_EOK then access will be allowed and no error
+	 * will be returned to the calling function. The status of if a previous handler has
+	 * returned an error is provided to the functions through the failed variable, and a
+	 * handler function can set abort_more to true to prevent calling any further handlers.
+	 */
+	SYS_SLIST_FOR_EACH_NODE_SAFE(&mgmt_callback_list, snp, sns) {
+		struct mgmt_callback *loop_group =
+			CONTAINER_OF(snp, struct mgmt_callback, node);
+
+		if (loop_group->event_id == event || loop_group->event_id == MGMT_EVT_OP_ALL ||
+		    (MGMT_EVT_GET_GROUP(loop_group->event_id) == group &&
+		     MGMT_EVT_GET_ID(loop_group->event_id) == MGMT_EVT_OP_ID_ALL)) {
+			rc = loop_group->callback(event, return_rc, &abort_more, data, data_size);
+
+			if (rc != MGMT_ERR_EOK && failed == false) {
+				failed = true;
+				return_rc = rc;
+			}
+
+			if (abort_more == true) {
+				break;
+			}
+		}
 	}
+
+	return return_rc;
 }
+#endif