Skip to content

Don't enable esp-hal/unstable in esp-storage - #5915

Open
bjoernQ wants to merge 2 commits into
esp-rs:mainfrom
bjoernQ:fix-esp-storage-enables-unstable
Open

Don't enable esp-hal/unstable in esp-storage#5915
bjoernQ wants to merge 2 commits into
esp-rs:mainfrom
bjoernQ:fix-esp-storage-enables-unstable

Conversation

@bjoernQ

@bjoernQ bjoernQ commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

This technically fixes #5875 - at least we are not violating our own guidelines anymore

For stabilization we will need a way to not rely on anything unstable - but that's a problem to solve for the stabilization story

Comment thread esp-storage/Cargo.toml Outdated

##
esp32c2 = ["esp-hal/esp32c2", "esp-hal/unstable", "esp-rom-sys/esp32c2", "esp-sync/esp32c2", "dep:esp32c2"]
esp32c2 = ["esp-hal/esp32c2", "esp-hal/requires-unstable", "esp-rom-sys/esp32c2", "esp-sync/esp32c2", "dep:esp32c2"]

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

requires-unstable can go to esp-hal, no need to repeat it

@bugadani

Copy link
Copy Markdown
Contributor

What do we need to stabilise, besides p.FLASH? Do we intend to stabilise esp-bootloader-esp-idf, or just esp-storage? If only one, how will we access partitions in stable code? If both, how will we introduce other bootloaders later, if we do everything in esp-hal still? We probably have a few questions to explore, still.

@bjoernQ

bjoernQ commented Jul 16, 2026

Copy link
Copy Markdown
Contributor Author

What do we need to stabilise, besides p.FLASH? Do we intend to stabilise esp-bootloader-esp-idf, or just esp-storage? If only one, how will we access partitions in stable code? If both, how will we introduce other bootloaders later, if we do everything in esp-hal still? We probably have a few questions to explore, still.

I guess we want to stabilize the API of both - at least most of it

We currently use

  • peripherals::FLASH, CPU_CTRL, CpuControl, is_running
  • efuse::flash_encryption()
  • peripherals::MMU_TABLE
  • peripherals::SPI1::regs()

A lot can just be turned into PAC access. Also the efuse thing seems innocent.

@bugadani

Copy link
Copy Markdown
Contributor

CPU_CTRL, CpuControl, is_running - multi-core functionality can be hidden behind esp_hal::if_unstable_hal! - with stable HAL, there is no chance of someone enabling the second core, so we don't need to deal with that problem.

@github-actions github-actions Bot added the merge-conflict Merge conflict detected. Automatically added/removed by CI. label Jul 16, 2026
@github-actions

Copy link
Copy Markdown

New commits in main have made this PR unmergeable. Please resolve the conflicts.

@bjoernQ
bjoernQ force-pushed the fix-esp-storage-enables-unstable branch from 73e474b to ed3f47d Compare July 16, 2026 13:23
@github-actions github-actions Bot removed the merge-conflict Merge conflict detected. Automatically added/removed by CI. label Jul 16, 2026
@bjoernQ
bjoernQ force-pushed the fix-esp-storage-enables-unstable branch from ed3f47d to f5639a6 Compare July 16, 2026 13:56
@bjoernQ
bjoernQ marked this pull request as ready for review July 16, 2026 14:51
@bjoernQ bjoernQ added the skip-changelog No changelog modification needed label Jul 16, 2026
@bjoernQ
bjoernQ enabled auto-merge July 16, 2026 15:03
@bjoernQ
bjoernQ added this pull request to the merge queue Jul 16, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Jul 16, 2026
@bugadani
bugadani added this pull request to the merge queue Jul 16, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Jul 16, 2026
@bjoernQ

bjoernQ commented Jul 17, 2026

Copy link
Copy Markdown
Contributor Author

aha - esp-bootloader requires esp-storage which requires unstable so the tests which don't enable esp-hal/unstable fail to build

but ... we cannot have tests without esp-bootloader since we need that macro

not sure what we want to do here now (and previously having esp-storage unconditionally enabling esp-hal/unstable means those tests technically never were testing only stable 🤔 )

@MabezDev given we might decide on moving things to esp-hal we might want to just close this PR knowing it will get sorted out by itself

@github-actions

Copy link
Copy Markdown

New commits in main have made this PR unmergeable. Please resolve the conflicts.

@github-actions github-actions Bot added the merge-conflict Merge conflict detected. Automatically added/removed by CI. label Jul 17, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-conflict Merge conflict detected. Automatically added/removed by CI. skip-changelog No changelog modification needed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

esp-storage enables esp-hal/unstable

2 participants