-
-
Notifications
You must be signed in to change notification settings - Fork 3k
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
feat(demos): add demo for the OSAL #6182
Conversation
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 Fabian!
Here you can see some errors reported by CI. Fortunately, they are easy to fix.
I feel like this update could be rather an example. Usually we add complex eye catching things in the demos
folder. So would you mind moving it to the examples
folder?
f095d89
to
9c099e3
Compare
Moved this to |
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.
Add
#include "porting/osal/lv_example_osal.h"
to lv_examples.h
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.
If we want to run it with the CI we need to add it here. However now the CI is running only with pthread
. Do you think it still makes sens to run it as a test?
|
Adds a example demonstrating the use of the OSAL. Resolves: lvgl#6049
The original example was failing for me so I added a little bit update. With the help What do you think? |
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.
Nice @kisvegabor, I think yes, this PR is actually a good place to add the global lock, so this PR doesn't have to wait for another PR. After this, we just have to update the Windows driver. Thanks!!
It surprised me that we have lv_mutex_lock_isr
. If it has to block, it will deadlock, because the OS can't suspend the ISR to switch to the thread holding it the lock.
We implement it with xSemaphoreTakeFromISR
in our FreeRTOS OSAL. Here's the description from the FreeRTOS docs (link):
A version of xSemaphoreTake() that can be called from
an ISR. Unlike xSemaphoreTake(), xSemaphoreTakeFromISR() does not permit a block
time to be specified.
So it's okay. It's just fallible. I think we can remove the LV_LOG_ERROR
since it's a possible and expected failure.
However for cmsis_rtos2 we call osMutexAcquire
which according to the docs, always fails in an ISR (link):
osErrorISR: cannot be called from interrupt service routines.
Mutex management functions cannot be called from Interrupt Service Routines (ISR), unlike a binary semaphore that can be released from an ISR.
All that to say, my feedback for your changes for now is to make the new function lv_lock_isr
return lv_result_t
and we can merge it.
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 faxe1008 and kisvegabor!
Thanks for moving the PR forward and making LVGL thread safe @kisvegabor, this is very good stuff 👍 |
Adds a demo demonstrating the use of the OSAL.
Resolves: #6049
Description of the feature or fix
A clear and concise description of what the bug or new feature is.
Notes
lv_conf_template.h
run lv_conf_internal_gen.py and update Kconfig.scripts/code-format.py
(astyle version v3.4.12 needs to be installed) and follow the Code Conventions.