Fixes and improvements - #620
Conversation
- custom lvgl alloc - keyboard drivers now uses codepoints only (no more hardwired LV_KEY_*) - fix lvgl file lock when lvgl is stopped
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe change standardizes keyboard input around Unicode codepoints and adds LVGL translation at the input boundary. Device keymaps and keyboard drivers update Enter, navigation, and control mappings. LVGL gains a custom memory backend with ESP32 fallback handling and whole-archive configuration. File-mutex registration now uses synchronized IDs with explicit cleanup. LVGL startup and shutdown manage mutex resources and keyboard devices. A generic GPIO encoder module replaces the T-Pager-specific implementation. Statusbar subscription ordering changes, and one boot log message is removed. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 7c220b4e-a7ab-40c4-944b-8f8083854484
📒 Files selected for processing (30)
Buildscripts/module.cmakeBuildscripts/sdkconfig/default.propertiesDevices/cl32/cl32.dtsDevices/lilygo-tdeck-max/lilygo,tdeck-max.dtsDevices/lilygo-tdeck-pro/lilygo,tdeck-pro.dtsDevices/lilygo-tlora-pager/lilygo,tlora-pager.dtsDevices/m5stack-tab5/Source/devices/tab5_keyboard.cppDevices/simulator/Source/drivers/sdl_input.cppDrivers/button-control-module/source/button_control.cppDrivers/m5stack-module/source/cardputer_adv_keyboard.cppDrivers/m5stack-module/source/cardputer_keyboard.cppDrivers/tca8418-module/bindings/ti,tca8418.yamlModules/lvgl-module/CMakeLists.txtModules/lvgl-module/README.mdModules/lvgl-module/include/lvgl/devices/keyboard.hModules/lvgl-module/source/devices/keyboard.cppModules/lvgl-module/source/lv_mem_custom.cPlatforms/platform-esp32/source/drivers/usb/esp32_usbhost_hid.cppTactility/Source/Tactility.cppTactility/Source/app/boot/Boot.cppTactility/Source/file/FileMutexLvgl.cppTactility/Source/lvgl/KeyboardDeviceListener.cppTactility/Source/lvgl/Statusbar.cppTactilityKernel/include/tactility/drivers/keyboard.hTactilityKernel/include/tactility/filesystem/file_mutex.hTactilityKernel/source/filesystem/file_mutex.cppTactilityKernel/source/memory_esp32.cppTactilityKernel/source/symbols.cTactilityKernel/tests/source/file_mutex_test.cpplv_conf.h
💤 Files with no reviewable changes (1)
- Tactility/Source/app/boot/Boot.cpp
Included review availability: Your plan provides up to 4 included reviews per hour; 3 remain after this review.
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Drivers/tca8418-module/bindings/ti,tca8418.yaml (1)
29-38: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftUse a keymap element type that preserves full Unicode codepoints.
If
keymap-*must carry allKeyboardKeyData::keyvalues,uint8_tis too narrow. The project defines arrows as U+2190–U+2193 and Home/End as U+21F1–U+21F2. These values cannot survive an 8-bit binding. (raw.githubusercontent.com)Change the binding and generated driver storage to a 32-bit element type. Otherwise, non-character keys will be truncated or rejected before LVGL translation.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 361903be-3641-4344-93aa-6bd8c0a4adf4
📒 Files selected for processing (8)
Drivers/m5stack-module/source/cardputer_adv_keyboard.cppDrivers/m5stack-module/source/cardputer_keyboard.cppDrivers/tca8418-module/bindings/ti,tca8418.yamlModules/lvgl-module/README.mdModules/lvgl-module/include/lvgl/devices/keyboard.hModules/lvgl-module/source/devices/keyboard.cppTactility/Source/lvgl/Statusbar.cppTactilityKernel/include/tactility/drivers/keyboard.h
🚧 Files skipped from review as they are similar to previous changes (2)
- Modules/lvgl-module/README.md
- TactilityKernel/include/tactility/drivers/keyboard.h
Included review availability: Your plan provides up to 4 included reviews per hour; 2 remain after this review.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Drivers/m5stack-module/source/cardputer_keyboard.cpp (1)
257-267: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy liftUse focus-navigation codepoints for Fn group navigation.
CODEPOINT_ARROW_UPandCODEPOINT_ARROW_DOWNare translated byModules/lvgl-module/source/devices/keyboard.cpptoLV_KEY_UPandLV_KEY_DOWN. Fn+;and Fn+.can therefore toggle widgets such aslv_switch, contrary to this comment’s stated focus-navigation behavior.Return dedicated focus-navigation codepoints and translate them to the corresponding LVGL focus keys at the LVGL boundary.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 4c923851-d3dd-4c1d-9b0d-350d5c863d5e
📒 Files selected for processing (1)
Drivers/m5stack-module/source/cardputer_keyboard.cpp
Included review availability: Your plan provides up to 4 included reviews per hour; 2 remain after this review.
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
TactilityKernel/source/memory_esp32.cpp (1)
31-46: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winNormalize zero capability masks before allocation.
When
required_caps | desired_caps == 0, ESP-IDF’s heap search finds no heap, so positive-size allocations returnnullptr. This affectsmemory_alloc_with_policy(),memory_calloc_with_policy(), andmemory_realloc_with_policy(nullptr, size, ...);desired_caps == 0prevents the retry. UseMALLOC_CAP_DEFAULTfor the initial call when the combined mask is zero, while retaining the optional-capability fallback.Source: MCP tools
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 4ca22211-85d5-4901-8363-0942b43ea2c3
📒 Files selected for processing (22)
Devices/lilygo-tlora-pager/CMakeLists.txtDevices/lilygo-tlora-pager/devicetree.yamlDevices/lilygo-tlora-pager/lilygo,tlora-pager.dtsDevices/lilygo-tlora-pager/source/module.cppDrivers/gpio-encoder-module/CMakeLists.txtDrivers/gpio-encoder-module/LICENSE-Apache-2.0.mdDrivers/gpio-encoder-module/bindings/tactility,gpio-encoder.yamlDrivers/gpio-encoder-module/devicetree.yamlDrivers/gpio-encoder-module/include/bindings/gpio_encoder.hDrivers/gpio-encoder-module/include/drivers/gpio_encoder.hDrivers/gpio-encoder-module/include/gpio_encoder_module.hDrivers/gpio-encoder-module/source/gpio_encoder.cppDrivers/gpio-encoder-module/source/module.cppDrivers/lilygo-module/bindings/lilygo,tpager-encoder.yamlDrivers/lilygo-module/include/lilygo/bindings/tpager_encoder.hDrivers/lilygo-module/include/lilygo/drivers/tpager_encoder.hDrivers/lilygo-module/include/lilygo/drivers/tpager_encoder_input.hDrivers/lilygo-module/source/module.cppDrivers/lilygo-module/source/tpager_encoder_input.cppModules/lvgl-module/source/devices/keyboard.cppTactilityKernel/source/memory_esp32.cppdevice.py
💤 Files with no reviewable changes (6)
- Drivers/lilygo-module/source/module.cpp
- Drivers/lilygo-module/bindings/lilygo,tpager-encoder.yaml
- Drivers/lilygo-module/include/lilygo/bindings/tpager_encoder.h
- Drivers/lilygo-module/include/lilygo/drivers/tpager_encoder.h
- Drivers/lilygo-module/include/lilygo/drivers/tpager_encoder_input.h
- Drivers/lilygo-module/source/tpager_encoder_input.cpp
Included review availability: Your plan provides up to 4 included reviews per hour; 3 remain after this review.
1 participant
LV_KEY_*anymore.Drivers/gpio-encoder-module/). Added more features to the config file.