Various improvements (#21)

* T-Deck stability and naming improvements

* allow main task to clean itself up

* remove unused includes

* various lvgl improvements

* added docs

app-sim/src/freertos.c

@@ -1,45 +1,31 @@
 #include "tactility.h"
+#include "thread.h"
 
 #include "FreeRTOS.h"
 #include "task.h"
 
 #define TAG "freertos"
 
-#define mainQUEUE_RECEIVE_TASK_PRIORITY (tskIDLE_PRIORITY + 2)
+void app_main();
-
-_Noreturn void app_main();
-
-bool lvgl_is_ready();
-void lvgl_task(void*);
-
-void app_main_task(TT_UNUSED void* parameter) {
-    while (!lvgl_is_ready()) {
-        TT_LOG_I(TAG, "waiting for lvgl task");
-        vTaskDelay(50);
-    }
 
+static void main_task(TT_UNUSED void* parameter) {
+    TT_LOG_I(TAG, "starting app_main()");
     app_main();
+    TT_LOG_I(TAG, "returned from app_main()");
+    vTaskDelete(NULL);
 }
 
 int main() {
-    // Create the main app loop, like ESP-IDF
+    BaseType_t task_result = xTaskCreate(
-    xTaskCreate(
+        main_task,
-        lvgl_task,
+        "main",
-        "lvgl",
         8192,
         NULL,
-        mainQUEUE_RECEIVE_TASK_PRIORITY + 2,
+        ThreadPriorityNormal,
         NULL
     );
 
-    xTaskCreate(
+    tt_assert(task_result == pdTRUE);
-        app_main_task,
-        "app_main",
-        8192,
-        NULL,
-        mainQUEUE_RECEIVE_TASK_PRIORITY + 1,
-        NULL
-    );
 
     // Blocks forever
     vTaskStartScheduler();
@@ -49,11 +35,11 @@ int main() {
  * Assert implementation as defined in the FreeRTOSConfig.h
  * It allows you to set breakpoints and debug asserts.
  */
-void vAssertCalled(TT_UNUSED unsigned long line, TT_UNUSED const char* const file) {
+void vAssertCalled(unsigned long line, const char* const file) {
     static portBASE_TYPE xPrinted = pdFALSE;
     volatile uint32_t set_to_nonzero_in_debugger_to_continue = 0;
 
-    TT_LOG_E(TAG, "assert triggered");
+    TT_LOG_E(TAG, "assert triggered at %s:%d", file, line);
     taskENTER_CRITICAL();
     {
         // Step out by attaching a debugger and setting set_to_nonzero_in_debugger_to_continue

app-sim/src/hardware_config.c

@@ -1,16 +1,28 @@
-/**
- * Placeholder hardware config.
- * The real one happens during FreeRTOS startup. See freertos.c and lvgl_*.c
- */
-#include <stdbool.h>
 #include "hardware_config.h"
+#include "lvgl_task.h"
+#include <src/core/lv_obj.h>
+#include <stdbool.h>
 
+#define TAG "hardware"
 
-// TODO: See if we can move the init from FreeRTOS to app_main()?
+static bool lvgl_init() {
-static bool init_lvgl() { return true; }
+    lv_init();
+    lvgl_task_start();
+    return true;
+}
+
+TT_UNUSED static void lvgl_deinit() {
+    lvgl_task_interrupt();
+    while (lvgl_task_is_running()) {
+        tt_delay_ms(10);
+    }
+
+#if LV_ENABLE_GC || !LV_MEM_CUSTOM
+    lv_deinit();
+#endif
+}
 
 HardwareConfig sim_hardware = {
     .bootstrap = NULL,
-    .init_lvgl = &init_lvgl,
+    .init_lvgl = &lvgl_init,
 };
-

app-sim/src/lvgl_hal.c

@@ -1,5 +1,3 @@
-#include "lvgl_hal.h"
-
 #include "lvgl.h"
 #include "tactility_core.h"
 #include <sdl/sdl.h>
@@ -8,16 +6,17 @@
 
 #define BUFFER_SIZE (SDL_HOR_RES * SDL_VER_RES * 3)
 
-static lv_disp_t* hal_init() {
+lv_disp_t* lvgl_hal_init() {
-    /* Use the 'monitor' driver which creates window on PC's monitor to simulate a display*/
+    // Use the 'monitor' driver to simulate a display on PC
+    // Note: this is part of lv_drivers and not SDL!
     sdl_init();
 
-    /*Create a display buffer*/
+    // Create display buffer
     static lv_disp_draw_buf_t disp_buf1;
     static lv_color_t buf1_1[BUFFER_SIZE];
     lv_disp_draw_buf_init(&disp_buf1, buf1_1, NULL, BUFFER_SIZE);
 
-    /*Create a display*/
+    // Create display
     static lv_disp_drv_t disp_drv;
     lv_disp_drv_init(&disp_drv); /*Basic initialization*/
     disp_drv.draw_buf = &disp_buf1;
@@ -25,20 +24,20 @@ static lv_disp_t* hal_init() {
     disp_drv.hor_res = SDL_HOR_RES;
     disp_drv.ver_res = SDL_VER_RES;
 
-    lv_disp_t* disp = lv_disp_drv_register(&disp_drv);
+    lv_disp_t* display = lv_disp_drv_register(&disp_drv);
 
-    lv_theme_t* th = lv_theme_default_init(
+    lv_theme_t* theme = lv_theme_default_init(
-        disp,
+        display,
         lv_palette_main(LV_PALETTE_BLUE),
         lv_palette_main(LV_PALETTE_RED),
         LV_THEME_DEFAULT_DARK,
         LV_FONT_DEFAULT
     );
 
-    lv_disp_set_theme(disp, th);
+    lv_disp_set_theme(display, theme);
 
-    lv_group_t* g = lv_group_create();
+    lv_group_t* group = lv_group_create();
-    lv_group_set_default(g);
+    lv_group_set_default(group);
 
     /* Add the mouse as input device
      * Use the 'mouse' driver which reads the PC's mouse*/
@@ -55,21 +54,15 @@ static lv_disp_t* hal_init() {
     indev_drv_2.type = LV_INDEV_TYPE_KEYPAD;
     indev_drv_2.read_cb = sdl_keyboard_read;
     lv_indev_t* kb_indev = lv_indev_drv_register(&indev_drv_2);
-    lv_indev_set_group(kb_indev, g);
+    lv_indev_set_group(kb_indev, group);
 
     static lv_indev_drv_t indev_drv_3;
     lv_indev_drv_init(&indev_drv_3); /*Basic initialization*/
     indev_drv_3.type = LV_INDEV_TYPE_ENCODER;
     indev_drv_3.read_cb = sdl_mousewheel_read;
     lv_indev_t* enc_indev = lv_indev_drv_register(&indev_drv_3);
-    lv_indev_set_group(enc_indev, g);
+    lv_indev_set_group(enc_indev, group);
 
-    return disp;
+    return display;
 }
 
-void lvgl_hal_init() {
-    TT_LOG_I(TAG, "init: started");
-    lv_init();
-    hal_init();
-    TT_LOG_I(TAG, "init: complete");
-}

app-sim/src/lvgl_hal.h

@@ -7,7 +7,7 @@
 extern "C" {
 #endif
 
-void lvgl_hal_init();
+lv_disp_t* lvgl_hal_init();
 
 #ifdef __cplusplus
 }

app-sim/src/lvgl_task.c

@@ -1,5 +1,6 @@
 #include "lvgl_task.h"
 
+#include "lvgl.h"
 #include "lvgl_hal.h"
 #include "tactility_core.h"
 #include "thread.h"
@@ -18,6 +19,8 @@ static uint32_t task_max_sleep_ms = 10;
 static QueueHandle_t task_mutex = NULL;
 static bool task_running = false;
 
+static void lvgl_task(TT_UNUSED void* arg);
+
 static bool task_lock(int timeout_ticks) {
     assert(task_mutex != NULL);
     return xSemaphoreTakeRecursive(task_mutex, timeout_ticks) == pdTRUE;
@@ -34,14 +37,14 @@ static void task_set_running(bool running) {
     task_unlock();
 }
 
-static bool task_is_running() {
+bool lvgl_task_is_running() {
     assert(task_lock(configTICK_RATE_HZ / 100));
     bool result = task_running;
     task_unlock();
     return result;
 }
 
-static bool lvgl_lock(int timeout_ticks) {
+static bool lvgl_lock(uint32_t timeout_ticks) {
     assert(lvgl_mutex != NULL);
     return xSemaphoreTakeRecursive(lvgl_mutex, timeout_ticks) == pdTRUE;
 }
@@ -51,7 +54,15 @@ static void lvgl_unlock() {
     xSemaphoreGiveRecursive(lvgl_mutex);
 }
 
-static void lvgl_task_init() {
+void lvgl_task_interrupt() {
+    tt_check(lvgl_lock(TtWaitForever));
+    task_set_running(false); // interrupt task with boolean as flag
+    lvgl_unlock();
+}
+
+void lvgl_task_start() {
+    TT_LOG_I(TAG, "lvgl task starting");
+
     if (lvgl_mutex == NULL) {
         TT_LOG_D(TAG, "init: creating lvgl mutex");
         lvgl_mutex = xSemaphoreCreateRecursiveMutex();
@@ -63,31 +74,30 @@ static void lvgl_task_init() {
     }
 
     tt_lvgl_sync_set(&lvgl_lock, &lvgl_unlock);
+
+    // Create the main app loop, like ESP-IDF
+    BaseType_t task_result = xTaskCreate(
+        lvgl_task,
+        "lvgl",
+        8192,
+        NULL,
+        ThreadPriorityHigh, // Should be higher than main app task
+        NULL
+    );
+
+    tt_assert(task_result == pdTRUE);
 }
 
-static void lvgl_task_deinit() {
+static void lvgl_task(TT_UNUSED void* arg) {
-    if (lvgl_mutex) {
+    TT_LOG_I(TAG, "lvgl task started");
-        vSemaphoreDelete(lvgl_mutex);
-        lvgl_mutex = NULL;
-    }
-    if (task_mutex) {
-        vSemaphoreDelete(task_mutex);
-        task_mutex = NULL;
-    }
-#if LV_ENABLE_GC || !LV_MEM_CUSTOM
-    lv_deinit();
-#endif
-}
 
-void lvgl_task(TT_UNUSED void* arg) {
+    lv_disp_t* display = lvgl_hal_init();
-    lvgl_hal_init();
-    lvgl_task_init();
 
     uint32_t task_delay_ms = task_max_sleep_ms;
 
     task_set_running(true);
 
-    while (task_is_running()) {
+    while (lvgl_task_is_running()) {
         if (lvgl_lock(0)) {
             task_delay_ms = lv_timer_handler();
             lvgl_unlock();
@@ -100,16 +110,8 @@ void lvgl_task(TT_UNUSED void* arg) {
         vTaskDelay(pdMS_TO_TICKS(task_delay_ms));
     }
 
-    lvgl_task_deinit();
+    lv_disp_remove(display);
+
     vTaskDelete(NULL);
 }
 
-bool lvgl_is_ready() {
-    return task_running;
-}
-
-void lvgl_interrupt() {
-    tt_check(lvgl_lock(TtWaitForever));
-    task_set_running(false); // interrupt task with boolean as flag
-    lvgl_unlock();
-}

app-sim/src/lvgl_task.h

@@ -6,8 +6,9 @@
 extern "C" {
 #endif
 
-bool lvgl_is_ready();
+void lvgl_task_start();
-void lvgl_interrupt();
+bool lvgl_task_is_running();
+void lvgl_task_interrupt();
 
 #ifdef __cplusplus
 }

app-sim/src/main.c

@@ -1,16 +1,13 @@
 #include "hello_world/hello_world.h"
-#include "lvgl_hal.h"
 #include "tactility.h"
-#include "ui/lvgl_sync.h"
 
 #include "FreeRTOS.h"
-#include "task.h"
 
 #define TAG "main"
 
 extern HardwareConfig sim_hardware;
 
-_Noreturn void app_main() {
+void app_main() {
     static const Config config = {
         .hardware = &sim_hardware,
         .apps = {
@@ -20,11 +17,5 @@ _Noreturn void app_main() {
         .auto_start_app_id = NULL
     };
 
-    TT_LOG_I("app", "Hello, world!");
-
     tt_init(&config);
-
-    while (true) {
-        vTaskDelay(1000);
-    }
 }

boards/lilygo_tdeck/bootstrap.c

@@ -5,7 +5,7 @@
 
 #define TAG "tdeck_bootstrap"
 
-lv_disp_t* lilygo_tdeck_init_display();
+lv_disp_t* tdeck_init_display();
 
 static bool tdeck_power_on() {
     ESP_LOGI(TAG, "power on");
@@ -42,14 +42,22 @@ static bool init_i2c() {
         && i2c_driver_install(TDECK_I2C_BUS_HANDLE, i2c_conf.mode, 0, 0, 0) == ESP_OK;
 }
 
-bool lilygo_tdeck_bootstrap() {
+bool tdeck_bootstrap() {
     if (!tdeck_power_on()) {
         TT_LOG_E(TAG, "failed to power on device");
     }
 
-    // Give keyboard's ESP time to boot
+    /**
-    // It uses I2C and seems to interfere with the touch driver
+     * Without this delay, the touch driver randomly fails when the device is USB-powered:
-    tt_delay_ms(500);
+     *  > lcd_panel.io.i2c: panel_io_i2c_rx_buffer(135): i2c transaction failed
+     *  > GT911: touch_gt911_read_cfg(352): GT911 read error!
+     * This might not be a problem with a lipo, but I haven't been able to test that.
+     * I tried to solve it just like I did with the keyboard:
+     * By reading from I2C until it succeeds and to then init the driver.
+     * It doesn't work, because it never recovers from the error.
+     */
+    TT_LOG_I(TAG, "waiting after power-on");
+    tt_delay_ms(2000);
 
     if (!init_i2c()) {
         TT_LOG_E(TAG, "failed to init I2C");

boards/lilygo_tdeck/display.c

@@ -32,7 +32,7 @@
 #define LCD_BACKLIGHT_LEDC_DUTY (191)
 #define LCD_BACKLIGHT_LEDC_FREQUENCY (1000)
 
-static void tdeck_backlight() {
+void tdeck_enable_backlight() {
     ESP_LOGI(TAG, "enable backlight");
 
     ledc_timer_config_t ledc_timer = {
@@ -58,7 +58,7 @@ static void tdeck_backlight() {
     ESP_ERROR_CHECK(ledc_set_duty(LCD_BACKLIGHT_LEDC_MODE, LCD_BACKLIGHT_LEDC_CHANNEL, LCD_BACKLIGHT_LEDC_DUTY));
 }
 
-lv_disp_t* lilygo_tdeck_init_display() {
+lv_disp_t* tdeck_init_display() {
     ESP_LOGI(TAG, "creating display");
 
     int max_transfer_size = LCD_HORIZONTAL_RESOLUTION * LCD_SPI_TRANSFER_HEIGHT * (LCD_BITS_PER_PIXEL / 8);
@@ -170,7 +170,7 @@ lv_disp_t* lilygo_tdeck_init_display() {
         }
     };
 
-    tdeck_backlight();
+    lv_disp_t* display = lvgl_port_add_disp(&disp_cfg);
 
-    return lvgl_port_add_disp(&disp_cfg);
+    return display;
 }

boards/lilygo_tdeck/lilygo_tdeck.c

@@ -1,10 +1,10 @@
 #include "lilygo_tdeck.h"
 #include <stdbool.h>
 
-bool lilygo_tdeck_bootstrap();
+bool tdeck_bootstrap();
-bool lilygo_init_lvgl();
+bool tdeck_init_lvgl();
 
 const HardwareConfig lilygo_tdeck = {
-    .bootstrap = &lilygo_tdeck_bootstrap,
+    .bootstrap = &tdeck_bootstrap,
-    .init_lvgl = &lilygo_init_lvgl
+    .init_lvgl = &tdeck_init_lvgl
 };

boards/lilygo_tdeck/lvgl.c

@@ -6,14 +6,17 @@
 
 #define TAG "tdeck_lvgl"
 
-lv_disp_t* lilygo_tdeck_init_display();
+lv_disp_t* tdeck_init_display();
-bool lilygo_tdeck_init_touch(esp_lcd_panel_io_handle_t* io_handle, esp_lcd_touch_handle_t* touch_handle);
+void tdeck_enable_backlight();
+bool tdeck_init_touch(esp_lcd_panel_io_handle_t* io_handle, esp_lcd_touch_handle_t* touch_handle);
 
-bool lilygo_init_lvgl() {
+bool tdeck_init_lvgl() {
     static lv_disp_t* display = NULL;
     static esp_lcd_panel_io_handle_t touch_io_handle;
     static esp_lcd_touch_handle_t touch_handle;
 
+    // Init LVGL Port library
+
     const lvgl_port_cfg_t lvgl_cfg = {
         .task_priority = ThreadPriorityHigh,
         .task_stack = 8096,
@@ -28,14 +31,16 @@ bool lilygo_init_lvgl() {
     }
 
     // Add display
-    display = lilygo_tdeck_init_display();
+
+    display = tdeck_init_display();
     if (display == NULL) {
         TT_LOG_E(TAG, "failed to add display");
         return false;
     }
 
     // Add touch
-    if (!lilygo_tdeck_init_touch(&touch_io_handle, &touch_handle)) {
+
+    if (!tdeck_init_touch(&touch_io_handle, &touch_handle)) {
         return false;
     }
 
@@ -55,5 +60,7 @@ bool lilygo_init_lvgl() {
 
     keyboard_alloc(display);
 
+    tdeck_enable_backlight();
+
     return true;
 }

boards/lilygo_tdeck/touch.c

@@ -1,21 +1,22 @@
 #include "config.h"
-#include "driver/i2c.h"
 #include "esp_err.h"
 #include "esp_lcd_touch_gt911.h"
-#include "esp_log.h"
+#include "esp_lcd_panel_io_interface.h"
+#include "log.h"
+#include <kernel.h>
 
 #define TAG "tdeck_touch"
 
-bool lilygo_tdeck_init_touch(esp_lcd_panel_io_handle_t* io_handle, esp_lcd_touch_handle_t* touch_handle) {
+bool tdeck_init_touch(esp_lcd_panel_io_handle_t* io_handle, esp_lcd_touch_handle_t* touch_handle) {
-    ESP_LOGI(TAG, "creating touch");
+    TT_LOG_I(TAG, "creating touch");
 
     const esp_lcd_panel_io_i2c_config_t touch_io_config = ESP_LCD_TOUCH_IO_I2C_GT911_CONFIG();
     if (esp_lcd_new_panel_io_i2c((esp_lcd_i2c_bus_handle_t)TDECK_I2C_BUS_HANDLE, &touch_io_config, io_handle) != ESP_OK) {
-        ESP_LOGE(TAG, "touch io i2c creation failed");
+        TT_LOG_E(TAG, "touch io i2c creation failed");
         return false;
     }
 
-    ESP_LOGI(TAG, "create_touch");
+    TT_LOG_I(TAG, "create_touch");
     esp_lcd_touch_config_t config = {
         .x_max = 240,
         .y_max = 320,
@@ -36,7 +37,7 @@ bool lilygo_tdeck_init_touch(esp_lcd_panel_io_handle_t* io_handle, esp_lcd_touch
     };
 
     if (esp_lcd_touch_new_i2c_gt911(*io_handle, &config, touch_handle) != ESP_OK) {
-        ESP_LOGE(TAG, "gt911 driver creation failed");
+        TT_LOG_E(TAG, "gt911 driver creation failed");
         return false;
     }
 

boards/waveshare_s3_touch/display.c

@@ -136,7 +136,7 @@ lv_disp_t* ws3t_display_create() {
     assert(lvgl_mux);
 
     Thread* thread = tt_thread_alloc_ex("display_task", 8192, &display_task, NULL);
-    tt_thread_set_priority(thread, ThreadPriorityHigh);
+    tt_thread_set_priority(thread, ThreadPriorityHigh); // TODO: try out THREAD_PRIORITY_RENDER
     tt_thread_start(thread);
 
     ESP_LOGI(TAG, "Install RGB LCD panel driver");

tactility-core/src/thread.c

@@ -24,6 +24,9 @@
 #define THREAD_FLAGS_INVALID_BITS (~((1UL << MAX_BITS_TASK_NOTIFY) - 1U))
 #define EVENT_FLAGS_INVALID_BITS (~((1UL << MAX_BITS_EVENT_GROUPS) - 1U))
 
+static_assert(ThreadPriorityHighest <= TT_CONFIG_THREAD_MAX_PRIORITIES, "highest thread priority is higher than max priority");
+static_assert(TT_CONFIG_THREAD_MAX_PRIORITIES <= configMAX_PRIORITIES, "highest tactility priority is higher than max FreeRTOS priority");
+
 struct Thread {
     ThreadState state;
     int32_t ret;
@@ -193,7 +196,7 @@ void tt_thread_set_context(Thread* thread, void* context) {
 void tt_thread_set_priority(Thread* thread, ThreadPriority priority) {
     tt_assert(thread);
     tt_assert(thread->state == ThreadStateStopped);
-    tt_assert(priority >= ThreadPriorityIdle && priority <= ThreadPriorityIsr);
+    tt_assert(priority >= 0 && priority <= TT_CONFIG_THREAD_MAX_PRIORITIES);
     thread->priority = priority;
 }
 

tactility-core/src/thread.h

@@ -19,17 +19,21 @@ typedef enum {
 
 /** ThreadPriority */
 typedef enum {
-    ThreadPriorityNone = 0,     /**< Uninitialized, choose system default */
+    ThreadPriorityNone = 0, /**< Uninitialized, choose system default */
-    ThreadPriorityIdle = 1,     /**< Idle priority */
+    ThreadPriorityIdle = 1,
-    ThreadPriorityLowest = 2,  /**< Lowest */
+    ThreadPriorityLowest = 2,
-    ThreadPriorityLow = 3,     /**< Low */
+    ThreadPriorityLow = 3,
-    ThreadPriorityNormal = 4,  /**< Normal */
+    ThreadPriorityNormal = 4,
-    ThreadPriorityHigh = 5,    /**< High */
+    ThreadPriorityHigh = 5,
-    ThreadPriorityHighest = 6, /**< Highest */
+    ThreadPriorityHigher = 6,
-    ThreadPriorityIsr =
+    ThreadPriorityHighest = 7
-        (TT_CONFIG_THREAD_MAX_PRIORITIES - 1), /**< Deferred ISR (highest possible) */
 } ThreadPriority;
 
+#define THREAD_PRIORITY_APP ThreadPriorityNormal
+#define THREAD_PRIORITY_SERVICE ThreadPriorityHigh
+#define THREAD_PRIORITY_RENDER ThreadPriorityHigher
+#define THREAD_PRIORITY_ISR (TT_CONFIG_THREAD_MAX_PRIORITIES - 1)
+
 /** Thread anonymous structure */
 typedef struct Thread Thread;
 

tactility/src/services/gui/gui.c

@@ -150,7 +150,7 @@ static int32_t gui_main(TT_UNUSED void* p) {
 static void gui_start(TT_UNUSED Service service) {
     gui = gui_alloc();
 
-    tt_thread_set_priority(gui->thread, ThreadPriorityNormal);
+    tt_thread_set_priority(gui->thread, THREAD_PRIORITY_SERVICE);
     tt_thread_start(gui->thread);
 }
 

tactility/src/services/loader/loader.c

@@ -299,7 +299,7 @@ static void loader_start(TT_UNUSED Service service) {
     tt_check(loader_singleton == NULL);
     loader_singleton = loader_alloc();
 
-    tt_thread_set_priority(loader_singleton->thread, ThreadPriorityNormal);
+    tt_thread_set_priority(loader_singleton->thread, THREAD_PRIORITY_SERVICE);
     tt_thread_start(loader_singleton->thread);
 }