net: lib: Erase pending may cause the modem DFU operation to stall

sigvartmh · carlescufi · commit 3a0e91e654e6 · 2020-02-11T15:08:39.000+01:00
Doing a modem DFU may cause the modem's erase operation to hang on
`DFU_ERASE_PENDING` this is due to the modem being too busy to do the
actual erase. Either reboot the device or turn of the LTE link until
the erase operation is complete to mitigate this issue.

Signed-off-by: Sigvart Hovland &lt;sigvart.hovland@nordicsemi.no&gt;
diff --git a/applications/asset_tracker/src/main.c b/applications/asset_tracker/src/main.c
@@ -902,6 +902,9 @@ void cloud_event_handler(const struct cloud_backend *const backend,
 		break;
 	case CLOUD_EVT_FOTA_DONE:
 		LOG_INF("CLOUD_EVT_FOTA_DONE");
+#if defined(CONFIG_LTE_LINK_CONTROL)
+		lte_lc_power_off();
+#endif
 		sys_reboot(SYS_REBOOT_COLD);
 		break;
 	default:
diff --git a/include/dfu/dfu_target.h b/include/dfu/dfu_target.h
@@ -23,10 +23,17 @@ extern "C" {
 #define DFU_TARGET_IMAGE_TYPE_MCUBOOT 1
 #define DFU_TARGET_IMAGE_TYPE_MODEM_DELTA 2
 
+enum dfu_target_evt_id {
+	DFU_TARGET_EVT_TIMEOUT,
+	DFU_TARGET_EVT_ERASE_DONE
+};
+
+typedef void (*dfu_target_callback_t)(enum dfu_target_evt_id evt_id);
+
 /** @brief Functions which needs to be supported by all DFU targets.
  */
 struct dfu_target {
-	int (*init)(size_t file_size);
+	int (*init)(size_t file_size, dfu_target_callback_t cb);
 	int (*offset_get)(size_t *offset);
 	int (*write)(const void *const buf, size_t len);
 	int (*done)(bool successful);
@@ -58,12 +65,14 @@ int dfu_target_img_type(const void *const buf, size_t len);
  *
  * @param[in] img_type Image type identifier.
  * @param[in] file_size Size of the current file being downloaded.
+ * @param[in] cb Callback function in case the DFU operation requires additional
+ *		 proceedures to be called.
  *
  * @return 0 for a supported image type or a negative error
  *	   code identicating reason of failure.
  *
  **/
-int dfu_target_init(int img_type, size_t file_size);
+int dfu_target_init(int img_type, size_t file_size, dfu_target_callback_t cb);
 
 /**
  * @brief Get offset of the firmware upgrade
diff --git a/include/net/aws_fota.h b/include/net/aws_fota.h
@@ -25,7 +25,11 @@ enum aws_fota_evt_id {
 	/** AWS FOTA complete and status reported to job document */
 	AWS_FOTA_EVT_DONE,
 	/** AWS FOTA error */
-	AWS_FOTA_EVT_ERROR
+	AWS_FOTA_EVT_ERROR,
+	/** AWS FOTA Erase pending*/
+	AWS_FOTA_EVT_ERASE_PENDING,
+	/** AWS FOTA Erase done*/
+	AWS_FOTA_EVT_ERASE_DONE
 };
 
 typedef void (*aws_fota_callback_t)(enum aws_fota_evt_id evt_id);
diff --git a/include/net/fota_download.h b/include/net/fota_download.h
@@ -34,6 +34,10 @@ enum fota_download_evt_id {
 	FOTA_DOWNLOAD_EVT_PROGRESS,
 	/** FOTA download finished. */
 	FOTA_DOWNLOAD_EVT_FINISHED,
+	/** FOTA download erase pending. */
+	FOTA_DOWNLOAD_EVT_ERASE_PENDING,
+	/** FOTA download erase done. */
+	FOTA_DOWNLOAD_EVT_ERASE_DONE,
 	/** FOTA download error. */
 	FOTA_DOWNLOAD_EVT_ERROR,
 };
diff --git a/samples/nrf9160/aws_fota/src/main.c b/samples/nrf9160/aws_fota/src/main.c
@@ -17,6 +17,9 @@
 #include <dfu/mcuboot.h>
 #include <power/reboot.h>
 
+BUILD_ASSERT_MSG(!IS_ENABLED(CONFIG_LTE_AUTO_INIT_AND_CONNECT),
+			"This sample does not support auto init and connect");
+
 #if defined(CONFIG_USE_NRF_CLOUD)
 #define NRF_CLOUD_SECURITY_TAG 16842753
 #endif
@@ -434,36 +437,44 @@ static int fds_init(struct mqtt_client *c)
 	return 0;
 }
 
-/**@brief Configures modem to provide LTE link.
- *
- * Blocks until link is successfully established.
- */
-static void modem_configure(void)
+/**@brief Configures AT Command interface to the modem link. */
+static void at_configure(void)
 {
-#if defined(CONFIG_LTE_LINK_CONTROL)
-	BUILD_ASSERT_MSG(!IS_ENABLED(CONFIG_LTE_AUTO_INIT_AND_CONNECT),
-			"This sample does not support auto init and connect");
 	int err;
 
 	err = at_notif_init();
 	__ASSERT(err == 0, "AT Notify could not be initialized.");
 	err = at_cmd_init();
 	__ASSERT(err == 0, "AT CMD could not be established.");
-	printk("LTE Link Connecting ...\n");
-	err = lte_lc_init_and_connect();
-	__ASSERT(err == 0, "LTE link could not be established.");
-	printk("LTE Link Connected!\n");
-#endif
 }
 
 static void aws_fota_cb_handler(enum aws_fota_evt_id evt)
 {
+	int err;
+
 	switch (evt) {
 	case AWS_FOTA_EVT_DONE:
 		printk("AWS_FOTA_EVT_DONE, rebooting to apply update.\n");
 		do_reboot = true;
 		break;
 
+	case AWS_FOTA_EVT_ERASE_PENDING:
+		printk("AWS_FOTA_EVT_ERASE_PENDING, reboot or disconnect the "
+		       "LTE link\n");
+		err = lte_lc_offline();
+		if (err) {
+			printk("Error turning off the LTE link\n");
+		}
+		break;
+
+	case AWS_FOTA_EVT_ERASE_DONE:
+		printk("AWS_FOTA_EVT_ERASE_DONE, reconnecting the LTE link\n");
+		err = lte_lc_connect();
+		if (err) {
+			printk("Error reconnecting the LTE link\n");
+		}
+		break;
+
 	case AWS_FOTA_EVT_ERROR:
 		printk("AWS_FOTA_EVT_ERROR\n");
 		break;
@@ -505,10 +516,14 @@ void main(void)
 	}
 	printk("Initialized bsdlib\n");
 
+	at_configure();
 #if defined(CONFIG_PROVISION_CERTIFICATES)
 	provision_certificates();
 #endif /* CONFIG_PROVISION_CERTIFICATES */
-	modem_configure();
+	printk("LTE Link Connecting ...\n");
+	err = lte_lc_init_and_connect();
+	__ASSERT(err == 0, "LTE link could not be established.");
+	printk("LTE Link Connected!\n");
 
 	client_init(&client, CONFIG_MQTT_BROKER_HOSTNAME);
 
diff --git a/subsys/dfu/Kconfig b/subsys/dfu/Kconfig
@@ -38,6 +38,22 @@ config DFU_TARGET_MODEM
 	help
 	  Enable support for updates to the modem firmware.
 
+if (DFU_TARGET_MODEM)
+
+config DFU_TARGET_MODEM_TIMEOUT
+	int "Erase pending timeout"
+	default 60
+	help
+	  Set the timeout in seconds for how long the code will wait when
+	  reading DFU_ERASE_PENDING from the modem. If the timeout is reached
+	  an DFU_TARGET_EVT_TIMEOUT is issued and a disconnect of the LTE link
+	  is recommended so that the modem has time to service the
+	  DFU_ERASE_PENDING request. It's also possible to reboot the device to
+	  achive the same desired behavior.
+
+
+endif # DFU_TARGET_MODEM
+
 module=DFU_TARGET
 module-dep=LOG
 module-str=Device Firmware Upgrade
diff --git a/subsys/dfu/include/dfu_target_mcuboot.h b/subsys/dfu/include/dfu_target_mcuboot.h
@@ -52,10 +52,11 @@ bool dfu_target_mcuboot_identify(const void *const buf);
  * @brief Initialize dfu target, perform steps necessary to receive firmware.
  *
  * @param[in] file_size Size of the current file being downloaded.
+ * @param[in] cb Callback for signaling events(unused).
  *
  * @retval 0 If successful, negative errno otherwise.
  */
-int dfu_target_mcuboot_init(size_t file_size);
+int dfu_target_mcuboot_init(size_t file_size, dfu_target_callback_t cb);
 
 /**
  * @brief Get offset of firmware
diff --git a/subsys/dfu/include/dfu_target_modem.h b/subsys/dfu/include/dfu_target_modem.h
@@ -31,10 +31,12 @@ bool dfu_target_modem_identify(const void *const buf);
  * @brief Initialize dfu target, perform steps necessary to receive firmware.
  *
  * @param[in] file_size Size of the current file being downloaded.
+ * @param[in] callback Callback function for signaling if the modem is not able
+ *		       to service the erase request.
  *
  * @retval 0 If successful, negative errno otherwise.
  */
-int dfu_target_modem_init(size_t file_size);
+int dfu_target_modem_init(size_t file_size, dfu_target_callback_t callback);
 
 /**
  * @brief Get offset of firmware
diff --git a/subsys/dfu/src/dfu_target.c b/subsys/dfu/src/dfu_target.c
@@ -54,7 +54,7 @@ int dfu_target_img_type(const void *const buf, size_t len)
 	return -ENOTSUP;
 }
 
-int dfu_target_init(int img_type, size_t file_size)
+int dfu_target_init(int img_type, size_t file_size, dfu_target_callback_t cb)
 {
 	const struct dfu_target *new_target = NULL;
 
@@ -83,7 +83,7 @@ int dfu_target_init(int img_type, size_t file_size)
 
 	current_target = new_target;
 
-	return current_target->init(file_size);
+	return current_target->init(file_size, cb);
 }
 
 int dfu_target_offset_get(size_t *offset)
@@ -124,4 +124,3 @@ int dfu_target_done(bool successful)
 
 	return 0;
 }
-
diff --git a/subsys/dfu/src/dfu_target_mcuboot.c b/subsys/dfu/src/dfu_target_mcuboot.c
@@ -21,6 +21,7 @@
 #include <pm_config.h>
 #include <logging/log.h>
 #include <dfu/mcuboot.h>
+#include <dfu/dfu_target.h>
 #include <dfu/flash_img.h>
 #include <settings/settings.h>
 
@@ -31,7 +32,8 @@ LOG_MODULE_REGISTER(dfu_target_mcuboot, CONFIG_DFU_TARGET_LOG_LEVEL);
 
 static struct flash_img_context flash_img;
 
-int dfu_ctx_mcuboot_set_b1_file(const char *file, bool s0_active, const char **update)
+int dfu_ctx_mcuboot_set_b1_file(const char *file, bool s0_active,
+				const char **update)
 {
 	if (file == NULL || update == NULL) {
 		return -EINVAL;
@@ -112,8 +114,9 @@ bool dfu_target_mcuboot_identify(const void *const buf)
 	return *((const u32_t *)buf) == MCUBOOT_HEADER_MAGIC;
 }
 
-int dfu_target_mcuboot_init(size_t file_size)
+int dfu_target_mcuboot_init(size_t file_size, dfu_target_callback_t cb)
 {
+	ARG_UNUSED(cb);
 	int err = flash_img_init(&flash_img);
 
 	if (err != 0) {
@@ -122,7 +125,8 @@ int dfu_target_mcuboot_init(size_t file_size)
 	}
 
 	if (file_size > PM_MCUBOOT_SECONDARY_SIZE) {
-		LOG_ERR("Requested file too big to fit in flash");
+		LOG_ERR("Requested file too big to fit in flash %d > %d",
+			file_size, PM_MCUBOOT_SECONDARY_SIZE);
 		return -EFBIG;
 	}
 
diff --git a/subsys/dfu/src/dfu_target_modem.c b/subsys/dfu/src/dfu_target_modem.c
@@ -4,6 +4,7 @@
 #include <net/socket.h>
 #include <nrf_socket.h>
 #include <logging/log.h>
+#include <dfu/dfu_target.h>
 
 LOG_MODULE_REGISTER(dfu_target_modem, CONFIG_DFU_TARGET_LOG_LEVEL);
 
@@ -18,6 +19,7 @@ struct modem_delta_header {
 
 static int  fd;
 static int  offset;
+static dfu_target_callback_t callback;
 
 static int get_modem_error(void)
 {
@@ -51,30 +53,36 @@ static int apply_modem_upgrade(void)
 	}
 	return 0;
 }
-
+#define SLEEP_TIME 1
 static int delete_banked_modem_fw(void)
 {
 	int err;
 	socklen_t len = sizeof(offset);
+	int timeout = CONFIG_DFU_TARGET_MODEM_TIMEOUT;
 
 	LOG_INF("Deleting firmware image, this can take several minutes");
 	err = setsockopt(fd, SOL_DFU, SO_DFU_BACKUP_DELETE, NULL, 0);
 	if (err < 0) {
 		LOG_ERR("Failed to delete backup, errno %d", errno);
 		return -EFAULT;
 	}
-
 	while (true) {
 		err = getsockopt(fd, SOL_DFU, SO_DFU_OFFSET, &offset, &len);
 		if (err < 0) {
+			if (timeout < 0) {
+				callback(DFU_TARGET_EVT_TIMEOUT);
+				timeout = CONFIG_DFU_TARGET_MODEM_TIMEOUT;
+			}
 			if (errno == ENOEXEC) {
 				err = get_modem_error();
 				if (err != DFU_ERASE_PENDING) {
 					LOG_ERR("DFU error: %d", err);
 				}
-				k_sleep(K_MSEC(500));
+				k_sleep(K_SECONDS(SLEEP_TIME));
 			}
+			timeout -= SLEEP_TIME;
 		} else {
+			callback(DFU_TARGET_EVT_ERASE_DONE);
 			LOG_INF("Modem FW delete complete");
 			break;
 		}
@@ -122,20 +130,34 @@ bool dfu_target_modem_identify(const void *const buf)
 
 }
 
-int dfu_target_modem_init(size_t file_size)
+int dfu_target_modem_init(size_t file_size, dfu_target_callback_t cb)
 {
-	/* We have no way of checking the amount of flash available in the modem
-	 * with the current API
-	 */
-	ARG_UNUSED(file_size);
 	int err;
+	size_t scratch_space;
 	socklen_t len = sizeof(offset);
 
+	callback = cb;
+
 	err = modem_dfu_socket_init();
 	if (err < 0) {
 		return err;
 	}
 
+	err = getsockopt(fd, SOL_DFU, SO_DFU_RESOURCES, &scratch_space, &len);
+	if (err < 0) {
+		if (errno == ENOEXEC) {
+			LOG_ERR("Modem error: %d", get_modem_error());
+		} else {
+			LOG_ERR("getsockopt(OFFSET) errno: %d", errno);
+		}
+	}
+
+	if (file_size > scratch_space) {
+		LOG_ERR("Requested file too big to fit in flash %d > %d",
+			file_size, scratch_space);
+		return -EFBIG;
+	}
+
 	/* Check offset, store to local variable */
 	err = getsockopt(fd, SOL_DFU, SO_DFU_OFFSET, &offset, &len);
 	if (err < 0) {
diff --git a/subsys/net/lib/aws_fota/src/aws_fota.c b/subsys/net/lib/aws_fota/src/aws_fota.c
@@ -486,6 +486,15 @@ static void http_fota_handler(const struct fota_download_evt *evt)
 			callback(AWS_FOTA_EVT_ERROR);
 		}
 		break;
+
+	case FOTA_DOWNLOAD_EVT_ERASE_PENDING:
+		callback(AWS_FOTA_EVT_ERASE_PENDING);
+		break;
+
+	case FOTA_DOWNLOAD_EVT_ERASE_DONE:
+		callback(AWS_FOTA_EVT_ERASE_DONE);
+		break;
+
 	case FOTA_DOWNLOAD_EVT_ERROR:
 		LOG_ERR("FOTA download failed, report back");
 		fota_state = NONE;
diff --git a/subsys/net/lib/fota_download/src/fota_download.c b/subsys/net/lib/fota_download/src/fota_download.c
diff --git a/subsys/net/lib/nrf_cloud/src/nrf_cloud_transport.c b/subsys/net/lib/nrf_cloud/src/nrf_cloud_transport.c

Original file line number	Diff line number	Diff line change
`@@ -52,10 +52,11 @@ bool dfu_target_mcuboot_identify(const void *const buf);`
`52`	`52`	`* @brief Initialize dfu target, perform steps necessary to receive firmware.`
`53`	`53`	`*`
`54`	`54`	`* @param[in] file_size Size of the current file being downloaded.`
	`55`	`+ * @param[in] cb Callback for signaling events(unused).`
`55`	`56`	`*`
`56`	`57`	`* @retval 0 If successful, negative errno otherwise.`
`57`	`58`	`*/`
`58`		`-int dfu_target_mcuboot_init(size_t file_size);`
	`59`	`+int dfu_target_mcuboot_init(size_t file_size, dfu_target_callback_t cb);`
`59`	`60`
`60`	`61`	`/**`
`61`	`62`	`* @brief Get offset of firmware`
Original file line number	Diff line number	Diff line change
`@@ -31,10 +31,12 @@ bool dfu_target_modem_identify(const void *const buf);`
`31`	`31`	`* @brief Initialize dfu target, perform steps necessary to receive firmware.`
`32`	`32`	`*`
`33`	`33`	`* @param[in] file_size Size of the current file being downloaded.`
	`34`	`+ * @param[in] callback Callback function for signaling if the modem is not able`
	`35`	`+ * to service the erase request.`
`34`	`36`	`*`
`35`	`37`	`* @retval 0 If successful, negative errno otherwise.`
`36`	`38`	`*/`
`37`		`-int dfu_target_modem_init(size_t file_size);`
	`39`	`+int dfu_target_modem_init(size_t file_size, dfu_target_callback_t callback);`
`38`	`40`
`39`	`41`	`/**`
`40`	`42`	`* @brief Get offset of firmware`
Original file line number	Diff line number	Diff line change
`@@ -54,7 +54,7 @@ int dfu_target_img_type(const void *const buf, size_t len)`
`54`	`54`	`return -ENOTSUP;`
`55`	`55`	`}`
`56`	`56`
`57`		`-int dfu_target_init(int img_type, size_t file_size)`
	`57`	`+int dfu_target_init(int img_type, size_t file_size, dfu_target_callback_t cb)`
`58`	`58`	`{`
`59`	`59`	`const struct dfu_target *new_target = NULL;`
`60`	`60`
`@@ -83,7 +83,7 @@ int dfu_target_init(int img_type, size_t file_size)`
`83`	`83`
`84`	`84`	`current_target = new_target;`
`85`	`85`
`86`		`- return current_target->init(file_size);`
	`86`	`+ return current_target->init(file_size, cb);`
`87`	`87`	`}`
`88`	`88`
`89`	`89`	`int dfu_target_offset_get(size_t *offset)`
`@@ -124,4 +124,3 @@ int dfu_target_done(bool successful)`
`124`	`124`
`125`	`125`	`return 0;`
`126`	`126`	`}`
`127`		`-`
Original file line number	Diff line number	Diff line change
`@@ -21,6 +21,7 @@`
`21`	`21`	`#include <pm_config.h>`
`22`	`22`	`#include <logging/log.h>`
`23`	`23`	`#include <dfu/mcuboot.h>`
	`24`	`+#include <dfu/dfu_target.h>`
`24`	`25`	`#include <dfu/flash_img.h>`
`25`	`26`	`#include <settings/settings.h>`
`26`	`27`
`@@ -31,7 +32,8 @@ LOG_MODULE_REGISTER(dfu_target_mcuboot, CONFIG_DFU_TARGET_LOG_LEVEL);`
`31`	`32`
`32`	`33`	`static struct flash_img_context flash_img;`
`33`	`34`
`34`		`-int dfu_ctx_mcuboot_set_b1_file(const char file, bool s0_active, const char *update)`
	`35`	`+int dfu_ctx_mcuboot_set_b1_file(const char *file, bool s0_active,`
	`36`	`+ const char **update)`
`35`	`37`	`{`
`36`	`38`	`if (file == NULL \|\| update == NULL) {`
`37`	`39`	`return -EINVAL;`
`@@ -112,8 +114,9 @@ bool dfu_target_mcuboot_identify(const void *const buf)`
`112`	`114`	`return ((const u32_t )buf) == MCUBOOT_HEADER_MAGIC;`
`113`	`115`	`}`
`114`	`116`
`115`		`-int dfu_target_mcuboot_init(size_t file_size)`
	`117`	`+int dfu_target_mcuboot_init(size_t file_size, dfu_target_callback_t cb)`
`116`	`118`	`{`
	`119`	`+ ARG_UNUSED(cb);`
`117`	`120`	`int err = flash_img_init(&flash_img);`
`118`	`121`
`119`	`122`	`if (err != 0) {`
`@@ -122,7 +125,8 @@ int dfu_target_mcuboot_init(size_t file_size)`
`122`	`125`	`}`
`123`	`126`
`124`	`127`	`if (file_size > PM_MCUBOOT_SECONDARY_SIZE) {`
`125`		`- LOG_ERR("Requested file too big to fit in flash");`
	`128`	`+ LOG_ERR("Requested file too big to fit in flash %d > %d",`
	`129`	`+ file_size, PM_MCUBOOT_SECONDARY_SIZE);`
`126`	`130`	`return -EFBIG;`
`127`	`131`	`}`
`128`	`132`