esp32: Fix stuck TouchPad readings on ESP32-S3, use version specifiers in code. #16169
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
|
@wangshujun-tj Thanks for submitting the bug report and the description of a fix, and for being patient while we followed up. @wangshujun-tj @brainstorm @SkaveRat do any of you have hardware that you can use to confirm this fix before we merge it? |
This was referenced Nov 10, 2024
brainstorm
added a commit
to CCHS-Melbourne/Spectrogram
that referenced
this pull request
Nov 12, 2024
…, mapped both touch buttons... LED and MIC still need to be fixed/mapped correctly to their pins
|
Tested on an aditional (different design/prototype) board with an esp32s3-mini-1u module: the difference between having this PR applied or not means reliable touch support or not. |
dpgeorge
reviewed
Nov 19, 2024
ESP32 has hardware V1 and S2/S3 has V2, and future chips may have different versions. This should still compile to the same binary before and after. This work was funded through GitHub Sponsors. Signed-off-by: Angus Gratton <angus@redyak.com.au>
Closes micropython#13178. TouchPad confirmed working on both chips, and fixes the the ESP32-S3 reading constant max value. Was unable to reproduce the bug on ESP32-S2 but this may be due to my test setup, and it still works with the fix. This work was funded through GitHub Sponsors. Signed-off-by: Angus Gratton <angus@redyak.com.au>
Signed-off-by: Angus Gratton <angus@redyak.com.au>
fb6c8da to
63e184e
Compare
|
Updated according to review, re-tested with dodgy dupont wire method on ESP32-S2 and original ESP32, rebased. |
dpgeorge
approved these changes
Nov 28, 2024
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.
Summary
machine.TouchPaddriver to use hardware version specifiers instead of chip names. This should make adding future SoCs easier.The new order of initialisation matches more closely the order in Espressif's example code, i.e.
https://github.com/espressif/esp-idf/blob/v5.2.2/examples/peripherals/touch_sensor/touch_sensor_v2/touch_pad_read/main/tp_read_main.c#L86
This work was funded through GitHub Sponsors.
Testing
The only buggy case of stuck max readings that I was able to reproduce was on ESP32-S3, and never reproduced once the fix was applied.
The hardware FSM did seem to get stuck once on ESP32-S3 in a different way (was reading correctly and then stopped updating and didn't start again). However I think this was due to my poor quality touch pad inputs (had moved the wires a lot and it would have changed capacitance by a huge factor).
Trade-offs and Alternatives