Skip to content

Conversation

wzhd
Copy link
Contributor

@wzhd wzhd commented Sep 5, 2025

Submission Checklist 📝

  • I have updated existing examples or added new ones.
  • I have used cargo xtask fmt-packages command to ensure that all changed code is formatted correctly.
  • My changes were added to the CHANGELOG.md in the proper section.
  • I have added necessary changes to user code to the Migration Guide.
  • My changes are in accordance to the esp-rs developer guidelines

Extra:

Pull Request Details 📖

Description

address_to could be just past the range, being exactly equal to the end that is exclusive.

Testing

I configured littlefs to use partition size / erase size blocks, and got an OutOfBounds error when it tried to erase the last block.

}

if !self.range().contains(&address_to) {
if !address_to <= self.range().end {
Copy link
Contributor

Choose a reason for hiding this comment

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

needs to be !(address_to <= self.range().end) or even better address_to > self.range().end

Copy link
Contributor Author

Choose a reason for hiding this comment

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

my bad

@bjoernQ
Copy link
Contributor

bjoernQ commented Sep 5, 2025

Thanks!

Ideally, we would add tests for this but that would require implementing NorFlash for MockFlash

the ! takes precedence so you need braces

@bugadani
Copy link
Contributor

bugadani commented Sep 5, 2025

According to the documentation:

Erase the given storage range, clearing all data within [from..to].

That's a closed-closed interval. Is our current check really incorrect?

Allow the exclusive address_to to be equal to the exclusive upper bound of a partition.
@wzhd
Copy link
Contributor Author

wzhd commented Sep 5, 2025

The check_erase helper function only rejects to > flash.capacity(), I think it's consistent with the way ranges are usually passed. Perhaps the documentation isn't using mathematical notation.

@bugadani
Copy link
Contributor

bugadani commented Sep 5, 2025

Maybe we should then ask for clarification on the documentation, instead of guessing something and risking doing the wrong thing. This may be a case where the rest of our implementation is wrong, but the documentation is right. If we don't know, we shouldn't change behaviour.

I guess the helper is a good hint, IMO we still should be sure.

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.

3 participants