esp-rs / esp-idf-part Goto Github PK

Parsing a byte vector should not require std.

The repository contains actions using Node12 which are producing a warning in every CI run. Node12 will stop working this summer, so we should update them, for more information see: https://github.blog/changelog/2022-09-22-github-actions-all-actions-will-begin-running-on-node16-instead-of-node12/

esp-idf-part still appears to depend on the std crate even when used without the std feature.

The serde_plain dependency pulls in a dependency on std, as it depends on serde with the default features.
This could be made an optional dependency, but there are still other std dependency requirements: serde appears to only support untagged enums when the std feature is enabled, and this is used for the SubType enum. It probably would be possible to put all derive(Serialize, Deserialize) attributes behind a cfg_attr check.

All that said, given that PartitionTable::try_from_bytes() is only available with the std feature I don't think that no_std support is going to be useful for what I was hoping to do anyway. The dependency on deku seems to be the main hurdle to making try_from_bytes() work with no-std and no-alloc. For my use case I know my buffer will be aligned, so it's probably easiest for me to just declare my own #[repr(C)] PartitionTableEntry struct and just use bytemuck::from_bytes() to read the table entries through that structure.

Using SubType::Custom when generating a partition table causes panic when flashing:

Partition::new(
  "fs",
  Type::Data,
  SubType::Custom(0x83), // littlefs 
  0x810_000, // App offsets must be multiples of 0x10000 (64KB)
  0x600_000,
  false,
);

a snippet used to generate a partition table file

Unfortunately, panic occurs when flashing:

Error:   × Main thread panicked.
  ├─▶ at /home/rencedm112/.cargo/registry/src/index.crates.io-6f17d22bba15001f/esp-idf-part-0.4.1/src/partition/mod.rs:171:56
  ╰─▶ called `Option::unwrap()` on a `None` value
  help: set the `RUST_BACKTRACE=1` environment variable to display a backtrace.

The panic occurs here:

/// Create a [SubType::Data] variant from an integer value
pub fn data(value: u8) -> Self {
    Self::Data(DataType::from_repr(value as usize).unwrap())
}

the panic occurs because 0x832 subtype is not recognized, which causes DataType::from_repr() to return Option::None

Currently our tests are run on input which is assumed to be correct. It would be good to add some intentionally invalid partition tables to ensure that parsing fails as expected.

Apart from the encrypted flag, the csv partition tables can also contain the readonly flag. The latter flag is currently not handled throughout the code-base.

EDIT:
It seems that the readonly flag is not yet part of esp-idf stable (only 5.2beta), so its quite understandable that this was not implemented so far.

how would one use this?

i currently need to increase the app partition size as the default 1MB is to small, how would i do this, using this library?

without using this library i would create the .csv by hand and apply it via cargo pio espidf menuconfig

The following partition table can't be parsed because the "data" partition type doesn't have a "data" subtype (note to self: it has an "undefined" subtype). It would be useful is espflash would be able to point this out to me, instead of just a simple "The partition table is invalid" error.

# Name,   Type, SubType, Offset,  Size, Flags
otadata,  data, ota,     ,        0x2000,
ota_0,    app,  ota_0,   ,        2M,
ota_1,    app,  ota_1,   ,        2M,
storage,  data, data,    ,        3M,

There should be reasonably complete descriptions of each type, and the top-level documentation should contains some basic examples and much more information than it currently does.

In the validate function in lib.rs there's a check that says partition sizes cannot be larger then 16MB:

This check should not apply to data partitions which can be used even when they exceed 16MB. I think this check should be
if partition.ty() == Type::App && partition.size() > MAX_PART_SIZE {

The IDF bootloader code referenced there does indeed check this 16MB limit, but if I read that code correctly it only does so for the partition it tries to boot from. If I write a partition table manually using espflash write-bin 0x8000 partitions.bin to a S3 with 32MB flash I can use a 28MB data partition just fine.

Currently there is nothing verifying that partitions are not overlapping with each other, for example.