Skip to content
New issue

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

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

Already on GitHub? # to your account

Async capacity #54

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

avsaase
Copy link

@avsaase avsaase commented Jul 19, 2024

Closes #37

Should the blocking version also be fallible?

@MathiasKoch
Copy link
Collaborator

As I said in the issue, I am personally okay with this change.

Not sure about others?
CC @lulf @Dirbaio @eldruin ?

@Dirbaio
Copy link
Contributor

Dirbaio commented Jul 25, 2024

is this to support flashes where you query the size by doing e.g. an SPI operation which requires async?

I think in general it'd be better to read it in new() and cache it in a field, then .capacity() can just return that and needs no async. This'd probably help perf, especially if you're using the capacity in read/write/erase to do bounds checks, so you don't do an extra spi transaction to read the capacity every time.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Consider making capacity async
3 participants