[ESP32 OTA] Critical fix - Make sure that the BT coreTask gets suspended when not needed #795
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.
After some tinkering with this project (which I very much like now so congrats for the great work) I spotted a flaw which seems to be pretty critical for me. Maybe someone else is experiencing it also but let's try to give some details first.
Steps to reproduce:
platformio run --target monitor
in another terminalExpected results:
Actual results:
When inspecting the monitoring terminal we see this:
After digging in I noticed that there's a dedicated coreTask that gets pinned to the core 0 of the ESP32 wonder chip and has a priority of 1. Added to that there's a flag called ProcessLock which makes the coreTask trigger the BLE detection or not. So far so good but its logic has a flaw in the sense that we cannot let a FreeRTOS task sitting in an infinite loop and just keep it doing almost nothing in the
else
branch when OTA is in progress.Let's paste here the original code so that we can see what the
else
branch does:If the debug level is not set to TRACE then the
else
branch does NOTHING !!! If I set the debug level to TRACE then the task does something more or less useful - I let you decide here 😃.So in the end it must be either suspended because it doesn't do anything useful anyway or maybe use a vTaskDelay so that other tasks pinned to the same core get a chance to run and in this case one is important in particular - the IDLE task which has priority 0 and that task also feeds the watchdog. See where I'm going? That's why the above ugly thing happens and the watchdog gets triggered which resets the chip and bye bye OTA.
This PR should fix this issue and I already tested it actually because I wasn't able to reflash my GW anymore via OTA after enabling BT support as it was crashing constantly with the above exception.
The fix is pretty simple without changing too much the existing logic and what it does basically is to suspend the coreTask when locking is requested and resume it when locking is not required anymore - simple as that. After applying this fix I was able to perform OTA updates without a glitch.
The only thing that happens and which is normal BTW is when your sketch occupies more that 90% of flash memory and that is - [E][ArduinoOTA.cpp:344] _runUpdate(): Update ERROR: Not Enough Space. But this is not related to the current issue anyways (for me it happened because I wanted a Gateway that supports both BLE and RF - but it worked after a few retries 😃 ).
Hope it helps. Please feel free to add comments or if there's something wrong in what I said above the corrections are welcome.
Thanks and keep up the good work!