net: fota_download: Add more error codes to callback

Add error codes for
* Unsupported protocol
* DFU failure
* Invalid configuration or invalid URI

This is because Downloader may give us -EPROTONOSUPPORT or
-EINVAL and these have no equivalent on FOTA Download callbacks.

DFU failures might also be usefull to see as a separate error
instead of generic INTERNAL error.

Signed-off-by: Seppo Takalo <[email protected]>

Author: SeppoTakalo

doc/nrf/releases_and_maturity/releases/release-notes-changelog.rst

@@ -633,6 +633,7 @@ Libraries for networking
 
 * :ref:`lib_fota_download` library:
 
+  * Added error codes related to unsupported protocol, DFU failures, and invalid configuration.
   * Updated to use the :ref:`lib_downloader` library for CoAP downloads.
 
 * :ref:`lib_nrf_cloud` library:

include/net/fota_download.h

@@ -83,6 +83,12 @@ enum fota_download_error_cause {
 	FOTA_DOWNLOAD_ERROR_CAUSE_TYPE_MISMATCH,
 	/** Generic error on device side. */
 	FOTA_DOWNLOAD_ERROR_CAUSE_INTERNAL,
+	/** Error on DFU library */
+	FOTA_DOWNLOAD_ERROR_CAUSE_DFU,
+	/** Protocol not supported */
+	FOTA_DOWNLOAD_ERROR_CAUSE_PROTO_NOT_SUPPORTED,
+	/** Invalid URI or invalid configuration */
+	FOTA_DOWNLOAD_ERROR_CAUSE_INVALID_CONFIGURATION,
 };
 
 /**

subsys/net/lib/fota_download/src/fota_download.c

@@ -187,7 +187,7 @@ static int downloader_callback(const struct downloader_evt *event)
 					set_error_state(FOTA_DOWNLOAD_ERROR_CAUSE_INVALID_UPDATE);
 				} else if (err < 0) {
 					LOG_ERR("dfu_target_init error %d", err);
-					set_error_state(FOTA_DOWNLOAD_ERROR_CAUSE_INTERNAL);
+					set_error_state(FOTA_DOWNLOAD_ERROR_CAUSE_DFU);
 				}
 			}
 
@@ -199,7 +199,7 @@ static int downloader_callback(const struct downloader_evt *event)
 			if (err != 0) {
 				LOG_DBG("unable to get dfu target offset err: "
 					"%d", err);
-				set_error_state(FOTA_DOWNLOAD_ERROR_CAUSE_DOWNLOAD_FAILED);
+				set_error_state(FOTA_DOWNLOAD_ERROR_CAUSE_DFU);
 				goto error_and_close;
 			}
 
@@ -213,15 +213,15 @@ static int downloader_callback(const struct downloader_evt *event)
 					if (err != 0) {
 						LOG_ERR("Unable to reset DFU target, err: %d", err);
 						set_error_state(
-							FOTA_DOWNLOAD_ERROR_CAUSE_DOWNLOAD_FAILED);
+							FOTA_DOWNLOAD_ERROR_CAUSE_DFU);
 						goto error_and_close;
 					}
 					err = dfu_target_init(img_type, 0, file_size,
 							      dfu_target_callback_handler);
 					if (err != 0) {
 						LOG_ERR("Failed to re-initialize target, err: %d",
 							err);
-						set_error_state(FOTA_DOWNLOAD_ERROR_CAUSE_INTERNAL);
+						set_error_state(FOTA_DOWNLOAD_ERROR_CAUSE_DFU);
 						goto error_and_close;
 					}
 				} else {
@@ -246,7 +246,7 @@ static int downloader_callback(const struct downloader_evt *event)
 			goto error_and_close;
 		} else if (err != 0) {
 			LOG_ERR("dfu_target_write error %d", err);
-			set_error_state(FOTA_DOWNLOAD_ERROR_CAUSE_INTERNAL);
+			set_error_state(FOTA_DOWNLOAD_ERROR_CAUSE_DFU);
 			goto error_and_close;
 		}
 
@@ -278,7 +278,7 @@ static int downloader_callback(const struct downloader_evt *event)
 
 		if (err != 0) {
 			LOG_ERR("dfu_target_done error: %d", err);
-			set_error_state(FOTA_DOWNLOAD_ERROR_CAUSE_INTERNAL);
+			set_error_state(FOTA_DOWNLOAD_ERROR_CAUSE_DFU);
 			goto error_and_close;
 		}
 
@@ -300,26 +300,44 @@ static int downloader_callback(const struct downloader_evt *event)
 			/* Fall through and return 0 below to tell
 			 * downloader to retry
 			 */
-		} else if ((event->error == -ECONNABORTED) ||
-			   (event->error == -ECONNREFUSED) ||
-			   (event->error == -EHOSTUNREACH)) {
-			LOG_ERR("Download client failed to connect to server");
-			set_error_state(FOTA_DOWNLOAD_ERROR_CAUSE_CONNECT_FAILED);
+			break;
+		}
+		LOG_ERR("Downloader error event %d", event->error);
+		err = dfu_target_done(false);
+		if (err == -EACCES) {
+			LOG_DBG("No DFU target was initialized");
+		} else if (err != 0) {
+			LOG_ERR("Unable to deinitialize resources "
+				"used by dfu_target.");
+		}
+		switch (event->error) {
 
-			goto error_and_close;
-		} else {
-			LOG_ERR("Downloader error event %d", event->error);
-			err = dfu_target_done(false);
-			if (err == -EACCES) {
-				LOG_DBG("No DFU target was initialized");
-			} else if (err != 0) {
-				LOG_ERR("Unable to deinitialze resources "
-					"used by dfu_target.");
-			}
+		case -ECONNABORTED:
+		case -ECONNREFUSED:
+		case -EHOSTUNREACH:
+		case -ENETDOWN:
+			set_error_state(FOTA_DOWNLOAD_ERROR_CAUSE_CONNECT_FAILED);
+			break;
+		case -ECONNRESET:
+		case -ETIMEDOUT:
+		case -EHOSTDOWN:
+		case -EBADMSG:
+		case -ERANGE:
+		case -E2BIG:
 			set_error_state(FOTA_DOWNLOAD_ERROR_CAUSE_DOWNLOAD_FAILED);
-			goto error_and_close;
+			break;
+		case -EPROTONOSUPPORT:
+			set_error_state(FOTA_DOWNLOAD_ERROR_CAUSE_PROTO_NOT_SUPPORTED);
+			break;
+		case -EINVAL:
+		case -EAFNOSUPPORT:
+			set_error_state(FOTA_DOWNLOAD_ERROR_CAUSE_INVALID_CONFIGURATION);
+			break;
+		default:
+			set_error_state(FOTA_DOWNLOAD_ERROR_CAUSE_INTERNAL);
+			break;
 		}
-		break;
+		goto error_and_close;
 	case DOWNLOADER_EVT_STOPPED:
 		atomic_set_bit(&flags, FLAG_STOPPED);
 		/* Only clear flags if we are not going to resume */

subsys/net/lib/fota_download/src/util/fota_download_util.c

@@ -184,10 +184,24 @@ static void start_fota_download(struct k_work *work)
 	ret = fota_download_start_with_image_type(fota_host, fota_path, fota_sec_tag, 0, 0,
 						  active_dfu_type);
 	if (ret) {
-		struct fota_download_evt evt;
+		struct fota_download_evt evt = {
+			.id = FOTA_DOWNLOAD_EVT_ERROR
+		};
+
+		switch (ret) {
+		case -EINVAL:
+		case -E2BIG:
+			evt.cause = FOTA_DOWNLOAD_ERROR_CAUSE_INVALID_CONFIGURATION;
+			break;
+		case -EPROTONOSUPPORT:
+			evt.cause = FOTA_DOWNLOAD_ERROR_CAUSE_PROTO_NOT_SUPPORTED;
+			break;
+		default:
+			evt.cause = FOTA_DOWNLOAD_ERROR_CAUSE_INTERNAL;
+			break;
+		}
 
 		LOG_ERR("fota_download_start() failed, return code %d", ret);
-		evt.id = FOTA_DOWNLOAD_EVT_CANCELLED;
 		fota_download_callback(&evt);
 	}
 }

subsys/net/lib/lwm2m_client_utils/lwm2m/lwm2m_firmware.c

@@ -902,6 +902,9 @@ static void fota_download_callback(const struct fota_download_evt *evt)
 		LOG_INF("FOTA download failed, target %d", dfu_image_type);
 		target_image_type_store(ongoing_obj_id, dfu_image_type);
 		switch (evt->cause) {
+		case FOTA_DOWNLOAD_ERROR_CAUSE_PROTO_NOT_SUPPORTED:
+			set_result(ongoing_obj_id, RESULT_UNSUP_PROTO);
+			break;
 		/* Connecting to the FOTA server failed. */
 		case FOTA_DOWNLOAD_ERROR_CAUSE_CONNECT_FAILED:
 			/* FALLTHROUGH */

tests/subsys/net/lib/fota_download/src/test_fota_download.c

@@ -13,6 +13,7 @@
 #include <pm_config.h>
 #include <fota_download.h>
 #include "test_fota_download_common.h"
+#include <fota_download_util.h>
 
 /* fota_download_start may modify the resource locator passed to it, so we cannot directly pass
  * test constants. Instead, we copy them to a modifiable buffer first.
@@ -32,6 +33,7 @@ static bool start_with_offset;
 static bool fail_on_offset_get;
 static bool fail_on_connect;
 static bool fail_on_start;
+static bool fail_on_proto;
 static bool download_with_offset_success;
 static downloader_callback_t downloader_event_handler;
 K_SEM_DEFINE(stop_sem, 0, 1);
@@ -101,6 +103,9 @@ int downloader_get_with_host_and_file(struct downloader *dl,
 				      const struct downloader_host_cfg *dl_host_cfg,
 				      const char *host, const char *file, size_t from)
 {
+	if (fail_on_proto == true) {
+		return -EPROTONOSUPPORT;
+	}
 	if (fail_on_connect == true) {
 		return -1;
 	}
@@ -209,8 +214,14 @@ void set_s0_active(bool s0_active)
 
 void client_callback(const struct fota_download_evt *evt)
 {
+	printk("Event: %d\n", evt->id);
 	switch (evt->id) {
 	case FOTA_DOWNLOAD_EVT_ERROR:
+		if (fail_on_proto) {
+			zassert_equal(evt->cause, FOTA_DOWNLOAD_ERROR_CAUSE_PROTO_NOT_SUPPORTED);
+			fail_on_proto = false;
+			k_sem_give(&download_with_offset_sem);
+		}
 		if (fail_on_offset_get == true) {
 			zassert_equal(evt->cause, FOTA_DOWNLOAD_ERROR_CAUSE_INTERNAL, NULL);
 			fail_on_offset_get = false;
@@ -253,10 +264,12 @@ static void init(void)
 	fail_on_offset_get = false;
 	fail_on_connect = false;
 	fail_on_start = false;
+	fail_on_proto = false;
 	downloader_get_file = NULL;
 	spm_s0_active_retval = false;
 
 	k_sem_reset(&stop_sem);
+	k_sem_reset(&download_with_offset_sem);
 
 	err = fota_download_init(client_callback);
 	zassert_equal(err, 0, NULL);
@@ -415,3 +428,63 @@ ZTEST(fota_download_tests, test_download_with_offset)
 	err = fota_download_cancel();
 	zassert_equal(err, -EAGAIN);
 }
+
+
+ZTEST(fota_download_tests, test_download_invalid_protocol)
+{
+	init();
+
+	fail_on_proto = true;
+	int err = fota_download_any("ftp://" BASE_DOMAIN, buf, NO_TLS, 0, 0, 0);
+
+	zassert_equal(err, -EPROTONOSUPPORT, "%d", err);
+	fota_download_cancel();
+
+}
+
+ZTEST_SUITE(fota_download_util, NULL, NULL, NULL, NULL, NULL);
+
+ZTEST(fota_download_util, test_download_util_invalid_protocol)
+{
+	init();
+
+	fail_on_proto = true;
+	int err = fota_download_util_download_start("ftp://host.com/file.bin",
+				      DFU_TARGET_IMAGE_TYPE_ANY, 0,
+				      client_callback);
+
+	zassert_ok(err, NULL);
+	k_sem_take(&download_with_offset_sem, K_SECONDS(2));
+	k_sem_take(&stop_sem, K_FOREVER);
+}
+
+ZTEST(fota_download_util, test_download_util_start)
+{
+	init();
+
+	uint8_t fragment_buf[1] = {0};
+	size_t  fragment_len = 1;
+	struct downloader_evt evt = {
+		.id = DOWNLOADER_EVT_FRAGMENT,
+		.fragment = {
+			.buf = fragment_buf,
+			.len = fragment_len,
+		}
+	};
+
+	start_with_offset = true;
+	int err = fota_download_util_download_start("http://host.com/file.bin",
+				      DFU_TARGET_IMAGE_TYPE_ANY, 0,
+				      client_callback);
+
+	zassert_ok(err);
+
+	err = downloader_event_handler(&evt);
+	zassert_ok(err);
+
+	evt.id = DOWNLOADER_EVT_DONE;
+	err = downloader_event_handler(&evt);
+	zassert_ok(err);
+
+	k_sem_take(&stop_sem, K_FOREVER);
+}