-
Notifications
You must be signed in to change notification settings - Fork 195
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Remove most PSRAM features #2178
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Really nice to make this runtime configurable!
Do you think we could write some basic tests? It might depend on what's currently on the runners but I'm sure we can swap out some bits if we need to.
f28878a
to
161c546
Compare
|
fde90d5
to
3fe55f6
Compare
03a10f7
to
f1d0a51
Compare
Co-authored-by: Scott Mabin <[email protected]>
} | ||
|
||
/// Provide exclusive access to the protected data to the given closure | ||
pub(crate) fn with<R>(&self, f: impl FnOnce(&mut T) -> R) -> R { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With this API you can call with
inside with
and LockState
is not reentrant, so we'll get a panic if we do that. I suppose it's an internal API so maybe it's okay to live with this limitation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Panicking is kind of the point here, that's what allows the mutable access.
Thank you for your contribution!
We appreciate the time and effort you've put into this pull request.
To help us review it efficiently, please ensure you've gone through the following checklist:
Submission Checklist 📝
cargo xtask fmt-packages
command to ensure that all changed code is formatted correctly.CHANGELOG.md
in the proper section.Extra:
Pull Request Details 📖
Description
This gets rid of most of the PSRAM related features. We still need
psram
andoctal-psram
howeverThis also adds auto-detection of PSRAM size (by using the RAM chip id on S2/S3, just probing memory on ESP32 since reading the chip id didn't work as expected)
The user now needs to pass a configuration parameter to psram_initialize - this way they could avoid RAM size auto-detect and configure things like the PSRAM SPI frequency
Testing
Examples still work