-
Notifications
You must be signed in to change notification settings - Fork 167
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Avoid esp32-s3 compile error #243
base: master
Are you sure you want to change the base?
Conversation
Hi @imliubo! Thank you for this contribution! Which display driver do you use with esp32-s3?
ESP32 compiles with your changes, but does not run correctly. What was your motivation for removing these headers from the espidf module?
If you complete the tasks in #227 you will be eligable to get paid for your contribution! Question for @kisvegabor: #227 offers 100 USD for adding support for ESP32-S2, ESP32-C3, ESP32-S3. |
I use a C code driver(not ili94341.py).
emmm... I use my own display driver,so I am not test the ili9341.py driver.I remove those headers beacuse it has some problem with ESP32-S3 compile, looks need figrue out why the error happen instead of just remove these files :), late I will try again.
Actually this also was my work, so no need any paid. But thanks for this awesome project. I would be happy to contribute some very little help for this. Anyway, I will try fix these mistakes late. |
I know it's not a life changer amount of money, but that's what we can offer now and I'd really like to give it to people who work on making LVGL better. We are a little bit tight on budget but 70 USD for the S3 part still work. |
A liitle process for esp32s3. Simple test with example3 and use ili9XXX.py driver, but esp32s3 has some problem. Left is esp32, right is esp32s3. |
It seems for some reason you DMA can't send out all the bytes in the flush_cb. Maybe the DMA has limit for the number of bytes in one transfer? |
@kisvegabor Yeah, looks had some limit, but I am not deep in check this,it can be work by reducing display buf_size (set factor=8). Test result with this example:
It need more test with different lcds, and still has some issuse with esp-idf module, because some API is not support esp32s3 for now,see related issues: All test is build with esp-idf v4.4.2-1b16ef6cfc2479a08136782f9dc57effefa86f66 @amirgon has any suggest? |
Could you tell us more about your display driver? Also, note that there are two different ili9341 drivers: one that uses DMA driver/esp32/ili9XXX.py and a slower pure Python one driver/generic/ili9xxx.py that is not ESP32-specific.
Thanks!!
It's ok to filter out some espidf headers that are not in use. These are the headers that we want and that are included explicitly: lv_binding_micropython/driver/esp32/espidf.h Lines 117 to 130 in c544c51
So I think you can filter out other headers such as If a header is required but specific functions are missing (For example, we need |
Yeah, it kind like TFT_eSPI, when not use LVGL, it still has many draw functions can be use.
This one driver/esp32/ili9XXX.py, not driver/generic/ili9xxx.py.
Yes,need espressif resolved this. |
@imliubo Is this ready from your side or do you plan to make more changes? |
@amirgon Yeah I can do this, but I only have ESP32 and ESP32-S3 for now, I will buy new C3 and S2 board test this. And the issue opened at esp-idf is not resolved for now. I will do more test at this weekend. |
I think that even if your PR covers only ESP32 and ESP32-S3, it's good enough for now. |
LGTM! @imliubo This patch is all I need for my S3 board. |
Related: #227
No guarantee that everyone‘s ESP32-S3 boards will work, but it worked for my ESP32-S3 board.It will not affect the ESP32 boards, because the new added content has a judgment on the chip type by like blow code:
But the this added may have some impact:Link. I do some simple test it no impact with my device(both esp32 and esp32-s3 device) and simple demos.