feat(demos): add demo for the OSAL #6182
Conversation
There was a problem hiding this comment.
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?
demos/osal/lv_demo_osal.c
Outdated
| uint32_t press_count = 0; | ||
|
|
||
| counter_label = lv_label_create(lv_scr_act()); | ||
| lv_obj_align(counter_label, LV_ALIGN_CENTER, 0, 0); |
There was a problem hiding this comment.
Don't we need a mutex here? What if this thread is running concurrently with the thread of lv_timer_handler?
There was a problem hiding this comment.
True, but this would mean that the lv_timer_handler thread would also need to take this lock. I do not understand how this can be achieved though.
There was a problem hiding this comment.
Correct. See the latest comments in #6059
There was a problem hiding this comment.
So if I followed the disussion correctly there will be some sort of global mutex that may be taken on the PR for that is ready, right?
There was a problem hiding this comment.
Yes, after that PR, there will be a new API you can use in this PR to acquire the global mutex.
FASTSHIFT
changed the title
faxe1008
force-pushed
the
osal_demo
branch
3 times, most recently
from
f095d89
to
9c099e3
Compare
|
Moved this to |
There was a problem hiding this comment.
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.
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.
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.hrun 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.