Implement lvgl-module for ESP32 and fix various subsystems

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 ()

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 <stdint.h>
+#include <stdbool.h>
 
+/**
+ * @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

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 <esp_lvgl_port.h>
+
+#include <tactility/error.h>
+#include <tactility/lvgl_module.h>
+
+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<UBaseType_t>(Thread::Priority::Critical),
+        .task_priority = lvgl_module_config.task_priority,
-        .task_stack = LVGL_TASK_STACK_DEPTH,
+        .task_stack = lvgl_module_config.task_stack_size,
-        .task_affinity = getCpuAffinityConfiguration().graphics,
+        .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;
 }
 

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
+    return recursive_mutex_try_lock_timed(&lvgl_mutex, timeout);
-    // 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);
 }
 
 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
     );
 

Modules/lvgl-module/Source/lvgl_module.c

@@ -1,23 +1,36 @@
 // SPDX-License-Identifier: GPL-3.0-only
+#include <lvgl.h>
 #include <string.h>
 #include <tactility/module.h>
 #include <tactility/lvgl_module.h>
 
+
 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,
+    NULL,
-    nullptr
+    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,

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<Lock> getSyncLock();

Tactility/Private/Tactility/lvgl/EspLvglPort.h

@@ -1,11 +0,0 @@
-#pragma once
-
-#ifdef ESP_PLATFORM
-
-namespace tt::lvgl {
-
-bool initEspLvglPort();
-
-}
-
-#endif

Tactility/Source/Tactility.cpp

@@ -2,11 +2,15 @@
 #include <sdkconfig.h>
 #endif
 
+#include <format>
+#include <map>
+
 #include <Tactility/Tactility.h>
 #include <Tactility/TactilityConfig.h>
 
 #include <Tactility/app/AppManifestParsing.h>
 #include <Tactility/app/AppRegistration.h>
+#include <Tactility/CpuAffinity.h>
 #include <Tactility/DispatcherThread.h>
 #include <Tactility/file/File.h>
 #include <Tactility/file/FileLock.h>
@@ -21,13 +25,11 @@
 #include <Tactility/service/ServiceRegistration.h>
 #include <Tactility/settings/TimePrivate.h>
 
+#include <tactility/concurrent/thread.h>
 #include <tactility/kernel_init.h>
 #include <tactility/hal_device_module.h>
 #include <tactility/lvgl_module.h>
 
-#include <format>
-#include <map>
-
 #ifdef ESP_PLATFORM
 #include <Tactility/InitEsp.h>
 #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();

Tactility/Source/lvgl/EspLvglPort.cpp

@@ -1,42 +0,0 @@
-#ifdef ESP_PLATFORM
-
-#include <Tactility/Thread.h>
-#include <Tactility/CpuAffinity.h>
-#include <Tactility/Logger.h>
-#include <Tactility/Mutex.h>
-#include <Tactility/lvgl/LvglSync.h>
-
-#include <esp_lvgl_port.h>
-
-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<UBaseType_t>(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

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);
 }
 

Tactility/Source/lvgl/LvglSync.cpp

@@ -1,39 +1,16 @@
 #include "Tactility/lvgl/LvglSync.h"
 
 #include <Tactility/Mutex.h>
+#include <tactility/lvgl_module.h>
 
 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();
     }
 };
 

Tactility/Source/service/gui/GuiService.cpp

@@ -40,6 +40,51 @@ void GuiService::onLoaderEvent(LoaderService::Event event) {
 int32_t GuiService::guiMain() {
     auto service = findServiceById<GuiService>(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));
+    lvgl::lock();
-    lv_group_delete(keyboardGroup);
+    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);
 }