samples: siwx917_ota: http/s OTAF application #111

Devika-Raju-Z · 2025-09-16T04:54:29Z

Updating the SiWx917 device's firmware including NWP, M4, and the combined M4 + TA images using an HTTP or HTTPS server. It supports both secure and non-secure firmware updates for all three types: NWP, M4, and the combined image.

samples/siwx917_https_otaf/CMakeLists.txt

samples/siwx917_https_otaf/README.rst

samples/siwx917_https_otaf/src/main.c

samples/siwx917_https_otaf/CMakeLists.txt

samples/siwx917_https_otaf/README.rst

samples/siwx917_https_otaf/prj.conf

samples/siwx917_https_otaf/src/main.c

samples/siwx91x_ota/prj.conf

samples/siwx917_https_otaf/src/app_config.h

samples/siwx917_https_otaf/src/main.c

jhedberg · 2025-09-26T12:38:51Z

Please rebase this on the latest main branch to see if the Compliance failure gets resolved

samples/siwx91x_ota/src/main.c

jerome-pouiller · 2025-10-06T14:34:27Z

samples/siwx91x_ota/src/main.c

+/* Message for HTTP data */
+typedef struct {
+	uint32_t length;
+	uint8_t buffer[RECV_BUFFER_SIZE];


RECV_BUFFER_SIZE is an arbitrary value and used only once. Drop the symbols and replace it the value.

samples/siwx91x_ota/src/main.c

jerome-pouiller · 2025-10-06T15:54:24Z

samples/siwx91x_ota/src/main.c

+	if (app_ctx_st->http_response_error) {
+		printf("Error occurred during HTTP download, skipping processing\n");
+		return -EPROTO;
+	}


Should probably handled by the caller.

samples/siwx91x_ota/src/main.c

jerome-pouiller

I stop here for today.

samples/siwx91x_ota/CMakeLists.txt

samples/siwx91x_ota/Kconfig

jerome-pouiller · 2025-10-23T10:01:02Z

samples/siwx91x_ota/Kconfig

+
+config OTA_UPDATE_URL
+	string "OTA update URL"
+	default "http://example.com:8080/firmware.rps"


Documentation talks about HTTPS, but the default value doesn't use it.

jerome-pouiller · 2025-10-23T10:02:24Z

samples/siwx91x_ota/README.rst

+
+Application demonstrates how to perform HTTP/HTTPS OTA firmware updates on the
+SiWx917 platform using Zephyr RTOS. It connects to a Wi-Fi network, establishes
+a secure HTTPS connection using a CA certificate, and downloads firmware


a secure HTTPS connection using a CA certificate

In simple words: an HTTPS connection.

samples/siwx91x_ota/src/main.c

jerome-pouiller · 2025-11-05T17:17:34Z

samples/siwx91x_ota/src/main.c

+
+error:
+	if (ctx->sock < 0) {
+		zsock_close(ctx->sock);


hu? probably you mean if (ctx->sock >= 0)?

In fact, even this test is useless since zsock_close() on negative value will properly catch the error.

jerome-pouiller · 2025-11-05T17:18:01Z

samples/siwx91x_ota/src/main.c

+	ret = zsock_connect(ctx->sock, res->ai_addr, res->ai_addrlen);
+	if (ret < 0) {
+		printf("Connection failed (%d): %s\n", -errno, strerror(errno));
+		ctx->sock = -1;


hu? you leak the socket ressource.

jerome-pouiller · 2025-11-05T17:19:43Z

samples/siwx91x_ota/src/main.c

+
+	ret = zsock_getaddrinfo(http_parse_data_info.host, http_parse_data_info.port, NULL, &res);
+	if (ret != 0) {
+		printf("Address resolution failed: %s\n", zsock_gai_strerror(ret));


Since the host and the port are easily reachable and provide a useful information, you can provide them:

printf("Cannot resolve %s:%s: %s\n", http_parse_data_info.host, http_parse_data_info.port, zsock_gai_strerror(ret));

jerome-pouiller · 2025-11-05T17:22:23Z

samples/siwx91x_ota/src/main.c

+ */
+static int ota_connect_to_server(struct app_ctx *ctx)
+{
+	struct http_parse_data http_parse_data_info = ctx->http_parse_data_info;


data and info are redundant. BTW, why don't you get a pointer rather than duplicating this structure?

struct url_data *url_data = &ctx->url_data;

This application demonstrates the support for OTA firmware upgrade. Signed-off-by: Devika Raju <[email protected]>

jerome-pouiller · 2025-11-06T08:44:11Z

samples/siwx91x_ota/prj.conf

+CONFIG_NET_TCP_MAX_RECV_WINDOW_SIZE=10240
+
+# Socket Support
+CONFIG_REQUIRES_FULL_LIBC=y


Can you explain why you required this parameter?

jerome-pouiller · 2025-11-06T10:30:29Z

samples/siwx91x_ota/prj.conf

+# TLS Security Configuration
+CONFIG_MBEDTLS=y
+CONFIG_MBEDTLS_BUILTIN=y
+CONFIG_MBEDTLS_HEAP_SIZE=60000


Is it required?

jerome-pouiller · 2025-11-06T10:32:49Z

samples/siwx91x_ota/prj.conf

+CONFIG_REBOOT=y
+
+CONFIG_OTA_WIFI_SSID="<AP-SSID>"
+CONFIG_OTA_WIFI_PSK="<AP-PASSWORD>"


You provide samples values in Kconfig and sample values in prj.conf. We probably want to keep them in only one place. I suggest to remove default attribute in Kconfig and add a default value for OTA_UPDATE_URL in prj.conf

jerome-pouiller · 2025-11-06T10:35:54Z

samples/siwx91x_ota/sample.yaml

+  sample.net.http_otaf:
+    harness: net
+    platform_allow:
+      - siwx917_rb4338a


This test also work with siwx917_rb4342a and dh2605a.
Maybe:

filter: CONFIG_SOC_FAMILY_SILABS_SIWX91X

jerome-pouiller · 2025-11-06T10:37:29Z

samples/siwx91x_ota/src/main.c

+};
+
+struct http_parse_data {
+	char *schema;


I would have keep is_tls_enabled and removed schema. But it's not a big deal.

jerome-pouiller · 2025-11-06T16:36:07Z

samples/siwx91x_ota/src/main.c

+	}
+	if (ctx->http_status_code == 301) {
+		ret = -EPROTO;
+	}


Is it useful to return a different errno? the caller does make the difference anyway.

jerome-pouiller · 2025-11-06T16:38:11Z

samples/siwx91x_ota/src/main.c

+		 ctx->ota_range_start_byte, ctx->ota_range_end_byte);
+
+	/* Send request */
+	ret = http_client_req(ctx->sock, &req, 30000, ctx);


30 seconds is huge. Does it make sense to not return an error earlier?

jerome-pouiller · 2025-11-06T16:39:25Z

samples/siwx91x_ota/src/main.c

+	return ret;
+}
+
+static bool ota_update_http_range_header(struct app_ctx *ctx)


Be uniform, return an errno.

jerome-pouiller · 2025-11-06T16:41:10Z

samples/siwx91x_ota/src/main.c

+
+static bool ota_update_http_range_header(struct app_ctx *ctx)
+{
+	ctx->ota_range_start_byte = ctx->ota_range_end_byte + 1;


This is probably unnecessary twisted. ctx->ota_range_start_byte += sizeof(ctx->flash_buffer); is easier to understand (sizeof(...) is a constant data while ctx->ota_range_end_byte may have been updated somewhere in the algorithm)

jerome-pouiller · 2025-11-06T16:48:47Z

samples/siwx91x_ota/src/main.c

+				if (!ota_handle_retry(ctx, operation, true)) {
+					ctx->retry_count++;
+					continue;
+				}


Nope, this is not correct. If sl_si91x_fwup_load() return an error, you have to properly restart the download from the beginning (because we wrote corrupted data). Currently, it will never recover the error.

Globally, the error management is difficult to review. I suggest to clean it up.

Devika-Raju-Z requested a review from a team September 16, 2025 04:54

Devika-Raju-Z force-pushed the https_otaf branch from 466273e to 6a581ad Compare September 16, 2025 05:19

jerome-pouiller requested changes Sep 16, 2025

View reviewed changes

jerome-pouiller reviewed Sep 17, 2025

View reviewed changes

Devika-Raju-Z force-pushed the https_otaf branch 24 times, most recently from 569c7b4 to dd7f3de Compare September 25, 2025 17:03

jhedberg changed the title ~~http/s otaf application~~ samples: siwx917_ota: http/s OTAF application Sep 26, 2025

Devika-Raju-Z force-pushed the https_otaf branch 7 times, most recently from 6025ad2 to 8cc35e9 Compare October 1, 2025 09:07

jerome-pouiller reviewed Oct 6, 2025

View reviewed changes

samples/siwx91x_ota/src/main.c Outdated Show resolved Hide resolved

Devika-Raju-Z force-pushed the https_otaf branch from 8cc35e9 to 00eb379 Compare October 8, 2025 04:59

Devika-Raju-Z requested a review from a team as a code owner October 8, 2025 04:59

Devika-Raju-Z force-pushed the https_otaf branch 6 times, most recently from 9c620ad to 7fe97ff Compare October 14, 2025 09:48

Devika-Raju-Z force-pushed the https_otaf branch 5 times, most recently from c3b2473 to 4bf41dc Compare October 21, 2025 09:50

jerome-pouiller reviewed Oct 23, 2025

View reviewed changes

Devika-Raju-Z force-pushed the https_otaf branch 2 times, most recently from 1b8ada9 to a13421c Compare October 29, 2025 11:54

jerome-pouiller reviewed Nov 5, 2025

View reviewed changes

Devika-Raju-Z force-pushed the https_otaf branch from a13421c to b2555db Compare November 6, 2025 05:45

samples: siwx917_ota: http/s OTAF application

4ad0105

This application demonstrates the support for OTA firmware upgrade. Signed-off-by: Devika Raju <[email protected]>

Devika-Raju-Z force-pushed the https_otaf branch from b2555db to 4ad0105 Compare November 6, 2025 05:57

jerome-pouiller reviewed Nov 6, 2025

View reviewed changes

samples: siwx917_ota: http/s OTAF application #111

Are you sure you want to change the base?

samples: siwx917_ota: http/s OTAF application #111

Uh oh!

Conversation

Devika-Raju-Z commented Sep 16, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jhedberg commented Sep 26, 2025

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jerome-pouiller left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment