PR feedback fixes

This commit is contained in:
Ken Van Hoeylandt 2026-02-01 21:47:58 +01:00
parent 219970c2ac
commit e61023e087
8 changed files with 59 additions and 36 deletions

View File

@ -6,7 +6,7 @@
class SdlDisplay final : public tt::hal::display::DisplayDevice { class SdlDisplay final : public tt::hal::display::DisplayDevice {
lv_disp_t* displayHandle; lv_disp_t* displayHandle = nullptr;
public: public:
@ -20,12 +20,14 @@ public:
bool supportsLvgl() const override { return true; } bool supportsLvgl() const override { return true; }
bool startLvgl() override { bool startLvgl() override {
if (displayHandle) return true; // already started
displayHandle = lv_sdl_window_create(320, 240); displayHandle = lv_sdl_window_create(320, 240);
lv_sdl_window_set_title(displayHandle, "Tactility"); lv_sdl_window_set_title(displayHandle, "Tactility");
return displayHandle != nullptr; return displayHandle != nullptr;
} }
bool stopLvgl() override { bool stopLvgl() override {
if (!displayHandle) return true;
lv_display_delete(displayHandle); lv_display_delete(displayHandle);
displayHandle = nullptr; displayHandle = nullptr;
return true; return true;

View File

@ -69,7 +69,7 @@ bool lvgl_lock(void);
/** /**
* @brief Tries to lock the LVGL mutex with a timeout. * @brief Tries to lock the LVGL mutex with a timeout.
* *
* @param timeout Timeout in milliseconds. * @param timeout Timeout in ticks
* @return true if the lock was acquired, false otherwise. * @return true if the lock was acquired, false otherwise.
*/ */
bool lvgl_try_lock_timed(uint32_t timeout); bool lvgl_try_lock_timed(uint32_t timeout);

View File

@ -3,6 +3,7 @@
#include <esp_lvgl_port.h> #include <esp_lvgl_port.h>
#include <tactility/time.h>
#include <tactility/error.h> #include <tactility/error.h>
#include <tactility/lvgl_module.h> #include <tactility/lvgl_module.h>
@ -17,7 +18,7 @@ bool lvgl_lock(void) {
bool lvgl_try_lock_timed(uint32_t timeout) { bool lvgl_try_lock_timed(uint32_t timeout) {
if (!initialized) return false; if (!initialized) return false;
return lvgl_port_lock(timeout); return lvgl_port_lock(millis_to_ticks(timeout));
} }
void lvgl_unlock(void) { void lvgl_unlock(void) {

View File

@ -5,8 +5,8 @@
#include <tactility/delay.h> #include <tactility/delay.h>
#include <tactility/concurrent/recursive_mutex.h> #include <tactility/concurrent/recursive_mutex.h>
#include <tactility/concurrent/thread.h> #include <tactility/concurrent/thread.h>
#include <tactility/freertos/task.h> #include <tactility/freertos/task.h>
#include <tactility/time.h>
#include <lvgl.h> #include <lvgl.h>
@ -28,42 +28,34 @@ static bool lvgl_task_interrupt_requested = false;
#define LVGL_STOP_TIMEOUT 5000 #define LVGL_STOP_TIMEOUT 5000
static void task_lock(void) { static void task_lock(void) {
if (!task_mutex_initialised) { if (!task_mutex_initialised) return;
recursive_mutex_construct(&task_mutex);
task_mutex_initialised = true;
}
recursive_mutex_lock(&task_mutex); recursive_mutex_lock(&task_mutex);
} }
static void task_unlock(void) { static void task_unlock(void) {
if (!task_mutex_initialised) return;
recursive_mutex_unlock(&task_mutex); recursive_mutex_unlock(&task_mutex);
} }
bool lvgl_lock(void) { bool lvgl_lock(void) {
if (!lvgl_mutex_initialised) { if (!lvgl_mutex_initialised) return false;
recursive_mutex_construct(&lvgl_mutex);
lvgl_mutex_initialised = true;
}
recursive_mutex_lock(&lvgl_mutex); recursive_mutex_lock(&lvgl_mutex);
return true; return true;
} }
bool lvgl_try_lock_timed(uint32_t timeout) { bool lvgl_try_lock_timed(uint32_t timeout) {
if (!lvgl_mutex_initialised) { if (!lvgl_mutex_initialised) return false;
recursive_mutex_construct(&lvgl_mutex); return recursive_mutex_try_lock_timed(&lvgl_mutex, millis_to_ticks(timeout));
lvgl_mutex_initialised = true;
}
return recursive_mutex_try_lock_timed(&lvgl_mutex, timeout);
} }
void lvgl_unlock(void) { void lvgl_unlock(void) {
if (!lvgl_mutex_initialised) return;
recursive_mutex_unlock(&lvgl_mutex); recursive_mutex_unlock(&lvgl_mutex);
} }
static void lvgl_task_interrupt() { static void lvgl_task_set_interrupted(bool interrupted) {
task_lock(); task_lock();
lvgl_task_interrupt_requested = false; // interrupt task with boolean as flag lvgl_task_interrupt_requested = interrupted;
task_unlock(); task_unlock();
} }
@ -79,7 +71,7 @@ static void lvgl_task(void* arg) {
check(!lvgl_task_is_interrupt_requested()); check(!lvgl_task_is_interrupt_requested());
// on_start must be called from the task, otherwhise the display doesn't work // on_start must be called from the task, otherwise the display doesn't work
if (lvgl_module_config.on_start) lvgl_module_config.on_start(); if (lvgl_module_config.on_start) lvgl_module_config.on_start();
while (!lvgl_task_is_interrupt_requested()) { while (!lvgl_task_is_interrupt_requested()) {
@ -97,10 +89,26 @@ static void lvgl_task(void* arg) {
if (lvgl_module_config.on_stop) lvgl_module_config.on_stop(); if (lvgl_module_config.on_stop) lvgl_module_config.on_stop();
task_lock();
lvgl_task_handle = nullptr;
task_unlock();
vTaskDelete(NULL); vTaskDelete(NULL);
} }
error_t lvgl_arch_start() { error_t lvgl_arch_start() {
if (!lvgl_mutex_initialised) {
recursive_mutex_construct(&lvgl_mutex);
lvgl_mutex_initialised = true;
}
if (!task_mutex_initialised) {
recursive_mutex_construct(&task_mutex);
task_mutex_initialised = true;
}
lvgl_task_set_interrupted(false);
lv_init(); lv_init();
// Create the main app loop, like ESP-IDF // Create the main app loop, like ESP-IDF
@ -108,9 +116,9 @@ error_t lvgl_arch_start() {
lvgl_task, lvgl_task,
"lvgl", "lvgl",
lvgl_module_config.task_stack_size, lvgl_module_config.task_stack_size,
&lvgl_task_handle, NULL,
lvgl_module_config.task_priority, lvgl_module_config.task_priority,
NULL &lvgl_task_handle
); );
return (task_result == pdTRUE) ? ERROR_NONE : ERROR_RESOURCE; return (task_result == pdTRUE) ? ERROR_NONE : ERROR_RESOURCE;
@ -118,9 +126,15 @@ error_t lvgl_arch_start() {
error_t lvgl_arch_stop() { error_t lvgl_arch_stop() {
TickType_t start_ticks = get_ticks(); TickType_t start_ticks = get_ticks();
lvgl_task_interrupt(); lvgl_task_set_interrupted(true);
while (lvgl_task_handle != NULL) { // TODO: make thread-safe while (true) {
task_lock();
bool done = (lvgl_task_handle == nullptr);
task_unlock();
if (done) break;
delay_ticks(LVGL_STOP_POLL_INTERVAL); delay_ticks(LVGL_STOP_POLL_INTERVAL);
if (get_ticks() - start_ticks > LVGL_STOP_TIMEOUT) { if (get_ticks() - start_ticks > LVGL_STOP_TIMEOUT) {
return ERROR_TIMEOUT; return ERROR_TIMEOUT;
} }

View File

@ -1,4 +1,7 @@
#include <stdlib.h>
#if defined(__GLIBC__)
#include <assert.h> #include <assert.h>
#endif
#include <tactility/freertos/task.h> #include <tactility/freertos/task.h>
#include <tactility/log.h> #include <tactility/log.h>
@ -9,5 +12,9 @@
*/ */
void vAssertCalled(unsigned long line, const char* const file) { void vAssertCalled(unsigned long line, const char* const file) {
LOG_E("Assert triggered at {}:{}", file, line); LOG_E("Assert triggered at {}:{}", file, line);
#if defined(__GLIBC__)
__assert_fail("assert failed", file, line, ""); __assert_fail("assert failed", file, line, "");
#else
abort();
#endif
} }

View File

@ -371,7 +371,7 @@ void run(const Configuration& config, Module* platformModule, Module* deviceModu
.task_affinity = getCpuAffinityConfiguration().graphics .task_affinity = getCpuAffinityConfiguration().graphics
#endif #endif
}); });
module_set_parent(&lvgl_module, &tactility_module_parent); check(module_set_parent(&lvgl_module, &tactility_module_parent) == ERROR_NONE);
lvgl::start(); lvgl::start();
registerAndStartSecondaryServices(); registerAndStartSecondaryServices();

View File

@ -43,7 +43,7 @@ std::shared_ptr<AppContext> getCurrentAppContext() {
std::shared_ptr<App> getCurrentApp() { std::shared_ptr<App> getCurrentApp() {
const auto app_context = getCurrentAppContext(); const auto app_context = getCurrentAppContext();
return (app_context != nullptr) ? app_context->getApp() : nullptr; return (app_context != nullptr) ? app_context->getApp() : nullptr;
} }
} }

View File

@ -59,10 +59,6 @@ int32_t GuiService::guiMain() {
lv_obj_set_style_border_width(screen_root, 0, LV_STATE_DEFAULT); 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_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_t* vertical_container = lv_obj_create(screen_root);
lv_obj_set_size(vertical_container, LV_PCT(100), LV_PCT(100)); 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_flex_flow(vertical_container, LV_FLEX_FLOW_COLUMN);
@ -214,12 +210,15 @@ void GuiService::onStop(ServiceContext& service) {
unlock(); unlock();
thread->join(); thread->join();
lvgl::lock(); if (lvgl::lock()) {
if (keyboardGroup != nullptr) { if (keyboardGroup != nullptr) {
lv_group_delete(keyboardGroup); lv_group_delete(keyboardGroup);
keyboardGroup = nullptr; keyboardGroup = nullptr;
}
lvgl::unlock();
} else {
LOGGER.error("Failed to unlock LVGL during GUI stop");
} }
lvgl::unlock();
delete thread; delete thread;
} }