diff --git a/Modules/lvgl-module/CMakeLists.txt b/Modules/lvgl-module/CMakeLists.txt index ae0a52ef..2f299fe6 100644 --- a/Modules/lvgl-module/CMakeLists.txt +++ b/Modules/lvgl-module/CMakeLists.txt @@ -7,7 +7,7 @@ if (DEFINED ENV{ESP_IDF_VERSION}) idf_component_register( SRCS ${SOURCE_FILES} INCLUDE_DIRS "Include/" - REQUIRES lvgl esp_lvgl_port + REQUIRES TactilityKernel lvgl esp_lvgl_port ) else () diff --git a/Modules/lvgl-module/Include/tactility/lvgl_module.h b/Modules/lvgl-module/Include/tactility/lvgl_module.h index 3051c102..bc818b68 100644 --- a/Modules/lvgl-module/Include/tactility/lvgl_module.h +++ b/Modules/lvgl-module/Include/tactility/lvgl_module.h @@ -1,29 +1,89 @@ // SPDX-License-Identifier: Apache-2.0 #pragma once +/** + * @file lvgl_module.h + * @brief LVGL module for Tactility. + * + * This module manages the lifecycle of the LVGL library, including initialization, + * task management, and thread-safety. + */ + #ifdef __cplusplus extern "C" { #endif #include +#include +/** + * @brief The LVGL module instance. + */ extern struct Module lvgl_module; +/** + * @brief Configuration for the LVGL module. + */ struct LvglModuleConfig { - /** Used to add devices like displays/pointers/etc. */ + /** + * @brief Callback invoked when the LVGL task starts. + * Use this to add devices (e.g. displays, pointers), start services, create widgets, etc. + */ void (*on_start)(void); - /** Used to remove devices */ + + /** + * @brief Callback invoked when the LVGL task stops. + * Use this to remove devices, stop services, etc. + */ void (*on_stop)(void); + + /** @brief Priority of the LVGL task. */ + int task_priority; + + /** @brief Stack size of the LVGL task in bytes. */ + int task_stack_size; + +#ifdef ESP_PLATFORM + /** @brief CPU affinity of the LVGL task (ESP32 specific). */ + int task_affinity; +#endif }; -void lvgl_module_configure(const struct LvglModuleConfig config); +/** + * @brief Configures the LVGL module. + * + * @warning This must be called before starting the module. + * @param config The configuration to apply. + */ +void lvgl_module_configure(struct LvglModuleConfig config); -void lvgl_lock(void); +/** + * @brief Locks the LVGL mutex. + * + * This should be called before any LVGL API calls from threads other than the LVGL task. + * It is a recursive mutex. + * @retval true when a lock was acquired, false otherwise + */ +bool lvgl_lock(void); +/** + * @brief Tries to lock the LVGL mutex with a timeout. + * + * @param timeout Timeout in milliseconds. + * @return true if the lock was acquired, false otherwise. + */ bool lvgl_try_lock_timed(uint32_t timeout); +/** + * @brief Unlocks the LVGL mutex. + */ void lvgl_unlock(void); +/** + * @brief Checks if the LVGL module is currently running. + * + * @return true if running, false otherwise. + */ bool lvgl_is_running(); #ifdef __cplusplus diff --git a/Modules/lvgl-module/Source/arch/lvgl_esp32.c b/Modules/lvgl-module/Source/arch/lvgl_esp32.c index d4b30146..09b667cf 100644 --- a/Modules/lvgl-module/Source/arch/lvgl_esp32.c +++ b/Modules/lvgl-module/Source/arch/lvgl_esp32.c @@ -1,20 +1,35 @@ // SPDX-License-Identifier: GPL-3.0-only #ifdef ESP_PLATFORM -void lvgl_lock(void) { +#include + +#include +#include + +extern struct LvglModuleConfig lvgl_module_config; + +static bool initialized = false; + +bool lvgl_lock(void) { + if (!initialized) return false; + return lvgl_port_lock(portMAX_DELAY); } bool lvgl_try_lock_timed(uint32_t timeout) { + if (!initialized) return false; + return lvgl_port_lock(timeout); } void lvgl_unlock(void) { + if (!initialized) return; + lvgl_port_unlock(); } error_t lvgl_arch_start() { const lvgl_port_cfg_t lvgl_cfg = { - .task_priority = static_cast(Thread::Priority::Critical), - .task_stack = LVGL_TASK_STACK_DEPTH, - .task_affinity = getCpuAffinityConfiguration().graphics, + .task_priority = lvgl_module_config.task_priority, + .task_stack = lvgl_module_config.task_stack_size, + .task_affinity = lvgl_module_config.task_affinity, .task_max_sleep_ms = 500, .timer_period_ms = 5 }; @@ -23,10 +38,27 @@ error_t lvgl_arch_start() { return ERROR_RESOURCE; } + // We must have the state set to "initialized" so that the lvgl lock works + // when we call listener functions. These functions could create new + // devices and services. The latter might start adding widgets immediately. + initialized = true; + + if (lvgl_module_config.on_start) lvgl_module_config.on_start(); + return ERROR_NONE; } error_t lvgl_arch_stop() { + if (lvgl_module_config.on_stop) lvgl_module_config.on_stop(); + + if (lvgl_port_deinit() != ESP_OK) { + // Call on_start again to recover + if (lvgl_module_config.on_start) lvgl_module_config.on_start(); + return ERROR_RESOURCE; + } + + initialized = false; + return ERROR_NONE; } diff --git a/Modules/lvgl-module/Source/arch/lvgl_posix.c b/Modules/lvgl-module/Source/arch/lvgl_posix.c index ed400853..513eb63b 100644 --- a/Modules/lvgl-module/Source/arch/lvgl_posix.c +++ b/Modules/lvgl-module/Source/arch/lvgl_posix.c @@ -39,12 +39,13 @@ static void task_unlock(void) { recursive_mutex_unlock(&task_mutex); } -void lvgl_lock(void) { +bool lvgl_lock(void) { if (!lvgl_mutex_initialised) { recursive_mutex_construct(&lvgl_mutex); lvgl_mutex_initialised = true; } recursive_mutex_lock(&lvgl_mutex); + return true; } bool lvgl_try_lock_timed(uint32_t timeout) { @@ -53,11 +54,7 @@ bool lvgl_try_lock_timed(uint32_t timeout) { lvgl_mutex_initialised = true; } - // esp_lvgl_port locks without timeout when timeout is set to 0 - // We want to keep it consistent so we do that here too - // TODO: Test if we can remove it - TickType_t safe_timeout = (timeout == 0) ? portMAX_DELAY : timeout; - return recursive_mutex_try_lock_timed(&lvgl_mutex, safe_timeout); + return recursive_mutex_try_lock_timed(&lvgl_mutex, timeout); } void lvgl_unlock(void) { @@ -82,6 +79,7 @@ static void lvgl_task(void* arg) { check(!lvgl_task_is_interrupt_requested()); + // on_start must be called from the task, otherwhise the display doesn't work if (lvgl_module_config.on_start) lvgl_module_config.on_start(); while (!lvgl_task_is_interrupt_requested()) { @@ -109,9 +107,9 @@ error_t lvgl_arch_start() { BaseType_t task_result = xTaskCreate( lvgl_task, "lvgl", - 8192, + lvgl_module_config.task_stack_size, &lvgl_task_handle, - (UBaseType_t)THREAD_PRIORITY_HIGH, + lvgl_module_config.task_priority, nullptr ); diff --git a/Modules/lvgl-module/Source/lvgl_module.c b/Modules/lvgl-module/Source/lvgl_module.c index 58508102..2c48c29c 100644 --- a/Modules/lvgl-module/Source/lvgl_module.c +++ b/Modules/lvgl-module/Source/lvgl_module.c @@ -1,23 +1,36 @@ // SPDX-License-Identifier: GPL-3.0-only +#include #include #include #include + error_t lvgl_arch_start(); error_t lvgl_arch_stop(); -static bool is_running; +static bool is_running = false; +static bool is_configured = false; struct LvglModuleConfig lvgl_module_config = { - nullptr, - nullptr + NULL, + NULL, + 0, + 0, +#ifdef ESP_PLATFORM + 0, +#endif }; void lvgl_module_configure(const struct LvglModuleConfig config) { + is_configured = true; memcpy(&lvgl_module_config, &config, sizeof(struct LvglModuleConfig)); } static error_t start() { + if (!is_configured) { + return ERROR_INVALID_STATE; + } + if (is_running) { return ERROR_NONE; } @@ -43,6 +56,10 @@ static error_t stop() { return error; } +bool lvgl_is_running() { + return is_running; +} + struct Module lvgl_module = { .name = "lvgl", .start = start, diff --git a/Tactility/Include/Tactility/lvgl/LvglSync.h b/Tactility/Include/Tactility/lvgl/LvglSync.h index 35a05797..f579358d 100644 --- a/Tactility/Include/Tactility/lvgl/LvglSync.h +++ b/Tactility/Include/Tactility/lvgl/LvglSync.h @@ -8,23 +8,13 @@ namespace tt::lvgl { constexpr TickType_t defaultLockTime = 500 / portTICK_PERIOD_MS; -/** - * LVGL locking function - * @param[in] timeoutMillis timeout in milliseconds. waits forever when 0 is passed. - * @warning this works with milliseconds, as opposed to every other FreeRTOS function that works in ticks! - * @warning when passing zero, we wait forever, as this is the default behaviour for esp_lvgl_port, and we want it to remain consistent - */ -typedef bool (*LvglLock)(uint32_t timeoutMillis); -typedef void (*LvglUnlock)(); - -void syncSet(LvglLock lock, LvglUnlock unlock); - /** * LVGL locking function * @param[in] timeout as ticks * @warning when passing zero, we wait forever, as this is the default behaviour for esp_lvgl_port, and we want it to remain consistent */ -bool lock(TickType_t timeout); +bool lock(TickType_t timeout = portMAX_DELAY); + void unlock(); std::shared_ptr getSyncLock(); diff --git a/Tactility/Private/Tactility/lvgl/EspLvglPort.h b/Tactility/Private/Tactility/lvgl/EspLvglPort.h deleted file mode 100644 index a1db1de0..00000000 --- a/Tactility/Private/Tactility/lvgl/EspLvglPort.h +++ /dev/null @@ -1,11 +0,0 @@ -#pragma once - -#ifdef ESP_PLATFORM - -namespace tt::lvgl { - -bool initEspLvglPort(); - -} - -#endif diff --git a/Tactility/Source/Tactility.cpp b/Tactility/Source/Tactility.cpp index d2665a80..3cc3ef99 100644 --- a/Tactility/Source/Tactility.cpp +++ b/Tactility/Source/Tactility.cpp @@ -2,11 +2,15 @@ #include #endif +#include +#include + #include #include #include #include +#include #include #include #include @@ -21,13 +25,11 @@ #include #include +#include #include #include #include -#include -#include - #ifdef ESP_PLATFORM #include #endif @@ -359,7 +361,15 @@ void run(const Configuration& config, Module* platformModule, Module* deviceModu lvgl_module_configure((LvglModuleConfig) { .on_start = lvgl::attachDevices, - .on_stop = lvgl::detachDevices + .on_stop = lvgl::detachDevices, + .task_priority = THREAD_PRIORITY_HIGHER, + /** Minimum seems to be about 3500. In some scenarios, the WiFi app crashes at 8192, + * so we now have 9120 to run in a stable manner. We should figure out a way to avoid this. + * Perhaps we can give apps their own stack space and deal with lvgl callback handlers in a clever way. */ + .task_stack_size = 9120, +#ifdef ESP_PLATFORM + .task_affinity = getCpuAffinityConfiguration().graphics +#endif }); module_set_parent(&lvgl_module, &tactility_module_parent); lvgl::start(); diff --git a/Tactility/Source/lvgl/EspLvglPort.cpp b/Tactility/Source/lvgl/EspLvglPort.cpp deleted file mode 100644 index 1114b3cd..00000000 --- a/Tactility/Source/lvgl/EspLvglPort.cpp +++ /dev/null @@ -1,42 +0,0 @@ -#ifdef ESP_PLATFORM - -#include -#include -#include -#include -#include - -#include - -namespace tt::lvgl { - -// LVGL -// The minimum task stack seems to be about 3500, but that crashes the wifi app in some scenarios -// At 8192, it sometimes crashes when wifi-auto enables and is busy connecting and then you open WifiManage -constexpr auto LVGL_TASK_STACK_DEPTH = 9216; - -static const auto LOGGER = Logger("EspLvglPort"); - -bool initEspLvglPort() { - LOGGER.debug("Init"); - const lvgl_port_cfg_t lvgl_cfg = { - .task_priority = static_cast(Thread::Priority::Critical), - .task_stack = LVGL_TASK_STACK_DEPTH, - .task_affinity = getCpuAffinityConfiguration().graphics, - .task_max_sleep_ms = 500, - .timer_period_ms = 5 - }; - - if (lvgl_port_init(&lvgl_cfg) != ESP_OK) { - LOGGER.error("Init failed"); - return false; - } - - syncSet(&lvgl_port_lock, &lvgl_port_unlock); - - return true; -} - -} // namespace tt::lvgl - -#endif diff --git a/Tactility/Source/lvgl/Lvgl.cpp b/Tactility/Source/lvgl/Lvgl.cpp index 8afc3d00..b6a3af20 100644 --- a/Tactility/Source/lvgl/Lvgl.cpp +++ b/Tactility/Source/lvgl/Lvgl.cpp @@ -174,7 +174,6 @@ void detachDevices() { } void start() { - tt::lvgl::syncSet(&lvgl_try_lock_timed, &lvgl_unlock); check(module_start(&lvgl_module) == ERROR_NONE); } diff --git a/Tactility/Source/lvgl/LvglSync.cpp b/Tactility/Source/lvgl/LvglSync.cpp index 45c3ab48..9d6f4fc5 100644 --- a/Tactility/Source/lvgl/LvglSync.cpp +++ b/Tactility/Source/lvgl/LvglSync.cpp @@ -1,39 +1,16 @@ #include "Tactility/lvgl/LvglSync.h" #include +#include namespace tt::lvgl { -static Mutex lockMutex; - -static bool defaultLock(uint32_t timeoutMillis) { - return lockMutex.lock(timeoutMillis); -} - -static void defaultUnlock() { - lockMutex.unlock(); -} - -static LvglLock lock_singleton = defaultLock; -static LvglUnlock unlock_singleton = defaultUnlock; - -void syncSet(LvglLock lock, LvglUnlock unlock) { - auto old_lock = lock_singleton; - auto old_unlock = unlock_singleton; - - // Ensure the old lock is not engaged when changing locks - old_lock((uint32_t)kernel::MAX_TICKS); - lock_singleton = lock; - unlock_singleton = unlock; - old_unlock(); -} - bool lock(TickType_t timeout) { - return lock_singleton(pdMS_TO_TICKS(timeout == 0 ? kernel::MAX_TICKS : timeout)); + return lvgl_try_lock_timed(timeout); } void unlock() { - unlock_singleton(); + lvgl_unlock(); } class LvglSync : public Lock { @@ -41,11 +18,11 @@ public: ~LvglSync() override = default; bool lock(TickType_t timeoutTicks) const override { - return lvgl::lock(timeoutTicks); + return lvgl_try_lock_timed(timeoutTicks); } void unlock() const override { - lvgl::unlock(); + lvgl_unlock(); } }; diff --git a/Tactility/Source/service/gui/GuiService.cpp b/Tactility/Source/service/gui/GuiService.cpp index c24adc29..c4f10d52 100644 --- a/Tactility/Source/service/gui/GuiService.cpp +++ b/Tactility/Source/service/gui/GuiService.cpp @@ -40,6 +40,51 @@ void GuiService::onLoaderEvent(LoaderService::Event event) { int32_t GuiService::guiMain() { auto service = findServiceById(manifest.id); + if (!lvgl::lock(5000)) { + LOGGER.error("LVGL guiMain start failed as LVGL couldn't be locked"); + return 0; + } + + // The screen root is created in the main task instead of during onStart because + // it allows onStart() to succeed faster and allows widget creation to happen in the background + + auto* screen_root = lv_screen_active(); + if (screen_root == nullptr) { + LOGGER.error("No display found, exiting GUI task"); + lvgl::unlock(); + return 0; + } + + service->keyboardGroup = lv_group_create(); + lv_obj_set_style_border_width(screen_root, 0, LV_STATE_DEFAULT); + lv_obj_set_style_pad_all(screen_root, 0, LV_STATE_DEFAULT); + + service->keyboardGroup = lv_group_create(); + lv_obj_set_style_border_width(screen_root, 0, LV_STATE_DEFAULT); + lv_obj_set_style_pad_all(screen_root, 0, LV_STATE_DEFAULT); + + lv_obj_t* vertical_container = lv_obj_create(screen_root); + lv_obj_set_size(vertical_container, LV_PCT(100), LV_PCT(100)); + lv_obj_set_flex_flow(vertical_container, LV_FLEX_FLOW_COLUMN); + lv_obj_set_style_pad_all(vertical_container, 0, LV_STATE_DEFAULT); + lv_obj_set_style_pad_gap(vertical_container, 0, LV_STATE_DEFAULT); + lv_obj_set_style_bg_color(vertical_container, lv_color_black(), LV_STATE_DEFAULT); + lv_obj_set_style_border_width(vertical_container, 0, LV_STATE_DEFAULT); + lv_obj_set_style_radius(vertical_container, 0, LV_STATE_DEFAULT); + + service->statusbarWidget = lvgl::statusbar_create(vertical_container); + + auto* app_container = lv_obj_create(vertical_container); + lv_obj_set_style_pad_all(app_container, 0, LV_STATE_DEFAULT); + lv_obj_set_style_border_width(app_container, 0, LV_STATE_DEFAULT); + lv_obj_set_width(app_container, LV_PCT(100)); + lv_obj_set_flex_grow(app_container, 1); + lv_obj_set_flex_flow(app_container, LV_FLEX_FLOW_COLUMN); + + service->appRootWidget = app_container; + + lvgl::unlock(); + while (true) { uint32_t flags = 0; if (service->threadFlags.wait(GUI_THREAD_FLAG_ALL, false, true, &flags, portMAX_DELAY)) { @@ -62,6 +107,9 @@ int32_t GuiService::guiMain() { } } + service->appRootWidget = nullptr; + service->statusbarWidget = nullptr; + return 0; } @@ -87,6 +135,12 @@ void GuiService::redraw() { // Lock GUI and LVGL lock(); + if (appRootWidget == nullptr) { + LOGGER.warn("No root widget"); + unlock(); + return; + } + if (lvgl::lock(1000)) { lv_obj_clean(appRootWidget); @@ -113,7 +167,7 @@ void GuiService::redraw() { lv_obj_t* container = createAppViews(appRootWidget); appToRender->getApp()->onShow(*appToRender, container); } else { - LOGGER.warn("nothing to draw"); + LOGGER.warn("Nothing to draw"); } // Unlock GUI and LVGL @@ -126,18 +180,11 @@ void GuiService::redraw() { } bool GuiService::onStart(ServiceContext& service) { - auto* screen_root = lv_screen_active(); - if (screen_root == nullptr) { - LOGGER.error("No display found"); - return false; - } - thread = new Thread( GUI_TASK_NAME, 4096, // Last known minimum was 2800 for launching desktop - []() { return guiMain(); } + guiMain ); - thread->setPriority(THREAD_PRIORITY_SERVICE); const auto loader = findLoaderService(); @@ -146,37 +193,8 @@ bool GuiService::onStart(ServiceContext& service) { onLoaderEvent(event); }); - lvgl::lock(portMAX_DELAY); - - keyboardGroup = lv_group_create(); - lv_obj_set_style_border_width(screen_root, 0, LV_STATE_DEFAULT); - lv_obj_set_style_pad_all(screen_root, 0, LV_STATE_DEFAULT); - - lv_obj_t* vertical_container = lv_obj_create(screen_root); - lv_obj_set_size(vertical_container, LV_PCT(100), LV_PCT(100)); - lv_obj_set_flex_flow(vertical_container, LV_FLEX_FLOW_COLUMN); - lv_obj_set_style_pad_all(vertical_container, 0, LV_STATE_DEFAULT); - lv_obj_set_style_pad_gap(vertical_container, 0, LV_STATE_DEFAULT); - lv_obj_set_style_bg_color(vertical_container, lv_color_black(), LV_STATE_DEFAULT); - lv_obj_set_style_border_width(vertical_container, 0, LV_STATE_DEFAULT); - lv_obj_set_style_radius(vertical_container, 0, LV_STATE_DEFAULT); - - statusbarWidget = lvgl::statusbar_create(vertical_container); - - auto* app_container = lv_obj_create(vertical_container); - lv_obj_set_style_pad_all(app_container, 0, LV_STATE_DEFAULT); - lv_obj_set_style_border_width(app_container, 0, LV_STATE_DEFAULT); - lv_obj_set_width(app_container, LV_PCT(100)); - lv_obj_set_flex_grow(app_container, 1); - lv_obj_set_flex_flow(app_container, LV_FLEX_FLOW_COLUMN); - - appRootWidget = app_container; - - lvgl::unlock(); - isStarted = true; - thread->setPriority(THREAD_PRIORITY_SERVICE); thread->start(); return true; @@ -192,20 +210,21 @@ void GuiService::onStop(ServiceContext& service) { appToRender = nullptr; isStarted = false; - auto task_handle = thread->getTaskHandle(); threadFlags.set(GUI_THREAD_FLAG_EXIT); - thread->join(); - delete thread; - unlock(); + thread->join(); - check(lvgl::lock(1000 / portTICK_PERIOD_MS)); - lv_group_delete(keyboardGroup); + lvgl::lock(); + if (keyboardGroup != nullptr) { + lv_group_delete(keyboardGroup); + keyboardGroup = nullptr; + } lvgl::unlock(); + + delete thread; } void GuiService::requestDraw() { - auto task_handle = thread->getTaskHandle(); threadFlags.set(GUI_THREAD_FLAG_DRAW); }