Add an example #5

DaneSlattery · 2024-02-22T11:59:58Z

Add an example that updates an OTA enabled app using another app encoded in the flash memory.

DaneSlattery · 2024-02-22T12:17:43Z

Hi Linus

I have a working HTTP OTA update that would make a good example, do you think it would be worthwhile to add it?

Additionally, I have built an extension that performs a checksum of the update using ShA256. I have called this HashedOtaUpdate. Would you be open to a contribution for this?

faern · 2024-03-11T21:14:29Z

.cargo/config.toml

@@ -1,5 +1,6 @@
 [build]
 # Uncomment the relevant target for your chip here (ESP32, ESP32-S2, ESP32-S3 or ESP32-C3)
+# you may need to change the toolchain in `rust-toolc


What is rust-toolc?

meant to be rust toolchain, xtensa chips need a custom toolchain afaik

Seems like an unfinished sentence then?

faern · 2024-03-11T21:15:45Z

.gitignore

+/.embuild
+/.devcontainer


You ignore the devcontainer folder here. But this PR also commits devcontainer definitions. I think you might want to remove the devcontainer files from this PR and keep it ignored?

faern · 2024-03-11T21:20:08Z

Thank you for wanting to contribute! I have not read the entire PR yet. I left some comments on some things I think might have been commited by mistake as a first iteration. Will read more of it soon!

Additionally, I have built an extension that performs a checksum of the update using ShA256. I have called this HashedOtaUpdate. Would you be open to a contribution for this?

Having both signature verification (chip will only trust firmware signed by trusted parties) and integrity verification is something I really want in general. I have not yet coded this up for myself. And I have not decided if it belongs in this crate or outside. If it's put inside the crate then it becomes very opinionated as to what checksum algorithm and/or signature method it uses. But yeah, sha256 is universally a pretty solid choice, so it might be good! As long as we keep it behind an opt-in feature flag and the dependency it pulls in is sane and fairly small.

faern

Really lovely that you want to contribute! This crate deserves some more love!

faern · 2024-03-11T21:25:08Z

examples/README.md

+Build: 
+```
+RUSTFLAGS="-C strip=symbols -C debuginfo=0" cargo build --example beta --release
+espflash save-image --chip <mcu_target> ./target/<mcu_target>/release/examples/beta ./beta.bin


This PR commits the beta.bin file. I'm guessing that's a mistake, since these instructions build it? I don't think we should carry around built binaries in the repository.

Agreed, will amend

beta.bin is still part of this PR

faern · 2024-03-11T21:27:25Z

examples/README.md

+
+Build: 
+```
+RUSTFLAGS="-C strip=symbols -C debuginfo=0" cargo build --example beta --release


Does the stripping etc really need to be done here? It significantly complicates the example instructions. Is it required to fit on some specific type of chip? Otherwise I think simple instructions is more valuable.

Doesn't need to happen, but it is an important aspect of OTA updates in my experience. We should mention it in the docs if not as part of the example, but it does greatly speed up the flashing.

faern · 2024-03-11T21:28:12Z

examples/README.md

+
+## ota_from_flash
+
+The app contains the update in it's flash memory at compile time. The `beta.bin` image in the ESP32 app image format must be available at compile time.


nit: s/it's/its/

faern · 2024-03-11T21:29:33Z

examples/README.md

+
+## partitions_factory.csv
+
+The default partition table shipped with the ESP32. _Not suitable for OTA._


Why is this partition table included, since it's not used?

I included it as a reference. Dodgy things might happen if a user doesn't re-write their original partition table.
For example, if you flash OTA alpha and OTA beta into partition slot ota1 and ota2 respectively. In this case, OTA alpha loads, writes OTA beta into the ota2 slot and selects that as the boot partition.

When a user wants to flash a non-ota app into what they expect to be the factory partition, the ota2 slot could still be selected as the boot partition, and the factory partition never opens :-(

I don't think it's the responsibility of this repository to track the default partition table. If there are potential caveats (which are currently not even mentioned) I think it's far better to describe how to get out of the bad situation, and link to an online resource with the default partition table. Then we don't have to maintain it here

examples/ota_from_flash.rs

faern · 2024-03-11T21:35:08Z

examples/ota_from_flash.rs

+        for app_chunk in NEW_APP.chunks(4096) {
+            if let Err(err) = ota.write(app_chunk) {
+                error!("Failed to write chunk");
+                break;


This will just exit the for loop and go directly to ota.finalize(). Here we know the update failed already, and I don't think it's the best course of action to try and finalize the update. I think it's better in general (and teach the readers) to exit with an error here. Only run the finalize code if all write calls succeeded.

faern · 2024-03-11T21:35:49Z

examples/ota_from_flash.rs

+        match ota.finalize() {
+            Err(x) => {
+                error!("Failed to validate image.");
+                ()


Returning unit is automatically implied when you don't return anything else. I believe this is superfluous here.

faern · 2024-03-16T09:34:19Z

Additionally, I have built an extension that performs a checksum of the update using ShA256. I have called this HashedOtaUpdate. Would you be open to a contribution for this?

Hmm.. I thought about this again. How is it implemented? I don't want this crate to be locked to a specific format in how the images are transferred to the chip, neither be biased on the transport method.

DaneSlattery · 2024-03-17T14:04:48Z

Having both signature verification (chip will only trust firmware signed by trusted parties) and integrity verification is something I really want in general. I have not yet coded this up for myself. And I have not decided if it belongs in this crate or outside. If it's put inside the crate then it becomes very opinionated as to what checksum algorithm and/or signature method it uses. But yeah, sha256 is universally a pretty solid choice, so it might be good! As long as we keep it behind an opt-in feature flag and the dependency it pulls in is sane and fairly small.

I agree that it is tough to be opionated regarding checksum algorithm. The ESP32 has CRC hardware support built in, for example. I think it would be a good idea to create a trait as the public interface, that we implement for SHA256 as a start, and allow future contributors to extend as desired.

Hmm.. I thought about this again. How is it implemented? I don't want this crate to be locked to a specific format in how the images are transferred to the chip, neither be biased on the transport method.

My implementation is an extension of ota.write, where the incremental checksum is calculated per chunk. This checksum could be any algorithm. Then, I extended ota.finalize(expected_checksum) to confirm the checksum against the expected one.

This is not locked into the specific format or transport method, but does require the user to also transfer the expected checksum over a side channel.

faern · 2024-03-18T15:45:40Z

.cargo/config.toml

@@ -1,5 +1,6 @@
 [build]
 # Uncomment the relevant target for your chip here (ESP32, ESP32-S2, ESP32-S3 or ESP32-C3)
+# you may need to change the toolchain in `rust-toolc


Seems like an unfinished sentence then?

faern · 2024-03-18T15:46:21Z

.devcontainer/Dockerfile

@@ -0,0 +1,66 @@
+ARG VARIANT=bookworm-slim


Please remove these files

faern · 2024-03-18T15:47:45Z

examples/README.md

+Build: 
+```
+RUSTFLAGS="-C strip=symbols -C debuginfo=0" cargo build --example beta --release
+espflash save-image --chip <mcu_target> ./target/<mcu_target>/release/examples/beta ./beta.bin


beta.bin is still part of this PR

faern · 2024-03-18T15:48:17Z

examples/README.md

+
+## partitions_factory.csv
+
+The default partition table shipped with the ESP32. _Not suitable for OTA._


I don't think it's the responsibility of this repository to track the default partition table. If there are potential caveats (which are currently not even mentioned) I think it's far better to describe how to get out of the bad situation, and link to an online resource with the default partition table. Then we don't have to maintain it here

Add an example

d47600c

DaneSlattery force-pushed the main branch from 3a30429 to d47600c Compare February 22, 2024 12:05

faern requested changes Mar 11, 2024

View reviewed changes

faern requested changes Mar 18, 2024

View reviewed changes

Dane Slattery added 2 commits June 27, 2024 12:05

update versions

f56205c

revert...

aa51f22

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add an example #5

Add an example #5

DaneSlattery commented Feb 22, 2024

DaneSlattery commented Feb 22, 2024

faern Mar 11, 2024

DaneSlattery Mar 17, 2024

faern Mar 18, 2024

faern Mar 11, 2024

faern commented Mar 11, 2024

faern left a comment

faern Mar 11, 2024

DaneSlattery Mar 17, 2024

faern Mar 18, 2024

faern Mar 11, 2024

DaneSlattery Mar 17, 2024

faern Mar 11, 2024

faern Mar 11, 2024

DaneSlattery Mar 17, 2024

faern Mar 18, 2024

faern Mar 11, 2024

faern Mar 11, 2024

faern commented Mar 16, 2024

DaneSlattery commented Mar 17, 2024

faern Mar 18, 2024

faern Mar 18, 2024

faern Mar 18, 2024

faern Mar 18, 2024


		## ota_from_flash

		The app contains the update in it's flash memory at compile time. The `beta.bin` image in the ESP32 app image format must be available at compile time.


		## partitions_factory.csv

		The default partition table shipped with the ESP32. _Not suitable for OTA._

		/.embuild
		/.devcontainer

Add an example #5

Are you sure you want to change the base?

Add an example #5

Conversation

DaneSlattery commented Feb 22, 2024

DaneSlattery commented Feb 22, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

faern commented Mar 11, 2024

faern left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

faern commented Mar 16, 2024

DaneSlattery commented Mar 17, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment