From 48b34ca2b58a037637fa7c5e932aa08c98013efa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Simen=20S=2E=20R=C3=B8stad?= Date: Thu, 12 Sep 2024 12:43:30 +0200 Subject: [PATCH 1/5] net: lib: nrf_cloud: Add FOTA status callback MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Convert error callback to include general status notifications from the FOTA library as well as error events. This gives the application the opportunity to block network related operations during FOTA updates. Add image type to reboot function to allow the application to reinitialize the modem after a firmware update. Its not nessecary to reboot the device after modem FOTA. Signed-off-by: Simen S. Røstad --- include/net/nrf_cloud.h | 2 +- include/net/nrf_cloud_fota_poll.h | 22 +++++----- .../lib/nrf_cloud/src/nrf_cloud_fota_poll.c | 44 ++++++++++++++----- 3 files changed, 46 insertions(+), 22 deletions(-) diff --git a/include/net/nrf_cloud.h b/include/net/nrf_cloud.h index 755bb859a517..21962e6bd77c 100644 --- a/include/net/nrf_cloud.h +++ b/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, diff --git a/include/net/nrf_cloud_fota_poll.h b/include/net/nrf_cloud_fota_poll.h index 4ba1a6ce93d0..29e34566cff1 100644 --- a/include/net/nrf_cloud_fota_poll.h +++ b/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; }; /** diff --git a/subsys/net/lib/nrf_cloud/src/nrf_cloud_fota_poll.c b/subsys/net/lib/nrf_cloud/src/nrf_cloud_fota_poll.c index 532e0a14d10f..68edd30353ed 100644 --- a/subsys/net/lib/nrf_cloud/src/nrf_cloud_fota_poll.c +++ b/subsys/net/lib/nrf_cloud/src/nrf_cloud_fota_poll.c @@ -13,6 +13,7 @@ #include #include #include +#include #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,7 +415,6 @@ 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; @@ -406,14 +422,14 @@ static int start_download(void) 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,6 +583,10 @@ 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) { @@ -570,7 +594,7 @@ int nrf_cloud_fota_poll_process(struct nrf_cloud_fota_poll_ctx *ctx) 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)); From dc9d8ac43a5a16c08ca7a2372a1549b94f6a117f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Simen=20S=2E=20R=C3=B8stad?= Date: Thu, 12 Sep 2024 12:48:37 +0200 Subject: [PATCH 2/5] memfault: Various fixes for FOTA MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Use nrf modem library CFUN callback to prevent modem info calls in offline and powered off states. This is needed instead of fetching the func mode due to the modem being shutdown when a lte handler notification is received. - Ease up on the logging when modem info API are called. Its expected that they fail in certain scenarios, and logging an error in those instances is not necessary. Signed-off-by: Simen S. Røstad --- .../memfault_lte_metrics.c | 62 +++++++++++-------- 1 file changed, 36 insertions(+), 26 deletions(-) diff --git a/modules/memfault-firmware-sdk/memfault_lte_metrics.c b/modules/memfault-firmware-sdk/memfault_lte_metrics.c index 5b42d3782458..4c50328b1b80 100644 --- a/modules/memfault-firmware-sdk/memfault_lte_metrics.c +++ b/modules/memfault-firmware-sdk/memfault_lte_metrics.c @@ -9,6 +9,7 @@ #include #include +#include #if defined(CONFIG_MODEM_INFO) #include @@ -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,32 +75,35 @@ 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 } @@ -97,12 +111,8 @@ static void modem_params_get(void) 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(); } From d0b445ed10318a0349d25b109d1f2bb48083a601 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Simen=20S=2E=20R=C3=B8stad?= Date: Thu, 12 Sep 2024 12:57:13 +0200 Subject: [PATCH 3/5] net: lib: nrf_cloud_coap: Add option to disconnect on a failed request MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add option, CONFIG_NRF_CLOUD_COAP_DISCONNECT_ON_FAILED_REQUEST, that disconnects the CoAP client on a failed request. Disabled by default. Signed-off-by: Simen S. Røstad --- subsys/net/lib/nrf_cloud/Kconfig.nrf_cloud_coap | 6 ++++++ .../net/lib/nrf_cloud/coap/src/nrf_cloud_coap_transport.c | 3 +++ 2 files changed, 9 insertions(+) diff --git a/subsys/net/lib/nrf_cloud/Kconfig.nrf_cloud_coap b/subsys/net/lib/nrf_cloud/Kconfig.nrf_cloud_coap index 0d42e4c469c9..5a02242b2c8c 100644 --- a/subsys/net/lib/nrf_cloud/Kconfig.nrf_cloud_coap +++ b/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" diff --git a/subsys/net/lib/nrf_cloud/coap/src/nrf_cloud_coap_transport.c b/subsys/net/lib/nrf_cloud/coap/src/nrf_cloud_coap_transport.c index 1d454d4fe8fa..78971e6f12c0 100644 --- a/subsys/net/lib/nrf_cloud/coap/src/nrf_cloud_coap_transport.c +++ b/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; } From 54c139d87bd4496462cd1ae60c481d3ce6c671ea Mon Sep 17 00:00:00 2001 From: Tommi Rantanen Date: Mon, 16 Sep 2024 15:15:50 +0300 Subject: [PATCH 4/5] lib: lte_link_control: eDRX request not in workqueue in power off lte_lc_edrx_req() will be run directly in lte_lc_edrx_on_modem_cfun() when power off is issued. Earlier it was submitted to a work queue and run in lte_lc_work_q. Work queue was originally added because we wanted 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. This was added in PR #15203 Jira: NCSIDB-1347 Signed-off-by: Tommi Rantanen --- lib/lte_link_control/lte_lc.c | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/lib/lte_link_control/lte_lc.c b/lib/lte_link_control/lte_lc.c index e000c154e99c..69d7038dad81 100644 --- a/lib/lte_link_control/lte_lc.c +++ b/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(<e_lc_work_q, <e_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); } } From 3b06b18005bd901e22b1b14402d0e20cd990bfa7 Mon Sep 17 00:00:00 2001 From: Tommi Rantanen Date: Mon, 16 Sep 2024 16:16:48 +0300 Subject: [PATCH 5/5] lib: lte_link_control: Remove power off from modem shutdown nrf_modem_lib uses 'AT+CFUN=0' in nrf_modem_lib_shutdown() if the mode is not already '0'. So doing this also in lte_lc is unnecessary. Signed-off-by: Tommi Rantanen --- lib/lte_link_control/lte_lc_modem_hooks.c | 14 -------------- tests/lib/lte_lc_api/src/lte_lc_api_test.c | 16 ---------------- 2 files changed, 30 deletions(-) diff --git a/lib/lte_link_control/lte_lc_modem_hooks.c b/lib/lte_link_control/lte_lc_modem_hooks.c index a6f0ee1bb7f9..958c2a2de90b 100644 --- a/lib/lte_link_control/lte_lc_modem_hooks.c +++ b/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 diff --git a/tests/lib/lte_lc_api/src/lte_lc_api_test.c b/tests/lib/lte_lc_api/src/lte_lc_api_test.c index 5ca158c634f1..4e4efc495b30 100644 --- a/tests/lib/lte_lc_api/src/lte_lc_api_test.c +++ b/tests/lib/lte_lc_api/src/lte_lc_api_test.c @@ -266,7 +266,6 @@ static int sys_init_helper(void) } extern void on_modem_init(int err, void *ctx); -extern void on_modem_shutdown(void *ctx); extern void lte_lc_on_modem_cfun(int mode, void *ctx); void test_lte_lc_on_modem_init_success(void) @@ -344,21 +343,6 @@ void test_lte_lc_on_modem_init_rai_fail(void) on_modem_init(0, NULL); } -void test_lte_lc_on_modem_shutdown(void) -{ - __cmock_nrf_modem_is_initialized_ExpectAndReturn(true); - __mock_nrf_modem_at_printf_ExpectAndReturn("AT+CFUN=0", EXIT_SUCCESS); - - on_modem_shutdown(NULL); -} - -void test_lte_lc_on_modem_shutdown_not_initialized(void) -{ - __cmock_nrf_modem_is_initialized_ExpectAndReturn(false); - - on_modem_shutdown(NULL); -} - static int lte_lc_on_modem_cfun_callback_count; static void on_modem_cfun_callback(enum lte_lc_func_mode, void *ctx)