From 700c3322191aed14a587b48aed97df0eb9d25c96 Mon Sep 17 00:00:00 2001 From: Ken Van Hoeylandt Date: Fri, 21 Aug 2026 21:18:20 +0200 Subject: [PATCH 01/14] Fix for USB keyboard --- .../lvgl-module/include/lvgl/devices/keyboard.h | 8 ++++++++ Modules/lvgl-module/source/devices/keyboard.cpp | 14 ++++++++++++++ Tactility/Source/lvgl/KeyboardDeviceListener.cpp | 16 ++++++++++------ 3 files changed, 32 insertions(+), 6 deletions(-) diff --git a/Modules/lvgl-module/include/lvgl/devices/keyboard.h b/Modules/lvgl-module/include/lvgl/devices/keyboard.h index d0c0af61e..66e62dc71 100644 --- a/Modules/lvgl-module/include/lvgl/devices/keyboard.h +++ b/Modules/lvgl-module/include/lvgl/devices/keyboard.h @@ -36,6 +36,14 @@ error_t lvgl_keyboard_add(struct Device* device, lv_display_t* display, lv_indev */ void lvgl_keyboard_remove(lv_indev_t* indev); +/** + * @brief Finds the indev previously created with lvgl_keyboard_add() for the given device, if any. + * @warning Caller must hold the LVGL lock. + * @param[in] device a device of type KEYBOARD_TYPE + * @return the bound indev, or NULL if none is bound to this device + */ +lv_indev_t* lvgl_keyboard_find_by_device(struct Device* device); + /** * @brief Assigns the indev to the shared keyboard input group, so it can drive focus * navigation and input for focused widgets. diff --git a/Modules/lvgl-module/source/devices/keyboard.cpp b/Modules/lvgl-module/source/devices/keyboard.cpp index 02bee3d89..a0b725295 100644 --- a/Modules/lvgl-module/source/devices/keyboard.cpp +++ b/Modules/lvgl-module/source/devices/keyboard.cpp @@ -100,6 +100,20 @@ void lvgl_keyboard_remove(lv_indev_t* indev) { delete wrapper; } +lv_indev_t* lvgl_keyboard_find_by_device(Device* device) { + lv_indev_t* indev = lv_indev_get_next(nullptr); + while (indev != nullptr) { + if (lv_indev_get_type(indev) == LV_INDEV_TYPE_KEYPAD) { + auto* wrapper = static_cast(lv_indev_get_driver_data(indev)); + if (wrapper != nullptr && wrapper->device == device) { + return indev; + } + } + indev = lv_indev_get_next(indev); + } + return nullptr; +} + void lvgl_keyboard_enable(lv_indev_t* indev) { check(keyboard_group != nullptr); lv_indev_set_group(indev, keyboard_group); diff --git a/Tactility/Source/lvgl/KeyboardDeviceListener.cpp b/Tactility/Source/lvgl/KeyboardDeviceListener.cpp index f017a366d..c237f7431 100644 --- a/Tactility/Source/lvgl/KeyboardDeviceListener.cpp +++ b/Tactility/Source/lvgl/KeyboardDeviceListener.cpp @@ -63,21 +63,25 @@ void onKeyboardDeviceStarted(Device* device) { void onKeyboardDeviceStopped(Device* device) { auto lock = bindingsMutex().asScopedLock(); lock.lock(); - lv_indev_t* indev = nullptr; auto& list = bindings(); for (auto it = list.begin(); it != list.end(); ++it) { if (it->device == device) { - indev = it->indev; list.erase(it); break; } } - if (indev == nullptr) { - return; - } + // Not every indev bound to a keyboard device goes through onKeyboardDeviceStarted() and + // bindings() above. lvgl_devices_attach() also binds whatever KEYBOARD_TYPE devices are + // already started at LVGL startup, before this listener is even registered (see + // startKeyboardDeviceListener()'s call site). Look the indev up directly so that path's + // devices still get detached before the kernel device is destructed, instead of leaving a + // dangling indev that reads a freed device on the next LVGL tick. lvgl_lock(); - lvgl_keyboard_remove(indev); + lv_indev_t* indev = lvgl_keyboard_find_by_device(device); + if (indev != nullptr) { + lvgl_keyboard_remove(indev); + } lvgl_unlock(); } From f77bea59bcb01a941f9516acf03f5c739066efb1 Mon Sep 17 00:00:00 2001 From: Ken Van Hoeylandt Date: Fri, 21 Aug 2026 21:28:11 +0200 Subject: [PATCH 02/14] Fix for LVGL file lock when LVGL is not running --- Modules/lvgl-module/source/arch/lvgl_esp32.c | 3 ++ Tactility/Source/file/FileMutexLvgl.cpp | 31 ++++++++++++++++---- 2 files changed, 29 insertions(+), 5 deletions(-) diff --git a/Modules/lvgl-module/source/arch/lvgl_esp32.c b/Modules/lvgl-module/source/arch/lvgl_esp32.c index 63d1134ce..a7f879844 100644 --- a/Modules/lvgl-module/source/arch/lvgl_esp32.c +++ b/Modules/lvgl-module/source/arch/lvgl_esp32.c @@ -20,16 +20,19 @@ static bool initialized = false; void lvgl_lock(void) { if (!initialized) { return; } + if (!lvgl_is_running()) { return; } lvgl_port_lock(portMAX_DELAY); } bool lvgl_try_lock(uint32_t timeoutTicks) { if (!initialized) { return false; } + if (!lvgl_is_running()) { return false; } // lvgl_port_lock expects milliseconds return lvgl_port_lock(timeoutTicks * portTICK_PERIOD_MS); } void lvgl_unlock(void) { + if (!lvgl_is_running()) { return; } if (!initialized) { return; } lvgl_port_unlock(); } diff --git a/Tactility/Source/file/FileMutexLvgl.cpp b/Tactility/Source/file/FileMutexLvgl.cpp index f8df9430b..8a4b8094f 100644 --- a/Tactility/Source/file/FileMutexLvgl.cpp +++ b/Tactility/Source/file/FileMutexLvgl.cpp @@ -9,14 +9,35 @@ constexpr auto* TAG = "file_mutex_lvgl"; struct Device; -namespace tt { -static const FileMutex lvgl_mutex = { - .lock = lvgl_lock, - .try_lock = lvgl_try_lock, - .unlock = lvgl_unlock, +namespace { + +void wrapped_lvgl_lock() { + if (!lvgl_is_running()) return; + lvgl_lock(); +} + +bool wrapped_lvgl_try_lock(uint32_t timeout) { + // Return lock success, so the file operation can continue when LVGL is not running + // lvgl_try_lock() fails to lock if LVGL is not running + if (!lvgl_is_running()) return true; + return lvgl_try_lock(timeout); +} +void wrapped_lvgl_unlock() { + if (!lvgl_is_running()) return; + lvgl_unlock(); +} + +const FileMutex lvgl_mutex = { + .lock = wrapped_lvgl_lock, + .try_lock = wrapped_lvgl_try_lock, + .unlock = wrapped_lvgl_unlock, }; +} + +namespace tt { + /** * Finds file systems with a device (e.g. sd card) that is owned by a SPI controller. * If the SPI controller has a display on the bus, we create an LVGL lock for the file system path. From 4b6d2b3d8f97e86b73cd1aff37e9eca68461d008 Mon Sep 17 00:00:00 2001 From: Ken Van Hoeylandt Date: Fri, 21 Aug 2026 23:28:46 +0200 Subject: [PATCH 03/14] Fix for crash in statusbar --- Tactility/Source/lvgl/Statusbar.cpp | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/Tactility/Source/lvgl/Statusbar.cpp b/Tactility/Source/lvgl/Statusbar.cpp index 9f192ff0f..ce2d4aaac 100644 --- a/Tactility/Source/lvgl/Statusbar.cpp +++ b/Tactility/Source/lvgl/Statusbar.cpp @@ -126,11 +126,12 @@ static void statusbar_constructor(const lv_obj_class_t* class_p, lv_obj_t* obj) LV_TRACE_OBJ_CREATE("begin"); lv_obj_remove_flag(obj, LV_OBJ_FLAG_SCROLLABLE); LV_TRACE_OBJ_CREATE("finished"); - auto* statusbar = (Statusbar*)obj; - statusbar->pubsub_subscription = statusbar_data.pubsub->subscribe([statusbar](auto) { - statusbar_pubsub_event(statusbar); - }); + // Deliberately does NOT subscribe to statusbar_data.pubsub here - that happens at the end of + // statusbar_create(), once statusbar->icons[] is actually populated. Subscribing this early + // would let a concurrent statusbar_icon_add()/_remove()/_set_image()/_set_visibility() call + // from another task publish and run update_icon() on a still-null icons[] slot before this + // instance's own creation loop below has had a chance to fill it in. if (!statusbar_data.time_update_timer->isRunning()) { statusbar_data.time_update_timer->start(); system_event_callback_add(KERNEL_EVENT_TIME_CHANGED, onTimeChanged, nullptr); @@ -191,6 +192,13 @@ lv_obj_t* statusbar_create(lv_obj_t* parent) { update_icon(image, &(statusbar_data.icons[i])); } statusbar_data.mutex.unlock(); + + // Only now is statusbar->icons[] fully populated - see statusbar_constructor()'s comment for + // why the subscription can't be registered any earlier. + statusbar->pubsub_subscription = statusbar_data.pubsub->subscribe([statusbar](auto) { + statusbar_pubsub_event(statusbar); + }); + return obj; } From 5707c2788290459b423729c02dc365479117737c Mon Sep 17 00:00:00 2001 From: Ken Van Hoeylandt Date: Sat, 22 Aug 2026 22:50:58 +0200 Subject: [PATCH 04/14] Improvements and fixes - custom lvgl alloc - keyboard drivers now uses codepoints only (no more hardwired LV_KEY_*) - fix lvgl file lock when lvgl is stopped --- Buildscripts/module.cmake | 17 +++- Buildscripts/sdkconfig/default.properties | 4 +- Devices/cl32/cl32.dts | 12 +-- Devices/lilygo-tdeck-max/lilygo,tdeck-max.dts | 6 +- Devices/lilygo-tdeck-pro/lilygo,tdeck-pro.dts | 6 +- .../lilygo-tlora-pager/lilygo,tlora-pager.dts | 4 +- .../Source/devices/tab5_keyboard.cpp | 50 +++++------ .../simulator/Source/drivers/sdl_input.cpp | 37 +++++---- .../source/button_control.cpp | 10 +-- .../source/cardputer_adv_keyboard.cpp | 22 +++-- .../source/cardputer_keyboard.cpp | 42 +++++----- .../tca8418-module/bindings/ti,tca8418.yaml | 5 +- Modules/lvgl-module/CMakeLists.txt | 4 + Modules/lvgl-module/README.md | 26 ++++++ Modules/lvgl-module/source/arch/lvgl_esp32.c | 3 - .../lvgl-module/source/devices/keyboard.cpp | 23 +++++- Modules/lvgl-module/source/lv_mem_custom.c | 57 +++++++++++++ .../source/drivers/usb/esp32_usbhost_hid.cpp | 44 +++++----- Tactility/Source/Tactility.cpp | 8 +- Tactility/Source/app/boot/Boot.cpp | 1 - Tactility/Source/file/FileMutexLvgl.cpp | 26 ++++-- .../include/tactility/drivers/keyboard.h | 46 +++++++++-- .../include/tactility/filesystem/file_mutex.h | 16 +++- .../source/filesystem/file_mutex.cpp | 66 +++++++++++++-- TactilityKernel/source/memory_esp32.cpp | 14 ++-- TactilityKernel/source/symbols.c | 3 +- .../tests/source/file_mutex_test.cpp | 82 +++++++++++++++++-- lv_conf.h | 4 +- 28 files changed, 472 insertions(+), 166 deletions(-) create mode 100644 Modules/lvgl-module/source/lv_mem_custom.c diff --git a/Buildscripts/module.cmake b/Buildscripts/module.cmake index 9c148b3a7..a950ccc2a 100644 --- a/Buildscripts/module.cmake +++ b/Buildscripts/module.cmake @@ -11,18 +11,33 @@ macro(tactility_get_module_name NAME OUT_NAME) endmacro() macro(tactility_add_module NAME) - set(options) + # WHOLE_ARCHIVE: force every object file in this module into the final link unconditionally, + # instead of only the ones some other already-scanned archive currently has a pending + # undefined reference to. Needed when this module provides symbols a component it depends on + # (e.g. lvgl__lvgl's custom-allocator hooks) calls back into - a reverse reference a normal + # single-pass static-archive link can't resolve, since that component is scanned after this + # one's archive has already been passed once. POSIX builds link everything as plain OBJECT + # libraries (no archive-pruning to begin with), so this is a no-op there. + set(options WHOLE_ARCHIVE) set(oneValueArgs) set(multiValueArgs SRCS INCLUDE_DIRS PRIV_INCLUDE_DIRS REQUIRES PRIV_REQUIRES) cmake_parse_arguments(ARG "${options}" "${oneValueArgs}" "${multiValueArgs}" ${ARGN}) if (DEFINED ENV{ESP_IDF_VERSION}) + # idf_component_register's WHOLE_ARCHIVE is a presence-based flag (no value) - only pass + # the token at all when requested, rather than passing ARG_WHOLE_ARCHIVE's TRUE/FALSE as + # a value, which idf_component_register doesn't expect. + set(whole_archive_arg) + if (ARG_WHOLE_ARCHIVE) + set(whole_archive_arg WHOLE_ARCHIVE) + endif() idf_component_register( SRCS ${ARG_SRCS} INCLUDE_DIRS ${ARG_INCLUDE_DIRS} PRIV_INCLUDE_DIRS ${ARG_PRIV_INCLUDE_DIRS} REQUIRES ${ARG_REQUIRES} PRIV_REQUIRES ${ARG_PRIV_REQUIRES} + ${whole_archive_arg} ) else() add_library(${NAME} OBJECT) diff --git a/Buildscripts/sdkconfig/default.properties b/Buildscripts/sdkconfig/default.properties index 268021b17..c20cec826 100644 --- a/Buildscripts/sdkconfig/default.properties +++ b/Buildscripts/sdkconfig/default.properties @@ -19,7 +19,9 @@ CONFIG_LV_FS_STDIO_PATH="" CONFIG_LV_FS_STDIO_CACHE_SIZE=4096 CONFIG_LV_USE_LODEPNG=y CONFIG_LV_USE_BUILTIN_MALLOC=n -CONFIG_LV_USE_CLIB_MALLOC=y +# Routes lv_malloc/realloc/free through Modules/lvgl-module/source/lv_mem_custom.c, which prefers +# PSRAM (falls back to internal RAM automatically) instead of a fixed-size pool or plain malloc. +CONFIG_LV_USE_CUSTOM_MALLOC=y CONFIG_LV_USE_MSGBOX=n CONFIG_LV_USE_SPINNER=n CONFIG_LV_USE_WIN=n diff --git a/Devices/cl32/cl32.dts b/Devices/cl32/cl32.dts index 9027c8e99..042b2256a 100644 --- a/Devices/cl32/cl32.dts +++ b/Devices/cl32/cl32.dts @@ -44,31 +44,31 @@ 37 49 50 51 52 53 54 55 56 0 // % 1 2 3 4 5 6 7 8 57 48 8 91 93 43 34 39 27 0 // 9 0 BKSP [ ] + " ' EXIT 9 113 119 101 114 116 121 117 105 0 // TAB q w e r t y u i - 111 112 10 40 41 45 59 58 3 0 // o p ENTER ( ) - ; : STOP + 111 112 13 40 41 45 59 58 3 0 // o p ENTER ( ) - ; : STOP 0 97 115 100 102 103 104 106 107 0 // a s d f g h j k 108 17 35 123 125 42 44 46 2 0 // l UP # { } * , . MENU 122 120 99 118 98 32 32 110 109 0 // z x c v b n m - 20 18 19 60 62 47 92 61 10 0 // LEFT DOWN RIGHT < > / \ = RUN + 20 18 19 60 62 47 92 61 13 0 // LEFT DOWN RIGHT < > / \ = RUN ]; keymap-uc = [ 37 49 50 51 52 53 54 55 56 0 // % 1 2 3 4 5 6 7 8 57 48 8 91 93 43 34 39 27 0 // 9 0 BKSP [ ] + " ' EXIT 9 81 87 69 82 84 89 85 73 0 // TAB Q W E R T Y U I - 79 80 10 40 41 45 59 58 3 0 // O P ENTER ( ) - ; : STOP + 79 80 13 40 41 45 59 58 3 0 // O P ENTER ( ) - ; : STOP 0 65 83 68 70 71 72 74 75 0 // A S D F G H J K 76 17 35 123 125 42 44 46 2 0 // L UP # { } * , . MENU 90 88 67 86 66 32 32 78 77 0 // Z X C V B N M - 20 18 19 60 62 47 92 61 10 0 // LEFT DOWN RIGHT < > / \ = RUN + 20 18 19 60 62 47 92 61 13 0 // LEFT DOWN RIGHT < > / \ = RUN ]; keymap-sy = [ 37 49 50 51 52 53 54 55 56 0 // % 1 2 3 4 5 6 7 8 57 48 8 91 93 43 34 39 27 0 // 9 0 BKSP [ ] + " ' EXIT 9 113 119 101 114 116 121 117 105 0 // TAB q w e r t y u i - 111 112 10 40 41 45 59 58 3 0 // o p ENTER ( ) - ; : STOP + 111 112 13 40 41 45 59 58 3 0 // o p ENTER ( ) - ; : STOP 0 97 115 100 102 103 104 106 107 0 // a s d f g h j k 108 17 35 123 125 42 44 46 2 0 // l UP # { } * , . MENU 122 120 99 118 98 32 32 110 109 0 // z x c v b n m - 20 18 19 60 62 47 92 61 10 0 // LEFT DOWN RIGHT < > / \ = RUN + 20 18 19 60 62 47 92 61 13 0 // LEFT DOWN RIGHT < > / \ = RUN ]; shift-row = <4>; shift-col = <0>; diff --git a/Devices/lilygo-tdeck-max/lilygo,tdeck-max.dts b/Devices/lilygo-tdeck-max/lilygo,tdeck-max.dts index 0be730b59..d8ca0f0ab 100644 --- a/Devices/lilygo-tdeck-max/lilygo,tdeck-max.dts +++ b/Devices/lilygo-tdeck-max/lilygo,tdeck-max.dts @@ -74,19 +74,19 @@ keymap-lc = [ 113 119 101 114 116 121 117 105 111 112 // q w e r t y u i o p 97 115 100 102 103 104 106 107 108 8 // a s d f g h j k l BACKSPACE - 0 122 120 99 118 98 110 109 36 10 // z x c v b n m $ ENTER + 0 122 120 99 118 98 110 109 36 13 // z x c v b n m $ ENTER 0 0 0 0 0 11 48 32 0 9 // PREV 0 SPC NEXT ]; keymap-uc = [ 81 87 69 82 84 89 85 73 79 80 // Q W E R T Y U I O P 65 83 68 70 71 72 74 75 76 8 // A S D F G H J K L BACKSPACE - 0 90 88 67 86 66 78 77 36 10 // Z X C V B N M $ ENTER + 0 90 88 67 86 66 78 77 36 13 // Z X C V B N M $ ENTER 0 0 0 0 0 11 48 32 0 9 // PREV 0 SPC NEXT ]; keymap-sy = [ 49 50 51 52 53 54 55 56 57 48 // 1 2 3 4 5 6 7 8 9 0 64 35 43 45 42 47 40 41 95 8 // @ # + - * / ( ) _ BACKSPACE - 0 33 63 59 58 39 34 44 46 10 // ! ? ; : ' " , . ENTER + 0 33 63 59 58 39 34 44 46 13 // ! ? ; : ' " , . ENTER 0 0 0 0 0 11 48 32 0 9 // PREV 0 SPC NEXT ]; shift-row = <2>; diff --git a/Devices/lilygo-tdeck-pro/lilygo,tdeck-pro.dts b/Devices/lilygo-tdeck-pro/lilygo,tdeck-pro.dts index dfa466002..a8db46f48 100644 --- a/Devices/lilygo-tdeck-pro/lilygo,tdeck-pro.dts +++ b/Devices/lilygo-tdeck-pro/lilygo,tdeck-pro.dts @@ -61,19 +61,19 @@ keymap-lc = [ 113 119 101 114 116 121 117 105 111 112 // q w e r t y u i o p 97 115 100 102 103 104 106 107 108 8 // a s d f g h j k l BACKSPACE - 0 122 120 99 118 98 110 109 36 10 // z x c v b n m $ ENTER + 0 122 120 99 118 98 110 109 36 13 // z x c v b n m $ ENTER 0 0 0 0 0 11 48 32 0 9 // PREV 0 SPC NEXT ]; keymap-uc = [ 81 87 69 82 84 89 85 73 79 80 // Q W E R T Y U I O P 65 83 68 70 71 72 74 75 76 8 // A S D F G H J K L BACKSPACE - 0 90 88 67 86 66 78 77 36 10 // Z X C V B N M $ ENTER + 0 90 88 67 86 66 78 77 36 13 // Z X C V B N M $ ENTER 0 0 0 0 0 11 48 32 0 9 // PREV 0 SPC NEXT ]; keymap-sy = [ 49 50 51 52 53 54 55 56 57 48 // 1 2 3 4 5 6 7 8 9 0 64 35 43 45 42 47 40 41 95 8 // @ # + - * / ( ) _ BACKSPACE - 0 33 63 59 58 39 34 44 46 10 // ! ? ; : ' " , . ENTER + 0 33 63 59 58 39 34 44 46 13 // ! ? ; : ' " , . ENTER 0 0 0 0 0 11 48 32 0 9 // PREV 0 SPC NEXT ]; shift-row = <2>; diff --git a/Devices/lilygo-tlora-pager/lilygo,tlora-pager.dts b/Devices/lilygo-tlora-pager/lilygo,tlora-pager.dts index 48c571c7c..d1bfc61be 100644 --- a/Devices/lilygo-tlora-pager/lilygo,tlora-pager.dts +++ b/Devices/lilygo-tlora-pager/lilygo,tlora-pager.dts @@ -60,13 +60,13 @@ columns = <10>; keymap-lc = [ 113 119 101 114 116 121 117 105 111 112 // q w e r t y u i o p - 97 115 100 102 103 104 106 107 108 10 // a s d f g h j k l ENTER + 97 115 100 102 103 104 106 107 108 13 // a s d f g h j k l ENTER 0 122 120 99 118 98 110 109 0 8 // z x c v b n m BACKSPACE 32 0 0 0 0 0 0 0 0 0 // SPC ]; keymap-uc = [ 81 87 69 82 84 89 85 73 79 80 // Q W E R T Y U I O P - 65 83 68 70 71 72 74 75 76 10 // A S D F G H J K L ENTER + 65 83 68 70 71 72 74 75 76 13 // A S D F G H J K L ENTER 0 90 88 67 86 66 78 77 0 8 // Z X C V B N M BACKSPACE 32 0 0 0 0 0 0 0 0 0 // SPC ]; diff --git a/Devices/m5stack-tab5/Source/devices/tab5_keyboard.cpp b/Devices/m5stack-tab5/Source/devices/tab5_keyboard.cpp index 429b317cd..9cfb27710 100644 --- a/Devices/m5stack-tab5/Source/devices/tab5_keyboard.cpp +++ b/Devices/m5stack-tab5/Source/devices/tab5_keyboard.cpp @@ -17,8 +17,6 @@ #include #include -#include - #include #include @@ -119,36 +117,38 @@ static constexpr HidMapping KEY_MATRIX_HID_SYM[70] = { }; // --------------------------------------------------------------------------- -// HID usage code + modifier → LVGL key -// Covers all codes present in the Tab5 matrix tables above. LV_KEY_* are plain uint32_t -// constants - matching KeyboardKeyData::key's driver-defined contract and the same convention -// m5stack-module's cardputer_keyboard.cpp kernel driver already uses. +// HID usage code + modifier → key (Unicode codepoint) +// Covers all codes present in the Tab5 matrix tables above. Every key returns a real Unicode +// codepoint per KeyboardKeyData::key's contract - never an LV_KEY_* constant. Keys with no +// ordinary character of their own (arrows, Tab/Shift+Tab and Ctrl+arrow as focus nav) use the +// CodePoint enum's standard Unicode symbols for the concept (CODEPOINT_FOCUS_PREV/NEXT reused for +// Ctrl+arrow's focus-move aliases). lvgl-module's keyboard.cpp translates all of these back to +// LVGL's own sentinels (CODEPOINT_ESCAPE/BACKSPACE/DELETE already equal their LV_KEY_* counterpart +// numerically, so no translation is needed for those). // -// `ctrl` only selects the LVGL focus-navigation aliases for the arrow keys. Ctrl chords on -// ordinary keys are NOT folded into the returned value - the C0 control codes a terminal wants -// (Ctrl+C = 0x03, Ctrl+K = 0x0B, ...) collide with the LVGL constants returned here (LV_KEY_END = 3, -// LV_KEY_PREV = 11, ...), so Ctrl is reported out-of-band via KeyboardKeyData::ctrl instead and -// consumers that want control codes derive them themselves. +// Ctrl chords on ordinary keys are NOT folded into the returned value - the C0 control codes a +// terminal wants (Ctrl+C = 0x03, Ctrl+K = 0x0B, ...) would collide with a codepoint-based scheme, +// so Ctrl is reported out-of-band via KeyboardKeyData::ctrl instead and consumers that want +// control codes derive them themselves. // --------------------------------------------------------------------------- static uint32_t tab5_translate_key(uint8_t keycode, uint8_t modifier, bool ctrl) { const bool shift = (modifier & 0x22U) != 0U; - // Navigation → LVGL key constants + // Navigation → key (Unicode codepoint) switch (keycode) { - case 0x29: return LV_KEY_ESC; - case 0x28: return LV_KEY_ENTER; - case 0x2A: return LV_KEY_BACKSPACE; - case 0x4C: return LV_KEY_DEL; - case 0x2B: return '\t'; - // Arrows: Ctrl+arrow = focus navigation, plain arrow = raw cursor movement - case 0x52: return ctrl ? (uint32_t)LV_KEY_PREV : (uint32_t)LV_KEY_UP; - case 0x51: return ctrl ? (uint32_t)LV_KEY_NEXT : (uint32_t)LV_KEY_DOWN; - case 0x50: return ctrl ? (uint32_t)LV_KEY_PREV : (uint32_t)LV_KEY_LEFT; - case 0x4F: return ctrl ? (uint32_t)LV_KEY_NEXT : (uint32_t)LV_KEY_RIGHT; + case 0x29: return CODEPOINT_ESCAPE; + case 0x28: return CODEPOINT_ENTER; + case 0x2A: return CODEPOINT_BACKSPACE; + case 0x4C: return CODEPOINT_DELETE; + case 0x2B: return CODEPOINT_TAB; + case 0x52: return CODEPOINT_ARROW_UP; + case 0x51: return CODEPOINT_ARROW_DOWN; + case 0x50: return CODEPOINT_ARROW_LEFT; + case 0x4F: return CODEPOINT_ARROW_RIGHT; default: break; } - // F1-F12 (Sym layer over the number row) have no LVGL/ASCII representation - callers that + // F1-F12 (Sym layer over the number row) have no Unicode/LVGL representation - callers that // want them read KeyboardKeyData::hid_keycode instead (see drain_events()), which is // populated for every key regardless of whether `key` itself has a meaningful value. @@ -378,9 +378,9 @@ static void drain_events(Device* device, Tab5KeyboardInternal* internal) { if (internal->ctrl_held) modifier |= 0x01U; // HID LeftCtrl if (internal->alt_held) modifier |= 0x04U; // HID LeftAlt const uint32_t lv_key = tab5_translate_key(m.keycode, modifier, internal->ctrl_held); - // Queue whenever there's a HID keycode, even if this key has no LVGL/ASCII + // Queue whenever there's a HID keycode, even if this key has no Unicode/LVGL // representation (e.g. F1-F12) - lv_key stays 0 for those, callers that only - // care about LVGL/ASCII text ignore a 0 key the same way they always have. + // care about the codepoint ignore a 0 key the same way they always have. if (pressed) { // A real key was pressed — this hold is a chord, not a tap internal->aa_tapped = false; diff --git a/Devices/simulator/Source/drivers/sdl_input.cpp b/Devices/simulator/Source/drivers/sdl_input.cpp index 86aea8c81..002a2d069 100644 --- a/Devices/simulator/Source/drivers/sdl_input.cpp +++ b/Devices/simulator/Source/drivers/sdl_input.cpp @@ -1,7 +1,7 @@ // SPDX-License-Identifier: Apache-2.0 #include "sdl_input.h" -#include +#include #include @@ -27,22 +27,27 @@ void push_key(uint32_t key) { key_queue_count++; } -// Mirrors LVGL's own lv_sdl_keyboard.c keycode_to_ctrl_key(): maps navigation/control keys to -// LV_KEY_* constants. Printable characters arrive separately via SDL_TEXTINPUT. -uint32_t keycode_to_key(SDL_Keycode sdl_key) { +// Mirrors LVGL's own lv_sdl_keyboard.c keycode_to_ctrl_key(): every key returns a real Unicode +// codepoint per KeyboardKeyData::key's contract - never an LV_KEY_* constant, even for keys with +// no ordinary character of their own (arrows, Tab/Shift+Tab as focus nav, Home/End), which use the +// CodePoint enum's standard Unicode symbols for the concept. lvgl-module's keyboard.cpp translates +// all of these back to LVGL's own sentinels (CODEPOINT_ESCAPE/BACKSPACE/DELETE already equal their +// LV_KEY_* counterpart numerically, so no translation is needed for those). Printable characters +// arrive separately via SDL_TEXTINPUT. +uint32_t keycode_to_key(SDL_Keycode sdl_key, bool shift) { switch (sdl_key) { - case SDLK_RIGHT: return LV_KEY_RIGHT; - case SDLK_LEFT: return LV_KEY_LEFT; - case SDLK_UP: return LV_KEY_UP; - case SDLK_DOWN: return LV_KEY_DOWN; - case SDLK_ESCAPE: return LV_KEY_ESC; - case SDLK_BACKSPACE: return LV_KEY_BACKSPACE; - case SDLK_DELETE: return LV_KEY_DEL; + case SDLK_RIGHT: return CODEPOINT_ARROW_RIGHT; + case SDLK_LEFT: return CODEPOINT_ARROW_LEFT; + case SDLK_UP: return CODEPOINT_ARROW_UP; + case SDLK_DOWN: return CODEPOINT_ARROW_DOWN; + case SDLK_ESCAPE: return CODEPOINT_ESCAPE; + case SDLK_BACKSPACE: return CODEPOINT_BACKSPACE; + case SDLK_DELETE: return CODEPOINT_DELETE; case SDLK_RETURN: - case SDLK_KP_ENTER: return LV_KEY_ENTER; - case SDLK_TAB: return LV_KEY_NEXT; - case SDLK_HOME: return LV_KEY_HOME; - case SDLK_END: return LV_KEY_END; + case SDLK_KP_ENTER: return CODEPOINT_ENTER; + case SDLK_TAB: return CODEPOINT_TAB; + case SDLK_HOME: return CODEPOINT_HOME; + case SDLK_END: return CODEPOINT_END; default: return 0; } } @@ -75,7 +80,7 @@ void sdl_input_pump() { } break; case SDL_KEYDOWN: - push_key(keycode_to_key(event.key.keysym.sym)); + push_key(keycode_to_key(event.key.keysym.sym, (event.key.keysym.mod & KMOD_SHIFT) != 0)); break; case SDL_TEXTINPUT: // ASCII only (first byte of event.text.text) - sufficient for a simulator keyboard. diff --git a/Drivers/button-control-module/source/button_control.cpp b/Drivers/button-control-module/source/button_control.cpp index 9facf5e60..4a32ffdb6 100644 --- a/Drivers/button-control-module/source/button_control.cpp +++ b/Drivers/button-control-module/source/button_control.cpp @@ -12,8 +12,6 @@ #include #include -#include - #include #define TAG "ButtonControl" @@ -105,8 +103,8 @@ static error_t start(Device* device) { error_t error = acquire_button( config->pin_primary, - two_button_mode ? LV_KEY_ENTER : LV_KEY_NEXT, - two_button_mode ? LV_KEY_ESC : LV_KEY_ENTER, + two_button_mode ? (uint32_t)CODEPOINT_ENTER : (uint32_t)CODEPOINT_ARROW_DOWN, + two_button_mode ? (uint32_t)CODEPOINT_ESCAPE : (uint32_t)CODEPOINT_ENTER, &internal->primary ); if (error != ERROR_NONE) { @@ -114,7 +112,7 @@ static error_t start(Device* device) { return error; } - error = acquire_button(config->pin_secondary, LV_KEY_NEXT, LV_KEY_PREV, &internal->secondary); + error = acquire_button(config->pin_secondary, CODEPOINT_ARROW_DOWN, CODEPOINT_ARROW_UP, &internal->secondary); if (error != ERROR_NONE) { if (internal->primary.in_use) { gpio_descriptor_release(internal->primary.descriptor); @@ -176,7 +174,7 @@ static void poll_button(const ButtonControlConfig* config, ButtonControlInternal } // Release: decide short vs. long press by elapsed hold duration, then queue a synthetic - // key tap (press followed by release) for the LVGL key this gesture maps to. + // key tap (press followed by release) for the key this gesture maps to. uint32_t held_ms = now - state->press_start_time; uint32_t key = held_ms < config->long_press_ms ? state->short_press_key : state->long_press_key; push_pending(internal, key, true); diff --git a/Drivers/m5stack-module/source/cardputer_adv_keyboard.cpp b/Drivers/m5stack-module/source/cardputer_adv_keyboard.cpp index 72db94b07..a7210bd2b 100644 --- a/Drivers/m5stack-module/source/cardputer_adv_keyboard.cpp +++ b/Drivers/m5stack-module/source/cardputer_adv_keyboard.cpp @@ -14,8 +14,6 @@ #include #include -#include - #include static constexpr const char* TAG = "CardputerAdvKeyboard"; @@ -46,24 +44,24 @@ static constexpr int CARDPUTER_ADV_COLS = 14; // [row][col] on the 4x14 grid, matching the base Cardputer's physical layout. 0 means the cell // emits nothing (used for the sym/shift cells themselves, and unwired cells on this board). static const uint32_t cardputer_adv_keymap_lc[CARDPUTER_ADV_ROWS][CARDPUTER_ADV_COLS] = { - { '`', '1', '2', '3', '4', '5', '6', '7', '8', '9', '0', '-', '=', LV_KEY_BACKSPACE }, - { '\t', 'q', 'w', 'e', 'r', 't', 'y', 'u', 'i', 'o', 'p', '[', ']', '\\' }, - { 0, 0, 'a', 's', 'd', 'f', 'g', 'h', 'j', 'k', 'l', ';', '\'', LV_KEY_ENTER }, + { '`', '1', '2', '3', '4', '5', '6', '7', '8', '9', '0', '-', '=', CODEPOINT_BACKSPACE }, + { CODEPOINT_TAB, 'q', 'w', 'e', 'r', 't', 'y', 'u', 'i', 'o', 'p', '[', ']', '\\' }, + { 0, 0, 'a', 's', 'd', 'f', 'g', 'h', 'j', 'k', 'l', ';', '\'', CODEPOINT_ENTER }, { 0, 0, 0, 'z', 'x', 'c', 'v', 'b', 'n', 'm', ',', '.', '/', ' ' }, }; static const uint32_t cardputer_adv_keymap_uc[CARDPUTER_ADV_ROWS][CARDPUTER_ADV_COLS] = { - { '~', '!', '@', '#', '$', '%', '^', '&', '*', '(', ')', '_', '+', LV_KEY_DEL }, - { '\t', 'Q', 'W', 'E', 'R', 'T', 'Y', 'U', 'I', 'O', 'P', '{', '}', '|' }, - { 0, 0, 'A', 'S', 'D', 'F', 'G', 'H', 'J', 'K', 'L', ':', '"', LV_KEY_ENTER }, + { '~', '!', '@', '#', '$', '%', '^', '&', '*', '(', ')', '_', '+', CODEPOINT_DELETE }, + { CODEPOINT_FOCUS_PREV, 'Q', 'W', 'E', 'R', 'T', 'Y', 'U', 'I', 'O', 'P', '{', '}', '|' }, + { 0, 0, 'A', 'S', 'D', 'F', 'G', 'H', 'J', 'K', 'L', ':', '"', CODEPOINT_ENTER }, { 0, 0, 0, 'Z', 'X', 'C', 'V', 'B', 'N', 'M', '<', '>', '?', ' ' }, }; static const uint32_t cardputer_adv_keymap_sym[CARDPUTER_ADV_ROWS][CARDPUTER_ADV_COLS] = { - { LV_KEY_ESC, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0 }, - { '\t', 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0 }, - { 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, LV_KEY_PREV, 0, LV_KEY_ENTER }, - { 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, LV_KEY_LEFT, LV_KEY_NEXT, LV_KEY_RIGHT, 0 }, + { CODEPOINT_ESCAPE, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0 }, + { CODEPOINT_TAB, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0 }, + { 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, CODEPOINT_ARROW_UP, 0, CODEPOINT_ENTER }, + { 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, CODEPOINT_ARROW_LEFT, CODEPOINT_ARROW_DOWN, CODEPOINT_ARROW_RIGHT, 0 }, }; struct CardputerAdvActiveKey { diff --git a/Drivers/m5stack-module/source/cardputer_keyboard.cpp b/Drivers/m5stack-module/source/cardputer_keyboard.cpp index 43722fae4..91d7bc0b5 100644 --- a/Drivers/m5stack-module/source/cardputer_keyboard.cpp +++ b/Drivers/m5stack-module/source/cardputer_keyboard.cpp @@ -14,8 +14,6 @@ #include #include -#include - #include static constexpr const char* TAG = "CardputerKeyboard"; @@ -74,8 +72,8 @@ struct CardputerKeyboardPendingEvent { struct CardputerKeyboardInternal { GpioDescriptor* output_descriptors[CARDPUTER_OUTPUT_COUNT]; GpioDescriptor* input_descriptors[CARDPUTER_INPUT_COUNT]; - // 0 when no actionable key is currently held; otherwise the LVGL key code last reported - // via read_key(). Only ever one actionable key at a time (matches original hardware driver: + // 0 when no actionable key is currently held; otherwise the key (Unicode codepoint) last + // reported via read_key(). Only ever one actionable key at a time (matches original hardware driver: // modifier keys are consumed internally, and only the first non-modifier key found in a // scan is reported). uint32_t active_key; @@ -195,10 +193,12 @@ static uint8_t read_input(CardputerKeyboardInternal* internal) { return mask; } -// Scans the full matrix and resolves it to a single LVGL key code (0 if none), applying the -// same priority as the original driver: enter > space > backspace > first regular character -// found in scan order, with fn changing the interpretation of backspace/enter/punctuation. -// Modifier keys (fn/shift/ctrl/opt/alt/tab) are never reported themselves. +// Scans the full matrix and resolves it to a single key (Unicode codepoint, 0 if none - never an +// LV_KEY_* constant, see KeyboardKeyData::key's contract), applying priority: enter > space > +// backspace > tab (U+21E5 tab-to-bar, or U+21E4 with shift - the closest standard Unicode symbol +// for focus navigation) > first regular character found in scan order, with fn changing the +// interpretation of backspace/enter/punctuation. Modifier keys (fn/shift/ctrl/opt/alt) are never +// reported themselves. static uint32_t scan_key(CardputerKeyboardInternal* internal) { bool fn = false, shift = false, ctrl = false; bool del_flag = false, enter_flag = false, space_flag = false; @@ -223,7 +223,6 @@ static uint32_t scan_key(CardputerKeyboardInternal* internal) { const auto& def = cardputer_key_map[row][col]; switch (def.role) { - case CARDPUTER_KEY_TAB: case CARDPUTER_KEY_OPT: case CARDPUTER_KEY_ALT: break; // consumed, never affects output @@ -259,24 +258,25 @@ static uint32_t scan_key(CardputerKeyboardInternal* internal) { char resolved_char = has_regular ? ((ctrl || shift) ? regular_shifted : regular_normal) : 0; if (!fn) { - if (enter_flag) return LV_KEY_ENTER; + if (enter_flag) return CODEPOINT_ENTER; if (space_flag) return (uint32_t)' '; - if (del_flag) return LV_KEY_BACKSPACE; - if (has_regular) return (uint32_t)(uint8_t)resolved_char; + if (del_flag) return CODEPOINT_BACKSPACE; + if (has_regular) return (uint32_t)resolved_char; return 0; } - // fn combos: forward-delete, enter, and group navigation (using PREV/NEXT rather than - // UP/DOWN so widgets like lv_switch that toggle on arrow keys aren't affected). - if (del_flag) return LV_KEY_DEL; - if (enter_flag) return LV_KEY_ENTER; + // fn combos: forward-delete, enter, and group navigation (using the tab-to-bar codepoints + // rather than arrow codepoints so widgets like lv_switch that toggle on arrow keys aren't + // affected). + if (del_flag) return CODEPOINT_DELETE; + if (enter_flag) return CODEPOINT_ENTER; if (has_regular) { switch (resolved_char) { - case '`': return LV_KEY_ESC; - case ',': return LV_KEY_LEFT; - case '/': return LV_KEY_RIGHT; - case ';': return LV_KEY_PREV; - case '.': return LV_KEY_NEXT; + case '`': return CODEPOINT_ESCAPE; + case ',': return CODEPOINT_ARROW_LEFT; + case '/': return CODEPOINT_ARROW_RIGHT; + case ';': return CODEPOINT_ARROW_UP; + case '.': return CODEPOINT_ARROW_DOWN; default: return 0; } } diff --git a/Drivers/tca8418-module/bindings/ti,tca8418.yaml b/Drivers/tca8418-module/bindings/ti,tca8418.yaml index f6b2018f6..5a7856043 100644 --- a/Drivers/tca8418-module/bindings/ti,tca8418.yaml +++ b/Drivers/tca8418-module/bindings/ti,tca8418.yaml @@ -34,8 +34,9 @@ properties: Base (lowercase) layer keymap, rows*columns bytes in row-major order (already in silkscreen/keymap column order - see reverse-columns). 0 = no key at this position (e.g. a blank matrix position, or a position handled as a modifier via shift-row/shift-col/ - sym-row/sym-col instead). Non-zero bytes are sent as-is via KeyboardKeyData::key (ASCII - or an LVGL LV_KEY_* code). + sym-row/sym-col instead). Non-zero bytes are sent as-is via KeyboardKeyData::key (a Unicode + codepoint - byte range covers Latin-1 - or an LVGL LV_KEY_* code for keys with no character + representation). keymap-uc: type: array element-type: uint8_t diff --git a/Modules/lvgl-module/CMakeLists.txt b/Modules/lvgl-module/CMakeLists.txt index b170cad1a..7a5bc78f8 100644 --- a/Modules/lvgl-module/CMakeLists.txt +++ b/Modules/lvgl-module/CMakeLists.txt @@ -94,6 +94,10 @@ tactility_add_module(lvgl-module INCLUDE_DIRS include/ PRIV_INCLUDE_DIRS private/ REQUIRES ${REQUIRES_LIST} + # lv_mem_custom.c provides lv_mem_init/lv_malloc_core/etc, which lvgl__lvgl's own lv_init.c/ + # lv_mem.c call back into (LV_STDLIB_CUSTOM) - a reverse reference into this module that a + # normal single-pass static link can't resolve. See module.cmake's WHOLE_ARCHIVE comment. + WHOLE_ARCHIVE ) tactility_get_module_name("lvgl-module" MODULE_NAME) diff --git a/Modules/lvgl-module/README.md b/Modules/lvgl-module/README.md index 121062d4e..d36133022 100644 --- a/Modules/lvgl-module/README.md +++ b/Modules/lvgl-module/README.md @@ -37,6 +37,32 @@ Font sizes and symbols are configurable: If you change an icon font size, ensure that a corresponding C file exists in `source-fonts/` (e.g., `material_symbols_shared_24.c`). These files are generated from TTF/OTF fonts using the LVGL font converter. +## Custom memory allocator + +LVGL's malloc/realloc/free can be routed through a custom backend instead of its built-in pool or +plain `malloc`. Three things are required: + +**1. Select the backend.** On ESP32, via Kconfig (`sdkconfig`): +``` +CONFIG_LV_USE_CUSTOM_MALLOC=y +``` +On Simulator/POSIX, ESP-IDF's Kconfig doesn't apply - select it in `lv_conf.h` instead: +```c +#define LV_USE_STDLIB_MALLOC LV_STDLIB_CUSTOM +``` + +**2. Force the module into the link.** LVGL's own init code calls back into the functions above +a reverse reference a normal single-pass static-archive link can't resolve on its own: +```cmake +tactility_add_module(lvgl-module + ... + WHOLE_ARCHIVE +) +``` +Without `WHOLE_ARCHIVE`, or without step 1 selecting the custom backend, ESP-IDF's LVGL component +compiles its own allocator using these same symbol names - whichever one the linker happens to +pull in first silently wins, with no error and no guarantee it's the intended one. + ## License This module is licensed under the [Apache v2.0](LICENSE-Apache-2.0.md) license. \ No newline at end of file diff --git a/Modules/lvgl-module/source/arch/lvgl_esp32.c b/Modules/lvgl-module/source/arch/lvgl_esp32.c index a7f879844..63d1134ce 100644 --- a/Modules/lvgl-module/source/arch/lvgl_esp32.c +++ b/Modules/lvgl-module/source/arch/lvgl_esp32.c @@ -20,19 +20,16 @@ static bool initialized = false; void lvgl_lock(void) { if (!initialized) { return; } - if (!lvgl_is_running()) { return; } lvgl_port_lock(portMAX_DELAY); } bool lvgl_try_lock(uint32_t timeoutTicks) { if (!initialized) { return false; } - if (!lvgl_is_running()) { return false; } // lvgl_port_lock expects milliseconds return lvgl_port_lock(timeoutTicks * portTICK_PERIOD_MS); } void lvgl_unlock(void) { - if (!lvgl_is_running()) { return; } if (!initialized) { return; } lvgl_port_unlock(); } diff --git a/Modules/lvgl-module/source/devices/keyboard.cpp b/Modules/lvgl-module/source/devices/keyboard.cpp index a0b725295..b50e88d84 100644 --- a/Modules/lvgl-module/source/devices/keyboard.cpp +++ b/Modules/lvgl-module/source/devices/keyboard.cpp @@ -41,6 +41,26 @@ void lvgl_keyboard_on_stop_lvgl() { keyboard_group = nullptr; } +// KeyboardKeyData::key is always a Unicode codepoint (see its doc comment / the CodePoint enum) - +// drivers never emit LV_KEY_* directly, including for pure focus-navigation concepts that have no +// ordinary character of their own. LVGL itself hardcodes specific sentinel values in its own +// indev/group/textarea code that don't match the real Unicode codepoint chosen for the same key, +// so those are translated here rather than each driver having to know about LVGL's internals. +// CODEPOINT_BACKSPACE/TAB/ESCAPE/DELETE already equal their LV_KEY_* counterpart numerically, so +// they need no case below - they fall through `default` unchanged. +static uint32_t codepoint_to_lv_key(uint32_t key) { + switch (key) { + case CODEPOINT_ENTER: return LV_KEY_ENTER; + case CODEPOINT_ARROW_LEFT: return LV_KEY_LEFT; + case CODEPOINT_ARROW_UP: return LV_KEY_UP; + case CODEPOINT_ARROW_RIGHT: return LV_KEY_RIGHT; + case CODEPOINT_ARROW_DOWN: return LV_KEY_DOWN; + case CODEPOINT_HOME: return LV_KEY_HOME; + case CODEPOINT_END: return LV_KEY_END; + default: return key; + } +} + static void lvgl_keyboard_read_cb(lv_indev_t* indev, lv_indev_data_t* data) { auto* wrapper = static_cast(lv_indev_get_driver_data(indev)); @@ -51,8 +71,7 @@ static void lvgl_keyboard_read_cb(lv_indev_t* indev, lv_indev_data_t* data) { return; } - // KeyboardKeyData deliberately mirrors lv_indev_data_t's key/continue_reading fields, so no translation is needed. - data->key = key_data.key; + data->key = codepoint_to_lv_key(key_data.key); data->state = key_data.pressed ? LV_INDEV_STATE_PRESSED : LV_INDEV_STATE_RELEASED; data->continue_reading = key_data.continue_reading; } diff --git a/Modules/lvgl-module/source/lv_mem_custom.c b/Modules/lvgl-module/source/lv_mem_custom.c new file mode 100644 index 000000000..64bcab1c6 --- /dev/null +++ b/Modules/lvgl-module/source/lv_mem_custom.c @@ -0,0 +1,57 @@ +// SPDX-License-Identifier: Apache-2.0 +// LVGL's custom stdlib allocator backend (LV_STDLIB_CUSTOM / CONFIG_LV_USE_CUSTOM_MALLOC) - routes +// lv_malloc()/lv_realloc()/lv_free() through the kernel's memory_*_with_policy() functions instead +// of a fixed-size private pool (LV_STDLIB_BUILTIN) or plain malloc (LV_STDLIB_CLIB), so LVGL grows +// dynamically and prefers PSRAM when available instead of contending with everything else for a +// small, fixed internal-RAM arena. +#include +#include + +// PSRAM is preferred, not required: MEMORY_CAPABILITY_EXTERNAL is `desired`, so +// memory_alloc_with_policy()/memory_realloc_with_policy() fall back to internal RAM automatically +// on boards without PSRAM, or if PSRAM is exhausted. +static const struct MemoryPolicy LVGL_MEMORY_POLICY = { + .required = 0, + .desired = MEMORY_CAPABILITY_EXTERNAL, + .alignment = 0, +}; + +void lv_mem_init(void) { +} + +void lv_mem_deinit(void) { +} + +lv_mem_pool_t lv_mem_add_pool(void* mem, size_t bytes) { + // Not supported - memory_*_with_policy() owns allocation, LVGL doesn't need to manage its own + // pools on top of it. + (void)mem; + (void)bytes; + return NULL; +} + +void lv_mem_remove_pool(lv_mem_pool_t pool) { + (void)pool; +} + +void* lv_malloc_core(size_t size) { + return memory_alloc_with_policy(size, &LVGL_MEMORY_POLICY); +} + +void* lv_realloc_core(void* p, size_t new_size) { + return memory_realloc_with_policy(p, new_size, &LVGL_MEMORY_POLICY); +} + +void lv_free_core(void* p) { + memory_free(p); +} + +void lv_mem_monitor_core(lv_mem_monitor_t* mon_p) { + // Not supported - memory_*_with_policy() doesn't expose LVGL-specific usage/fragmentation + // stats (memory_print_stats() covers overall heap state instead). + (void)mon_p; +} + +lv_result_t lv_mem_test_core(void) { + return LV_RESULT_OK; +} diff --git a/Platforms/platform-esp32/source/drivers/usb/esp32_usbhost_hid.cpp b/Platforms/platform-esp32/source/drivers/usb/esp32_usbhost_hid.cpp index 00dbe6524..25b973f12 100644 --- a/Platforms/platform-esp32/source/drivers/usb/esp32_usbhost_hid.cpp +++ b/Platforms/platform-esp32/source/drivers/usb/esp32_usbhost_hid.cpp @@ -85,6 +85,12 @@ static const uint8_t keycode2ascii[57][2] = { {'`', '~'}, {',', '<'}, {'.', '>'}, {'/', '?'}, }; +// Every key returns a real Unicode codepoint per KeyboardKeyData::key's contract - never an +// LV_KEY_* (or USB_HID_KEY_*, which mirrors it) constant. Keys with no ordinary character of +// their own use the CodePoint enum's standard Unicode symbol for the concept. lvgl-module's +// keyboard.cpp translates all of these back to LVGL's own sentinels (CODEPOINT_ESCAPE/BACKSPACE/ +// DELETE already equal their LV_KEY_* counterpart numerically, so no translation is needed for +// those). static uint32_t hid_keycode_to_key(uint8_t modifier, uint8_t key_code, bool caps_lock, bool num_lock) { bool shift = (modifier & HID_LEFT_SHIFT) || (modifier & HID_RIGHT_SHIFT); @@ -92,33 +98,33 @@ static uint32_t hid_keycode_to_key(uint8_t modifier, uint8_t key_code, bool alt = (modifier & HID_LEFT_ALT) || (modifier & HID_RIGHT_ALT); switch (key_code) { - case HID_KEY_ENTER: return USB_HID_KEY_ENTER; - case HID_KEY_ESC: return USB_HID_KEY_ESC; - case HID_KEY_DEL: return USB_HID_KEY_BACKSPACE; - case HID_KEY_DELETE: return USB_HID_KEY_DEL; - case HID_KEY_TAB: return shift ? USB_HID_KEY_PREV : USB_HID_KEY_NEXT; - case HID_KEY_UP: return USB_HID_KEY_UP; - case HID_KEY_DOWN: return USB_HID_KEY_DOWN; - case HID_KEY_LEFT: return USB_HID_KEY_LEFT; - case HID_KEY_RIGHT: return USB_HID_KEY_RIGHT; - case HID_KEY_HOME: return USB_HID_KEY_HOME; - case HID_KEY_END: return USB_HID_KEY_END; - case HID_KEY_KEYPAD_ENTER: return USB_HID_KEY_ENTER; + case HID_KEY_ENTER: return CODEPOINT_ENTER; + case HID_KEY_ESC: return CODEPOINT_ESCAPE; + case HID_KEY_DEL: return CODEPOINT_BACKSPACE; + case HID_KEY_DELETE: return CODEPOINT_DELETE; + case HID_KEY_TAB: return '\t'; + case HID_KEY_UP: return CODEPOINT_ARROW_UP; + case HID_KEY_DOWN: return CODEPOINT_ARROW_DOWN; + case HID_KEY_LEFT: return CODEPOINT_ARROW_LEFT; + case HID_KEY_RIGHT: return CODEPOINT_ARROW_RIGHT; + case HID_KEY_HOME: return CODEPOINT_HOME; + case HID_KEY_END: return CODEPOINT_END; + case HID_KEY_KEYPAD_ENTER: return CODEPOINT_ENTER; case HID_KEY_KEYPAD_ADD: return '+'; case HID_KEY_KEYPAD_SUB: return '-'; case HID_KEY_KEYPAD_MUL: return '*'; case HID_KEY_KEYPAD_DIV: return '/'; case HID_KEY_KEYPAD_0: return num_lock ? (uint32_t)'0' : 0u; - case HID_KEY_KEYPAD_1: return num_lock ? (uint32_t)'1' : (uint32_t)USB_HID_KEY_END; - case HID_KEY_KEYPAD_2: return num_lock ? (uint32_t)'2' : (uint32_t)USB_HID_KEY_DOWN; + case HID_KEY_KEYPAD_1: return num_lock ? (uint32_t)'1' : (uint32_t)CODEPOINT_END; + case HID_KEY_KEYPAD_2: return num_lock ? (uint32_t)'2' : (uint32_t)CODEPOINT_ARROW_DOWN; case HID_KEY_KEYPAD_3: return num_lock ? (uint32_t)'3' : 0u; - case HID_KEY_KEYPAD_4: return num_lock ? (uint32_t)'4' : (uint32_t)USB_HID_KEY_LEFT; + case HID_KEY_KEYPAD_4: return num_lock ? (uint32_t)'4' : (uint32_t)CODEPOINT_ARROW_LEFT; case HID_KEY_KEYPAD_5: return num_lock ? (uint32_t)'5' : 0u; - case HID_KEY_KEYPAD_6: return num_lock ? (uint32_t)'6' : (uint32_t)USB_HID_KEY_RIGHT; - case HID_KEY_KEYPAD_7: return num_lock ? (uint32_t)'7' : (uint32_t)USB_HID_KEY_HOME; - case HID_KEY_KEYPAD_8: return num_lock ? (uint32_t)'8' : (uint32_t)USB_HID_KEY_UP; + case HID_KEY_KEYPAD_6: return num_lock ? (uint32_t)'6' : (uint32_t)CODEPOINT_ARROW_RIGHT; + case HID_KEY_KEYPAD_7: return num_lock ? (uint32_t)'7' : (uint32_t)CODEPOINT_HOME; + case HID_KEY_KEYPAD_8: return num_lock ? (uint32_t)'8' : (uint32_t)CODEPOINT_ARROW_UP; case HID_KEY_KEYPAD_9: return num_lock ? (uint32_t)'9' : 0u; - case HID_KEY_KEYPAD_DELETE: return num_lock ? (uint32_t)'.' : (uint32_t)USB_HID_KEY_DEL; + case HID_KEY_KEYPAD_DELETE: return num_lock ? (uint32_t)'.' : (uint32_t)CODEPOINT_DELETE; default: break; } diff --git a/Tactility/Source/Tactility.cpp b/Tactility/Source/Tactility.cpp index d28d32705..7d8041ac0 100644 --- a/Tactility/Source/Tactility.cpp +++ b/Tactility/Source/Tactility.cpp @@ -74,6 +74,7 @@ constexpr auto* TAG = "Tactility"; static DispatcherHandle_t mainDispatcherHandle = dispatcher_alloc(); void initFileMutexForLvgl(); +void deinitFileMutexForLvgl(); namespace { @@ -423,6 +424,8 @@ static void applySavedTouchCalibration() { #endif // CONFIG_TT_TOUCH_CALIBRATION_SUPPORTED static void onLvglStarted() { + initFileMutexForLvgl(); + window_manager_configure(windowManagerScreenInit); check(module_ensure_started(&lvgl_window_manager_module) == ERROR_NONE); @@ -453,6 +456,8 @@ static void onLvglStarted() { } static void onLvglStopped() { + deinitFileMutexForLvgl(); + if (softwareKeyboard.object != nullptr) { lvgl_software_keyboard_destruct(&softwareKeyboard); } @@ -510,9 +515,6 @@ void run(Module* const dtsModules[], const DtsDevice dtsDevices[]) { registerAndStartServices(); - // Must start right before LVGL - initFileMutexForLvgl(); - lvgl_module_configure((LvglModuleConfig) { .on_start = onLvglStarted, .on_stop = onLvglStopped, diff --git a/Tactility/Source/app/boot/Boot.cpp b/Tactility/Source/app/boot/Boot.cpp index 6c6356b78..4b5631494 100644 --- a/Tactility/Source/app/boot/Boot.cpp +++ b/Tactility/Source/app/boot/Boot.cpp @@ -273,7 +273,6 @@ void runBootSequence(TickType_t startTime) { #endif if (!setupUsbBootMode()) { - LOG_I(TAG, "initFromBootApp"); registerApps(); waitForMinimalSplashDuration(startTime); startNextApp(); diff --git a/Tactility/Source/file/FileMutexLvgl.cpp b/Tactility/Source/file/FileMutexLvgl.cpp index 8a4b8094f..3d4eeffcc 100644 --- a/Tactility/Source/file/FileMutexLvgl.cpp +++ b/Tactility/Source/file/FileMutexLvgl.cpp @@ -6,12 +6,16 @@ #include #include +#include + constexpr auto* TAG = "file_mutex_lvgl"; struct Device; namespace { +std::vector registered_ids; + void wrapped_lvgl_lock() { if (!lvgl_is_running()) return; lvgl_lock(); @@ -43,7 +47,7 @@ namespace tt { * If the SPI controller has a display on the bus, we create an LVGL lock for the file system path. */ void initFileMutexForLvgl() { - file_system_for_each(nullptr, [](FileSystem* fs, void* context) { + file_system_for_each(®istered_ids, [](FileSystem* fs, void* context) { char mount_path[64]; if (file_system_get_path(fs, mount_path, sizeof(mount_path)) != ERROR_NONE) { return true; @@ -77,17 +81,15 @@ void initFileMutexForLvgl() { struct Context { const char* mountPath; + std::vector* registeredIds; }; - Context ctx = { .mountPath = mount_path }; + Context ctx = { .mountPath = mount_path, .registeredIds = static_cast*>(context) }; device_for_each_child(parent, &ctx, [](Device* child, void* context) -> bool { Context* ctx = static_cast(context); if (device_get_type(child) == &DISPLAY_TYPE) { LOG_I(TAG, "Adding file mutex for %s as it shares a bus with a display", ctx->mountPath); - file_mutex_register( - &lvgl_mutex, - ctx->mountPath - ); + ctx->registeredIds->push_back(file_mutex_add(&lvgl_mutex, ctx->mountPath)); return false; } else { LOG_D(TAG, "child of parent, %s: not DISPLAY_TYPE", child->name); @@ -105,7 +107,7 @@ void initFileMutexForLvgl() { return; } - file_system_for_each(nullptr, [](FileSystem* fs, void* context) { + file_system_for_each(®istered_ids, [](FileSystem* fs, void* context) { char mount_path[64]; if (file_system_get_path(fs, mount_path, sizeof(mount_path)) != ERROR_NONE) { return true; @@ -117,9 +119,17 @@ void initFileMutexForLvgl() { } LOG_I(TAG, "Adding file mutex for %s (SD card) - a display is present and may contend for bus/DMA resources", mount_path); - file_mutex_register(&lvgl_mutex, mount_path); + auto* ids = static_cast*>(context); + ids->push_back(file_mutex_add(&lvgl_mutex, mount_path)); return true; }); } +void deinitFileMutexForLvgl() { + for (FileMutexId id : registered_ids) { + file_mutex_remove(id); + } + registered_ids.clear(); +} + } diff --git a/TactilityKernel/include/tactility/drivers/keyboard.h b/TactilityKernel/include/tactility/drivers/keyboard.h index 20d4e849d..74978441f 100644 --- a/TactilityKernel/include/tactility/drivers/keyboard.h +++ b/TactilityKernel/include/tactility/drivers/keyboard.h @@ -11,11 +11,43 @@ extern "C" { #include #include +/** + * @brief Named Unicode codepoints for KeyboardKeyData::key. + * + * Keys that produce an ordinary character use that character's own codepoint directly (e.g. 'a', + * ' ') and don't need a name here. These are the standard Unicode symbol codepoints used instead + * of LVGL's LV_KEY_* sentinels for keys with no character of their own (arrows, Home/End, + * Tab/Shift+Tab-as-focus-navigation), plus names for the handful of C0 control keys every driver + * needs (Enter/Escape/Backspace/Delete/Tab). Modules/lvgl-module/source/devices/keyboard.cpp + * translates these back into LV_KEY_* internally for LVGL. + */ +typedef enum { + CODEPOINT_ENTER = '\r', + CODEPOINT_ESCAPE = '\x1B', + CODEPOINT_BACKSPACE = '\b', + CODEPOINT_DELETE = '\x7F', + CODEPOINT_TAB = '\t', + CODEPOINT_ARROW_LEFT = 0x2190, + CODEPOINT_ARROW_UP = 0x2191, + CODEPOINT_ARROW_RIGHT = 0x2192, + CODEPOINT_ARROW_DOWN = 0x2193, + CODEPOINT_HOME = 0x21F1, + CODEPOINT_END = 0x21F2, +} CodePoint; + /** * @brief A single key event read from a keyboard device. */ struct KeyboardKeyData { - /** @brief The key code. Driver-defined (e.g. ASCII/UTF-8 codepoint, scan code, or LVGL key code). */ + /** + * @brief The key. Always a Unicode codepoint - never a raw scan code. + * + * For a key that produces a character, this is that character's codepoint (e.g. 'a', ' '). For + * a key with no character representation, or one of the handful of C0 control keys every + * driver needs, this is one of the CodePoint enum values above - never an LVGL LV_KEY_* + * constant directly. See ctrl's doc comment below for the CodePoint values that numerically + * collide with real C0 control codes. + */ uint32_t key; /** @brief True if the key was pressed, false if released. */ bool pressed; @@ -29,9 +61,9 @@ struct KeyboardKeyData { * * Reported separately rather than folded into `key` because the two encodings collide: the C0 * control codes a terminal expects for Ctrl chords (Ctrl+C is 0x03, Ctrl+K is 0x0B, ...) overlap - * the LVGL key constants drivers emit in the same field (LV_KEY_END is 3, LV_KEY_PREV is 11, - * LV_KEY_UP is 17, ...), so a single uint32_t cannot express both. Consumers that want control - * codes derive them here, e.g. + * the four CodePoint values that remain in true C0 range (CODEPOINT_ENTER is Ctrl+M/13, + * CODEPOINT_BACKSPACE is Ctrl+H/8, CODEPOINT_TAB is Ctrl+I/9, CODEPOINT_ESCAPE is Ctrl+[/27), so + * a single uint32_t cannot express both. Consumers that want control codes derive them here, e.g. * `((key >= 'a' && key <= 'z') || (key >= 'A' && key <= 'Z')) ? (key & 0x1F) : key` * when ctrl is set. * @@ -48,8 +80,8 @@ struct KeyboardKeyData { * or 0 if this driver doesn't compute one (most don't - `key` is the only field most consumers * need). Populated by drivers whose hardware layout maps cleanly onto HID usage codes, so * consumers that want to mirror physical key presses as real HID reports (e.g. USB HID output) - * don't have to reverse-engineer one out of `key`'s LVGL/ASCII encoding - which is lossy for - * keys with no ASCII/LVGL representation at all, e.g. F1-F12. + * don't have to reverse-engineer one out of `key`'s codepoint encoding - which is lossy for + * keys with no Unicode/LVGL representation at all, e.g. F1-F12. */ uint8_t hid_keycode; /** @@ -83,7 +115,7 @@ struct KeyboardApi { /** * @brief Optional: reports whether the keyboard is physically present right now. Only - * meaningful for hot-pluggable/detachable keyboards (e.g. a removable accessory) whose + * meaningful for hot-pluggable/detachable keyboarcardputer_keyboard.cppds (e.g. a removable accessory) whose * kernel device is constructed and started once at boot regardless of physical attachment - * leave NULL for a keyboard that's always physically present whenever its device is active * (the common case; callers must treat NULL the same as "always present"). diff --git a/TactilityKernel/include/tactility/filesystem/file_mutex.h b/TactilityKernel/include/tactility/filesystem/file_mutex.h index cec151878..c0a165b93 100644 --- a/TactilityKernel/include/tactility/filesystem/file_mutex.h +++ b/TactilityKernel/include/tactility/filesystem/file_mutex.h @@ -19,13 +19,25 @@ struct FileMutex { void (*unlock)(); }; +typedef uint32_t FileMutexId; + +#define FILE_MUTEX_ID_INVALID ((FileMutexId)0) + /** * @brief Registers a mutex for a mount path (e.g. "/sdcard") and its descendants. * @param[in] mutex callbacks to associate with the path; a copy is stored * @param[in] path mount path this mutex serializes access to - * @note No-op if a mutex is already registered for this exact path. + * @return the id of the new entry, or the id of the existing entry if path is already registered + * @note If a mutex is already registered for this exact path, no new entry is created and the + * existing entry's id is returned; the existing callbacks are left unchanged. + */ +FileMutexId file_mutex_add(const struct FileMutex* mutex, const char* path); + +/** + * @brief Removes a previously added mutex registration. + * @param[in] id id returned by file_mutex_add(); a stale or unknown id is a no-op */ -void file_mutex_register(const struct FileMutex* mutex, const char* path); +void file_mutex_remove(FileMutexId id); /** * @brief Looks up the mutex registered for path or one of its ancestor mount paths. diff --git a/TactilityKernel/source/filesystem/file_mutex.cpp b/TactilityKernel/source/filesystem/file_mutex.cpp index a8bbc9f92..72f648d16 100644 --- a/TactilityKernel/source/filesystem/file_mutex.cpp +++ b/TactilityKernel/source/filesystem/file_mutex.cpp @@ -1,6 +1,8 @@ // SPDX-License-Identifier: Apache-2.0 #include +#include +#include #include #include #include @@ -12,41 +14,89 @@ static const FileMutex no_mutex = { }; struct FileMutexEntry { + FileMutexId id; std::string path; FileMutex mutex; }; -static std::vector mutex_entries; +// Guards mutex_entries against concurrent add/get/remove; unrelated to whether a FileMutex's own +// lock/unlock is currently held (file_mutex_get() hands out a copy that stays valid regardless of +// later registry changes - see file_mutex_remove()). +struct FileMutexLedger { + std::vector entries; + FileMutexId next_id = 1; + Mutex mutex {}; + + FileMutexLedger() { mutex_construct(&mutex); } + ~FileMutexLedger() { mutex_destruct(&mutex); } + + void lock() { mutex_lock(&mutex); } + void unlock() { mutex_unlock(&mutex); } +}; + +static FileMutexLedger& get_ledger() { + static FileMutexLedger ledger; + return ledger; +} extern "C" { -void file_mutex_register(const FileMutex* mutex, const char* path) { - // Skip if entry for path exists - for (auto& entry : mutex_entries) { +FileMutexId file_mutex_add(const FileMutex* mutex, const char* path) { + auto& ledger = get_ledger(); + ledger.lock(); + + for (auto& entry : ledger.entries) { if (entry.path == path) { - return; + FileMutexId existing_id = entry.id; + ledger.unlock(); + return existing_id; } } - // Store a copy of the entry - mutex_entries.push_back({ + FileMutexId new_id = ledger.next_id++; + ledger.entries.push_back({ + .id = new_id, .path = path, .mutex = *mutex }); + + ledger.unlock(); + return new_id; +} + +void file_mutex_remove(FileMutexId id) { + auto& ledger = get_ledger(); + ledger.lock(); + + const auto iterator = std::ranges::find_if(ledger.entries, [id](const FileMutexEntry& entry) { + return entry.id == id; + }); + if (iterator != ledger.entries.end()) { + // Plain erase, not swap-and-pop: file_mutex_get() matches first-registered-wins, so + // removal must preserve the relative order of the remaining entries. + ledger.entries.erase(iterator); + } + + ledger.unlock(); } void file_mutex_get(FileMutex* mutex, const char* path) { + auto& ledger = get_ledger(); std::string path_string = path; - for (auto& entry : mutex_entries) { + + ledger.lock(); + for (auto& entry : ledger.entries) { // Match the mount path itself, or a descendant (e.g. "/sdcard" registered, "/sdcard/config.json" requested). bool is_match = path_string == entry.path || (entry.path == "/" && !path_string.empty() && path_string[0] == '/') || (path_string.rfind(entry.path, 0) == 0 && path_string[entry.path.size()] == '/'); if (is_match) { memcpy(mutex, &entry.mutex, sizeof(FileMutex)); + ledger.unlock(); return; } } + ledger.unlock(); *mutex = no_mutex; } diff --git a/TactilityKernel/source/memory_esp32.cpp b/TactilityKernel/source/memory_esp32.cpp index f52ab4620..44d467925 100644 --- a/TactilityKernel/source/memory_esp32.cpp +++ b/TactilityKernel/source/memory_esp32.cpp @@ -25,18 +25,22 @@ void* memory_alloc_with_policy(size_t size, const struct MemoryPolicy* policy) { uint32_t required_caps = toHeapCaps(policy->required); uint32_t desired_caps = toHeapCaps(policy->desired); + // heap_caps matches heaps via (heap->caps[prio] & caps) != 0 - a caps value of 0 (e.g. + // required_caps when policy->required wasn't set) can never match any heap, so the fallback + // must OR in MALLOC_CAP_DEFAULT to actually reach a general-purpose heap, same as ESP-IDF's + // own heap_caps_malloc_default() does. void* ptr; if (policy->alignment > 0) { ptr = heap_caps_aligned_alloc(policy->alignment, size, required_caps | desired_caps); if (ptr == nullptr && desired_caps != 0) { // Desired caps couldn't be satisfied alongside the required ones - retry with // required only, since desired is explicitly optional. - ptr = heap_caps_aligned_alloc(policy->alignment, size, required_caps); + ptr = heap_caps_aligned_alloc(policy->alignment, size, required_caps | MALLOC_CAP_DEFAULT); } } else { ptr = heap_caps_malloc(size, required_caps | desired_caps); if (ptr == nullptr && desired_caps != 0) { - ptr = heap_caps_malloc(size, required_caps); + ptr = heap_caps_malloc(size, required_caps | MALLOC_CAP_DEFAULT); } } return ptr; @@ -50,7 +54,7 @@ void* memory_realloc_with_policy(void* ptr, size_t size, const struct MemoryPoli // on fresh allocations (memory_alloc_with_policy/memory_calloc_with_policy). void* result = heap_caps_realloc(ptr, size, required_caps | desired_caps); if (result == nullptr && desired_caps != 0) { - result = heap_caps_realloc(ptr, size, required_caps); + result = heap_caps_realloc(ptr, size, required_caps | MALLOC_CAP_DEFAULT); } return result; } @@ -63,12 +67,12 @@ void* memory_calloc_with_policy(size_t count, size_t size, const struct MemoryPo if (policy->alignment > 0) { ptr = heap_caps_aligned_calloc(policy->alignment, count, size, required_caps | desired_caps); if (ptr == nullptr && desired_caps != 0) { - ptr = heap_caps_aligned_calloc(policy->alignment, count, size, required_caps); + ptr = heap_caps_aligned_calloc(policy->alignment, count, size, required_caps | MALLOC_CAP_DEFAULT); } } else { ptr = heap_caps_calloc(count, size, required_caps | desired_caps); if (ptr == nullptr && desired_caps != 0) { - ptr = heap_caps_calloc(count, size, required_caps); + ptr = heap_caps_calloc(count, size, required_caps | MALLOC_CAP_DEFAULT); } } return ptr; diff --git a/TactilityKernel/source/symbols.c b/TactilityKernel/source/symbols.c index 0f1d93030..d55076948 100644 --- a/TactilityKernel/source/symbols.c +++ b/TactilityKernel/source/symbols.c @@ -169,7 +169,8 @@ const struct ModuleSymbol KERNEL_SYMBOLS[] = { DEFINE_MODULE_SYMBOL(display_get_backlight), DEFINE_MODULE_SYMBOL(DISPLAY_TYPE), // file_mutex - DEFINE_MODULE_SYMBOL(file_mutex_register), + DEFINE_MODULE_SYMBOL(file_mutex_add), + DEFINE_MODULE_SYMBOL(file_mutex_remove), DEFINE_MODULE_SYMBOL(file_mutex_get), DEFINE_MODULE_SYMBOL(file_mutex_lock), DEFINE_MODULE_SYMBOL(file_mutex_try_lock), diff --git a/TactilityKernel/tests/source/file_mutex_test.cpp b/TactilityKernel/tests/source/file_mutex_test.cpp index 8e92e009c..a67f14c55 100644 --- a/TactilityKernel/tests/source/file_mutex_test.cpp +++ b/TactilityKernel/tests/source/file_mutex_test.cpp @@ -48,10 +48,10 @@ TEST_CASE("file_mutex_get with zero registrations returns a no-op mutex") { file_mutex_unlock(&mutex); } -TEST_CASE("file_mutex_register/get with a single registration") { +TEST_CASE("file_mutex_add/get with a single registration") { reset_mocks(); FileMutex registered = { .lock = mock_lock, .try_lock = mock_try_lock, .unlock = mock_unlock }; - file_mutex_register(®istered, "/mock1"); + file_mutex_add(®istered, "/mock1"); FileMutex mutex; @@ -87,19 +87,19 @@ TEST_CASE("file_mutex_register/get with a single registration") { // Re-registering the same path is a no-op: original callbacks remain in place. FileMutex replacement = { .lock = nullptr, .try_lock = nullptr, .unlock = nullptr }; - file_mutex_register(&replacement, "/mock1"); + file_mutex_add(&replacement, "/mock1"); file_mutex_get(&mutex, "/mock1"); CHECK_EQ(mutex.lock, mock_lock); } -TEST_CASE("file_mutex_register/get with two registrations resolves to the matching path") { +TEST_CASE("file_mutex_add/get with two registrations resolves to the matching path") { reset_mocks(); FileMutex mutex_a = { .lock = mock_lock_a, .try_lock = nullptr, .unlock = nullptr }; FileMutex mutex_b = { .lock = mock_lock_b, .try_lock = nullptr, .unlock = nullptr }; - file_mutex_register(&mutex_a, "/mock2a"); - file_mutex_register(&mutex_b, "/mock2b"); + file_mutex_add(&mutex_a, "/mock2a"); + file_mutex_add(&mutex_b, "/mock2b"); FileMutex resolved; @@ -116,7 +116,75 @@ TEST_CASE("file_mutex_register/get with two registrations resolves to the matchi // Registration order matters: the first matching entry wins, not the longest // prefix. A mount nested under an earlier one is shadowed by it. FileMutex mutex_nested = { .lock = nullptr, .try_lock = nullptr, .unlock = nullptr }; - file_mutex_register(&mutex_nested, "/mock2a/nested"); + file_mutex_add(&mutex_nested, "/mock2a/nested"); file_mutex_get(&resolved, "/mock2a/nested/file.txt"); CHECK_EQ(resolved.lock, mock_lock_a); // still /mock2a, registered first } + +TEST_CASE("file_mutex_add returns a valid id") { + reset_mocks(); + FileMutex registered = { .lock = mock_lock, .try_lock = nullptr, .unlock = nullptr }; + FileMutexId id = file_mutex_add(®istered, "/mockA"); + CHECK_NE(id, FILE_MUTEX_ID_INVALID); +} + +TEST_CASE("file_mutex_add with a duplicate path returns the existing id") { + reset_mocks(); + FileMutex mutex_1 = { .lock = mock_lock, .try_lock = nullptr, .unlock = nullptr }; + FileMutex mutex_2 = { .lock = mock_lock_a, .try_lock = nullptr, .unlock = nullptr }; + + FileMutexId id_1 = file_mutex_add(&mutex_1, "/mockB"); + FileMutexId id_2 = file_mutex_add(&mutex_2, "/mockB"); + CHECK_EQ(id_1, id_2); + + FileMutex resolved; + file_mutex_get(&resolved, "/mockB"); + CHECK_EQ(resolved.lock, mock_lock); // first registration's callbacks win, unchanged +} + +TEST_CASE("file_mutex_remove removes a registration") { + reset_mocks(); + FileMutex registered = { .lock = mock_lock, .try_lock = nullptr, .unlock = nullptr }; + FileMutexId id = file_mutex_add(®istered, "/mockC"); + + FileMutex before; + file_mutex_get(&before, "/mockC"); + CHECK_EQ(before.lock, mock_lock); + + file_mutex_remove(id); + + FileMutex after; + file_mutex_get(&after, "/mockC"); + CHECK_EQ(after.lock, nullptr); +} + +TEST_CASE("file_mutex_remove with an unknown id is a safe no-op") { + reset_mocks(); + FileMutex registered = { .lock = mock_lock, .try_lock = nullptr, .unlock = nullptr }; + file_mutex_add(®istered, "/mockD"); + + file_mutex_remove(999999); // never issued + file_mutex_remove(FILE_MUTEX_ID_INVALID); + + FileMutex resolved; + file_mutex_get(&resolved, "/mockD"); + CHECK_EQ(resolved.lock, mock_lock); // untouched +} + +TEST_CASE("file_mutex_remove of one registration leaves others intact") { + reset_mocks(); + FileMutex mutex_a = { .lock = mock_lock_a, .try_lock = nullptr, .unlock = nullptr }; + FileMutex mutex_b = { .lock = mock_lock_b, .try_lock = nullptr, .unlock = nullptr }; + FileMutexId id_a = file_mutex_add(&mutex_a, "/mockE1"); + file_mutex_add(&mutex_b, "/mockE2"); + + file_mutex_remove(id_a); + + FileMutex resolved_a; + file_mutex_get(&resolved_a, "/mockE1"); + CHECK_EQ(resolved_a.lock, nullptr); + + FileMutex resolved_b; + file_mutex_get(&resolved_b, "/mockE2"); + CHECK_EQ(resolved_b.lock, mock_lock_b); +} diff --git a/lv_conf.h b/lv_conf.h index b7f3421f3..d5ba25e75 100644 --- a/lv_conf.h +++ b/lv_conf.h @@ -38,14 +38,14 @@ * - LV_STDLIB_RTTHREAD: RT-Thread implementation * - LV_STDLIB_CUSTOM: Implement the functions externally */ -#define LV_USE_STDLIB_MALLOC LV_STDLIB_CLIB +#define LV_USE_STDLIB_MALLOC LV_STDLIB_CUSTOM #define LV_USE_STDLIB_STRING LV_STDLIB_BUILTIN #define LV_USE_STDLIB_SPRINTF LV_STDLIB_BUILTIN #if LV_USE_STDLIB_MALLOC == LV_STDLIB_BUILTIN /*Size of the memory available for `lv_malloc()` in bytes (>= 2kB)*/ - #define LV_MEM_SIZE (64 * 1024U) /*[bytes]*/ + #define LV_MEM_SIZE (128 * 1024U) /*[bytes]*/ /*Size of the memory expand for `lv_malloc()` in bytes*/ #define LV_MEM_POOL_EXPAND_SIZE 0 From 670d6ab062c633489491e6c5759e87928fe4a683 Mon Sep 17 00:00:00 2001 From: Ken Van Hoeylandt Date: Sat, 22 Aug 2026 23:23:36 +0200 Subject: [PATCH 05/14] Compile fixes --- .../source/cardputer_adv_keyboard.cpp | 6 +++--- .../source/cardputer_keyboard.cpp | 20 ++++--------------- 2 files changed, 7 insertions(+), 19 deletions(-) diff --git a/Drivers/m5stack-module/source/cardputer_adv_keyboard.cpp b/Drivers/m5stack-module/source/cardputer_adv_keyboard.cpp index a7210bd2b..03b0b22bc 100644 --- a/Drivers/m5stack-module/source/cardputer_adv_keyboard.cpp +++ b/Drivers/m5stack-module/source/cardputer_adv_keyboard.cpp @@ -45,21 +45,21 @@ static constexpr int CARDPUTER_ADV_COLS = 14; // emits nothing (used for the sym/shift cells themselves, and unwired cells on this board). static const uint32_t cardputer_adv_keymap_lc[CARDPUTER_ADV_ROWS][CARDPUTER_ADV_COLS] = { { '`', '1', '2', '3', '4', '5', '6', '7', '8', '9', '0', '-', '=', CODEPOINT_BACKSPACE }, - { CODEPOINT_TAB, 'q', 'w', 'e', 'r', 't', 'y', 'u', 'i', 'o', 'p', '[', ']', '\\' }, + { '\t', 'q', 'w', 'e', 'r', 't', 'y', 'u', 'i', 'o', 'p', '[', ']', '\\' }, { 0, 0, 'a', 's', 'd', 'f', 'g', 'h', 'j', 'k', 'l', ';', '\'', CODEPOINT_ENTER }, { 0, 0, 0, 'z', 'x', 'c', 'v', 'b', 'n', 'm', ',', '.', '/', ' ' }, }; static const uint32_t cardputer_adv_keymap_uc[CARDPUTER_ADV_ROWS][CARDPUTER_ADV_COLS] = { { '~', '!', '@', '#', '$', '%', '^', '&', '*', '(', ')', '_', '+', CODEPOINT_DELETE }, - { CODEPOINT_FOCUS_PREV, 'Q', 'W', 'E', 'R', 'T', 'Y', 'U', 'I', 'O', 'P', '{', '}', '|' }, + { '\t', 'Q', 'W', 'E', 'R', 'T', 'Y', 'U', 'I', 'O', 'P', '{', '}', '|' }, { 0, 0, 'A', 'S', 'D', 'F', 'G', 'H', 'J', 'K', 'L', ':', '"', CODEPOINT_ENTER }, { 0, 0, 0, 'Z', 'X', 'C', 'V', 'B', 'N', 'M', '<', '>', '?', ' ' }, }; static const uint32_t cardputer_adv_keymap_sym[CARDPUTER_ADV_ROWS][CARDPUTER_ADV_COLS] = { { CODEPOINT_ESCAPE, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0 }, - { CODEPOINT_TAB, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0 }, + { '\t', 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0 }, { 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, CODEPOINT_ARROW_UP, 0, CODEPOINT_ENTER }, { 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, CODEPOINT_ARROW_LEFT, CODEPOINT_ARROW_DOWN, CODEPOINT_ARROW_RIGHT, 0 }, }; diff --git a/Drivers/m5stack-module/source/cardputer_keyboard.cpp b/Drivers/m5stack-module/source/cardputer_keyboard.cpp index 91d7bc0b5..17defde32 100644 --- a/Drivers/m5stack-module/source/cardputer_keyboard.cpp +++ b/Drivers/m5stack-module/source/cardputer_keyboard.cpp @@ -29,15 +29,12 @@ static constexpr int CARDPUTER_PENDING_CAPACITY = 2; enum CardputerKeyRole { CARDPUTER_KEY_CHAR, - CARDPUTER_KEY_TAB, CARDPUTER_KEY_FN, CARDPUTER_KEY_SHIFT, CARDPUTER_KEY_CTRL, CARDPUTER_KEY_OPT, CARDPUTER_KEY_ALT, CARDPUTER_KEY_DEL, - CARDPUTER_KEY_ENTER, - CARDPUTER_KEY_SPACE, }; struct CardputerKeyDef { @@ -54,12 +51,12 @@ struct CardputerKeyDef { static const CardputerKeyDef cardputer_key_map[CARDPUTER_ROWS][CARDPUTER_COLS] = { { K('`', '~'), K('1', '!'), K('2', '@'), K('3', '#'), K('4', '$'), K('5', '%'), K('6', '^'), K('7', '&'), K('8', '*'), K('9', '('), K('0', ')'), K('-', '_'), K('=', '+'), { CARDPUTER_KEY_DEL, 0, 0 } }, - { { CARDPUTER_KEY_TAB, 0, 0 }, K('q', 'Q'), K('w', 'W'), K('e', 'E'), K('r', 'R'), K('t', 'T'), K('y', 'Y'), + { K('\t', '\t'), K('q', 'Q'), K('w', 'W'), K('e', 'E'), K('r', 'R'), K('t', 'T'), K('y', 'Y'), K('u', 'U'), K('i', 'I'), K('o', 'O'), K('p', 'P'), K('[', '{'), K(']', '}'), K('\\', '|') }, { { CARDPUTER_KEY_FN, 0, 0 }, { CARDPUTER_KEY_SHIFT, 0, 0 }, K('a', 'A'), K('s', 'S'), K('d', 'D'), K('f', 'F'), K('g', 'G'), - K('h', 'H'), K('j', 'J'), K('k', 'K'), K('l', 'L'), K(';', ':'), K('\'', '"'), { CARDPUTER_KEY_ENTER, 0, 0 } }, + K('h', 'H'), K('j', 'J'), K('k', 'K'), K('l', 'L'), K(';', ':'), K('\'', '"'), K(0x10, 0x10) }, { { CARDPUTER_KEY_CTRL, 0, 0 }, { CARDPUTER_KEY_OPT, 0, 0 }, { CARDPUTER_KEY_ALT, 0, 0 }, K('z', 'Z'), K('x', 'X'), K('c', 'C'), K('v', 'V'), - K('b', 'B'), K('n', 'N'), K('m', 'M'), K(',', '<'), K('.', '>'), K('/', '?'), { CARDPUTER_KEY_SPACE, 0, 0 } }, + K('b', 'B'), K('n', 'N'), K('m', 'M'), K(',', '<'), K('.', '>'), K('/', '?'), K(' ', ' ') }, }; #undef K @@ -201,7 +198,7 @@ static uint8_t read_input(CardputerKeyboardInternal* internal) { // reported themselves. static uint32_t scan_key(CardputerKeyboardInternal* internal) { bool fn = false, shift = false, ctrl = false; - bool del_flag = false, enter_flag = false, space_flag = false; + bool del_flag = false; bool has_regular = false; char regular_normal = 0, regular_shifted = 0; @@ -238,12 +235,6 @@ static uint32_t scan_key(CardputerKeyboardInternal* internal) { case CARDPUTER_KEY_DEL: del_flag = true; break; - case CARDPUTER_KEY_ENTER: - enter_flag = true; - break; - case CARDPUTER_KEY_SPACE: - space_flag = true; - break; case CARDPUTER_KEY_CHAR: if (!has_regular) { has_regular = true; @@ -258,8 +249,6 @@ static uint32_t scan_key(CardputerKeyboardInternal* internal) { char resolved_char = has_regular ? ((ctrl || shift) ? regular_shifted : regular_normal) : 0; if (!fn) { - if (enter_flag) return CODEPOINT_ENTER; - if (space_flag) return (uint32_t)' '; if (del_flag) return CODEPOINT_BACKSPACE; if (has_regular) return (uint32_t)resolved_char; return 0; @@ -269,7 +258,6 @@ static uint32_t scan_key(CardputerKeyboardInternal* internal) { // rather than arrow codepoints so widgets like lv_switch that toggle on arrow keys aren't // affected). if (del_flag) return CODEPOINT_DELETE; - if (enter_flag) return CODEPOINT_ENTER; if (has_regular) { switch (resolved_char) { case '`': return CODEPOINT_ESCAPE; From 0a2bab4e38e2607235f89df5efc528b9f81c508f Mon Sep 17 00:00:00 2001 From: Ken Van Hoeylandt Date: Sat, 22 Aug 2026 23:34:32 +0200 Subject: [PATCH 06/14] Fixes --- Drivers/tca8418-module/bindings/ti,tca8418.yaml | 3 +-- Modules/lvgl-module/README.md | 2 +- Modules/lvgl-module/include/lvgl/devices/keyboard.h | 5 ++++- Modules/lvgl-module/source/devices/keyboard.cpp | 12 ++++++++++++ Tactility/Source/lvgl/Statusbar.cpp | 8 +++++--- TactilityKernel/include/tactility/drivers/keyboard.h | 2 +- 6 files changed, 24 insertions(+), 8 deletions(-) diff --git a/Drivers/tca8418-module/bindings/ti,tca8418.yaml b/Drivers/tca8418-module/bindings/ti,tca8418.yaml index 5a7856043..a6b217cef 100644 --- a/Drivers/tca8418-module/bindings/ti,tca8418.yaml +++ b/Drivers/tca8418-module/bindings/ti,tca8418.yaml @@ -35,8 +35,7 @@ properties: silkscreen/keymap column order - see reverse-columns). 0 = no key at this position (e.g. a blank matrix position, or a position handled as a modifier via shift-row/shift-col/ sym-row/sym-col instead). Non-zero bytes are sent as-is via KeyboardKeyData::key (a Unicode - codepoint - byte range covers Latin-1 - or an LVGL LV_KEY_* code for keys with no character - representation). + codepoint - byte range covers Latin-1 - for character and non-character keys). keymap-uc: type: array element-type: uint8_t diff --git a/Modules/lvgl-module/README.md b/Modules/lvgl-module/README.md index d36133022..2db25aacb 100644 --- a/Modules/lvgl-module/README.md +++ b/Modules/lvgl-module/README.md @@ -43,7 +43,7 @@ LVGL's malloc/realloc/free can be routed through a custom backend instead of its plain `malloc`. Three things are required: **1. Select the backend.** On ESP32, via Kconfig (`sdkconfig`): -``` +```sdkconfig CONFIG_LV_USE_CUSTOM_MALLOC=y ``` On Simulator/POSIX, ESP-IDF's Kconfig doesn't apply - select it in `lv_conf.h` instead: diff --git a/Modules/lvgl-module/include/lvgl/devices/keyboard.h b/Modules/lvgl-module/include/lvgl/devices/keyboard.h index 66e62dc71..b134178f3 100644 --- a/Modules/lvgl-module/include/lvgl/devices/keyboard.h +++ b/Modules/lvgl-module/include/lvgl/devices/keyboard.h @@ -21,9 +21,12 @@ struct LvglSoftwareKeyboard { * @warning Caller must hold the LVGL lock (see lvgl_lock() in lvgl_module.h) — call this from * LvglModuleConfig.on_start, or after calling lvgl_lock() explicitly. * + * @note Idempotent per device: if an indev is already bound to this device (see + * lvgl_keyboard_find_by_device()), that indev is returned instead of creating a second one. + * * @param[in] device a device of type KEYBOARD_TYPE * @param[in] display the display this indev should be associated with, or NULL to leave it unset - * @param[out] out_indev the created indev, valid only when ERROR_NONE is returned + * @param[out] out_indev the created (or already-existing) indev, valid only when ERROR_NONE is returned * @retval ERROR_NONE on success * @retval ERROR_INVALID_ARGUMENT if device or out_indev is NULL, or device is not of type KEYBOARD_TYPE * @retval ERROR_OUT_OF_MEMORY if allocation failed diff --git a/Modules/lvgl-module/source/devices/keyboard.cpp b/Modules/lvgl-module/source/devices/keyboard.cpp index b50e88d84..107a5d1d6 100644 --- a/Modules/lvgl-module/source/devices/keyboard.cpp +++ b/Modules/lvgl-module/source/devices/keyboard.cpp @@ -84,6 +84,18 @@ error_t lvgl_keyboard_add(struct Device* device, lv_display_t* display, lv_indev return ERROR_INVALID_ARGUMENT; } + // A device can reach here twice: lvgl_devices_attach()'s boot scan binds every KEYBOARD_TYPE + // device unconditionally (started or not), and a device that wasn't started yet at that point + // fires DEVICE_EVENT_STARTED later, driving a second call through + // KeyboardDeviceListener::onKeyboardDeviceStarted(). Without this check that would create a + // second indev for the same device, and onKeyboardDeviceStopped() only ever removes one of + // them, leaving the other dangling - polling a destructed device on the next LVGL tick. + lv_indev_t* existing = lvgl_keyboard_find_by_device(device); + if (existing != NULL) { + *out_indev = existing; + return ERROR_NONE; + } + auto* wrapper = new(std::nothrow) LvglDeviceContext(nullptr); if (wrapper == NULL) { return ERROR_OUT_OF_MEMORY; diff --git a/Tactility/Source/lvgl/Statusbar.cpp b/Tactility/Source/lvgl/Statusbar.cpp index ce2d4aaac..716e85e42 100644 --- a/Tactility/Source/lvgl/Statusbar.cpp +++ b/Tactility/Source/lvgl/Statusbar.cpp @@ -191,13 +191,15 @@ lv_obj_t* statusbar_create(lv_obj_t* parent) { update_icon(image, &(statusbar_data.icons[i])); } - statusbar_data.mutex.unlock(); - // Only now is statusbar->icons[] fully populated - see statusbar_constructor()'s comment for - // why the subscription can't be registered any earlier. + // why the subscription can't be registered any earlier. Subscribing before unlocking (rather + // than after) closes a narrow window where a concurrent statusbar_icon_add()/_remove()/ + // _set_image()/_set_visibility() call could publish - and be missed by this instance - after + // the icons are populated but before it's subscribed. statusbar->pubsub_subscription = statusbar_data.pubsub->subscribe([statusbar](auto) { statusbar_pubsub_event(statusbar); }); + statusbar_data.mutex.unlock(); return obj; } diff --git a/TactilityKernel/include/tactility/drivers/keyboard.h b/TactilityKernel/include/tactility/drivers/keyboard.h index 74978441f..067596ea3 100644 --- a/TactilityKernel/include/tactility/drivers/keyboard.h +++ b/TactilityKernel/include/tactility/drivers/keyboard.h @@ -115,7 +115,7 @@ struct KeyboardApi { /** * @brief Optional: reports whether the keyboard is physically present right now. Only - * meaningful for hot-pluggable/detachable keyboarcardputer_keyboard.cppds (e.g. a removable accessory) whose + * meaningful for hot-pluggable/detachable keyboards (e.g. a removable accessory) whose * kernel device is constructed and started once at boot regardless of physical attachment - * leave NULL for a keyboard that's always physically present whenever its device is active * (the common case; callers must treat NULL the same as "always present"). From dc81063300e0ebccea586eb590fbb5c16f2fdcd0 Mon Sep 17 00:00:00 2001 From: Ken Van Hoeylandt Date: Sat, 22 Aug 2026 23:52:37 +0200 Subject: [PATCH 07/14] Fix for return key --- Drivers/m5stack-module/source/cardputer_keyboard.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Drivers/m5stack-module/source/cardputer_keyboard.cpp b/Drivers/m5stack-module/source/cardputer_keyboard.cpp index 17defde32..dcaf86bea 100644 --- a/Drivers/m5stack-module/source/cardputer_keyboard.cpp +++ b/Drivers/m5stack-module/source/cardputer_keyboard.cpp @@ -54,7 +54,7 @@ static const CardputerKeyDef cardputer_key_map[CARDPUTER_ROWS][CARDPUTER_COLS] = { K('\t', '\t'), K('q', 'Q'), K('w', 'W'), K('e', 'E'), K('r', 'R'), K('t', 'T'), K('y', 'Y'), K('u', 'U'), K('i', 'I'), K('o', 'O'), K('p', 'P'), K('[', '{'), K(']', '}'), K('\\', '|') }, { { CARDPUTER_KEY_FN, 0, 0 }, { CARDPUTER_KEY_SHIFT, 0, 0 }, K('a', 'A'), K('s', 'S'), K('d', 'D'), K('f', 'F'), K('g', 'G'), - K('h', 'H'), K('j', 'J'), K('k', 'K'), K('l', 'L'), K(';', ':'), K('\'', '"'), K(0x10, 0x10) }, + K('h', 'H'), K('j', 'J'), K('k', 'K'), K('l', 'L'), K(';', ':'), K('\'', '"'), K('\r', '\r') }, { { CARDPUTER_KEY_CTRL, 0, 0 }, { CARDPUTER_KEY_OPT, 0, 0 }, { CARDPUTER_KEY_ALT, 0, 0 }, K('z', 'Z'), K('x', 'X'), K('c', 'C'), K('v', 'V'), K('b', 'B'), K('n', 'N'), K('m', 'M'), K(',', '<'), K('.', '>'), K('/', '?'), K(' ', ' ') }, }; From cf549bf14a440d3b29b12c49b34ef7573bf13bc4 Mon Sep 17 00:00:00 2001 From: Ken Van Hoeylandt Date: Sun, 23 Aug 2026 00:10:22 +0200 Subject: [PATCH 08/14] Fix for CDC on P4/Tab5 --- .../source/drivers/usb/esp32_usb_device_controller.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/Platforms/platform-esp32/source/drivers/usb/esp32_usb_device_controller.cpp b/Platforms/platform-esp32/source/drivers/usb/esp32_usb_device_controller.cpp index bccf09d19..344919c63 100644 --- a/Platforms/platform-esp32/source/drivers/usb/esp32_usb_device_controller.cpp +++ b/Platforms/platform-esp32/source/drivers/usb/esp32_usb_device_controller.cpp @@ -167,7 +167,9 @@ static error_t claim(struct Device* device, enum UsbDeviceClass usb_class, const return ERROR_INVALID_STATE; } - const bool cdc_enabled = is_cdc_enabled(device); + // MSC is deliberately excluded from CDC compositing as it doesn't work on Tab5/P4 devices. + // It could be re-enabled for other architectures after more extensive future testing. + const bool cdc_enabled = usb_class != USB_DEVICE_CLASS_MSC && is_cdc_enabled(device); // ---- String table: primary's table, plus CDC's own interface string appended last. size_t string_count = config->string_descriptor_count; From 850f982c1eab7145fb619f8ab311143083ee24db Mon Sep 17 00:00:00 2001 From: Ken Van Hoeylandt Date: Sun, 23 Aug 2026 10:21:39 +0200 Subject: [PATCH 09/14] Update comment --- Drivers/m5stack-module/source/cardputer_keyboard.cpp | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/Drivers/m5stack-module/source/cardputer_keyboard.cpp b/Drivers/m5stack-module/source/cardputer_keyboard.cpp index dcaf86bea..704fe4cc2 100644 --- a/Drivers/m5stack-module/source/cardputer_keyboard.cpp +++ b/Drivers/m5stack-module/source/cardputer_keyboard.cpp @@ -190,12 +190,7 @@ static uint8_t read_input(CardputerKeyboardInternal* internal) { return mask; } -// Scans the full matrix and resolves it to a single key (Unicode codepoint, 0 if none - never an -// LV_KEY_* constant, see KeyboardKeyData::key's contract), applying priority: enter > space > -// backspace > tab (U+21E5 tab-to-bar, or U+21E4 with shift - the closest standard Unicode symbol -// for focus navigation) > first regular character found in scan order, with fn changing the -// interpretation of backspace/enter/punctuation. Modifier keys (fn/shift/ctrl/opt/alt) are never -// reported themselves. +// Scans the full matrix and resolves it to a Unicode codepoint static uint32_t scan_key(CardputerKeyboardInternal* internal) { bool fn = false, shift = false, ctrl = false; bool del_flag = false; From fe31889f8065f7fd85b59b59d8141dca1f643a44 Mon Sep 17 00:00:00 2001 From: Ken Van Hoeylandt Date: Sun, 23 Aug 2026 11:22:35 +0200 Subject: [PATCH 10/14] Remove unused code --- Devices/lilygo-tlora-pager/source/module.cpp | 3 - .../lilygo/drivers/tpager_encoder_input.h | 20 ---- .../source/tpager_encoder_input.cpp | 95 ------------------- 3 files changed, 118 deletions(-) delete mode 100644 Drivers/lilygo-module/include/lilygo/drivers/tpager_encoder_input.h delete mode 100644 Drivers/lilygo-module/source/tpager_encoder_input.cpp diff --git a/Devices/lilygo-tlora-pager/source/module.cpp b/Devices/lilygo-tlora-pager/source/module.cpp index ca93edd98..2d9458615 100644 --- a/Devices/lilygo-tlora-pager/source/module.cpp +++ b/Devices/lilygo-tlora-pager/source/module.cpp @@ -2,8 +2,6 @@ #include #include -#include - constexpr auto* TAG = "T-Lora Pager"; extern "C" { @@ -12,7 +10,6 @@ static void on_boot_completed(struct SystemEvent* /*event*/, void* /*context*/) // The kernel tpager_encoder device is already started by kernel_init(); this just // registers it as an LVGL input device, which requires LVGL to be up first. lvgl_lock(); - tpager_encoder::init(); lvgl_unlock(); } diff --git a/Drivers/lilygo-module/include/lilygo/drivers/tpager_encoder_input.h b/Drivers/lilygo-module/include/lilygo/drivers/tpager_encoder_input.h deleted file mode 100644 index f26dbb5af..000000000 --- a/Drivers/lilygo-module/include/lilygo/drivers/tpager_encoder_input.h +++ /dev/null @@ -1,20 +0,0 @@ -// SPDX-License-Identifier: Apache-2.0 -#pragma once - -#include - -namespace tpager_encoder { - -/** - * @brief Initialize the encoder wheel as an LVGL input device, backed by the kernel - * tpager_encoder driver. - * @return LVGL input device pointer, or nullptr if the kernel device isn't found/started - */ -lv_indev_t* init(); - -/** - * @brief Deinitialize the encoder wheel's LVGL input device. - */ -void deinit(); - -} diff --git a/Drivers/lilygo-module/source/tpager_encoder_input.cpp b/Drivers/lilygo-module/source/tpager_encoder_input.cpp deleted file mode 100644 index f8a5236b4..000000000 --- a/Drivers/lilygo-module/source/tpager_encoder_input.cpp +++ /dev/null @@ -1,95 +0,0 @@ -// SPDX-License-Identifier: Apache-2.0 -#include -#include - -#include -#include - -constexpr auto* TAG = "tpager_encoder"; - -namespace tpager_encoder { - -static lv_indev_t* g_indev = nullptr; -static Device* g_device = nullptr; - -// Ported from the deprecated HAL's TpagerEncoder::readCallback(). g_raw_total reconstructs the -// old absolute PCNT counter value (the kernel driver's read_delta() consumes/resets on every -// call instead of accumulating forever), so the hysteresis below behaves identically: only a -// run of more than pulses_click raw pulses commits to a detent, and any remainder is discarded -// rather than carried into the next read (matches the original's pulses_prev = pulses jump). -static void read_cb(lv_indev_t*, lv_indev_data_t* data) { - constexpr int32_t pulses_click = 4; - static int32_t raw_total = 0; - static int32_t committed_total = 0; - - constexpr int enter_filter_threshold = 2; - static int enter_filter = 0; - - data->enc_diff = 0; - data->state = LV_INDEV_STATE_RELEASED; - - int32_t delta = 0; - tpager_encoder_read_delta(g_device, &delta); - raw_total += delta; - - int32_t pulse_diff = raw_total - committed_total; - if (pulse_diff > pulses_click || pulse_diff < -pulses_click) { - data->enc_diff = static_cast(pulse_diff / pulses_click); - committed_total = raw_total; - } - - bool pressed = false; - tpager_encoder_get_button_pressed(g_device, &pressed); - if (pressed && enter_filter < enter_filter_threshold) { - enter_filter++; - } - if (!pressed && enter_filter > 0) { - enter_filter--; - } - - if (enter_filter == enter_filter_threshold) { - data->state = LV_INDEV_STATE_PRESSED; - } -} - -lv_indev_t* init() { - if (g_indev != nullptr) { - LOG_W(TAG, "Already initialized"); - return g_indev; - } - - if (device_get_first_active_by_type(&TPAGER_ENCODER_TYPE, &g_device) != ERROR_NONE) { - LOG_E(TAG, "tpager_encoder kernel device not found or not started"); - return nullptr; - } - - g_indev = lv_indev_create(); - if (g_indev == nullptr) { - LOG_E(TAG, "Failed to register LVGL input device"); - device_put(g_device); - g_device = nullptr; - return nullptr; - } - - lv_indev_set_type(g_indev, LV_INDEV_TYPE_ENCODER); - lv_indev_set_read_cb(g_indev, read_cb); - LOG_I(TAG, "Initialized"); - - return g_indev; -} - -void deinit() { - if (g_indev == nullptr) { - return; - } - - lv_indev_delete(g_indev); - g_indev = nullptr; - - device_put(g_device); - g_device = nullptr; - - LOG_I(TAG, "Deinitialized"); -} - -} From 899139eb5126462c3b9d8f99e03651a2fbecba29 Mon Sep 17 00:00:00 2001 From: Ken Van Hoeylandt Date: Sun, 23 Aug 2026 11:22:59 +0200 Subject: [PATCH 11/14] Arrow up/down on keyboard maps to prev/next in lvgl driver mapping --- Modules/lvgl-module/source/devices/keyboard.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Modules/lvgl-module/source/devices/keyboard.cpp b/Modules/lvgl-module/source/devices/keyboard.cpp index 107a5d1d6..96f16bd24 100644 --- a/Modules/lvgl-module/source/devices/keyboard.cpp +++ b/Modules/lvgl-module/source/devices/keyboard.cpp @@ -52,9 +52,9 @@ static uint32_t codepoint_to_lv_key(uint32_t key) { switch (key) { case CODEPOINT_ENTER: return LV_KEY_ENTER; case CODEPOINT_ARROW_LEFT: return LV_KEY_LEFT; - case CODEPOINT_ARROW_UP: return LV_KEY_UP; + case CODEPOINT_ARROW_UP: return LV_KEY_PREV; case CODEPOINT_ARROW_RIGHT: return LV_KEY_RIGHT; - case CODEPOINT_ARROW_DOWN: return LV_KEY_DOWN; + case CODEPOINT_ARROW_DOWN: return LV_KEY_NEXT; case CODEPOINT_HOME: return LV_KEY_HOME; case CODEPOINT_END: return LV_KEY_END; default: return key; From 16dd02f097e0ca2fa299fe222441ff7658da3934 Mon Sep 17 00:00:00 2001 From: Ken Van Hoeylandt Date: Sun, 23 Aug 2026 12:34:34 +0200 Subject: [PATCH 12/14] Refactor encoder driver --- Devices/lilygo-tlora-pager/CMakeLists.txt | 2 +- Devices/lilygo-tlora-pager/devicetree.yaml | 2 +- .../lilygo-tlora-pager/lilygo,tlora-pager.dts | 4 +- Devices/lilygo-tlora-pager/source/module.cpp | 2 +- Drivers/gpio-encoder-module/CMakeLists.txt | 11 + .../gpio-encoder-module/LICENSE-Apache-2.0.md | 195 ++++++++++++++++++ .../bindings/tactility,gpio-encoder.yaml | 29 +++ Drivers/gpio-encoder-module/devicetree.yaml | 3 + .../include/bindings/gpio_encoder.h | 7 + .../include/drivers/gpio_encoder.h | 25 +++ .../include/gpio_encoder_module.h | 14 ++ .../source/gpio_encoder.cpp} | 170 ++++++++++----- Drivers/gpio-encoder-module/source/module.cpp | 19 ++ .../bindings/lilygo,tpager-encoder.yaml | 22 -- .../include/lilygo/bindings/tpager_encoder.h | 7 - .../include/lilygo/drivers/tpager_encoder.h | 59 ------ Drivers/lilygo-module/source/module.cpp | 2 - 17 files changed, 432 insertions(+), 141 deletions(-) create mode 100644 Drivers/gpio-encoder-module/CMakeLists.txt create mode 100644 Drivers/gpio-encoder-module/LICENSE-Apache-2.0.md create mode 100644 Drivers/gpio-encoder-module/bindings/tactility,gpio-encoder.yaml create mode 100644 Drivers/gpio-encoder-module/devicetree.yaml create mode 100644 Drivers/gpio-encoder-module/include/bindings/gpio_encoder.h create mode 100644 Drivers/gpio-encoder-module/include/drivers/gpio_encoder.h create mode 100644 Drivers/gpio-encoder-module/include/gpio_encoder_module.h rename Drivers/{lilygo-module/source/tpager_encoder.cpp => gpio-encoder-module/source/gpio_encoder.cpp} (52%) create mode 100644 Drivers/gpio-encoder-module/source/module.cpp delete mode 100644 Drivers/lilygo-module/bindings/lilygo,tpager-encoder.yaml delete mode 100644 Drivers/lilygo-module/include/lilygo/bindings/tpager_encoder.h delete mode 100644 Drivers/lilygo-module/include/lilygo/drivers/tpager_encoder.h diff --git a/Devices/lilygo-tlora-pager/CMakeLists.txt b/Devices/lilygo-tlora-pager/CMakeLists.txt index 0ec63364a..1afb97ba9 100644 --- a/Devices/lilygo-tlora-pager/CMakeLists.txt +++ b/Devices/lilygo-tlora-pager/CMakeLists.txt @@ -3,5 +3,5 @@ file(GLOB_RECURSE SOURCE_FILES source/*.c*) idf_component_register( SRCS ${SOURCE_FILES} INCLUDE_DIRS "source" - REQUIRES TactilityKernel lvgl-module lilygo-module + REQUIRES TactilityKernel lvgl-module gpio-encoder-module ) diff --git a/Devices/lilygo-tlora-pager/devicetree.yaml b/Devices/lilygo-tlora-pager/devicetree.yaml index d38da1a52..2ed8e0a22 100644 --- a/Devices/lilygo-tlora-pager/devicetree.yaml +++ b/Devices/lilygo-tlora-pager/devicetree.yaml @@ -6,5 +6,5 @@ dependencies: - Drivers/tca8418-module - Drivers/bq25896-module - Drivers/drv2605-module - - Drivers/lilygo-module + - Drivers/gpio-encoder-module dts: lilygo,tlora-pager.dts diff --git a/Devices/lilygo-tlora-pager/lilygo,tlora-pager.dts b/Devices/lilygo-tlora-pager/lilygo,tlora-pager.dts index d1bfc61be..d42846f31 100644 --- a/Devices/lilygo-tlora-pager/lilygo,tlora-pager.dts +++ b/Devices/lilygo-tlora-pager/lilygo,tlora-pager.dts @@ -18,7 +18,7 @@ #include #include #include -#include +#include // Reference: https://wiki.lilygo.cc/get_started/en/LoRa_GPS/T-LoraPager/T-LoraPager.html / { @@ -201,7 +201,7 @@ // Encoder wheel next to the display. encoder { - compatible = "lilygo,tpager-encoder"; + compatible = "tactility,gpio-encoder"; pin-a = <&gpio0 40 GPIO_FLAG_NONE>; pin-b = <&gpio0 41 GPIO_FLAG_NONE>; pin-enter = <&gpio0 7 GPIO_FLAG_NONE>; diff --git a/Devices/lilygo-tlora-pager/source/module.cpp b/Devices/lilygo-tlora-pager/source/module.cpp index 2d9458615..f249bc7a7 100644 --- a/Devices/lilygo-tlora-pager/source/module.cpp +++ b/Devices/lilygo-tlora-pager/source/module.cpp @@ -7,7 +7,7 @@ constexpr auto* TAG = "T-Lora Pager"; extern "C" { static void on_boot_completed(struct SystemEvent* /*event*/, void* /*context*/) { - // The kernel tpager_encoder device is already started by kernel_init(); this just + // The kernel gpio_encoder device is already started by kernel_init(); this just // registers it as an LVGL input device, which requires LVGL to be up first. lvgl_lock(); lvgl_unlock(); diff --git a/Drivers/gpio-encoder-module/CMakeLists.txt b/Drivers/gpio-encoder-module/CMakeLists.txt new file mode 100644 index 000000000..bb3934b07 --- /dev/null +++ b/Drivers/gpio-encoder-module/CMakeLists.txt @@ -0,0 +1,11 @@ +cmake_minimum_required(VERSION 3.20) + +include("${CMAKE_CURRENT_LIST_DIR}/../../Buildscripts/module.cmake") + +file(GLOB_RECURSE SOURCE_FILES "source/*.c*") + +tactility_add_module(gpio-encoder-module + SRCS ${SOURCE_FILES} + INCLUDE_DIRS include/ + REQUIRES TactilityKernel driver +) diff --git a/Drivers/gpio-encoder-module/LICENSE-Apache-2.0.md b/Drivers/gpio-encoder-module/LICENSE-Apache-2.0.md new file mode 100644 index 000000000..f5f4b8b5e --- /dev/null +++ b/Drivers/gpio-encoder-module/LICENSE-Apache-2.0.md @@ -0,0 +1,195 @@ +Apache License +============== + +_Version 2.0, January 2004_ +_<>_ + +### Terms and Conditions for use, reproduction, and distribution + +#### 1. Definitions + +“License” shall mean the terms and conditions for use, reproduction, and +distribution as defined by Sections 1 through 9 of this document. + +“Licensor” shall mean the copyright owner or entity authorized by the copyright +owner that is granting the License. + +“Legal Entity” shall mean the union of the acting entity and all other entities +that control, are controlled by, or are under common control with that entity. +For the purposes of this definition, “control” means **(i)** the power, direct or +indirect, to cause the direction or management of such entity, whether by +contract or otherwise, or **(ii)** ownership of fifty percent (50%) or more of the +outstanding shares, or **(iii)** beneficial ownership of such entity. + +“You” (or “Your”) shall mean an individual or Legal Entity exercising +permissions granted by this License. + +“Source” form shall mean the preferred form for making modifications, including +but not limited to software source code, documentation source, and configuration +files. + +“Object” form shall mean any form resulting from mechanical transformation or +translation of a Source form, including but not limited to compiled object code, +generated documentation, and conversions to other media types. + +“Work” shall mean the work of authorship, whether in Source or Object form, made +available under the License, as indicated by a copyright notice that is included +in or attached to the work (an example is provided in the Appendix below). + +“Derivative Works” shall mean any work, whether in Source or Object form, that +is based on (or derived from) the Work and for which the editorial revisions, +annotations, elaborations, or other modifications represent, as a whole, an +original work of authorship. For the purposes of this License, Derivative Works +shall not include works that remain separable from, or merely link (or bind by +name) to the interfaces of, the Work and Derivative Works thereof. + +“Contribution” shall mean any work of authorship, including the original version +of the Work and any modifications or additions to that Work or Derivative Works +thereof, that is intentionally submitted to Licensor for inclusion in the Work +by the copyright owner or by an individual or Legal Entity authorized to submit +on behalf of the copyright owner. For the purposes of this definition, +“submitted” means any form of electronic, verbal, or written communication sent +to the Licensor or its representatives, including but not limited to +communication on electronic mailing lists, source code control systems, and +issue tracking systems that are managed by, or on behalf of, the Licensor for +the purpose of discussing and improving the Work, but excluding communication +that is conspicuously marked or otherwise designated in writing by the copyright +owner as “Not a Contribution.” + +“Contributor” shall mean Licensor and any individual or Legal Entity on behalf +of whom a Contribution has been received by Licensor and subsequently +incorporated within the Work. + +#### 2. Grant of Copyright License + +Subject to the terms and conditions of this License, each Contributor hereby +grants to You a perpetual, worldwide, non-exclusive, no-charge, royalty-free, +irrevocable copyright license to reproduce, prepare Derivative Works of, +publicly display, publicly perform, sublicense, and distribute the Work and such +Derivative Works in Source or Object form. + +#### 3. Grant of Patent License + +Subject to the terms and conditions of this License, each Contributor hereby +grants to You a perpetual, worldwide, non-exclusive, no-charge, royalty-free, +irrevocable (except as stated in this section) patent license to make, have +made, use, offer to sell, sell, import, and otherwise transfer the Work, where +such license applies only to those patent claims licensable by such Contributor +that are necessarily infringed by their Contribution(s) alone or by combination +of their Contribution(s) with the Work to which such Contribution(s) was +submitted. If You institute patent litigation against any entity (including a +cross-claim or counterclaim in a lawsuit) alleging that the Work or a +Contribution incorporated within the Work constitutes direct or contributory +patent infringement, then any patent licenses granted to You under this License +for that Work shall terminate as of the date such litigation is filed. + +#### 4. Redistribution + +You may reproduce and distribute copies of the Work or Derivative Works thereof +in any medium, with or without modifications, and in Source or Object form, +provided that You meet the following conditions: + +* **(a)** You must give any other recipients of the Work or Derivative Works a copy of +this License; and +* **(b)** You must cause any modified files to carry prominent notices stating that You +changed the files; and +* **(c)** You must retain, in the Source form of any Derivative Works that You distribute, +all copyright, patent, trademark, and attribution notices from the Source form +of the Work, excluding those notices that do not pertain to any part of the +Derivative Works; and +* **(d)** If the Work includes a “NOTICE” text file as part of its distribution, then any +Derivative Works that You distribute must include a readable copy of the +attribution notices contained within such NOTICE file, excluding those notices +that do not pertain to any part of the Derivative Works, in at least one of the +following places: within a NOTICE text file distributed as part of the +Derivative Works; within the Source form or documentation, if provided along +with the Derivative Works; or, within a display generated by the Derivative +Works, if and wherever such third-party notices normally appear. The contents of +the NOTICE file are for informational purposes only and do not modify the +License. You may add Your own attribution notices within Derivative Works that +You distribute, alongside or as an addendum to the NOTICE text from the Work, +provided that such additional attribution notices cannot be construed as +modifying the License. + +You may add Your own copyright statement to Your modifications and may provide +additional or different license terms and conditions for use, reproduction, or +distribution of Your modifications, or for any such Derivative Works as a whole, +provided Your use, reproduction, and distribution of the Work otherwise complies +with the conditions stated in this License. + +#### 5. Submission of Contributions + +Unless You explicitly state otherwise, any Contribution intentionally submitted +for inclusion in the Work by You to the Licensor shall be under the terms and +conditions of this License, without any additional terms or conditions. +Notwithstanding the above, nothing herein shall supersede or modify the terms of +any separate license agreement you may have executed with Licensor regarding +such Contributions. + +#### 6. Trademarks + +This License does not grant permission to use the trade names, trademarks, +service marks, or product names of the Licensor, except as required for +reasonable and customary use in describing the origin of the Work and +reproducing the content of the NOTICE file. + +#### 7. Disclaimer of Warranty + +Unless required by applicable law or agreed to in writing, Licensor provides the +Work (and each Contributor provides its Contributions) on an “AS IS” BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied, +including, without limitation, any warranties or conditions of TITLE, +NON-INFRINGEMENT, MERCHANTABILITY, or FITNESS FOR A PARTICULAR PURPOSE. You are +solely responsible for determining the appropriateness of using or +redistributing the Work and assume any risks associated with Your exercise of +permissions under this License. + +#### 8. Limitation of Liability + +In no event and under no legal theory, whether in tort (including negligence), +contract, or otherwise, unless required by applicable law (such as deliberate +and grossly negligent acts) or agreed to in writing, shall any Contributor be +liable to You for damages, including any direct, indirect, special, incidental, +or consequential damages of any character arising as a result of this License or +out of the use or inability to use the Work (including but not limited to +damages for loss of goodwill, work stoppage, computer failure or malfunction, or +any and all other commercial damages or losses), even if such Contributor has +been advised of the possibility of such damages. + +#### 9. Accepting Warranty or Additional Liability + +While redistributing the Work or Derivative Works thereof, You may choose to +offer, and charge a fee for, acceptance of support, warranty, indemnity, or +other liability obligations and/or rights consistent with this License. However, +in accepting such obligations, You may act only on Your own behalf and on Your +sole responsibility, not on behalf of any other Contributor, and only if You +agree to indemnify, defend, and hold each Contributor harmless for any liability +incurred by, or claims asserted against, such Contributor by reason of your +accepting any such warranty or additional liability. + +_END OF TERMS AND CONDITIONS_ + +### APPENDIX: How to apply the Apache License to your work + +To apply the Apache License to your work, attach the following boilerplate +notice, with the fields enclosed by brackets `[]` replaced with your own +identifying information. (Don't include the brackets!) The text should be +enclosed in the appropriate comment syntax for the file format. We also +recommend that a file or class name and description of purpose be included on +the same “printed page” as the copyright notice for easier identification within +third-party archives. + + Copyright [yyyy] [name of copyright owner] + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. + diff --git a/Drivers/gpio-encoder-module/bindings/tactility,gpio-encoder.yaml b/Drivers/gpio-encoder-module/bindings/tactility,gpio-encoder.yaml new file mode 100644 index 000000000..d6321e3e8 --- /dev/null +++ b/Drivers/gpio-encoder-module/bindings/tactility,gpio-encoder.yaml @@ -0,0 +1,29 @@ +description: > + GPIO-attached rotary encoder wheel - a 2-phase quadrature encoder (decoded via the ESP32 + hardware PCNT peripheral) plus an optional separate click/enter button. Exposes a + KEYBOARD_TYPE device: each wheel detent is translated to an arrow up/down key, and the + button (if present) press/release is translated to the enter key. + +compatible: "tactility,gpio-encoder" + +properties: + pin-a: + type: phandles + required: true + description: Quadrature phase A GPIO pin + pin-b: + type: phandles + required: true + description: Quadrature phase B GPIO pin + pin-enter: + type: phandles + default: GPIO_PIN_SPEC_NONE + description: Optional click/enter button GPIO pin (active low) + pulses-per-detent: + type: int + default: 4 + description: Quadrature pulses per mechanical detent + pending-capacity: + type: int + default: 16 + description: Capacity of the queue buffering key events between read_key() polls diff --git a/Drivers/gpio-encoder-module/devicetree.yaml b/Drivers/gpio-encoder-module/devicetree.yaml new file mode 100644 index 000000000..a07d6f334 --- /dev/null +++ b/Drivers/gpio-encoder-module/devicetree.yaml @@ -0,0 +1,3 @@ +dependencies: + - TactilityKernel +bindings: bindings diff --git a/Drivers/gpio-encoder-module/include/bindings/gpio_encoder.h b/Drivers/gpio-encoder-module/include/bindings/gpio_encoder.h new file mode 100644 index 000000000..458012a60 --- /dev/null +++ b/Drivers/gpio-encoder-module/include/bindings/gpio_encoder.h @@ -0,0 +1,7 @@ +// SPDX-License-Identifier: Apache-2.0 +#pragma once + +#include +#include + +DEFINE_DEVICETREE(gpio_encoder, struct GpioEncoderConfig) diff --git a/Drivers/gpio-encoder-module/include/drivers/gpio_encoder.h b/Drivers/gpio-encoder-module/include/drivers/gpio_encoder.h new file mode 100644 index 000000000..6afdb6aa8 --- /dev/null +++ b/Drivers/gpio-encoder-module/include/drivers/gpio_encoder.h @@ -0,0 +1,25 @@ +// SPDX-License-Identifier: Apache-2.0 +#pragma once + +#ifdef __cplusplus +extern "C" { +#endif + +#include + +struct GpioEncoderConfig { + // First pin of encoder wheel + struct GpioPinSpec pin_a; + // Second pin of encoder wheel + struct GpioPinSpec pin_b; + // "Button" pin of encoder wheel. Optional: GPIO_PIN_SPEC_NONE when the wheel has no click/enter button. + struct GpioPinSpec pin_enter; + // Quadrature pulses per mechanical detent (x4 decode gives 4 pulses/detent for a standard EC11-style encoder). + uint8_t pulses_per_detent; + // Capacity of the queue buffering key events between read_key() polls. + uint8_t pending_capacity; +}; + +#ifdef __cplusplus +} +#endif diff --git a/Drivers/gpio-encoder-module/include/gpio_encoder_module.h b/Drivers/gpio-encoder-module/include/gpio_encoder_module.h new file mode 100644 index 000000000..86bec5158 --- /dev/null +++ b/Drivers/gpio-encoder-module/include/gpio_encoder_module.h @@ -0,0 +1,14 @@ +// SPDX-License-Identifier: Apache-2.0 +#pragma once + +#include + +#ifdef __cplusplus +extern "C" { +#endif + +extern struct Module gpio_encoder_module; + +#ifdef __cplusplus +} +#endif diff --git a/Drivers/lilygo-module/source/tpager_encoder.cpp b/Drivers/gpio-encoder-module/source/gpio_encoder.cpp similarity index 52% rename from Drivers/lilygo-module/source/tpager_encoder.cpp rename to Drivers/gpio-encoder-module/source/gpio_encoder.cpp index c5f92680d..92ec65472 100644 --- a/Drivers/lilygo-module/source/tpager_encoder.cpp +++ b/Drivers/gpio-encoder-module/source/gpio_encoder.cpp @@ -1,49 +1,60 @@ // SPDX-License-Identifier: Apache-2.0 -#include +#include #include #include #include #include #include +#include #include #include #include -#define TAG "tpager_encoder" -#define GET_CONFIG(device) (static_cast((device)->config)) -#define GET_INTERNAL(device) (static_cast(device_get_driver_data(device))) +#define TAG "gpio_encoder" +#define GET_CONFIG(device) (static_cast((device)->config)) +#define GET_INTERNAL(device) (static_cast(device_get_driver_data(device))) -struct TpagerEncoderInternal { +struct GpioEncoderPendingEvent { + uint32_t key; + bool pressed; +}; + +struct GpioEncoderInternal { pcnt_unit_handle_t pcnt_unit = nullptr; GpioDescriptor* pin_enter = nullptr; + int32_t pulse_remainder = 0; + bool button_pressed = false; + int32_t pulses_per_detent = 0; + GpioEncoderPendingEvent* pending = nullptr; + uint32_t pending_capacity = 0; + uint32_t pending_head = 0; + uint32_t pending_count = 0; }; -extern "C" { - -static error_t read_delta(Device* device, int32_t* out_pulses) { - auto* internal = GET_INTERNAL(device); - int pulses = 0; - pcnt_unit_get_count(internal->pcnt_unit, &pulses); - pcnt_unit_clear_count(internal->pcnt_unit); - *out_pulses = pulses; - return ERROR_NONE; -} - -static error_t get_button_pressed(Device* device, bool* out_pressed) { - auto* internal = GET_INTERNAL(device); - return gpio_descriptor_get_level(internal->pin_enter, out_pressed); +static void push_pending(GpioEncoderInternal* internal, uint32_t key, bool pressed) { + if (internal->pending_count >= internal->pending_capacity) { + LOG_W(TAG, "Pending event queue full, dropping event"); + return; + } + uint32_t tail = (internal->pending_head + internal->pending_count) % internal->pending_capacity; + internal->pending[tail] = { .key = key, .pressed = pressed }; + internal->pending_count++; } -error_t tpager_encoder_read_delta(Device* device, int32_t* out_pulses) { - return read_delta(device, out_pulses); +static bool pop_pending(GpioEncoderInternal* internal, GpioEncoderPendingEvent* out_event) { + if (internal->pending_count == 0) { + return false; + } + *out_event = internal->pending[internal->pending_head]; + internal->pending_head = (internal->pending_head + 1) % internal->pending_capacity; + internal->pending_count--; + return true; } -error_t tpager_encoder_get_button_pressed(Device* device, bool* out_pressed) { - return get_button_pressed(device, out_pressed); -} +extern "C" { // region Driver lifecycle @@ -53,7 +64,7 @@ error_t tpager_encoder_get_button_pressed(Device* device, bool* out_pressed) { static constexpr int PCNT_LOW_LIMIT = -127; static constexpr int PCNT_HIGH_LIMIT = 126; -static error_t init_pcnt_unit(const TpagerEncoderConfig* config, pcnt_unit_handle_t* out_unit) { +static error_t init_pcnt_unit(const GpioEncoderConfig* config, pcnt_unit_handle_t* out_unit) { pcnt_unit_config_t unit_config = { .low_limit = PCNT_LOW_LIMIT, .high_limit = PCNT_HIGH_LIMIT, @@ -131,23 +142,35 @@ static error_t init_pcnt_unit(const TpagerEncoderConfig* config, pcnt_unit_handl static error_t start(Device* device) { const auto* config = GET_CONFIG(device); - auto* internal = new (std::nothrow) TpagerEncoderInternal(); + auto* internal = new (std::nothrow) GpioEncoderInternal(); if (internal == nullptr) { return ERROR_OUT_OF_MEMORY; } + internal->pulses_per_detent = static_cast(config->pulses_per_detent); + internal->pending_capacity = config->pending_capacity; + + internal->pending = new (std::nothrow) GpioEncoderPendingEvent[internal->pending_capacity]; + if (internal->pending == nullptr) { + delete internal; + return ERROR_OUT_OF_MEMORY; + } error_t error = init_pcnt_unit(config, &internal->pcnt_unit); if (error != ERROR_NONE) { + delete[] internal->pending; delete internal; return error; } - internal->pin_enter = gpio_descriptor_acquire(config->pin_enter.gpio_controller, config->pin_enter.pin, GPIO_FLAG_DIRECTION_INPUT | GPIO_FLAG_ACTIVE_LOW, GPIO_OWNER_GPIO); - if (internal->pin_enter == nullptr) { - pcnt_unit_stop(internal->pcnt_unit); - pcnt_del_unit(internal->pcnt_unit); - delete internal; - return ERROR_RESOURCE; + if (config->pin_enter.gpio_controller != nullptr) { + internal->pin_enter = gpio_descriptor_acquire(config->pin_enter.gpio_controller, config->pin_enter.pin, GPIO_FLAG_DIRECTION_INPUT | GPIO_FLAG_ACTIVE_LOW, GPIO_OWNER_GPIO); + if (internal->pin_enter == nullptr) { + pcnt_unit_stop(internal->pcnt_unit); + pcnt_del_unit(internal->pcnt_unit); + delete[] internal->pending; + delete internal; + return ERROR_RESOURCE; + } } device_set_driver_data(device, internal); @@ -157,7 +180,9 @@ static error_t start(Device* device) { static error_t stop(Device* device) { auto* internal = GET_INTERNAL(device); - gpio_descriptor_release(internal->pin_enter); + if (internal->pin_enter != nullptr) { + gpio_descriptor_release(internal->pin_enter); + } if (pcnt_unit_stop(internal->pcnt_unit) != ESP_OK) { LOG_W(TAG, "Failed to stop encoder"); @@ -167,31 +192,84 @@ static error_t stop(Device* device) { } device_set_driver_data(device, nullptr); + delete[] internal->pending; delete internal; return ERROR_NONE; } // endregion -static constexpr TpagerEncoderApi TPAGER_ENCODER_API = { - .read_delta = read_delta, - .get_button_pressed = get_button_pressed, -}; +// region KeyboardApi + +// Wheel rotation is a discrete notch, not a held key, so each detent is reported as an +// immediate press+release pair rather than a persistent pressed state. +static void poll_wheel(GpioEncoderInternal* internal) { + int pulses = 0; + pcnt_unit_get_count(internal->pcnt_unit, &pulses); + pcnt_unit_clear_count(internal->pcnt_unit); + + int32_t total = internal->pulse_remainder + pulses; + int32_t detents = total / internal->pulses_per_detent; + internal->pulse_remainder = total % internal->pulses_per_detent; + + uint32_t key = detents >= 0 ? CODEPOINT_ARROW_DOWN : CODEPOINT_ARROW_UP; + for (int32_t i = 0; i < (detents >= 0 ? detents : -detents); i++) { + push_pending(internal, key, true); + push_pending(internal, key, false); + } +} + +static void poll_button(GpioEncoderInternal* internal) { + if (internal->pin_enter == nullptr) { + return; + } + + bool pressed = false; + if (gpio_descriptor_get_level(internal->pin_enter, &pressed) != ERROR_NONE) { + return; + } + if (pressed != internal->button_pressed) { + internal->button_pressed = pressed; + push_pending(internal, CODEPOINT_ENTER, pressed); + } +} + +static error_t gpio_encoder_read_key(Device* device, KeyboardKeyData* data) { + auto* internal = GET_INTERNAL(device); + + poll_wheel(internal); + poll_button(internal); + + GpioEncoderPendingEvent event; + if (pop_pending(internal, &event)) { + data->key = event.key; + data->pressed = event.pressed; + data->continue_reading = internal->pending_count > 0; + } else { + data->key = 0; + data->pressed = false; + data->continue_reading = false; + } + + return ERROR_NONE; +} + +// endregion -const struct DeviceType TPAGER_ENCODER_TYPE { - .name = "tpager-encoder" +static constexpr KeyboardApi GPIO_ENCODER_API = { + .read_key = gpio_encoder_read_key, }; -extern Module lilygo_module; +extern Module gpio_encoder_module; -Driver tpager_encoder_driver = { - .name = "tpager_encoder", - .compatible = (const char*[]) { "lilygo,tpager-encoder", nullptr }, +Driver gpio_encoder_driver = { + .name = "gpio_encoder", + .compatible = (const char*[]) { "tactility,gpio-encoder", nullptr }, .start_device = start, .stop_device = stop, - .api = &TPAGER_ENCODER_API, - .device_type = &TPAGER_ENCODER_TYPE, - .owner = &lilygo_module, + .api = &GPIO_ENCODER_API, + .device_type = &KEYBOARD_TYPE, + .owner = &gpio_encoder_module, .internal = nullptr }; diff --git a/Drivers/gpio-encoder-module/source/module.cpp b/Drivers/gpio-encoder-module/source/module.cpp new file mode 100644 index 000000000..4de7b72f6 --- /dev/null +++ b/Drivers/gpio-encoder-module/source/module.cpp @@ -0,0 +1,19 @@ +// SPDX-License-Identifier: Apache-2.0 +#include +#include + +extern "C" { + +extern Driver gpio_encoder_driver; + +static Driver* const gpio_encoder_drivers[] = { + &gpio_encoder_driver, + nullptr +}; + +Module gpio_encoder_module = { + .name = "gpio-encoder", + .drivers = gpio_encoder_drivers +}; + +} // extern "C" diff --git a/Drivers/lilygo-module/bindings/lilygo,tpager-encoder.yaml b/Drivers/lilygo-module/bindings/lilygo,tpager-encoder.yaml deleted file mode 100644 index 48db9625f..000000000 --- a/Drivers/lilygo-module/bindings/lilygo,tpager-encoder.yaml +++ /dev/null @@ -1,22 +0,0 @@ -description: > - LilyGO T-Lora Pager encoder wheel next to the display - a 2-phase quadrature encoder - (decoded via the ESP32 hardware PCNT peripheral) plus a separate click/enter button. - Reports raw, unscaled pulses and button level: the pulses-per-detent scaling and enter-press - debounce are UI concerns layered on top by the consumer (see tpager_encoder_input.h), not - something this driver knows about. - -compatible: "lilygo,tpager-encoder" - -properties: - pin-a: - type: phandles - required: true - description: Quadrature phase A GPIO pin - pin-b: - type: phandles - required: true - description: Quadrature phase B GPIO pin - pin-enter: - type: phandles - required: true - description: Click/enter button GPIO pin (active low) diff --git a/Drivers/lilygo-module/include/lilygo/bindings/tpager_encoder.h b/Drivers/lilygo-module/include/lilygo/bindings/tpager_encoder.h deleted file mode 100644 index 1d4f5e08a..000000000 --- a/Drivers/lilygo-module/include/lilygo/bindings/tpager_encoder.h +++ /dev/null @@ -1,7 +0,0 @@ -// SPDX-License-Identifier: Apache-2.0 -#pragma once - -#include -#include - -DEFINE_DEVICETREE(tpager_encoder, struct TpagerEncoderConfig) diff --git a/Drivers/lilygo-module/include/lilygo/drivers/tpager_encoder.h b/Drivers/lilygo-module/include/lilygo/drivers/tpager_encoder.h deleted file mode 100644 index 982af26b8..000000000 --- a/Drivers/lilygo-module/include/lilygo/drivers/tpager_encoder.h +++ /dev/null @@ -1,59 +0,0 @@ -// SPDX-License-Identifier: Apache-2.0 -#pragma once - -#ifdef __cplusplus -extern "C" { -#endif - -#include -#include -#include -#include - -struct Device; -struct DeviceType; - -struct TpagerEncoderConfig { - struct GpioPinSpec pin_a; - struct GpioPinSpec pin_b; - struct GpioPinSpec pin_enter; -}; - -/** - * @brief API for the T-Lora Pager encoder wheel driver. - * Reports raw, unscaled pulses: pulses-per-detent scaling and enter-press debounce are UI - * concerns layered on top by the consumer, not something this driver knows about. - */ -struct TpagerEncoderApi { - /** - * @brief Reads the accumulated pulse count since the last read, then resets it to zero. - * @param[in] device the encoder device - * @param[out] out_pulses accumulated quadrature pulses (positive/negative by direction) - * @retval ERROR_NONE when the operation was successful - */ - error_t (*read_delta)(struct Device* device, int32_t* out_pulses); - - /** - * @brief Gets whether the enter button is currently pressed. - * @param[in] device the encoder device - * @param[out] out_pressed true when pressed - * @retval ERROR_NONE when the operation was successful - */ - error_t (*get_button_pressed)(struct Device* device, bool* out_pressed); -}; - -/** - * @brief Reads the accumulated pulse count using the specified encoder device. - */ -error_t tpager_encoder_read_delta(struct Device* device, int32_t* out_pulses); - -/** - * @brief Gets whether the enter button is currently pressed on the specified encoder device. - */ -error_t tpager_encoder_get_button_pressed(struct Device* device, bool* out_pressed); - -extern const struct DeviceType TPAGER_ENCODER_TYPE; - -#ifdef __cplusplus -} -#endif diff --git a/Drivers/lilygo-module/source/module.cpp b/Drivers/lilygo-module/source/module.cpp index 87b301abf..32947a397 100644 --- a/Drivers/lilygo-module/source/module.cpp +++ b/Drivers/lilygo-module/source/module.cpp @@ -6,12 +6,10 @@ extern "C" { extern Driver tdeck_keyboard_driver; extern Driver tdeck_keyboard_backlight_driver; -extern Driver tpager_encoder_driver; static Driver* const lilygo_drivers[] = { &tdeck_keyboard_driver, &tdeck_keyboard_backlight_driver, - &tpager_encoder_driver, nullptr }; From 30271b09393cd7cfe3693272231c82eec0a1abf6 Mon Sep 17 00:00:00 2001 From: Ken Van Hoeylandt Date: Sun, 23 Aug 2026 12:34:48 +0200 Subject: [PATCH 13/14] Fix for memory alloc --- TactilityKernel/source/memory_esp32.cpp | 5 ++++- device.py | 4 ++++ 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/TactilityKernel/source/memory_esp32.cpp b/TactilityKernel/source/memory_esp32.cpp index 44d467925..af6f15632 100644 --- a/TactilityKernel/source/memory_esp32.cpp +++ b/TactilityKernel/source/memory_esp32.cpp @@ -9,7 +9,10 @@ namespace { uint32_t toHeapCaps(uint16_t capabilityFlags) { uint32_t caps = 0; - if (capabilityFlags & MEMORY_CAPABILITY_INTERNAL) caps |= MALLOC_CAP_INTERNAL; + // MALLOC_CAP_INTERNAL alone can be satisfied by IRAM (tagged INTERNAL on ESP32's heap + // layout), which is word-only and fails FreeRTOS's byte-accessibility checks for things + // like a static task's TCB. 8BIT keeps this capability meaning genuinely internal RAM. + if (capabilityFlags & MEMORY_CAPABILITY_INTERNAL) caps |= MALLOC_CAP_INTERNAL | MALLOC_CAP_8BIT; if (capabilityFlags & MEMORY_CAPABILITY_EXTERNAL) caps |= MALLOC_CAP_SPIRAM; if (capabilityFlags & MEMORY_CAPABILITY_EXECUTABLE) caps |= MALLOC_CAP_EXEC; if (capabilityFlags & MEMORY_CAPABILITY_DMA) caps |= MALLOC_CAP_DMA; diff --git a/device.py b/device.py index 1cae58b5c..aaa6c58e7 100644 --- a/device.py +++ b/device.py @@ -192,6 +192,10 @@ def write_spiram_variables(output_file, device_properties: dict): output_file.write("CONFIG_SPIRAM_MEMTEST=n\n") # Enable output_file.write("CONFIG_SPIRAM=y\n") + # Defaults to n on the classic ESP32 target (unlike S2/S3/etc), but Tactility relies on it + # to put some task stacks in SPIRAM (see UsbHidInput.cpp) - without it, xTaskCreateStatic() + # asserts at runtime when handed a SPIRAM-backed stack buffer. + output_file.write("CONFIG_FREERTOS_TASK_CREATE_ALLOW_EXT_MEM=y\n") output_file.write(f"CONFIG_{idf_target.upper()}_SPIRAM_SUPPORT=y\n") mode = get_property_or_exit(device_properties, "hardware.spiRamMode") if mode == "OPI": From 6b713d6d6cdff419865618fe75987145b2e0cced Mon Sep 17 00:00:00 2001 From: Ken Van Hoeylandt Date: Sun, 23 Aug 2026 13:25:44 +0200 Subject: [PATCH 14/14] Fixes --- Drivers/gpio-encoder-module/CMakeLists.txt | 1 + .../bindings/tactility,gpio-encoder.yaml | 6 +- .../source/gpio_encoder.cpp | 83 ++++++++++++++++--- TactilityKernel/source/memory_esp32.cpp | 14 +++- 4 files changed, 88 insertions(+), 16 deletions(-) diff --git a/Drivers/gpio-encoder-module/CMakeLists.txt b/Drivers/gpio-encoder-module/CMakeLists.txt index bb3934b07..0e57e42e8 100644 --- a/Drivers/gpio-encoder-module/CMakeLists.txt +++ b/Drivers/gpio-encoder-module/CMakeLists.txt @@ -8,4 +8,5 @@ tactility_add_module(gpio-encoder-module SRCS ${SOURCE_FILES} INCLUDE_DIRS include/ REQUIRES TactilityKernel driver + PRIV_REQUIRES esp_driver_pcnt ) diff --git a/Drivers/gpio-encoder-module/bindings/tactility,gpio-encoder.yaml b/Drivers/gpio-encoder-module/bindings/tactility,gpio-encoder.yaml index d6321e3e8..aa22d3f89 100644 --- a/Drivers/gpio-encoder-module/bindings/tactility,gpio-encoder.yaml +++ b/Drivers/gpio-encoder-module/bindings/tactility,gpio-encoder.yaml @@ -21,9 +21,13 @@ properties: description: Optional click/enter button GPIO pin (active low) pulses-per-detent: type: int + min: 1 + max: 255 default: 4 description: Quadrature pulses per mechanical detent pending-capacity: type: int + min: 2 + max: 255 default: 16 - description: Capacity of the queue buffering key events between read_key() polls + description: Capacity of the queue buffering key events between read_key() polls. Must be at least 2 to hold one wheel press/release pair. diff --git a/Drivers/gpio-encoder-module/source/gpio_encoder.cpp b/Drivers/gpio-encoder-module/source/gpio_encoder.cpp index 92ec65472..613a20411 100644 --- a/Drivers/gpio-encoder-module/source/gpio_encoder.cpp +++ b/Drivers/gpio-encoder-module/source/gpio_encoder.cpp @@ -24,6 +24,8 @@ struct GpioEncoderPendingEvent { struct GpioEncoderInternal { pcnt_unit_handle_t pcnt_unit = nullptr; + GpioDescriptor* pin_a = nullptr; + GpioDescriptor* pin_b = nullptr; GpioDescriptor* pin_enter = nullptr; int32_t pulse_remainder = 0; bool button_pressed = false; @@ -34,14 +36,15 @@ struct GpioEncoderInternal { uint32_t pending_count = 0; }; -static void push_pending(GpioEncoderInternal* internal, uint32_t key, bool pressed) { +static bool push_pending(GpioEncoderInternal* internal, uint32_t key, bool pressed) { if (internal->pending_count >= internal->pending_capacity) { LOG_W(TAG, "Pending event queue full, dropping event"); - return; + return false; } uint32_t tail = (internal->pending_head + internal->pending_count) % internal->pending_capacity; internal->pending[tail] = { .key = key, .pressed = pressed }; internal->pending_count++; + return true; } static bool pop_pending(GpioEncoderInternal* internal, GpioEncoderPendingEvent* out_event) { @@ -64,7 +67,7 @@ extern "C" { static constexpr int PCNT_LOW_LIMIT = -127; static constexpr int PCNT_HIGH_LIMIT = 126; -static error_t init_pcnt_unit(const GpioEncoderConfig* config, pcnt_unit_handle_t* out_unit) { +static error_t init_pcnt_unit(int pin_a, int pin_b, pcnt_unit_handle_t* out_unit) { pcnt_unit_config_t unit_config = { .low_limit = PCNT_LOW_LIMIT, .high_limit = PCNT_HIGH_LIMIT, @@ -86,13 +89,13 @@ static error_t init_pcnt_unit(const GpioEncoderConfig* config, pcnt_unit_handle_ } pcnt_chan_config_t chan_a_config = { - .edge_gpio_num = static_cast(config->pin_b.pin), - .level_gpio_num = static_cast(config->pin_a.pin), + .edge_gpio_num = pin_b, + .level_gpio_num = pin_a, .flags = {}, }; pcnt_chan_config_t chan_b_config = { - .edge_gpio_num = static_cast(config->pin_a.pin), - .level_gpio_num = static_cast(config->pin_b.pin), + .edge_gpio_num = pin_a, + .level_gpio_num = pin_b, .flags = {}, }; @@ -142,6 +145,17 @@ static error_t init_pcnt_unit(const GpioEncoderConfig* config, pcnt_unit_handle_ static error_t start(Device* device) { const auto* config = GET_CONFIG(device); + // Backstop for values the devicetree compiler doesn't currently validate: 0 divides by + // zero in poll_wheel(), and a capacity below 2 can never hold one press/release pair. + if (config->pulses_per_detent == 0) { + LOG_E(TAG, "pulses_per_detent must be > 0"); + return ERROR_INVALID_ARGUMENT; + } + if (config->pending_capacity < 2) { + LOG_E(TAG, "pending_capacity must be >= 2"); + return ERROR_INVALID_ARGUMENT; + } + auto* internal = new (std::nothrow) GpioEncoderInternal(); if (internal == nullptr) { return ERROR_OUT_OF_MEMORY; @@ -155,8 +169,39 @@ static error_t start(Device* device) { return ERROR_OUT_OF_MEMORY; } - error_t error = init_pcnt_unit(config, &internal->pcnt_unit); + internal->pin_a = gpio_descriptor_acquire(config->pin_a.gpio_controller, config->pin_a.pin, config->pin_a.flags | GPIO_FLAG_DIRECTION_INPUT, GPIO_OWNER_GPIO); + if (internal->pin_a == nullptr) { + LOG_E(TAG, "Failed to acquire pin_a"); + delete[] internal->pending; + delete internal; + return ERROR_RESOURCE; + } + + internal->pin_b = gpio_descriptor_acquire(config->pin_b.gpio_controller, config->pin_b.pin, config->pin_b.flags | GPIO_FLAG_DIRECTION_INPUT, GPIO_OWNER_GPIO); + if (internal->pin_b == nullptr) { + LOG_E(TAG, "Failed to acquire pin_b"); + gpio_descriptor_release(internal->pin_a); + delete[] internal->pending; + delete internal; + return ERROR_RESOURCE; + } + + int native_pin_a = 0; + int native_pin_b = 0; + if (gpio_descriptor_get_native_pin_number(internal->pin_a, &native_pin_a) != ERROR_NONE || + gpio_descriptor_get_native_pin_number(internal->pin_b, &native_pin_b) != ERROR_NONE) { + LOG_E(TAG, "Failed to resolve native pin numbers"); + gpio_descriptor_release(internal->pin_b); + gpio_descriptor_release(internal->pin_a); + delete[] internal->pending; + delete internal; + return ERROR_RESOURCE; + } + + error_t error = init_pcnt_unit(native_pin_a, native_pin_b, &internal->pcnt_unit); if (error != ERROR_NONE) { + gpio_descriptor_release(internal->pin_b); + gpio_descriptor_release(internal->pin_a); delete[] internal->pending; delete internal; return error; @@ -167,6 +212,8 @@ static error_t start(Device* device) { if (internal->pin_enter == nullptr) { pcnt_unit_stop(internal->pcnt_unit); pcnt_del_unit(internal->pcnt_unit); + gpio_descriptor_release(internal->pin_b); + gpio_descriptor_release(internal->pin_a); delete[] internal->pending; delete internal; return ERROR_RESOURCE; @@ -191,6 +238,9 @@ static error_t stop(Device* device) { LOG_W(TAG, "Failed to delete encoder"); } + gpio_descriptor_release(internal->pin_b); + gpio_descriptor_release(internal->pin_a); + device_set_driver_data(device, nullptr); delete[] internal->pending; delete internal; @@ -213,7 +263,14 @@ static void poll_wheel(GpioEncoderInternal* internal) { internal->pulse_remainder = total % internal->pulses_per_detent; uint32_t key = detents >= 0 ? CODEPOINT_ARROW_DOWN : CODEPOINT_ARROW_UP; - for (int32_t i = 0; i < (detents >= 0 ? detents : -detents); i++) { + int32_t count = detents >= 0 ? detents : -detents; + for (int32_t i = 0; i < count; i++) { + // A press without its matching release would leave the consumer thinking the key + // is stuck down, so only enqueue the pair when both fit. + if (internal->pending_count + 2 > internal->pending_capacity) { + LOG_W(TAG, "Pending event queue full, dropping remaining wheel events"); + break; + } push_pending(internal, key, true); push_pending(internal, key, false); } @@ -228,9 +285,13 @@ static void poll_button(GpioEncoderInternal* internal) { if (gpio_descriptor_get_level(internal->pin_enter, &pressed) != ERROR_NONE) { return; } + // Only commit the new state once its event is actually queued - a full FIFO here + // leaves button_pressed unchanged so the same transition is retried next poll instead + // of being lost. if (pressed != internal->button_pressed) { - internal->button_pressed = pressed; - push_pending(internal, CODEPOINT_ENTER, pressed); + if (push_pending(internal, CODEPOINT_ENTER, pressed)) { + internal->button_pressed = pressed; + } } } diff --git a/TactilityKernel/source/memory_esp32.cpp b/TactilityKernel/source/memory_esp32.cpp index af6f15632..6228779b0 100644 --- a/TactilityKernel/source/memory_esp32.cpp +++ b/TactilityKernel/source/memory_esp32.cpp @@ -9,10 +9,16 @@ namespace { uint32_t toHeapCaps(uint16_t capabilityFlags) { uint32_t caps = 0; - // MALLOC_CAP_INTERNAL alone can be satisfied by IRAM (tagged INTERNAL on ESP32's heap - // layout), which is word-only and fails FreeRTOS's byte-accessibility checks for things - // like a static task's TCB. 8BIT keeps this capability meaning genuinely internal RAM. - if (capabilityFlags & MEMORY_CAPABILITY_INTERNAL) caps |= MALLOC_CAP_INTERNAL | MALLOC_CAP_8BIT; + if (capabilityFlags & MEMORY_CAPABILITY_INTERNAL) { + caps |= MALLOC_CAP_INTERNAL; + // MALLOC_CAP_INTERNAL alone can be satisfied by IRAM (tagged INTERNAL on ESP32's heap + // layout), which is word-only and fails FreeRTOS's byte-accessibility checks for things + // like a static task's TCB. 8BIT keeps this capability meaning genuinely byte-accessible + // internal RAM - but skip it when EXECUTABLE is also requested, since executable IRAM + // isn't 8-bit accessible on some ESP32 targets and combining both caps could make an + // otherwise-satisfiable request (plain executable internal memory) fail outright. + if (!(capabilityFlags & MEMORY_CAPABILITY_EXECUTABLE)) caps |= MALLOC_CAP_8BIT; + } if (capabilityFlags & MEMORY_CAPABILITY_EXTERNAL) caps |= MALLOC_CAP_SPIRAM; if (capabilityFlags & MEMORY_CAPABILITY_EXECUTABLE) caps |= MALLOC_CAP_EXEC; if (capabilityFlags & MEMORY_CAPABILITY_DMA) caps |= MALLOC_CAP_DMA;