New kernel drivers and device implementation updates by KenVanHoeylandt · Pull Request #561 · TactilityProject/Tactility

KenVanHoeylandt · 2026-07-12T16:10:35Z

Added modular device support and devicetree bindings for ILI9341, ILI9488, CST816S, XPT2046, and GPIO button input, updating several board configurations for display/touch/backlight/keyboard/battery.
Added a setting to control deprecated HAL usage (device property + Kconfig).

- add cst816s kernel driver - add ili9341 kernel driver

coderabbitai · 2026-07-12T16:10:54Z

📝 Walkthrough

Walkthrough

The change migrates several devices from legacy board-specific HAL implementations to device-tree-configured kernel modules. It adds display, touch, keyboard, and button drivers with bindings, lifecycle modules, and public configuration structures. Device trees now describe concrete peripherals, while device dependencies and CMake registration use modular targets. Firmware gains deprecated-HAL configuration handling, and LVGL plus SPI paths are updated for partial buffering, RGB565 byte swapping, and MISO pull-ups.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 13.43% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Title check	✅ Passed	The title matches the main scope: adding new kernel drivers and updating device implementations.

✨ Finishing Touches

📝 Generate docstrings

Create stacked PR
Commit on current branch

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch develop

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.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

coderabbitai

Actionable comments posted: 6

🧹 Nitpick comments (2)

Devices/m5stack-stickc-plus2/source/module.cpp (1)
10-26: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low value

Consider checking GPIO return values or simplifying init_power().

init_power() returns bool but always returns true regardless of whether the gpio_set_direction/gpio_set_level calls succeed. Additionally, start() ignores the return value entirely. Either check the ESP-IDF error codes and propagate failures, or simplify init_power() to return void to avoid misleading callers.
♻️ Optional refactor: propagate GPIO errors
 static bool init_power() {
     // CH552 applies 4 V to GPIO 0, which reduces Wi-Fi sensitivity.
     // Setting output to high adds a bias of 3.3 V and suppresses over-voltage:
-    gpio_set_direction(GPIO_NUM_0, GPIO_MODE_OUTPUT);
-    gpio_set_level(GPIO_NUM_0, 1);
+    if (gpio_set_direction(GPIO_NUM_0, GPIO_MODE_OUTPUT) != ESP_OK) return false;
+    if (gpio_set_level(GPIO_NUM_0, 1) != ESP_OK) return false;

     // "Hold power" pin: must be set to high to keep the device powered on:
-    gpio_set_direction(GPIO_NUM_4, GPIO_MODE_OUTPUT);
-    gpio_set_level(GPIO_NUM_4, 1);
+    if (gpio_set_direction(GPIO_NUM_4, GPIO_MODE_OUTPUT) != ESP_OK) return false;
+    if (gpio_set_level(GPIO_NUM_4, 1) != ESP_OK) return false;

     return true;
 }

 static error_t start() {
-    init_power();
-    return ERROR_NONE;
+    return init_power() ? ERROR_NONE : ERROR_FAILURE;
 }
Drivers/m5stack-module/source/module.cpp (1)
19-25: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win

Consider reverse-order teardown in stop().

start() constructs cardputer_keyboard_driver first, then cardputer_adv_keyboard_driver. stop() removes them in the same order. If the advanced keyboard driver depends on the base keyboard driver being alive, removing the base first could cause issues. LIFO (reverse) order is the safer convention for resource teardown.
♻️ Proposed reverse-order teardown
 static error_t stop() {
     /* We crash when destruct fails, because if a single driver fails to destruct,
      * there is no guarantee that the previously destroyed drivers can be recovered */
-    check(driver_remove_destruct(&cardputer_keyboard_driver) == ERROR_NONE);
     check(driver_remove_destruct(&cardputer_adv_keyboard_driver) == ERROR_NONE);
+    check(driver_remove_destruct(&cardputer_keyboard_driver) == ERROR_NONE);
     return ERROR_NONE;
 }

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 0c82dfa6-ac87-4a62-a155-29f343e7efee

📥 Commits

Reviewing files that changed from the base of the PR and between 50c0a14 and 94699ef.

📒 Files selected for processing (119)

Devices/cyd-2432s024c/CMakeLists.txt
Devices/cyd-2432s024c/Source/Configuration.cpp
Devices/cyd-2432s024c/Source/devices/Display.cpp
Devices/cyd-2432s024c/Source/devices/Display.h
Devices/cyd-2432s024c/Source/module.cpp
Devices/cyd-2432s024c/cyd,2432s024c.dts
Devices/cyd-2432s024c/device.properties
Devices/cyd-2432s024c/devicetree.yaml
Devices/cyd-2432s024c/source/module.cpp
Devices/elecrow-crowpanel-advance-35/CMakeLists.txt
Devices/elecrow-crowpanel-advance-35/Source/Configuration.cpp
Devices/elecrow-crowpanel-advance-35/Source/devices/Display.cpp
Devices/elecrow-crowpanel-advance-35/Source/devices/Display.h
Devices/elecrow-crowpanel-advance-35/device.properties
Devices/elecrow-crowpanel-advance-35/devicetree.yaml
Devices/elecrow-crowpanel-advance-35/elecrow,crowpanel-advance-35.dts
Devices/elecrow-crowpanel-advance-35/source/module.cpp
Devices/m5stack-cardputer-adv/CMakeLists.txt
Devices/m5stack-cardputer-adv/Source/Configuration.cpp
Devices/m5stack-cardputer-adv/Source/devices/CardputerKeyboard.cpp
Devices/m5stack-cardputer-adv/Source/devices/CardputerKeyboard.h
Devices/m5stack-cardputer-adv/Source/devices/CardputerPower.cpp
Devices/m5stack-cardputer-adv/Source/devices/CardputerPower.h
Devices/m5stack-cardputer-adv/Source/devices/Display.cpp
Devices/m5stack-cardputer-adv/Source/devices/Display.h
Devices/m5stack-cardputer-adv/device.properties
Devices/m5stack-cardputer-adv/devicetree.yaml
Devices/m5stack-cardputer-adv/m5stack,cardputer-adv.dts
Devices/m5stack-cardputer-adv/source/module.cpp
Devices/m5stack-cardputer/CMakeLists.txt
Devices/m5stack-cardputer/Source/Configuration.cpp
Devices/m5stack-cardputer/Source/devices/CardputerEncoder.cpp
Devices/m5stack-cardputer/Source/devices/CardputerEncoder.h
Devices/m5stack-cardputer/Source/devices/CardputerKeyboard.cpp
Devices/m5stack-cardputer/Source/devices/CardputerKeyboard.h
Devices/m5stack-cardputer/Source/devices/CardputerPower.cpp
Devices/m5stack-cardputer/Source/devices/CardputerPower.h
Devices/m5stack-cardputer/Source/devices/Display.cpp
Devices/m5stack-cardputer/Source/devices/Display.h
Devices/m5stack-cardputer/Source/keyboard/README.md
Devices/m5stack-cardputer/Source/keyboard/keyboard.cpp
Devices/m5stack-cardputer/Source/keyboard/keyboard.h
Devices/m5stack-cardputer/Source/keyboard/keymap.h
Devices/m5stack-cardputer/device.properties
Devices/m5stack-cardputer/devicetree.yaml
Devices/m5stack-cardputer/m5stack,cardputer.dts
Devices/m5stack-cardputer/source/module.cpp
Devices/m5stack-stickc-plus2/CMakeLists.txt
Devices/m5stack-stickc-plus2/Source/Configuration.cpp
Devices/m5stack-stickc-plus2/Source/devices/Display.cpp
Devices/m5stack-stickc-plus2/Source/devices/Display.h
Devices/m5stack-stickc-plus2/Source/module.cpp
Devices/m5stack-stickc-plus2/device.properties
Devices/m5stack-stickc-plus2/devicetree.yaml
Devices/m5stack-stickc-plus2/m5stack,stickc-plus2.dts
Devices/m5stack-stickc-plus2/source/module.cpp
Documentation/ideas.md
Drivers/ButtonControl/Source/ButtonControl.h
Drivers/button-control-module/CMakeLists.txt
Drivers/button-control-module/LICENSE-Apache-2.0.md
Drivers/button-control-module/README.md
Drivers/button-control-module/bindings/tactility,button-control.yaml
Drivers/button-control-module/devicetree.yaml
Drivers/button-control-module/include/bindings/button_control.h
Drivers/button-control-module/include/button_control_module.h
Drivers/button-control-module/include/drivers/button_control.h
Drivers/button-control-module/source/button_control.cpp
Drivers/button-control-module/source/module.cpp
Drivers/cst816s-module/CMakeLists.txt
Drivers/cst816s-module/bindings/hynitron,cst816s.yaml
Drivers/cst816s-module/devicetree.yaml
Drivers/cst816s-module/include/bindings/cst816s.h
Drivers/cst816s-module/include/cst816s_module.h
Drivers/cst816s-module/include/drivers/cst816s.h
Drivers/cst816s-module/source/cst816s.cpp
Drivers/cst816s-module/source/module.cpp
Drivers/gt911-module/LICENSE-Apache-2.0.md
Drivers/ili9341-module/CMakeLists.txt
Drivers/ili9341-module/LICENSE-Apache-2.0.md
Drivers/ili9341-module/bindings/ilitek,ili9341.yaml
Drivers/ili9341-module/devicetree.yaml
Drivers/ili9341-module/include/bindings/ili9341.h
Drivers/ili9341-module/include/drivers/ili9341.h
Drivers/ili9341-module/include/ili9341_module.h
Drivers/ili9341-module/source/ili9341.cpp
Drivers/ili9341-module/source/module.cpp
Drivers/ili9488-module/CMakeLists.txt
Drivers/ili9488-module/LICENSE-Apache-2.0.md
Drivers/ili9488-module/bindings/ilitek,ili9488.yaml
Drivers/ili9488-module/devicetree.yaml
Drivers/ili9488-module/include/bindings/ili9488.h
Drivers/ili9488-module/include/drivers/ili9488.h
Drivers/ili9488-module/include/ili9488_module.h
Drivers/ili9488-module/source/ili9488.cpp
Drivers/ili9488-module/source/module.cpp
Drivers/ina226-module/LICENSE-Apache-2.0.md
Drivers/lilygo-module/LICENSE-Apache-2.0.md
Drivers/m5stack-module/CMakeLists.txt
Drivers/m5stack-module/LICENSE-Apache-2.0.md
Drivers/m5stack-module/bindings/m5stack,cardputer-adv-keyboard.yaml
Drivers/m5stack-module/bindings/m5stack,cardputer-keyboard.yaml
Drivers/m5stack-module/devicetree.yaml
Drivers/m5stack-module/include/bindings/cardputer_adv_keyboard.h
Drivers/m5stack-module/include/bindings/cardputer_keyboard.h
Drivers/m5stack-module/include/drivers/cardputer_adv_keyboard.h
Drivers/m5stack-module/include/drivers/cardputer_keyboard.h
Drivers/m5stack-module/include/m5stack_module.h
Drivers/m5stack-module/source/cardputer_adv_keyboard.cpp
Drivers/m5stack-module/source/cardputer_keyboard.cpp
Drivers/m5stack-module/source/module.cpp
Drivers/py32ioexpander-module/LICENSE-Apache-2.0.md
Drivers/st7789-module/LICENSE-Apache-2.0.md
Drivers/st7789-module/source/st7789.cpp
Firmware/Kconfig
Firmware/Source/Main.cpp
Modules/lvgl-module/source/lvgl_devices.c
Modules/lvgl-module/source/lvgl_display.c
Platforms/platform-esp32/source/drivers/esp32_spi.cpp
device.py

💤 Files with no reviewable changes (32)

Devices/elecrow-crowpanel-advance-35/Source/Configuration.cpp
Devices/m5stack-cardputer-adv/Source/devices/Display.cpp
Devices/m5stack-cardputer/Source/devices/Display.h
Devices/m5stack-cardputer/Source/Configuration.cpp
Devices/elecrow-crowpanel-advance-35/Source/devices/Display.cpp
Devices/m5stack-cardputer/Source/devices/CardputerEncoder.h
Devices/m5stack-cardputer/Source/devices/CardputerKeyboard.h
Devices/m5stack-cardputer/Source/keyboard/README.md
Devices/m5stack-cardputer-adv/Source/devices/Display.h
Devices/m5stack-cardputer/Source/devices/Display.cpp
Devices/m5stack-stickc-plus2/Source/module.cpp
Devices/cyd-2432s024c/Source/Configuration.cpp
Devices/m5stack-cardputer-adv/Source/Configuration.cpp
Devices/m5stack-cardputer-adv/Source/devices/CardputerPower.h
Devices/m5stack-stickc-plus2/Source/Configuration.cpp
Devices/m5stack-cardputer-adv/Source/devices/CardputerKeyboard.h
Devices/m5stack-stickc-plus2/Source/devices/Display.cpp
Devices/m5stack-cardputer/Source/devices/CardputerEncoder.cpp
Drivers/ButtonControl/Source/ButtonControl.h
Devices/cyd-2432s024c/Source/devices/Display.cpp
Devices/m5stack-cardputer/Source/devices/CardputerPower.cpp
Devices/cyd-2432s024c/Source/module.cpp
Devices/m5stack-cardputer/Source/devices/CardputerKeyboard.cpp
Devices/m5stack-cardputer/Source/keyboard/keyboard.cpp
Devices/m5stack-cardputer/Source/keyboard/keymap.h
Devices/m5stack-stickc-plus2/Source/devices/Display.h
Devices/cyd-2432s024c/Source/devices/Display.h
Devices/m5stack-cardputer-adv/Source/devices/CardputerPower.cpp
Devices/m5stack-cardputer-adv/Source/devices/CardputerKeyboard.cpp
Devices/elecrow-crowpanel-advance-35/Source/devices/Display.h
Devices/m5stack-cardputer/Source/keyboard/keyboard.h
Devices/m5stack-cardputer/Source/devices/CardputerPower.h

coderabbitai

Actionable comments posted: 2

🧹 Nitpick comments (2)

TactilityKernel/include/tactility/device_listener.h (1)
20-27: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

DeviceEventListener and callback parameter design leak implementation details.

Two API design observations:

DeviceEventListener (lines 20–23) is defined in the public header but never appears in any public function signature — add/remove take raw DeviceListenerCallback* and void*. This struct is pure internal storage and should live in device_listener_internal.h or the .cpp file.

DeviceListenerCallback* (pointer-to-function-pointer) forces callers to write &my_func and the implementation to dereference. Passing DeviceListenerCallback by value is simpler for callers and equivalent in cost.
♻️ Proposed refactor
 // device_listener.h — remove the struct, simplify signatures
-struct DeviceEventListener {
-    DeviceListenerCallback callback;
-    void* callback_context;
-};
-
-void device_listener_add(DeviceListenerCallback* callback, void* context);
-void device_listener_remove(DeviceListenerCallback* callback);
+void device_listener_add(DeviceListenerCallback callback, void* context);
+void device_listener_remove(DeviceListenerCallback callback);
 // device_listener_internal.h — move struct here
+struct DeviceEventListener {
+    DeviceListenerCallback callback;
+    void* callback_context;
+};
+
 void device_listener_notify(struct Device* dev, enum DeviceEvent event);
 // device_listener.cpp — update implementation
 void device_listener_add(DeviceListenerCallback callback, void* context) {
     ledger.lock();
-    ledger.listeners.push_back(DeviceEventListener{ *callback, context });
+    ledger.listeners.push_back(DeviceEventListener{ callback, context });
     ledger.unlock();
 }

 void device_listener_remove(DeviceListenerCallback callback) {
     ledger.lock();
     const auto iterator = std::ranges::find_if(ledger.listeners, [callback](const DeviceEventListener& listener) {
-        return listener.callback == *callback;
+        return listener.callback == callback;
     });
Tests/TactilityKernel/Source/DeviceGetPutTest.cpp (1)

102-104: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win

Busy-wait can hang the test suite instead of failing cleanly.

If the worker's device_get(&device) returns non-ERROR_NONE, it returns 1 without ever setting acquired, so this loop spins forever and the CI run hangs rather than reporting a failed assertion. Consider bounding the wait (e.g. spin with an iteration/time cap, then CHECK the worker actually acquired) so a regression surfaces as a test failure.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 79486dbe-fc7f-4ca3-8262-4ee9ec38abd4

📥 Commits

Reviewing files that changed from the base of the PR and between 2691872 and 1dcdbda.

📒 Files selected for processing (15)

Drivers/button-control-module/source/button_control.cpp
Drivers/ili9341-module/source/ili9341.cpp
Drivers/m5stack-module/source/cardputer_adv_keyboard.cpp
Drivers/m5stack-module/source/cardputer_keyboard.cpp
Modules/lvgl-module/source/lvgl_devices.c
TactilityKernel/CMakeLists.txt
TactilityKernel/include/tactility/device.h
TactilityKernel/include/tactility/device_listener.h
TactilityKernel/include/tactility/error.h
TactilityKernel/private/tactility/device_listener_internal.h
TactilityKernel/source/device.cpp
TactilityKernel/source/device_listener.cpp
TactilityKernel/source/error.cpp
Tests/TactilityKernel/Source/DeviceGetPutTest.cpp
Tests/TactilityKernel/Source/DeviceTest.cpp

🚧 Files skipped from review as they are similar to previous changes (5)

Drivers/m5stack-module/source/cardputer_keyboard.cpp
Modules/lvgl-module/source/lvgl_devices.c
Drivers/button-control-module/source/button_control.cpp
Drivers/ili9341-module/source/ili9341.cpp
Drivers/m5stack-module/source/cardputer_adv_keyboard.cpp

KenVanHoeylandt added 11 commits July 12, 2026 14:24

Add missing license files

3592f05

Chang display driver to kernel driver

1ec6c13

Fix for glitches

0fe9bbc

Remove unused include

5bb70ab

Create button-control-module driver

0a92085

Update stickc-plus2 for dts drivers

efe7feb

Temp move

95c9385

Update stickc plus2

d7eacd6

m5stack cardputer and adv drivers migrated to kernel drivers

1206cb1

Implement cyd-2432s024c with kernel drivers

32cc642

- add cst816s kernel driver - add ili9341 kernel driver

Implement crowpanel adv 3.5" with kernel drivers

94699ef

coderabbitai Bot reviewed Jul 12, 2026

View reviewed changes

KenVanHoeylandt added 5 commits July 12, 2026 18:47

Implement xpt2046-module and convert crowpanel basic 3.5"

bd9826a

Fix for pc builds

2691872

Implement device listener interface

b8e3bf5

Implement device ref count

e8b4a78

Fixes

1dcdbda

coderabbitai Bot reviewed Jul 12, 2026

View reviewed changes

Comment thread TactilityKernel/source/device_listener.cpp

Comment thread TactilityKernel/source/device.cpp

Fixes for deadlocks

c2a9055

KenVanHoeylandt merged commit fa4a6e2 into main Jul 12, 2026
60 checks passed

KenVanHoeylandt deleted the develop branch July 12, 2026 21:54

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

New kernel drivers and device implementation updates#561

New kernel drivers and device implementation updates#561
KenVanHoeylandt merged 17 commits into
mainfrom
develop

KenVanHoeylandt commented Jul 12, 2026 •

edited

Loading

Uh oh!

coderabbitai Bot commented Jul 12, 2026 •

edited

Loading

Walkthrough

❌ Failed checks (1 warning)

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Uh oh!

Conversation

KenVanHoeylandt commented Jul 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

coderabbitai Bot commented Jul 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

❌ Failed checks (1 warning)

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

KenVanHoeylandt commented Jul 12, 2026 •

edited

Loading

coderabbitai Bot commented Jul 12, 2026 •

edited

Loading