CL-32, mesh, mono theme, SPI API, logos, and more by KenVanHoeylandt · Pull Request #724 · TactilityProject/Tactility

KenVanHoeylandt · 2026-10-28T19:19:09Z

Summary

Fixes for the CL-32 and mesh, a mono theme focus fix, two small UI improvements, and a platform-neutral SPI API for external apps.

Changes

CL-32 v04: GPIO conflict warnings

cl32_v4_display.cpp: display detection no longer configures the CS pin, which the SPI driver already owns.
Detection revokes its reservation of the DC and reset pins when done, so the display driver can claim them without a gpio: conflict warning.

Mesh: radio selection at boot

The mesh service used to start with its module, before the SX1262 was up, so it bound to ESP-NOW while the settings apps later reported LoRa.
mesh-module/module.cpp: the service is now registered without auto-start and starts on KERNEL_EVENT_BOOT_COMPLETED if it was enabled.

Mono theme: focus ring on checked icon buttons

A checked (inverted) icon button hid its focus ring, since both were ink.
material_mono.lss: a focused, checked or pressed icon button gets a paper gap between the ring and the fill (@focus-inverted-gap, 2 dpx, 1 px in compact mode).
Regenerated lv_theme_material_mono.c.

Wi-Fi Connect

Pressing Return in the password field now connects, like the Connect button.

Boot logo

The logo size is selected from the display width in the current orientation, not the smallest dimension (BootScreen::getLogicalWidth()).

SPI controller API

spi_controller in TactilityKernel gets add_device, remove_device, transmit, acquire_bus and release_bus, with an opaque SpiDevice and a plain-C SpiDeviceConfig. Bus init and free stay in the driver's start and stop.
Exported in symbols.c so ELF apps can use SPI without ESP-IDF symbols.
esp32_spi.cpp implements it and drives chip select through the GPIO descriptors it owns. cs_descriptors now matches cs_gpios index for index, with null for unavailable pins.

coderabbitai · 2026-10-28T19:20:19Z

Walkthrough

The changes add SPI peripheral operations to the kernel API and implement them in the ESP32 driver. They also adjust mesh service startup, boot logo selection, icon-button focus styles, and Wi-Fi password submission. The LVGL module exports three additional display functions. Display detection changes its GPIO setup and cleanup, and the license document adds a vendor logo notice. Other changes update GPIO controller memory allocation, boot-screen dimension cleanup, and a LodePNG test asset path.

Priority: Normal

Merge Risk: Low · up to 28249

App actions can be blocked when a device host is saved only in the app directory; entering the host manually works around this. The mesh rollback concern remains conditional, so the known issues are bounded but merit owner attention.

Pre-merge checks: 4 · 1

Failed checks: 1 warning

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 21.74% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documenta…	Write docstrings for the functions missing them to satisfy the coverage threshold.

Passed checks: 4 passed

Check name	Status	Explanation
Description Check		Check skipped - CodeRabbit’s high-level summary is enabled.
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.
Title check		The title identifies several major changes, including CL-32, mesh, the mono theme, the SPI API, and logos. It is broad but remains clearly related to the changeset.

Full details: Docstring Coverage

Explanation

Docstring coverage is 21.74% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 23 functions across 13 files.

Resolution

Write docstrings for the functions missing them to satisfy the coverage threshold.

Finishing Touches: 💡 1

📝 Generate docstrings: 💡

Commit to this branch
Create a new PR

🧪 Generate unit tests (beta)

Commit to this branch
Create a new PR

Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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: 7

Review info

Run configuration

Source mode: Auto
Repository: Unavailable
Branch: Unavailable
File: No configuration file from this repository applied
Commit: Unavailable
Layers: Organization UI
Review profile: CHILL
Plan: Advanced
Run ID: 83f12d71-d856-4e68-af2b-16bd19c04e23

Commits

Reviewing files that changed from the base of the PR and between bf902be and b35769c.

Files ignored due to path filters: 24

Data/system/boot/logo_large.png is excluded by !**/*.png
Data/system/boot/logo_medium.png is excluded by !**/*.png
Data/system/boot/logo_small.png is excluded by !**/*.png
Data/system/boot/logo_usb_large.png is excluded by !**/*.png
Data/system/boot/logo_usb_medium.png is excluded by !**/*.png
Data/system/boot/logo_usb_small.png is excluded by !**/*.png
Devices/elecrow-crowpanel-advance-28/system/boot/logo_large.png is excluded by !**/*.png
Devices/elecrow-crowpanel-advance-28/system/boot/logo_medium.png is excluded by !**/*.png
Devices/elecrow-crowpanel-advance-28/system/boot/logo_small.png is excluded by !**/*.png
Devices/elecrow-crowpanel-advance-35/system/boot/logo_large.png is excluded by !**/*.png
Devices/elecrow-crowpanel-advance-35/system/boot/logo_medium.png is excluded by !**/*.png
Devices/elecrow-crowpanel-advance-35/system/boot/logo_small.png is excluded by !**/*.png
Devices/elecrow-crowpanel-advance-50/system/boot/logo_large.png is excluded by !**/*.png
Devices/elecrow-crowpanel-advance-50/system/boot/logo_medium.png is excluded by !**/*.png
Devices/elecrow-crowpanel-advance-50/system/boot/logo_small.png is excluded by !**/*.png
Devices/elecrow-crowpanel-basic-28/system/boot/logo_large.png is excluded by !**/*.png
Devices/elecrow-crowpanel-basic-28/system/boot/logo_medium.png is excluded by !**/*.png
Devices/elecrow-crowpanel-basic-28/system/boot/logo_small.png is excluded by !**/*.png
Devices/elecrow-crowpanel-basic-35/system/boot/logo_large.png is excluded by !**/*.png
Devices/elecrow-crowpanel-basic-35/system/boot/logo_medium.png is excluded by !**/*.png
Devices/elecrow-crowpanel-basic-35/system/boot/logo_small.png is excluded by !**/*.png
Devices/elecrow-crowpanel-basic-50/system/boot/logo_large.png is excluded by !**/*.png
Devices/elecrow-crowpanel-basic-50/system/boot/logo_medium.png is excluded by !**/*.png
Devices/elecrow-crowpanel-basic-50/system/boot/logo_small.png is excluded by !**/*.png

Files selected for processing: 13

Devices/cl32-v04/source/cl32_v4_display.cpp
LICENSE.md
Modules/lvgl-module/source/symbols.c
Modules/lvgl-module/source/themes/lv_theme_material_mono.c
Modules/lvgl-module/themes/material_mono.lss
Modules/mesh-module/source/module.cpp
Platforms/platform-esp32/source/drivers/esp32_spi.cpp
Tactility/Private/Tactility/app/boot/BootScreen.h
Tactility/Source/app/boot/BootInit.cpp
Tactility/Source/app/wificonnect/WifiConnect.cpp
TactilityKernel/include/tactility/drivers/spi_controller.h
TactilityKernel/source/drivers/spi_controller.cpp
TactilityKernel/source/symbols.c

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 3 remain after this review.

coderabbitai

Caution

Some comments are outside the diff and can’t be posted inline due to GitHub limitations.

⚠️ Outside diff range comments (2)

Major · Keep the device registered until spi_bus_remove_device() succeeds. · esp32_spi.cpp:130-142

Platforms/platform-esp32/source/drivers/esp32_spi.cpp:130-142
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Keep the device registered until spi_bus_remove_device() succeeds.

When removal overlaps an unfinished transfer, ESP-IDF can return ESP_ERR_INVALID_STATE. The current code already erases the device and deletes its wrapper before checking that result. stop() can then pass its empty-device guard and call spi_bus_free() while the ESP-IDF device remains attached. The caller also cannot retry removal.
Suggested fix
     auto* data = GET_DATA(device);
-    recursive_mutex_lock(&data->mutex);
-    data->devices.erase(std::remove(data->devices.begin(), data->devices.end(), spi_device), data->devices.end());
-    recursive_mutex_unlock(&data->mutex);

     if (spi_device->bus_acquired) {
         if (spi_device->cs != nullptr) gpio_descriptor_set_level(spi_device->cs, false);
         spi_device_release_bus(spi_device->handle);
+        spi_device->bus_acquired = false;
     }
     const esp_err_t result = spi_bus_remove_device(spi_device->handle);
+    if (result != ESP_OK) return ERROR_RESOURCE;
+
+    recursive_mutex_lock(&data->mutex);
+    data->devices.erase(std::remove(data->devices.begin(), data->devices.end(), spi_device), data->devices.end());
+    recursive_mutex_unlock(&data->mutex);
     delete spi_device;
-    return result == ESP_OK ? ERROR_NONE : ERROR_RESOURCE;
+    return ERROR_NONE;

Minor · Keep mesh retryable when cleanup fails. · module.cpp:86-96

Modules/mesh-module/source/module.cpp:86-96
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win

Keep mesh retryable when cleanup fails.

If mesh_set_enabled(true) races stop(), service_manager_remove() can return ERROR_INVALID_STATE after the service starts again. stop() has already set module_active to false and removed the boot callback. The kernel then keeps the module marked started because module_stop() received an error. A later module_start() returns early, leaving mesh without its callback and without a recovery attempt.

Restore the active state and retry the service before returning the cleanup error. Remove the callback only after cleanup succeeds.
Suggested fix
 static error_t stop() {
-    system_event_callback_remove(KERNEL_EVENT_BOOT_COMPLETED, on_boot_completed);
-
     // Waits for a callback that is running, and makes one that is still pending do nothing
     mutex_lock(&lifecycle_mutex.handle);
     module_active = false;
     error_t error = service_manager_stop(mesh_service_manifest.id);
@@
     if (error == ERROR_NONE) {
         error = app_manager_remove(mesh_command_manifest.id);
     }
+    if (error != ERROR_NONE) {
+        module_active = true;
+        if (boot_completed) {
+            start_service_if_enabled();
+        }
+        mutex_unlock(&lifecycle_mutex.handle);
+        return error;
+    }
     mutex_unlock(&lifecycle_mutex.handle);
+    system_event_callback_remove(KERNEL_EVENT_BOOT_COMPLETED, on_boot_completed);
     return error;
 }

Review info

Run configuration

Source mode: Auto
Repository: Unavailable
Branch: Unavailable
File: No configuration file from this repository applied
Commit: Unavailable
Layers: Organization UI
Review profile: CHILL
Plan: Advanced
Run ID: c849cd91-1a43-48e5-9f5b-da8643eda23b

Commits

Reviewing files that changed from the base of the PR and between b35769c and 2112944.

Files selected for processing: 6

Documentation/ideas.md
Modules/lodepng-module/tests/CMakeLists.txt
Modules/mesh-module/source/module.cpp
Platforms/platform-esp32/source/drivers/esp32_spi.cpp
Tactility/Source/app/boot/BootScreen.cpp
TactilityKernel/source/drivers/gpio_controller.cpp

Files skipped from review as they are similar to previous changes: 2

Platforms/platform-esp32/source/drivers/esp32_spi.cpp
Modules/mesh-module/source/module.cpp

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 2 remain after this review.

coderabbitai

Caution

Some comments are outside the diff and can’t be posted inline due to GitHub limitations.

⚠️ Outside diff range comments (1)

Minor · Check the app directory's stored host before refusing a blank host field. · ui.py:2029-2038

Buildscripts/TactilitySDK/Tools/ui.py:2029-2038
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Check the app directory's stored host before refusing a blank host field.

The new guard in DevConsole._run refuses install, run, uninstall, bir, and brrr whenever the Device host field is blank. app.py can still resolve a host without --host. It reads .tactility/device_host relative to the process working directory. The new UI starts app actions in the app directory, but it fills and checks the field from <repo>/.tactility/device_host. A host stored only in the app directory therefore cannot fill the field, and the guard blocks a run that app.py would complete.
Suggested fix
-        if action in ("install", "run", "uninstall", "bir", "brrr") and not self.host_var.get().strip():
+        if (action in ("install", "run", "uninstall", "bir", "brrr")
+                and not self.host_var.get().strip()
+                and not read_host(Path(app.path))):
             self._warn("Device host", "Enter the device host first.")
             return
With this change, the field stays the primary source. The guard still applies when neither location holds a host.

🔇 Additional comments: 1

Modules/mesh-module/source/module.cpp-95-97: 1

95-97: Major · Stability & Availability · Heavy lift

⚠️ Unverified finding
Verification ran but could not confirm this finding. It is shown for review, not as a verified issue.

Restore the service manifest before restarting the service.

If the command app manifest is absent, app_manager_remove() returns ERROR_NOT_FOUND after service_manager_remove() has succeeded. This branch sets module_active to true, but service_manager_start() cannot find the removed service manifest. module_stop() then leaves the module marked started without a running mesh service. Treat an absent app manifest as already removed, or re-register the service manifest before rolling back. (raw.githubusercontent.com)

Review info

Run configuration

Source mode: Auto
Repository: Unavailable
Branch: Unavailable
File: No configuration file from this repository applied
Commit: Unavailable
Layers: Organization UI
Review profile: CHILL
Plan: Advanced
Run ID: 7e02864f-0dd3-427a-be13-fb60d1c85421

Commits

Reviewing files that changed from the base of the PR and between 2112944 and 28249f6.

Files selected for processing: 3

Buildscripts/TactilitySDK/Tools/ui.py
Modules/mesh-module/source/module.cpp
Platforms/platform-esp32/source/drivers/esp32_spi.cpp

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 2 remain after this review.

KenVanHoeylandt added 7 commits October 28, 2026 17:52

Fix for pin warnings

5444f94

Fix for selection in mono theme

a2a5f43

Fix for mesh radio selection on device startup

0878b32

WifiConnect: make enter key work

2b71b23

Improve boot logo logic and add Elecrow logo to crowpanels

33f85f2

Implement spi device

6a4d686

Add lv_display symbols

b35769c

coderabbitai Bot reviewed Oct 28, 2026

View reviewed changes

KenVanHoeylandt added 7 commits October 28, 2026 20:39

Fix for logo path in test

493a93a

Fix for boot signal vs module restart

0a89c75

Lifecycle fix

48601cd

Fix for width on panel end

35a6e4c

Use kernel allocator

53708b8

Fixes and memory improvements

7a993e4

Update ideas.md

2112944

coderabbitai Bot reviewed Oct 28, 2026

View reviewed changes

KenVanHoeylandt added 2 commits October 28, 2026 21:26

New UI for TactilitySDK

4b152dc

Fixes

28249f6

coderabbitai Bot reviewed Oct 28, 2026

View reviewed changes

KenVanHoeylandt changed the title ~~Various improvements~~ CL-32, mesh, mono theme, SPI API, logos, and more Oct 28, 2026

KenVanHoeylandt merged commit da147b3 into main Oct 28, 2026
78 checks passed

KenVanHoeylandt deleted the develop branch October 28, 2026 21:21

Navigation Menu

Repository navigation

CL-32, mesh, mono theme, SPI API, logos, and more - #724

CL-32, mesh, mono theme, SPI API, logos, and more#724
KenVanHoeylandt merged 16 commits into
mainfrom
develop

KenVanHoeylandt commented Oct 28, 2026 •

edited

Loading

Uh oh!

coderabbitai Bot commented Oct 28, 2026 •

edited

Loading

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

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

Uh oh!

Summary

Changes

CL-32 v04: GPIO conflict warnings

Mesh: radio selection at boot

Mono theme: focus ring on checked icon buttons

Wi-Fi Connect

Boot logo

SPI controller API

Uh oh!

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

Uh oh!

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!

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

KenVanHoeylandt commented Oct 28, 2026 •

edited

Loading

coderabbitai Bot commented Oct 28, 2026 •

edited

Loading