Add esp nvs partition tool by niclo · Pull Request #21 · lhemala/esp-nvs

niclo · 2026-02-23T13:05:45Z

No description provided.

esp-nvs-partition-tool/src/bin/main.rs

esp-nvs-partition-tool/src/csv/parser.rs

esp-nvs-partition-tool/src/csv/writer.rs

renkenono · 2026-02-23T14:55:51Z

esp-nvs-partition-tool/src/csv/writer.rs

+    for entry in &partition.entries {
+        // Emit a namespace row when the namespace changes
+        if current_namespace != Some(&entry.namespace) {
+            wtr.write_record([&entry.namespace, "namespace", "", ""])?;


We should migrate to relying on serialization in the future to simplify the maintenance of this code in the future, see https://docs.rs/csv/latest/csv/struct.Writer.html#method.serialize.

renkenono · 2026-02-23T15:14:41Z

esp-nvs-partition-tool/src/partition/consts.rs

+// Page states
+pub const PAGE_STATE_ACTIVE: u32 = 0xFFFFFFFE;
+pub const PAGE_STATE_FULL: u32 = 0xFFFFFFFC;
+pub const PAGE_STATE_FREEING: u32 = 0xFFFFFFF8;
+pub const PAGE_STATE_CORRUPT: u32 = 0x00000000;


You should use esp_nvs::PageState, don't hesitate to modify esp_nvs to expose these types to avoid duplication.

renkenono · 2026-02-23T15:15:37Z

esp-nvs-partition-tool/src/partition/consts.rs

+// NVS page layout
+pub const FLASH_SECTOR_SIZE: usize = 4096;
+pub const PAGE_HEADER_SIZE: usize = 32;
+pub const ENTRY_STATE_BITMAP_SIZE: usize = 32;
+pub const ENTRY_SIZE: usize = 32;
+pub const ENTRIES_PER_PAGE: usize = 126;


You should expose the consts defined at https://github.com/lhemala/esp-nvs/blob/main/src/raw.rs#L15-L21 and re-use them here.

renkenono · 2026-02-23T15:21:07Z

esp-nvs-partition-tool/src/partition/crc.rs

+///
+/// This function is intentionally public so that callers can verify or compute
+/// CRCs over NVS data independently of the higher-level partition APIs.
+pub fn crc32(data: &[u8]) -> u32 {


If this is a good software fallback implementation of CRC32 that can be used anywhere, I'd recommend that we create an issue to move it to esp_nvs and use it for the tests as well. This allows us to drop the dependency on libz-sys which would also make us drop the dependency on pkg-config in devenv.nix. BTW I think libz-sys is missing in the devenv.nix then?

This is where it'd be called in tests

esp-nvs/tests/common.rs

Line 198 in 85cc96e

libz_sys::crc32(init as core::ffi::c_ulong, data.as_ptr(), data.len() as u32) as u32

.

renkenono · 2026-02-23T15:36:06Z

esp-nvs-partition-tool/src/partition/generator.rs

+/// Generate an NVS partition binary in memory and return it as a `Vec<u8>`.
+///
+/// `size` must be a multiple of 4096 (the ESP-IDF flash sector size).
+pub(crate) fn generate_partition_data(


IMO you should esp_nvs underneath to implement the generator to avoid partially re-implementing it. I think re-using Flash from the test suite or something similar would reduce this implementation by a lot since it uses the abstractions that esp_nvs is based on such as NorFlash and esp_nvs::platform::Crc.

Because Flash contains the binary representation in memory, you can dump it whenever you want similarly to what you do here by returning Vec<u8>.

renkenono · 2026-02-23T15:42:06Z

esp-nvs-partition-tool/src/partition/parser.rs

+/// Parse an NVS partition binary file at the given `path`.
+pub(crate) fn parse_binary<P: AsRef<Path>>(path: P) -> Result<NvsPartition, Error> {
+    let data = fs::read(path)?;
+    parse_binary_data(&data)


Again, this implementation should re-use esp_nvs as well as the in-memory implementation Flash::new_from_file() to parse the binary file. However, the difficulty here would be the fact that there is currently no API to iterate the partition AFAIK which is why #10 exists. We could work in the direction of merging that PR and re-using its API here as well to simplify the implementation, I'd like @lhemala opinion since he knows the codebase better than me.

Yes it would make sense to push #10 . I commented there as it also needs to be rebased. I need to take another thorough look again as well

lhemala

I'm not fully through yet, I'll continue tomorrow

lhemala · 2026-02-23T16:23:39Z

.github/workflows/check.yml

  fmt:
    runs-on: ubuntu-latest
-    name: stable / fmt
+    name: nightly / fmt


I'm not going to merge things that depend on nightly

This only pertains to the code formatting, nightly is not used for anything else. esp-hal has a similar rustfmt.toml which also depends on nightly.

All of the rustfmt options used are not yet stable (as of rustfmt v1.8.0):

comment_width

doc_comment_code_block_width

format_code_in_doc_comments

group_imports

imports_granularity

imports_layout

normalize_comments

wrap_comments

lhemala · 2026-02-23T16:35:52Z

esp-nvs-partition-tool/src/partition/generator.rs

+/// Generate an NVS partition binary in memory and return it as a `Vec<u8>`.
+///
+/// `size` must be a multiple of 4096 (the ESP-IDF flash sector size).
+pub(crate) fn generate_partition_data(


lhemala · 2026-02-23T16:50:49Z

esp-nvs-partition-tool/src/partition/parser.rs

+/// Parse an NVS partition binary file at the given `path`.
+pub(crate) fn parse_binary<P: AsRef<Path>>(path: P) -> Result<NvsPartition, Error> {
+    let data = fs::read(path)?;
+    parse_binary_data(&data)


Yes it would make sense to push #10 . I commented there as it also needs to be rebased. I need to take another thorough look again as well

lhemala · 2026-02-23T16:52:29Z

esp-nvs-partition-tool/tests/roundtrip.rs

+
+#[test]
+fn test_parse_legacy_blob() {
+    // Hand-craft a minimal NVS binary containing a legacy blob (0x41) entry.


I'd rather not support them

lhemala · 2026-02-23T16:53:47Z

esp-nvs-partition-tool/tests/roundtrip.rs

@@ -0,0 +1,507 @@
+use std::fs;


It seems to me that a lot of tests are redundant, especially as we should reuse more code from esp-nvs direcly as @renkenono has mentioned

lhemala · 2026-02-23T16:54:26Z

esp-nvs-partition-tool/Cargo.toml

+[dev-dependencies]
+embedded-storage = "0.3.1"
+esp-nvs = { path = "../esp-nvs" }
+libz-sys = "1.1.22"


didn't you implement crc32 yourself? why is libz still needed?

niclo requested review from lhemala and renkenono February 23, 2026 13:20

niclo self-assigned this Feb 23, 2026

renkenono requested changes Feb 23, 2026

View reviewed changes

lhemala requested changes Feb 23, 2026

View reviewed changes

lhemala requested changes Feb 24, 2026

View reviewed changes

niclo added 4 commits March 2, 2026 13:44

build: update devenv

8218fe3

refactor: introduce workspace and rustfmt configuration

404c261

ci: fix github workflows

46a6bc1

feat: add esp nvs partition tool

23da6b6

niclo force-pushed the main branch from 8839d87 to 23da6b6 Compare March 2, 2026 13:22

Conversation

niclo commented Feb 23, 2026

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!

lhemala left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

niclo Feb 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

niclo Feb 24, 2026 •

edited

Loading