Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add a base hardware config without 'factory' #2933

Closed
wants to merge 1 commit into from

Conversation

pljakobs
Copy link
Contributor

@pljakobs pljakobs commented Jan 4, 2025

partition to allow two large OTA partitions

when working with OTA on the esp32c3, it became clear that factory would never be overwritten by OTA, making it difficult to fully upgrade the device over OTA when running an OTA version already.
ESP-IDF suggests to drop the factory partition altogether to be able to configure two large OTA.

    factory (0x00) is the default app partition. The bootloader will execute the factory app unless there it sees a partition of type data/ota, in which case it reads this partition to determine which OTA image to boot.

        OTA never updates the factory partition.

        If you want to conserve flash usage in an OTA project, you can remove the factory partition and use ota_0 instead.

this hardware definition does provide a basis for a factory less OTA setup including the OTADATA partition the bootloader needs. For size considerations, I have moved it to the end of the 4M flash area to avoid moving around the first partition.

Copy link

what-the-diff bot commented Jan 4, 2025

PR Summary

  • New Hardware Configuration Added
    The PR includes a newly introduced hardware configuration file for ESP32. This caters to an Over-The-Air (OTA) base setting, which allows updates to be sent and installed automatically on the ESP32 over a wireless network.

  • Modification in Partition Scheme
    The updated configuration avoids the use of a factory partition and instead, makes use of two ROM partitions. This allows for more efficient usage of the available memory by actively swap between the two ROM sections.

  • Defines Various Parameters
    The PR also includes specific definitions for parameters such as the bootloader size, the offset of the partition table, and the types of devices supported. This introduces improvements in the scalability, compatibility, and overall performance of the ESP32.

  • Detailed Memory Partition Configuration
    This PR also brings forward a well-detailed configuration for different memory partitions namely - phy_init, nvs, rom0, and otadata. These specifications are given exact addresses and sizes, thus enhancing the precision and efficiency of the system's memory operations.

@mikee47
Copy link
Contributor

mikee47 commented Jan 6, 2025

Sounds like a good use case to add support for removing partitions in inherited maps. For example:

{
	"name": "OTA base config with single ROM",
	"base_config": "standard",
	"comment": "ota base config to eliminate the factory partition and instead build two rom partitions.",
	"partitions": {
		"factory": {
			"size": 0
		},
		"rom0": {
			"address": "0x010000",
			"size": "0x0f0000",
			"type": "app",
			"subtype": "ota_0",
			"filename": "$(TARGET_BIN)"
		},
		"otadata":{
			"address":"0x3fe000",
			"size":"8k",
			"type":"data",
			"subtype":"ota"
		}
	}
}

This would avoid the need to duplicate definitions from the standard map.

@mikee47
Copy link
Contributor

mikee47 commented Jan 6, 2025

Here's a patch you can try:

diff --git a/Sming/Components/Storage/Tools/hwconfig/partition.py b/Sming/Components/Storage/Tools/hwconfig/partition.py
index f7b5e312..187ab9ad 100644
--- a/Sming/Components/Storage/Tools/hwconfig/partition.py
+++ b/Sming/Components/Storage/Tools/hwconfig/partition.py
@@ -146,7 +146,11 @@ class Table(list):
             if part is None:
                 part = Entry(devices[0], name)
                 self.append(part)
-            part.parse_dict(entry, devices)
+                part.parse_dict(entry, devices)
+            else:
+                part.parse_dict(entry, devices)
+                if part.size == 0:
+                    self.remove(part)
 
     def sort(self):
         def get_key(p):
@@ -498,7 +502,7 @@ class Entry(object):
         align = self.alignment(arch)
         if self.address % align:
             raise ValidationError(self, "Offset 0x%x is not aligned to 0x%x" % (self.address, align))
-        if self.size is None or self.size == 0:
+        if self.size is None:
             raise ValidationError(self, "Size field is not set")
         if self.size % align and secure:
             raise ValidationError(self, "Size 0x%x is not aligned to 0x%x" % (self.size, align))

@pljakobs
Copy link
Contributor Author

pljakobs commented Jan 7, 2025

sounds like a good solution.

@pljakobs
Copy link
Contributor Author

pljakobs commented Jan 7, 2025

quick test shows expected result. Thank you

@pljakobs
Copy link
Contributor Author

pljakobs commented Jan 7, 2025

one thought:
setting the size to 0 is certainly good shorthand for dropping the partition within the given schema.
I would, however, prefer to see a more explicit definition here, such as

"partitions":{
  "factory":{
    "drop":true
  }
}

although I could see this to be more change than the use case justifies, especially if there is no other similar "action" that might get it's own explicit statement.

@mikee47
Copy link
Contributor

mikee47 commented Jan 7, 2025

As there's not really any other useful interpretation of a zero-length partition I think it's probably the most appropriate choice.

The main purpose of these layered configurations is so that small changes or additions can be accommodated without having to replicate the entire schema. It also allows new architectures to be supported.

The programmer is always free to write a complete config from scratch of course.

I'll raise a PR with the change.

@pljakobs
Copy link
Contributor Author

pljakobs commented Jan 7, 2025

I'll raise a PR with the change.

I will go ahead and close this, then

[edit]

actually, since you say The programmer is always free to write a complete config from scratch of course. - I tried that and it seems the tooling expects a valid "base_config" property, at least for me, it did fail without one.

@pljakobs pljakobs closed this Jan 7, 2025
@mikee47
Copy link
Contributor

mikee47 commented Jan 7, 2025

Thank you for raising this issue!

slaff pushed a commit that referenced this pull request Jan 13, 2025
This PR adds the ability to 'drop' a previously defined partition from a hardware configuration by setting `"size": 0` in an inherited config. This is silently ignored if the partition is undefined.

Issue #2933 highlighted the need to be able to eliminate the `factory` partition and use OTA partitions instead. 

A couple of sample applications contained redundant `subtype` overrides since these are already defined in `spiffs-two-roms`, so this has been tidied.

Note: Typically the Esp32 `spiffs-two-roms` configuration is used as a base for OTA updates. This retains the `factory` partition so if not required this can be dropped to liberate the 1M allocation.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants