esp32: Restore ESP32-C3 brownout detector settings to IDF defaults. #15610
Conversation
|
@xorbit Hi! I don't suppose you remember why you needed to increase the brownout detector voltage when you added C3 USB support? |
There was a problem hiding this comment.
This change looks good to me.
At least for our generic board definitions we should use the IDF defaults where possible. The IDF has a reason for them having the defaults they do, and unless we have a good reason to change, they should stay as the default.
For the Lolin board, that probably was just copied from the generic definition, so should be changed to the default as well.
I also sometimes see timeouts on the |
Interesting, I think the only timeouts I've seen with this round of testing were on this one board. All my other ESP32 boards are genuine Espressif boards though, using their modules. If timeouts are more commonplace then maybe it is worth bumping up some of the timeouts, though. |
|
In CircuitPython, we noticed wifi had problems connecting when using maximum power as the default on some ESP32-C3 boards with chip antennas, and lowered the default power. We speculate this may be due to less-than-ideal impedance matching. At higher powers RF could be getting back into the chip. We didn't see resets. BLE wouldn't normally run at such high power, I think, but I wonder about the RF network on these boards and its effect on BLE and WiFi performance. |
From memory the main timeouts are on the initial connect, so it may be worth bumping those particular timeouts. |
|
Thanks @dhalbert, maybe we should consider adjusting default power as well. Weirdly I only saw the brownout reset running bluetooth tests, not any wifi ones. Although the wifi performance does seem a little lackluster (nothing I measured, just slow to associate with the AP - I expect there's a lot of packet loss). So not sure what to put that down to. |
There is a MicroPython |
No it's been a while, I don't remember why I added the brownout settings that I did. If I had to guess, it's probably because I was using an ESP32-C3-MINI-1 module which lists the minimum supply voltage as 3.0V in section 4.2. Interestingly, the ESP32-C3 itself also lists 3.0V as minimum supply voltage in section 5.2. Personally I would feel like properly keeping the chip in a known good state by brownout reset might be preferred over marginal/intermittent operation, but that's straying into policy, so that's for Damien to decide. I could understand trying to support cheap boards as well as possible, on the other hand if it's "working" but flaky it could also damage MicroPython's reputation if someone tried it. |
|
Thanks @xorbit, appreciate the insight into your reasoning. Seems sensible!
I can see this reasoning, although actually I think it could work both ways here. The board does appear to work (with poor but functional RF performance) on the default ESP-IDF settings. On MicroPython it currently randomly resets due to the BOD, and it doesn't do that on ESP-IDF and likely won't do that on Arduino, platform.io, etc. if the ESP-IDF defaults are used. So the naive conclusion could be that there's a problem with MicroPython. Although I totally agree that these are poor quality boards and we shouldn't go out of our way to support them. (An aside, I'm convinced one of the reasons ESP8266 had an early reputation for being flaky was the generally garbage ESP-01 breakouts that often only had one tiny ceramic cap on them. It's amazing a Wi-Fi radio works at all in that environment, let alone works mostly OK!) |
|
You are right, BOR reset looks flaky as well, bad connection is more functional. I agree the generic image should use the IDF defaults. |
Commit a66bd7a added the ESP32_GENERIC_C3_USB board (now merged with ESP32_GENERIC_C3) and changed the brownout detector from the default level 7 (~2.51V) to level 4 (~2.92V). Raising the level again seems to fix random BOD resets on some of the cheaper ESP32-C3 dev boards (that likely skimp on power supply capacitance). Specifically, this change prevents random resets running multi_bluetooth tests on ESP32-C3 "SuperMini" board. Also removed from the LOLIN_C3_MINI board as it seems this config is a copy of the generic one. This work was funded through GitHub Sponsors. Signed-off-by: Angus Gratton <angus@redyak.com.au>
8c4db65 to
e71a324
Compare
|
Rebased on master (IDF 5.2.2 changes) and tested WiFi on ESP32-C3-DevKitC-02. |
Summary
PR #7715 added the ESP32_GENERIC_C3_USB board (now merged with ESP32_GENERIC_C3) and changed the brownout detector from the default level 7 (~2.51V) to level 4 (~2.92V).
Going back to the default level seems to fix random BOD resets on some of the cheaper ESP32-C3 dev boards (that likely skimp on power supply capacitance.) Specifically, this change prevents random resets running multi_bluetooth tests on ESP32-C3 "SuperMini" board.
Also removed from the LOLIN_C3_MINI board (#8396) as it seems this config is a copy of the generic one.
Found while testing #15523.
Testing
On both master (IDF V5.0.4) and the IDF V5.2.2 update branch, running multi_bluetooth tests on the "SuperMini" board triggers random software resets. The same tests pass on Espressif DevKit-C3 boards. With this fix, the software resets go away on the cheap board.
The multi_bluetooth tests still randomly fail on the "SuperMini" board, but the new failures are timeouts, and increasing the test timeouts mostly makes those failures go away. I think the signal integrity is bad enough that a lot of BLE transfers have to be retried. I was going to submit a PR that raises the timeouts in the multi_bluetooth tests, but I think they're pretty high already - the root cause is that boards like this "SuperMini" are only just good enough to work, and not good enough to work well.
Trade-offs and Alternatives
I don't know why the brownout detector settings were changed for the C3_USB board (they were never changed for the generic C3 board). If it was to fix some other issue then maybe it should be left alone, but I can't find any reference to what that issue might have been.
This work was funded through GitHub Sponsors.