Update EEPROM.cpp #6599
Merged
Update EEPROM.cpp #6599
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
use InterruptLock class for scoped interrupts instead of blindly disabling/enabling interrupts, which doesn't support nesting nor restore previous state.
Add forgotten include
earlephilhower
requested changes
Oct 4, 2019
There was a problem hiding this comment.
I hate to be that guy but this whole code implies that spi_flash_read(), spi_flash_write(), and spi_flash_erase() need interrupts disabled.
They don't, unless you allow people to use IRQ service routines not in IRAM. So the noInterrupts/Interrupts would better be completely dropped.
If you're saying that we must disable interrupts on spi_flash_xxx, then I'd still drop the nointerrupts/interrupts here and place the IRQ lock inside the spi_xxx routines (where they actually used to be disabled, check the history).
Remove locks, simplify code
Fix typo
earlephilhower
approved these changes
Oct 6, 2019
Drop needless include
devyte
added a commit
that referenced
this pull request
Oct 6, 2019
* Minor cleanups in ESPClass Related to #6599 Fix inverted conditions Remove useless comments
6 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
use InterruptLock class for scoped interrupts instead of blindly disabling/enabling interrupts, which doesn't support nesting nor restore previous state.
Fixes #6589