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.

Note

Adds a new Zephyr sample for SiWx917 that performs HTTP/HTTPS OTA firmware updates over Wi‑Fi with ranged downloads and firmware flashing.

New sample: samples/siwx91x_ota
- src/main.c: Implements Wi‑Fi connect/scan, IP config handling, DNS logging, and an OTA state machine (scan → server connect → ranged download) using http_client with TLS and Range headers; buffers chunks, validates image size, loads via sl_si91x_fwup_*, and reboots on success.
- prj.conf: Enables networking, sockets, DHCPv4, HTTP client, URL parser, mbedTLS (TLS 1.2), DNS resolver, and SiWx91x firmware upgrade support.
- Kconfig: Adds configurable CONFIG_OTA_WIFI_SSID, CONFIG_OTA_WIFI_PSK, and CONFIG_OTA_UPDATE_URL.
- CMakeLists.txt: Sets up app build and sources.
- README.rst: Documents purpose, configuration, build/run, and test steps.
- sample.yaml: Registers sample and net test for siwx917_rb4338a.

^{Written by Cursor Bugbot for commit 8c6da5c. This will update automatically on new commits. Configure here.}

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

jerome-pouiller

I stop here for today.

samples/siwx91x_ota/CMakeLists.txt

samples/siwx91x_ota/Kconfig

samples/siwx91x_ota/README.rst

samples/siwx91x_ota/src/main.c

samples/siwx91x_ota/prj.conf

samples/siwx91x_ota/sample.yaml

samples/siwx91x_ota/src/main.c

jerome-pouiller · 2025-11-07T10:55:59Z

samples/siwx91x_ota/src/main.c

+		case OTA_STATE_SCAN:
+			/* Scan for networks and connect */
+			ota_wifi_start_scan(ctx);
+			ret = ota_wifi_connect(ssid, pwd, ctx);


Management of ctx->security is not correct:

The network can use hidden SSID. In this case, ctx->security won't be filled.

There is a race condition between the scan results and the wifi_connect.

We don't have idea of how to handle the security for hidden SSID.
Do you have any suggestion?

Probably you could init security with WIFI_SECURITY_TYPE_PSK. So, it will try to connect using WPA2 (the most common case in 2025, I think) if the SSID is not found during the scan.

I believe error management for security field is still not properly managed (security is set during the first scan, but not reverted on error).

(note ipv4 and ipv6 status are correct because we receive the events from Zephyr in case of disconnection)

samples/siwx91x_ota/src/main.c

jerome-pouiller · 2025-11-13T10:00:21Z

samples/siwx91x_ota/src/main.c

+			break;
+		}
+	}
+}


With all the simplification we made, I start to see how error management should be done. Instead of calling ota_handle_retry() with various parameters, manage the error inline in the function. Thus, the error handling is transparent and easy to follow the error path:

if (ret < 0) { ctx->state = OTA_STATE_SERVER_CONNECT; goto out_close; } [...] if (ret < 0) { ctx->state = OTA_STATE_SERVER_CONNECT; goto out_reset; } [...] return 0; out_reset: ctx->range_start = 0; ctx->range_end = 1023; out_close: zsock_close(ctx->sock); ctx->sock = -1; out: return ret;

Ti work properly, the retry loop has be placed in main(). This change also allow to save one level on of indentation in ota_application_start() and change retry_count in a local variable. It also solve the issue with retry_count decremented even if operation success.

I also wonder if it make sense to break the loop on success. You could just fallback to the next state. Finally, we can see that the state OTA_STATE_SCAN is in fact the same than ctx->disconnected and OTA_STATE_SERVER_CONNECT is the same than ctx->sock < 0. Because the states were not properly updated, it blurries the global algorithm. At the end, this FSM is just a violation of the single source of trust principle. At the end, algorithm appear clearly if we remove the FSM:

if (ctx->disconnected /* OTA_STATE_SCAN */) { [manage wifi connection and goto out on error] } if (ctx->sock < 0 /* OTA_STATE_SERVER_CONNECT */) { [manage server connection and goto out on error] } [manage download and goto out_close on error] [manage flash and goto out_reset on error]

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

jerome-pouiller

I have not yet finish my review, but the things start to have a good shape.

jerome-pouiller · 2025-11-22T22:11:21Z

samples/siwx91x_ota/src/main.c

+
+	ctx->ota_image_size = rsp->content_range.total;
+
+	if (rsp->body_frag_len > 0) {


I believe you can remove the condition without changing the behavior of the code, but it is not a big deal.

jerome-pouiller · 2025-11-22T22:13:43Z

samples/siwx91x_ota/src/main.c

+		       ctx->range_start, ctx->range_start + sizeof(ctx->flash_buffer) - 1);
+		return -EPROTO;
+	}
+	return ret;


return 0;. Not a big deal.

Addressed the comment

jerome-pouiller · 2025-11-22T22:18:26Z

samples/siwx91x_ota/src/main.c

+		buf_offset = sizeof(sl_si91x_firmware_header_t);
+		status = sl_wifi_get_firmware_size((void *)ctx->flash_buffer, &ota_calc_image_size);
+		if (status != SL_STATUS_OK) {
+			printf("Unable to fetch firmware size. Status: 0x%x\n", status);


Generally, we pad the hexadecimal numbers on 2, 4 or 8 characters (0x%08x).

jerome-pouiller · 2025-11-22T22:18:58Z

samples/siwx91x_ota/src/main.c

+
+		status = sl_si91x_fwup_start(ctx->flash_buffer);
+		if (status != SL_STATUS_OK) {
+			printf("Failed to load firmware header (0x%x)\n", status);


jerome-pouiller · 2025-11-23T11:58:18Z

samples/siwx91x_ota/src/main.c

+	ret = ota_load_firmware(ctx);
+	if (ret < 0) {
+		goto out_reset;
+	} else {


This else statement is useless. The idea is to use early return to keep the nominal case linear:

ret = fn1(); if (ret < 0) { goto early_return1; } ret = fn2(); if (ret < 0) { goto early_return2; }

jerome-pouiller · 2025-11-23T11:58:32Z

samples/siwx91x_ota/src/main.c

+			goto out_reset;
+		}
+	}
+	return ret;


It does not change the behavior, but return 0; is easier to understand.

(for the coding style, keep a blank between normal return and error management)

jerome-pouiller · 2025-11-23T12:02:46Z

samples/siwx91x_ota/src/main.c

+	return ret;
+out_reset:
+	ctx->range_start = 0;
+	return ret;


If we reset, don't we want to reconnect? The idea is to use fallbacks between the label to reset the various state (as you would do with an exception in python).

jerome-pouiller · 2025-11-23T12:04:28Z

samples/siwx91x_ota/src/main.c

+		ret = ota_connect_to_server(ctx);
+		if (ret < 0) {
+			goto out_close;
+		}


ota_connect_to_server() is well written. It does not change the context in case of error. You don't have to close the socket. return ret; is sufficient.

jerome-pouiller · 2025-11-23T12:17:24Z

samples/siwx91x_ota/src/main.c

+			k_sleep(K_MSEC(1000));
+		}
+	}
+	return (ret < 0) ? ret : 0;


Are you sure you need a suche complex code for a such simple thing?

while (retry_count < 3) { ret = ota_application_start(&app_ctx); if (ret < 0) { retry_count++; k_sleep(K_MSEC(1000)); } } printf("Maximum retries exceeded, aborting OTA\n"); return -EIO;

(Note the detailed errors are already displayed in the earlier functions, having yet another error message here has little benefit).

(It would be even easier if ota_application_start() would take care of the download loop and only returned in case of error)

jerome-pouiller · 2025-11-23T12:21:19Z

samples/siwx91x_ota/src/main.c

+	if (!ctx->connected) {
+		ota_wifi_start_scan(ctx);
+		ret = ota_wifi_connect(ctx);
+		if (ret) {


ret < 0 (not a big deal, but the other check in the function test against minus values, so for uniformity...)

jerome-pouiller

I have finished the review.

jerome-pouiller · 2025-11-23T12:42:23Z

samples/siwx91x_ota/src/main.c

+static int ota_http_response_cb(struct http_response *rsp, enum http_final_call final_data,
+				void *user_data)
+{
+	ARG_UNUSED(final_data);


jerome-pouiller · 2025-11-23T12:56:22Z

samples/siwx91x_ota/src/main.c

+
+	if (ctx->range_start == 0) {
+		buf_offset = sizeof(sl_si91x_firmware_header_t);
+		status = sl_wifi_get_firmware_size((void *)ctx->flash_buffer, &ota_calc_image_size);


Are you sure you need to cast the pointer?

jerome-pouiller · 2025-11-23T13:00:32Z

samples/siwx91x_ota/src/main.c

+	zsock_close(ctx->sock);
+	ctx->sock = -1;
+	ctx->flash_buffer_len = 0;
+	ctx->http_status_code = 0;


If I understand right, flash_buffer_len and http_status_code are reset each time you call ota_download_firmware_chunk()

We are resetting the flash_buffer_len, but not the http_status_code. We will reset that as well.

jerome-pouiller · 2025-11-23T13:06:22Z

samples/siwx91x_ota/src/main.c

+
+		if (ctx->image_size != ota_calc_image_size) {
+			printf("Image size mismatch - expected: %u, calculated: %u (file may be "
+			       "corrupted)\n",


Some user may understand file may be corrupted means the device is now corrupted. I suggest (corrupted payload?) (or Corrupted payload (image size mismatch: %d bytes != %d bytes)\n)

jerome-pouiller · 2025-11-23T13:11:49Z

samples/siwx91x_ota/src/main.c

+
+	/* Prepare headers */
+	snprintf(range_header, sizeof(range_header), "Range: bytes=%d-%d\r\n", ctx->range_start,
+		 ctx->range_start + sizeof(ctx->flash_buffer) - 1);


Can you confirm the HTTP server is fine when we request range_end beyond image_size?

HTTP server returns 416 http status code when we request range_end beyond image_size

So this code is not correct, that's it?

Once HTTP server responds 416 error code, application closes the connection

jerome-pouiller · 2025-11-23T13:14:39Z

samples/siwx91x_ota/src/main.c

+
+	if (ctx->range_start >= (int)ctx->image_size) {
+		return -ENODATA;
+	}


IMHO, this error is directly related to the http download and it make more send to handle it in ota_download_firmware_chunk().

(Then this function will be only one line, and maybe you will prefer to inline it)

jerome-pouiller · 2025-11-23T13:18:25Z

samples/siwx91x_ota/src/main.c

+	if (ret != 0) {
+		printf("Cannot resolve %s:%s: %s\n", ctx->http_parse_info.host,
+		       ctx->http_parse_info.port, zsock_gai_strerror(ret));
+		return ret;


Indeed, so you can't return ret. You have to return -EINVAL or something like this.

jerome-pouiller · 2025-11-23T13:26:45Z

samples/siwx91x_ota/src/main.c

+			if (!ctx->ipv4_addr_config_done || !ctx->ipv6_addr_config_done ||
+			    ctx->disconnected) {
+				ota_handle_retry(ctx, "network connection", true);
+				ctx->state = OTA_STATE_SCAN;


Not addressed.

jerome-pouiller · 2025-11-23T13:33:49Z

samples/siwx91x_ota/src/main.c

+static int ota_download_firmware_chunk(struct app_ctx *ctx)
+{
+	char range_header[64];
+	const char *headers[2] = {range_header, NULL};


Not addressed.

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 5 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

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

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

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

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

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

jerome-pouiller reviewed Nov 6, 2025

View reviewed changes

jerome-pouiller reviewed Nov 7, 2025

View reviewed changes

Devika-Raju-Z force-pushed the https_otaf branch from 4ad0105 to 8c6da5c Compare November 12, 2025 10:55

cursor bot reviewed Nov 12, 2025

View reviewed changes

jerome-pouiller reviewed Nov 12, 2025

View reviewed changes

jerome-pouiller reviewed Nov 13, 2025

View reviewed changes

Devika-Raju-Z force-pushed the https_otaf branch 2 times, most recently from ee3fcc1 to ec8d3a0 Compare November 20, 2025 12:37

samples: siwx917_ota: http/s OTAF application

5b70aa4

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

jerome-pouiller reviewed Nov 22, 2025

View reviewed changes

jerome-pouiller reviewed Nov 23, 2025

View reviewed changes

Devika-Raju-Z force-pushed the https_otaf branch 2 times, most recently from fc77cd2 to 5b70aa4 Compare November 24, 2025 15:29


		ctx->ota_image_size = rsp->content_range.total;

		if (rsp->body_frag_len > 0) {

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 • edited by cursor bot Loading Uh oh! There was an error while loading. Please reload this page.

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!

Uh oh!

jhedberg commented Sep 26, 2025

Uh oh!

jerome-pouiller left a comment

Choose a reason for hiding this comment

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!

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!

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!

Devika-Raju-Z commented Sep 16, 2025 •

edited by cursor bot

Loading

jerome-pouiller Nov 13, 2025 •

edited

Loading

jerome-pouiller Nov 23, 2025 •

edited

Loading

jerome-pouiller Nov 23, 2025 •

edited

Loading