Don't review

include/net/nrf_cloud.h

@@ -280,7 +280,7 @@ enum nrf_cloud_topic_type {
 	NRF_CLOUD_TOPIC_BIN
 };
 
-/** @brief FOTA status reported to nRF Cloud. */
+/** @brief FOTA status reported to nRF Cloud and notified in @ref nrf_cloud_fota_poll_handler_t */
 enum nrf_cloud_fota_status {
 	NRF_CLOUD_FOTA_QUEUED = 0,
 	NRF_CLOUD_FOTA_IN_PROGRESS = 1,

include/net/nrf_cloud_fota_poll.h

@@ -38,14 +38,14 @@ enum nrf_cloud_fota_reboot_status {
 typedef void (*fota_reboot_handler_t)(enum nrf_cloud_fota_reboot_status status);
 
 /**
- * @brief  Error event handler registered with the module to handle asynchronous
- * error events from the module.
+ * @brief  Status event handler registered with the module to handle asynchronous
+ * status events from the module.
  *
- * @param[in]  status The FOTA status for the error event.
- * @param[in]  status_details Details about the error event.
+ * @param[in]  status Status event.
+ * @param[in]  status_details Details about the event, NULL if no details are provided.
  */
-typedef void (*fota_error_handler_t)(enum nrf_cloud_fota_status status,
-				     const char *const status_details);
+typedef void (*nrf_cloud_fota_poll_handler_t)(enum nrf_cloud_fota_status status,
+					      const char *const status_details);
 
 struct nrf_cloud_fota_poll_ctx {
 	/* Internal variables */
@@ -54,6 +54,7 @@ struct nrf_cloud_fota_poll_ctx {
 	bool is_nonblocking;
 	bool full_modem_fota_supported;
 	const char *device_id;
+	enum dfu_target_image_type img_type;
 
 	/* Public variables */
 
@@ -71,14 +72,13 @@ struct nrf_cloud_fota_poll_ctx {
 	/** User-provided callback function to handle reboots */
 	fota_reboot_handler_t reboot_fn;
 
-	/** Optional, user-provided callback function to handle errors.
+	/** Optional, user-provided callback function to handle FOTA status events.
 	 *  If the function is provided, @ref nrf_cloud_fota_poll_process will be non-blocking and
-	 *  the user will receive error events asynchronously.
+	 *  the user will receive status events asynchronously.
 	 *
-	 * If the function is not provided, @ref nrf_cloud_fota_poll_process will be blocking and
-	 * return an error code when an error occurs.
+	 * If the function is not provided, @ref nrf_cloud_fota_poll_process will be blocking.
 	 */
-	fota_error_handler_t error_fn;
+	nrf_cloud_fota_poll_handler_t status_fn;
 };
 
 /**

lib/lte_link_control/lte_lc.c

@@ -135,9 +135,6 @@ K_WORK_DEFINE(lte_lc_psm_get_work, lte_lc_psm_get_work_fn);
 static void lte_lc_edrx_ptw_send_work_fn(struct k_work *work_item);
 K_WORK_DEFINE(lte_lc_edrx_ptw_send_work, lte_lc_edrx_ptw_send_work_fn);
 
-static void lte_lc_edrx_req_work_fn(struct k_work *work_item);
-K_WORK_DEFINE(lte_lc_edrx_req_work, lte_lc_edrx_req_work_fn);
-
 K_THREAD_STACK_DEFINE(lte_lc_work_q_stack, CONFIG_LTE_LC_WORKQUEUE_STACK_SIZE);
 
 static struct k_work_q lte_lc_work_q;
@@ -1107,11 +1104,6 @@ int lte_lc_edrx_get(struct lte_lc_edrx_cfg *edrx_cfg)
 	return 0;
 }
 
-static void lte_lc_edrx_req_work_fn(struct k_work *work_item)
-{
-	lte_lc_edrx_req(requested_edrx_enable);
-}
-
 #if defined(CONFIG_UNITY)
 void lte_lc_edrx_on_modem_cfun(int mode, void *ctx)
 #else
@@ -1128,7 +1120,11 @@ static void lte_lc_edrx_on_modem_cfun(int mode, void *ctx)
 	 */
 	if (mode == LTE_LC_FUNC_MODE_POWER_OFF && requested_edrx_enable) {
 		lte_lc_edrx_current_values_clear();
-		k_work_submit_to_queue(&lte_lc_work_q, &lte_lc_edrx_req_work);
+		/* We want to avoid sending AT commands in the callback. However,
+		 * when modem is powered off, we are not expecting AT notifications
+		 * that could cause an assertion or missing notification.
+		 */
+		lte_lc_edrx_req(requested_edrx_enable);
 	}
 }
 

lib/lte_link_control/lte_lc_modem_hooks.c

@@ -131,20 +131,6 @@ static void on_modem_init(int err, void *ctx)
 	}
 }
 
-#if defined(CONFIG_UNITY)
-void on_modem_shutdown(void *ctx)
-#else
-NRF_MODEM_LIB_ON_SHUTDOWN(lte_lc_shutdown_hook, on_modem_shutdown, NULL);
-
-static void on_modem_shutdown(void *ctx)
-#endif
-{
-	/* Make sure the Modem library was in normal mode and not in bootloader mode. */
-	if (nrf_modem_is_initialized()) {
-		(void)lte_lc_power_off();
-	}
-}
-
 #if defined(CONFIG_UNITY)
 void lte_lc_on_modem_cfun(int mode, void *ctx)
 #else

modules/memfault-firmware-sdk/memfault_lte_metrics.c

@@ -9,6 +9,7 @@
 
 #include <modem/lte_lc.h>
 #include <modem/lte_lc_trace.h>
+#include <modem/nrf_modem_lib.h>
 
 #if defined(CONFIG_MODEM_INFO)
 #include <modem/modem_info.h>
@@ -23,6 +24,16 @@
 LOG_MODULE_DECLARE(memfault_ncs_metrics, CONFIG_MEMFAULT_NCS_LOG_LEVEL);
 
 static bool connected;
+static enum lte_lc_func_mode current_func_mode;
+
+static void memfault_on_modem_cfun(int mode, void *ctx)
+{
+	ARG_UNUSED(ctx);
+
+	current_func_mode = mode;
+}
+
+NRF_MODEM_LIB_ON_CFUN(memfault_cfun_hook, memfault_on_modem_cfun, NULL);
 
 #if CONFIG_MEMFAULT_NCS_STACK_METRICS
 static struct memfault_ncs_metrics_thread lte_metrics_thread = {
@@ -64,45 +75,44 @@ static void modem_params_get(void)
 
 	err = modem_info_get_rsrp(&rsrp);
 	if (err) {
-		LOG_WRN("LTE RSRP value collection failed, error: %d", err);
-	} else {
-		err = MEMFAULT_METRIC_SET_SIGNED(ncs_lte_rsrp_dbm, rsrp);
-		if (err) {
-			LOG_ERR("Failed to set ncs_lte_rsrp_dbm");
-		}
-	};
+		return;
+	}
+
+	err = MEMFAULT_METRIC_SET_SIGNED(ncs_lte_rsrp_dbm, rsrp);
+	if (err) {
+		LOG_ERR("Failed to set ncs_lte_rsrp_dbm");
+		return;
+	}
 
 	err = modem_info_get_current_band(&band);
-	if (err != 0) {
-		LOG_WRN("Network band collection failed, error: %d", err);
-	} else {
-		err = MEMFAULT_METRIC_SET_UNSIGNED(ncs_lte_band, band);
-		if (err) {
-			LOG_ERR("Failed to set nce_lte_band");
-		}
+	if (err) {
+		return;
+	}
+
+	err = MEMFAULT_METRIC_SET_UNSIGNED(ncs_lte_band, band);
+	if (err) {
+		LOG_ERR("Failed to set nce_lte_band");
+		return;
 	}
 
 	err = modem_info_get_snr(&snr);
-	if (err != 0) {
-		LOG_WRN("SNR collection failed, error: %d", err);
-	} else {
-		err = MEMFAULT_METRIC_SET_SIGNED(ncs_lte_snr_decibels, snr);
-		if (err) {
-			LOG_ERR("Failed to set ncs_lte_snr_decibels");
-		}
+	if (err) {
+		return;
+	}
+
+	err = MEMFAULT_METRIC_SET_SIGNED(ncs_lte_snr_decibels, snr);
+	if (err) {
+		LOG_ERR("Failed to set ncs_lte_snr_decibels");
+		return;
 	}
 #endif
 }
 
 static void lte_handler(const struct lte_lc_evt *const evt)
 {
 	int err;
-	enum lte_lc_func_mode mode = LTE_LC_FUNC_MODE_OFFLINE;
 
-	err = lte_lc_func_mode_get(&mode);
-	if (err) {
-		LOG_ERR("Failed to get LTE mode, error: %d", err);
-	} else if (mode != LTE_LC_FUNC_MODE_OFFLINE) {
+	if (current_func_mode == LTE_LC_FUNC_MODE_NORMAL) {
 		modem_params_get();
 	}
 

subsys/net/lib/nrf_cloud/Kconfig.nrf_cloud_coap

@@ -57,6 +57,12 @@ config NRF_CLOUD_COAP_DOWNLOADS
 	  When enabled, the Kconfig option COAP_EXTENDED_OPTIONS_LEN_VALUE must be increased
 	  from the default value. Safe values are 80 for P-GPS and 192 for FOTA.
 
+config NRF_CLOUD_COAP_DISCONNECT_ON_FAILED_REQUEST
+	bool "Disconnect on failed request"
+	help
+	  Enabling this option will ensure that the CoAP client is disconnected when a request
+	  fails to be sent. (Maximum retransmissions reached).
+
 module = NRF_CLOUD_COAP
 module-str = nRF Cloud COAP
 source "subsys/logging/Kconfig.template.log_config"

subsys/net/lib/nrf_cloud/coap/src/nrf_cloud_coap_transport.c

@@ -509,6 +509,9 @@ static int client_transfer(enum coap_method method,
 		k_sem_give(&serial_sem);
 	}
 	xfer_ctx_release(xfer);
+	if (err == -ETIMEDOUT && IS_ENABLED(CONFIG_NRF_CLOUD_COAP_DISCONNECT_ON_FAILED_REQUEST)) {
+		nrf_cloud_coap_disconnect();
+	}
 	return err;
 }
 

subsys/net/lib/nrf_cloud/src/nrf_cloud_fota_poll.c

@@ -13,6 +13,7 @@
 #include <net/nrf_cloud_rest.h>
 #include <zephyr/logging/log.h>
 #include <net/fota_download.h>
+#include <fota_download_util.h>
 #include "nrf_cloud_download.h"
 #include "nrf_cloud_fota.h"
 
@@ -137,6 +138,7 @@ static void http_fota_dl_handler(const struct fota_download_evt *evt)
 		fota_status_details = FOTA_STATUS_DETAILS_SUCCESS;
 
 		if (ctx_ptr->is_nonblocking) {
+			k_work_cancel_delayable(&ctx_ptr->timeout_work);
 			handle_download_succeeded_and_reboot(ctx_ptr);
 		} else {
 			k_sem_give(&fota_download_sem);
@@ -153,6 +155,18 @@ static void http_fota_dl_handler(const struct fota_download_evt *evt)
 		LOG_ERR("FOTA download error: %d", evt->cause);
 
 		nrf_cloud_download_end();
+
+		/* Do an image reset if full modem FOTA fails. This is to avoid resuming the
+		 * download when doing a full modem FOTA, which does not currently work.
+		 */
+		if (ctx_ptr->img_type == DFU_TARGET_IMAGE_TYPE_FULL_MODEM) {
+			int err = fota_download_util_image_reset(DFU_TARGET_IMAGE_TYPE_FULL_MODEM);
+
+			if (err) {
+				LOG_ERR("fota_download_util_image_reset() failed: %d\n", err);
+			}
+		}
+
 		fota_status = NRF_CLOUD_FOTA_FAILED;
 		fota_status_details = FOTA_STATUS_DETAILS_DL_ERR;
 
@@ -168,7 +182,8 @@ static void http_fota_dl_handler(const struct fota_download_evt *evt)
 		}
 
 		if (ctx_ptr->is_nonblocking) {
-			ctx_ptr->error_fn(fota_status, fota_status_details);
+			k_work_cancel_delayable(&ctx_ptr->timeout_work);
+			ctx_ptr->status_fn(fota_status, fota_status_details);
 
 			(void)update_job_status(ctx_ptr);
 		} else {
@@ -188,7 +203,8 @@ static void http_fota_dl_handler(const struct fota_download_evt *evt)
 			fota_status = NRF_CLOUD_FOTA_TIMED_OUT;
 			fota_status_details = FOTA_STATUS_DETAILS_TIMEOUT;
 
-			ctx_ptr->error_fn(fota_status, fota_status_details);
+			k_work_cancel_delayable(&ctx_ptr->timeout_work);
+			ctx_ptr->status_fn(fota_status, fota_status_details);
 
 			(void)update_job_status(ctx_ptr);
 		}
@@ -209,7 +225,8 @@ static void http_fota_dl_handler(const struct fota_download_evt *evt)
 			fota_status_details = FOTA_STATUS_DETAILS_DL_ERR;
 
 			if (ctx_ptr->is_nonblocking) {
-				ctx_ptr->error_fn(fota_status, fota_status_details);
+				k_work_cancel_delayable(&ctx_ptr->timeout_work);
+				ctx_ptr->status_fn(fota_status, fota_status_details);
 
 				(void)update_job_status(ctx_ptr);
 			} else {
@@ -295,7 +312,7 @@ int nrf_cloud_fota_poll_init(struct nrf_cloud_fota_poll_ctx *ctx)
 	 * so the timeout must be detected via a delayable work item instead of a semaphore.
 	 * Calls to nrf_cloud_fota_poll_process() will then be nonblocking.
 	 */
-	if (ctx->error_fn) {
+	if (ctx->status_fn) {
 		ctx->is_nonblocking = true;
 
 		k_work_init_delayable(&ctx->timeout_work, fota_dl_timeout_work_fn);
@@ -398,22 +415,21 @@ static int update_job_status(struct nrf_cloud_fota_poll_ctx *ctx)
 
 static int start_download(void)
 {
-	enum dfu_target_image_type img_type;
 	static int sec_tag;
 	int ret = 0;
 
 	/* Start the FOTA download, specifying the job/image type */
 	switch (job.type) {
 	case NRF_CLOUD_FOTA_BOOTLOADER:
 	case NRF_CLOUD_FOTA_APPLICATION:
-		img_type = DFU_TARGET_IMAGE_TYPE_MCUBOOT;
+		ctx_ptr->img_type = DFU_TARGET_IMAGE_TYPE_MCUBOOT;
 		break;
 	case NRF_CLOUD_FOTA_MODEM_DELTA:
-		img_type = DFU_TARGET_IMAGE_TYPE_MODEM_DELTA;
+		ctx_ptr->img_type = DFU_TARGET_IMAGE_TYPE_MODEM_DELTA;
 		break;
 	case NRF_CLOUD_FOTA_MODEM_FULL:
 		if (IS_ENABLED(CONFIG_NRF_CLOUD_FOTA_FULL_MODEM_UPDATE)) {
-			img_type = DFU_TARGET_IMAGE_TYPE_FULL_MODEM;
+			ctx_ptr->img_type = DFU_TARGET_IMAGE_TYPE_FULL_MODEM;
 		} else {
 			LOG_ERR("Not configured for full modem FOTA");
 			ret = -EFTYPE;
@@ -446,7 +462,7 @@ static int start_download(void)
 					      CONFIG_NRF_CLOUD_FOTA_DOWNLOAD_FRAGMENT_SIZE,
 		},
 		.fota = {
-			.expected_type = img_type,
+			.expected_type = ctx_ptr->img_type,
 			.img_sz = job.file_size
 		}
 	};
@@ -521,6 +537,10 @@ static void handle_download_succeeded_and_reboot(struct nrf_cloud_fota_poll_ctx
 		(void)update_job_status(ctx);
 	}
 
+	if (ctx_ptr->status_fn) {
+		ctx_ptr->status_fn(NRF_CLOUD_FOTA_SUCCEEDED, NULL);
+	}
+
 	if (ctx->reboot_fn) {
 		ctx->reboot_fn(FOTA_REBOOT_SUCCESS);
 	}
@@ -563,14 +583,18 @@ int nrf_cloud_fota_poll_process(struct nrf_cloud_fota_poll_ctx *ctx)
 		return -EAGAIN;
 	}
 
+	if (ctx_ptr->status_fn) {
+		ctx_ptr->status_fn(NRF_CLOUD_FOTA_DOWNLOADING, NULL);
+	}
+
 	/* Start the FOTA download process and wait for completion (or timeout) */
 	err = start_download();
 	if (err) {
 		LOG_ERR("Failed to start FOTA download");
 		return -ENOTRECOVERABLE;
 	}
 
-	/* Set timeout and return if this call is asynchronous, ie error_fn is defined */
+	/* Set timeout and return if this call is asynchronous, ie status_fn is defined */
 	if (ctx->is_nonblocking) {
 		k_work_schedule(&ctx->timeout_work, K_MINUTES(CONFIG_FOTA_DL_TIMEOUT_MIN));