mgmt: mcumgr: img_mgmt: Switch to new event callback system

Switches to the new event callback system for the img_mgmt
functionality and removes the old code.

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

Author: nordicjm

include/zephyr/mgmt/mcumgr/grp/img_mgmt/img_mgmt_callbacks.h

@@ -0,0 +1,46 @@
+/*
+ * Copyright (c) 2022 Laird Connectivity
+ * Copyright (c) 2022 Nordic Semiconductor ASA
+ *
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+#ifndef H_MCUMGR_IMG_MGMT_CALLBACKS_
+#define H_MCUMGR_IMG_MGMT_CALLBACKS_
+
+#include <img_mgmt/img_mgmt.h>
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+/**
+ * @brief MCUmgr img_mgmt callback API
+ * @defgroup mcumgr_callback_api_img_mgmt MCUmgr img_mgmt callback API
+ * @ingroup mcumgr_callback_api
+ * @{
+ */
+
+/**
+ * Structure provided in the MGMT_EVT_OP_IMG_MGMT_DFU_CHUNK notification callback: This callback
+ * function is used to notify the application about a pending firmware upload packet from a client
+ * and authorise or deny it. Upload will be allowed so long as all notification handlers return
+ * MGMT_ERR_EOK, if one returns an error then the upload will be denied.
+ */
+struct img_mgmt_upload_check {
+	/** Action to take */
+	struct img_mgmt_upload_action *action;
+
+	/** Upload request information */
+	struct img_mgmt_upload_req *req;
+};
+
+/**
+ * @}
+ */
+
+#ifdef __cplusplus
+}
+#endif
+
+#endif

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

@@ -15,6 +15,10 @@
 #include <zephyr/mgmt/mcumgr/grp/fs_mgmt/fs_mgmt_callbacks.h>
 #endif
 
+#ifdef CONFIG_MCUMGR_CMD_IMG_MGMT
+#include <zephyr/mgmt/mcumgr/grp/img_mgmt/img_mgmt_callbacks.h>
+#endif
+
 #ifdef __cplusplus
 extern "C" {
 #endif

subsys/mgmt/mcumgr/lib/cmd/img_mgmt/Kconfig

@@ -84,6 +84,22 @@ config IMG_MGMT_FRUGAL_LIST
 	  a device but requires support in client software, which has to default omitted values.
 	  Works correctly with mcumgr-cli.
 
+config MCUMGR_GRP_IMG_UPLOAD_CHECK_HOOK
+	bool "Upload check hook"
+	depends on MCUMGR_MGMT_NOTIFICATION_HOOKS
+	help
+	  This will enable the upload check hook which will send image upload requests to
+	  registered callbacks to check with the user application if an upload should be accepted
+	  or rejected.
+
+config MCUMGR_GRP_IMG_STATUS_HOOKS
+	bool "Status hooks"
+	depends on MCUMGR_MGMT_NOTIFICATION_HOOKS
+	help
+	  This will enable DFU status hooks which can be checked by the application to monitor DFU
+	  uploads. Note that these are status checking only, to allow inspecting of a file upload
+	  or prevent it, CONFIG_MCUMGR_GRP_IMG_UPLOAD_CHECK_HOOK must be used.
+
 module = MCUMGR_IMG_MGMT
 module-str = mcumgr_img_mgmt
 source "subsys/logging/Kconfig.template.log_config"

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

@@ -1,5 +1,6 @@
 /*
  * Copyright (c) 2018-2021 mcumgr authors
+ * Copyright (c) 2022 Nordic Semiconductor ASA
  *
  * SPDX-License-Identifier: Apache-2.0
  */
@@ -192,53 +193,6 @@ int img_mgmt_state_set_pending(int slot, int permanent);
  */
 int img_mgmt_state_confirm(void);
 
-/** @brief Generic callback function for events */
-typedef void (*img_mgmt_dfu_cb)(void);
-
-/** Callback function pointers */
-struct img_mgmt_dfu_callbacks_t {
-	img_mgmt_dfu_cb dfu_started_cb;
-	img_mgmt_dfu_cb dfu_stopped_cb;
-	img_mgmt_dfu_cb dfu_pending_cb;
-	img_mgmt_dfu_cb dfu_confirmed_cb;
-};
-
-/** @typedef img_mgmt_upload_fn
- * @brief Application callback that is executed when an image upload request is
- * received.
- *
- * The callback's return code determines whether the upload request is accepted
- * or rejected.  If the callback returns 0, processing of the upload request
- * proceeds.  If the callback returns nonzero, the request is rejected with a
- * response containing an `rc` value equal to the return code.
- *
- * @param req		Image upload request structure
- * @param action	Image upload action structure
- *
- * @return	0 if the upload request should be accepted; nonzero to reject
- *		the request with the specified status.
- */
-typedef int (*img_mgmt_upload_fn)(const struct img_mgmt_upload_req req,
-				  const struct img_mgmt_upload_action action);
-
-/**
- * @brief Configures a callback that gets called whenever a valid image upload
- * request is received.
- *
- * The callback's return code determines whether the upload request is accepted
- * or rejected.  If the callback returns 0, processing of the upload request
- * proceeds.  If the callback returns nonzero, the request is rejected with a
- * response containing an `rc` value equal to the return code.
- *
- * @param cb	The callback to execute on rx of an upload request.
- */
-void img_mgmt_set_upload_cb(img_mgmt_upload_fn cb);
-void img_mgmt_register_callbacks(const struct img_mgmt_dfu_callbacks_t *cb_struct);
-void img_mgmt_dfu_stopped(void);
-void img_mgmt_dfu_started(void);
-void img_mgmt_dfu_pending(void);
-void img_mgmt_dfu_confirmed(void);
-
 /**
  * Compares two image version numbers in a semver-compatible way.
  *

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

@@ -27,9 +27,9 @@
 #include <zephyr/dfu/flash_img.h>
 #endif
 
-static img_mgmt_upload_fn img_mgmt_upload_cb;
-
-const struct img_mgmt_dfu_callbacks_t *img_mgmt_dfu_callbacks_fn;
+#ifdef CONFIG_MCUMGR_MGMT_NOTIFICATION_HOOKS
+#include <zephyr/mgmt/mcumgr/mgmt/callbacks.h>
+#endif
 
 struct img_mgmt_state g_img_mgmt_state;
 
@@ -295,7 +295,9 @@ img_mgmt_erase(struct smp_streamer *ctxt)
 	img_mgmt_reset_upload();
 
 	if (rc != 0) {
-		img_mgmt_dfu_stopped();
+#if defined(CONFIG_MCUMGR_GRP_IMG_STATUS_HOOKS)
+		(void)mgmt_callback_notify(MGMT_EVT_OP_IMG_MGMT_DFU_STOPPED, NULL, 0);
+#endif
 		return rc;
 	}
 
@@ -355,7 +357,6 @@ img_mgmt_upload_log(bool is_first, bool is_last, int status)
 static int
 img_mgmt_upload(struct smp_streamer *ctxt)
 {
-	struct mgmt_evt_op_cmd_status_arg cmd_status_arg;
 	zcbor_state_t *zsd = ctxt->reader->zs;
 	bool ok;
 	size_t decoded = 0;
@@ -381,6 +382,20 @@ img_mgmt_upload(struct smp_streamer *ctxt)
 		ZCBOR_MAP_DECODE_KEY_VAL(upgrade, zcbor_bool_decode, &req.upgrade)
 	};
 
+#if defined(CONFIG_MCUMGR_SMP_COMMAND_STATUS_HOOKS)
+	struct mgmt_evt_op_cmd_arg cmd_status_arg = {
+		.group = MGMT_GROUP_ID_IMAGE,
+		.id = IMG_MGMT_ID_UPLOAD,
+	};
+#endif
+
+#if defined(CONFIG_MCUMGR_GRP_IMG_UPLOAD_CHECK_HOOK)
+	struct img_mgmt_upload_check upload_check_data = {
+		.action = &action,
+		.req = &req,
+	};
+#endif
+
 	ok = zcbor_map_decode_bulk(zsd, image_upload_decode,
 		ARRAY_SIZE(image_upload_decode), &decoded) == 0;
 
@@ -393,7 +408,10 @@ img_mgmt_upload(struct smp_streamer *ctxt)
 	/* Determine what actions to take as a result of this request. */
 	rc = img_mgmt_upload_inspect(&req, &action);
 	if (rc != 0) {
-		img_mgmt_dfu_stopped();
+#if defined(CONFIG_MCUMGR_GRP_IMG_STATUS_HOOKS)
+		(void)mgmt_callback_notify(MGMT_EVT_OP_IMG_MGMT_DFU_STOPPED, NULL, 0);
+#endif
+
 		MGMT_CTXT_SET_RC_RSN(ctxt, IMG_MGMT_UPLOAD_ACTION_RC_RSN(&action));
 		return rc;
 	}
@@ -405,17 +423,18 @@ img_mgmt_upload(struct smp_streamer *ctxt)
 		return img_mgmt_upload_good_rsp(ctxt);
 	}
 
+#if defined(CONFIG_MCUMGR_GRP_IMG_UPLOAD_CHECK_HOOK)
 	/* Request is valid.  Give the application a chance to reject this upload
 	 * request.
 	 */
-	if (img_mgmt_upload_cb != NULL) {
-		rc = img_mgmt_upload_cb(req, action);
+	rc = mgmt_callback_notify(MGMT_EVT_OP_IMG_MGMT_DFU_CHUNK, &upload_check_data,
+				  sizeof(upload_check_data));
 
-		if (rc != 0) {
-			IMG_MGMT_UPLOAD_ACTION_SET_RC_RSN(&action, img_mgmt_err_str_app_reject);
-			goto end;
-		}
+	if (rc != MGMT_ERR_EOK) {
+		IMG_MGMT_UPLOAD_ACTION_SET_RC_RSN(&action, img_mgmt_err_str_app_reject);
+		goto end;
 	}
+#endif
 
 	/* Remember flash area ID and image size for subsequent upload requests. */
 	g_img_mgmt_state.area_id = action.area_id;
@@ -432,8 +451,13 @@ img_mgmt_upload(struct smp_streamer *ctxt)
 
 		g_img_mgmt_state.off = 0;
 
-		img_mgmt_dfu_started();
+#if defined(CONFIG_MCUMGR_GRP_IMG_STATUS_HOOKS)
+		(void)mgmt_callback_notify(MGMT_EVT_OP_IMG_MGMT_DFU_STARTED, NULL, 0);
+#endif
+
+#if defined(CONFIG_MCUMGR_SMP_COMMAND_STATUS_HOOKS)
 		cmd_status_arg.status = IMG_MGMT_ID_UPLOAD_STATUS_START;
+#endif
 
 		/*
 		 * We accept SHA trimmed to any length by client since it's up to client
@@ -456,8 +480,12 @@ img_mgmt_upload(struct smp_streamer *ctxt)
 			 */
 			g_img_mgmt_state.off = g_img_mgmt_state.size;
 			img_mgmt_dfu_pending();
-			cmd_status_arg.status = IMG_MGMT_ID_UPLOAD_STATUS_COMPLETE;
 			reset = true;
+
+#if defined(CONFIG_MCUMGR_SMP_COMMAND_STATUS_HOOKS)
+			cmd_status_arg.status = IMG_MGMT_ID_UPLOAD_STATUS_COMPLETE;
+#endif
+
 			goto end;
 		}
 #endif
@@ -474,7 +502,9 @@ img_mgmt_upload(struct smp_streamer *ctxt)
 		}
 #endif
 	} else {
+#if defined(CONFIG_MCUMGR_SMP_COMMAND_STATUS_HOOKS)
 		cmd_status_arg.status = IMG_MGMT_ID_UPLOAD_STATUS_ONGOING;
+#endif
 	}
 
 	/* Write the image data to flash. */
@@ -490,31 +520,46 @@ img_mgmt_upload(struct smp_streamer *ctxt)
 			g_img_mgmt_state.off += action.write_bytes;
 		} else {
 			/* Write failed, currently not able to recover from this */
+#if defined(CONFIG_MCUMGR_SMP_COMMAND_STATUS_HOOKS)
 			cmd_status_arg.status = IMG_MGMT_ID_UPLOAD_STATUS_COMPLETE;
+#endif
+
 			IMG_MGMT_UPLOAD_ACTION_SET_RC_RSN(&action,
 				img_mgmt_err_str_flash_write_failed);
 			reset = true;
+			IMG_MGMT_UPLOAD_ACTION_SET_RC_RSN(&action,
+				img_mgmt_err_str_flash_write_failed);
+
 			goto end;
 		}
 
 		if (g_img_mgmt_state.off == g_img_mgmt_state.size) {
 			/* Done */
-			img_mgmt_dfu_pending();
-			cmd_status_arg.status = IMG_MGMT_ID_UPLOAD_STATUS_COMPLETE;
+#if defined(CONFIG_MCUMGR_GRP_IMG_STATUS_HOOKS)
+			(void)mgmt_callback_notify(MGMT_EVT_OP_IMG_MGMT_DFU_PENDING, NULL, 0);
+#endif
+
 			reset = true;
+
+#if defined(CONFIG_MCUMGR_SMP_COMMAND_STATUS_HOOKS)
+			cmd_status_arg.status = IMG_MGMT_ID_UPLOAD_STATUS_COMPLETE;
+#endif
 		}
 	}
 end:
 
 	img_mgmt_upload_log(req.off == 0, g_img_mgmt_state.off == g_img_mgmt_state.size, rc);
 
 #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);
+	(void)mgmt_callback_notify(MGMT_EVT_OP_CMD_STATUS, &cmd_status_arg,
+				   sizeof(cmd_status_arg));
 #endif
 
 	if (rc != 0) {
-		img_mgmt_dfu_stopped();
+#if defined(CONFIG_MCUMGR_GRP_IMG_STATUS_HOOKS)
+		(void)mgmt_callback_notify(MGMT_EVT_OP_IMG_MGMT_DFU_STOPPED, NULL, 0);
+#endif
+
 		return rc;
 	}
 
@@ -528,50 +573,6 @@ img_mgmt_upload(struct smp_streamer *ctxt)
 	return rc;
 }
 
-void
-img_mgmt_dfu_stopped(void)
-{
-	if (img_mgmt_dfu_callbacks_fn && img_mgmt_dfu_callbacks_fn->dfu_stopped_cb) {
-		img_mgmt_dfu_callbacks_fn->dfu_stopped_cb();
-	}
-}
-
-void
-img_mgmt_dfu_started(void)
-{
-	if (img_mgmt_dfu_callbacks_fn && img_mgmt_dfu_callbacks_fn->dfu_started_cb) {
-		img_mgmt_dfu_callbacks_fn->dfu_started_cb();
-	}
-}
-
-void
-img_mgmt_dfu_pending(void)
-{
-	if (img_mgmt_dfu_callbacks_fn && img_mgmt_dfu_callbacks_fn->dfu_pending_cb) {
-		img_mgmt_dfu_callbacks_fn->dfu_pending_cb();
-	}
-}
-
-void
-img_mgmt_dfu_confirmed(void)
-{
-	if (img_mgmt_dfu_callbacks_fn && img_mgmt_dfu_callbacks_fn->dfu_confirmed_cb) {
-		img_mgmt_dfu_callbacks_fn->dfu_confirmed_cb();
-	}
-}
-
-void
-img_mgmt_set_upload_cb(img_mgmt_upload_fn cb)
-{
-	img_mgmt_upload_cb = cb;
-}
-
-void
-img_mgmt_register_callbacks(const struct img_mgmt_dfu_callbacks_t *cb_struct)
-{
-	img_mgmt_dfu_callbacks_fn = cb_struct;
-}
-
 int
 img_mgmt_my_version(struct image_version *ver)
 {