-
-
Notifications
You must be signed in to change notification settings - Fork 3.1k
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
Add support for FreeRTOS ported by Espressif #5862
Conversation
The FreeRTOS ported by Espressif changes the standard FreeRTOS API to support multi-core SoCs. Therefore, this change generate compatibility with the new Espressif API Fixes lvgl#5810
Fix coding style break in two files fixes lvgl#5810
Code-format.py introduces an error when trying to resolve LVGL coding style fixes lvgl#5810
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you!
It just came to my mind that we could automatically detect the ESP environment via an #ifdef
. E.g. #ifdef ESP_PLATFORM
I'm not sure if really ESP_PLATFORM
shall be used, but we are using it in Cmake.
What do you think about it?
Kconfig
Outdated
@@ -135,12 +135,14 @@ menu "LVGL configuration" | |||
bool "1: PTHREAD" | |||
config LV_OS_FREERTOS | |||
bool "2: FREERTOS" | |||
config LV_OS_FREERTOS_ESP |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Check the indentation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just minor indentation corrections are needed, the config and code changes seem OK.
Hi @kisvegabor I've done some research on the ESP_PLATFORM macro. As far as I can see the macro is set to 1 when the CMake file is processed within the ESP-IDF build system Documentation ESP-IDF. My doubt is if we use the Arduino framework instead of ESP-IDF, I'll test it in my development environment while writing in the issue to ask the users if they can test it also. |
A different approach to detect with you are using the FreeRTOS ported by Espressif. We detect it using the ESP_PLATFORM macro defined by ESP-IDF framework. fixes lvgl#5810
Hi @kisvegabor I've tested the approach you commented using the ESP_PLATFORM macro and it works perfectly. I think that it is a better approach because the modified files are less. |
Great! Please update PR accordingly. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great, these new changes are even better!
@kisvegabor What do you mean by updating the PR accordingly? I already uploaded the changes when I replied to you. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My bad, all look good! Thank you!
Description of the feature or fix
The FreeRTOS ported by Espressif changes the standard FreeRTOS API to support multi-core SoCs. Therefore, this change generate compatibility with the new Espressif API.
Fixes issue #5810