Pin ESP32 to core 1 once again ftw #7258
Closed
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
dpgeorge
reviewed
May 12, 2021
This follows up on micropython#5489, where we changed the esp32 core pinning to core 0 in order to work around an issue with IDF < 4.2.0. Now that IDF > 4.2.0 is available, we allow pinning back to core 1, which eliminates some problematic callback latency with wifi enabled.
|
I tested BLE with this PR and it does not work, events get lost. It starts working again if NimBLE is pinned to core 1. |
|
It's probably OK to move NimBLE to core 1 as well. Otherwise wait for #7046 which may resolve the issue of pinning NImBLE. |
This ensures that NimBLE is on the same core as Micropython, even in IDF versions above 4.2.
|
@dpgeorge Oh yes, I initially missed that NimBLE pinning is in another file. Keeping them on the same core for now seems like a good agreeable choice, so thanks for the advice. I have made the corresponding change, so please let me know if there is anything else to address in this PR. |
dpgeorge
reviewed
May 14, 2021
Whoops, probably should not have expected c macros to work in kconfig files. This undoes that and just plain pins to core 1.
tannewt
pushed a commit
to tannewt/circuitpython
that referenced
this pull request
Jan 4, 2023
Esp32 dev kit v1
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.
Hi friends. New contributor, but pretty experienced...and hopefully I have done my homework. Phew, there is a lot of context/history in a very small PR here... :)
Originally we had some latency issues that were resolved by pinning micropython to a core, specifically opposite of the other stuff going on (BLE/Wifi). That "other stuff" could introduce latency or jitter in timer or interrupt callback delivery. All good, cool workaround.
Some of this seemed to be rooted in an upstream bug in IDF < 4.2, so we pinned everything to core0 in the esp32 port. Related.. There were comments to indicate that when this upstream situation was resolved we could undo this...
Time passed. A major branch got integrated. The core pinning stayed or otherwise got left behind as IDF improved (I couldn't track down the root cause in IDF).
Fast forward to the future...and I boot up a project based on the 1.15 release, and I'm doing what I think to be a pretty simple thing -- 1ms pin toggles based on a
Timer. It's rock solid, until I turn on wifi...at which point it becomes unstable (even with no traffic). I make a pretty basic test case that I can put on the oscilloscope and it is solid without wifi, significantly jittery with wifi on.LOTS of digging ensues, and I end up finding the above-related issues.
I checked out the main branch and built with IDF 4.2.1, same problem. I made the change in this PR and presto the problem goes away and the latency is solid again. Woohoo! Let's do this!
Sadly, my target device doesn't leverage BLE so I'm able to verify that this doesn't regress the original issue...but I can verify that
Timer()instances now deliver with negligible jitter when WiFi is on. Can others with BLE-based targets confirm that this patch doesn't impact them? Are we ready to roll with pinning to core 1 again please, cuz this 100% helps me.Thanks thanks.