You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
App launcher, settings, and file tiles now display SVG icons, with colour or monochrome variants matching the current theme and fallback icons when images are unavailable.
Launcher icons refresh automatically when packages change.
Touch calibration accounts for display rotation and can run before the launcher starts an app or setup.
Added configurable display draw-buffer sizing and IPS panel support.
Bug Fixes
Large display updates are sent in buffer-sized strips.
The software keyboard now better keeps active text fields visible and hides cleanly when a field is removed.
It 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 reviews.auto_review.auto_pause_after_reviewed_commits setting.
The changes add NanoSVG parsing and rasterization, LVGL APIs for rasterizing SVG data and files, and SVG icons in file browser and launcher tiles. The launcher loads and refreshes package and settings icons. The app manager notifies listeners after successful package registration or removal. The app paths API exposes an app’s installation directory. Other changes update touch calibration, device configuration, display transfers, memory allocation, and application behavior.
Merge Risk: Moderate · previous review 28240
The new SVG icon pipeline still has memory-pressure crash paths and an unbounded dash-pattern loop when parsing or rasterizing untrusted icons. A few device and UI behaviors may also misbehave: the Advance 3.5 touch mirror, stale package icons, and the calibration result screen. Fix or explicitly accept these before merging.
Pre-merge checks: 4
Check name
Status
Explanation
Description Check
Check skipped - CodeRabbit’s high-level summary is enabled.
Title check
The title names three themes: SVG app icons, display improvements, and flash ROM size optimization. The changeset does add SVG icon rasterization for launcher and settings tiles, plus the nanosvg libr…
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.
Full details: Title check
Explanation
The title names three themes: SVG app icons, display improvements, and flash ROM size optimization. The changeset does add SVG icon rasterization for launcher and settings tiles, plus the nanosvg library and an LVGL SVG API. It also adds display changes (ILI9488 IPS and strip-based drawing, draw-buffer line config, rotation handling) and flash size changes (the ESP32-P4 flash size goes from 8MB to 16MB, and 4MB builds get size-optimization settings). The title is related to the main change, but it lists the three themes as a flat list and does not convey the scale of the changes, such as the touch calibration rework, the package listener API, or audio module gating. This makes it partially related and broad rather than a full summary, so it passes.
Finishing Touches
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.
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 12
Caution
Some comments are outside the diff and can’t be posted inline due to GitHub limitations.
⚠️ Outside diff range comments (1)
Major · Export the SVG API through lvgl_module_symbols. · symbols.c:39
Modules/lvgl-module/source/symbols.c:39 Major · Data Integrity & Integration · Quick win
Export the SVG API through lvgl_module_symbols.
lvgl_module_symbols is the list of symbols the LVGL module exports. This change includes <lvgl/svg.h> in symbols.c but adds no entries for the lvgl_svg_rasterize_* functions, and no entry exists for lv_draw_buf_destroy either. An app that imports these functions through the export table cannot resolve them.
Register each public lvgl_svg_rasterize_* function with DEFINE_MODULE_SYMBOL in the same table, following the existing entries. If callers of the new API must also call lv_draw_buf_destroy, register that function too.
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: 7defafa0-9923-4597-b46d-c1cbadf73c75
Commits
Reviewing files that changed from the base of the PR and between 74dee43 and e530408.
Files ignored due to path filters: 76
Data/system/app_icon/default.svg is excluded by !**/*.svg
Data/system/app_icon/default_mono.svg is excluded by !**/*.svg
Data/system/app_icon/tactility.appearance.svg is excluded by !**/*.svg
Data/system/app_icon/tactility.appearance_mono.svg is excluded by !**/*.svg
Data/system/app_icon/tactility.apphub.svg is excluded by !**/*.svg
Data/system/app_icon/tactility.apphub_mono.svg is excluded by !**/*.svg
Data/system/app_icon/tactility.audiosettings.svg is excluded by !**/*.svg
Data/system/app_icon/tactility.audiosettings_mono.svg is excluded by !**/*.svg
Data/system/app_icon/tactility.btmanage.svg is excluded by !**/*.svg
Data/system/app_icon/tactility.btmanage_mono.svg is excluded by !**/*.svg
Data/system/app_icon/tactility.camera.svg is excluded by !**/*.svg
Data/system/app_icon/tactility.camera_mono.svg is excluded by !**/*.svg
Data/system/app_icon/tactility.chat.svg is excluded by !**/*.svg
Data/system/app_icon/tactility.chat_mono.svg is excluded by !**/*.svg
Data/system/app_icon/tactility.development.svg is excluded by !**/*.svg
Data/system/app_icon/tactility.development_mono.svg is excluded by !**/*.svg
Data/system/app_icon/tactility.display.svg is excluded by !**/*.svg
Data/system/app_icon/tactility.display_mono.svg is excluded by !**/*.svg
Data/system/app_icon/tactility.files.svg is excluded by !**/*.svg
Data/system/app_icon/tactility.files_mono.svg is excluded by !**/*.svg
Data/system/app_icon/tactility.grovesettings.svg is excluded by !**/*.svg
Data/system/app_icon/tactility.grovesettings_mono.svg is excluded by !**/*.svg
Data/system/app_icon/tactility.i2cscanner.svg is excluded by !**/*.svg
Data/system/app_icon/tactility.i2cscanner_mono.svg is excluded by !**/*.svg
Data/system/app_icon/tactility.keyboardsettings.svg is excluded by !**/*.svg
Data/system/app_icon/tactility.keyboardsettings_mono.svg is excluded by !**/*.svg
Data/system/app_icon/tactility.ledstripsettings.svg is excluded by !**/*.svg
Data/system/app_icon/tactility.ledstripsettings_mono.svg is excluded by !**/*.svg
Data/system/app_icon/tactility.locationsettings.svg is excluded by !**/*.svg
Data/system/app_icon/tactility.locationsettings_mono.svg is excluded by !**/*.svg
Data/system/app_icon/tactility.maps.svg is excluded by !**/*.svg
Data/system/app_icon/tactility.maps_mono.svg is excluded by !**/*.svg
Data/system/app_icon/tactility.meshchannels.svg is excluded by !**/*.svg
Data/system/app_icon/tactility.meshchannels_mono.svg is excluded by !**/*.svg
Data/system/app_icon/tactility.meshcontacts.svg is excluded by !**/*.svg
Data/system/app_icon/tactility.meshcontacts_mono.svg is excluded by !**/*.svg
Data/system/app_icon/tactility.meshsettings.svg is excluded by !**/*.svg
Data/system/app_icon/tactility.meshsettings_mono.svg is excluded by !**/*.svg
Data/system/app_icon/tactility.notes.svg is excluded by !**/*.svg
Data/system/app_icon/tactility.notes_mono.svg is excluded by !**/*.svg
Data/system/app_icon/tactility.power.svg is excluded by !**/*.svg
Data/system/app_icon/tactility.power_mono.svg is excluded by !**/*.svg
Data/system/app_icon/tactility.systeminfo.svg is excluded by !**/*.svg
Data/system/app_icon/tactility.systeminfo_mono.svg is excluded by !**/*.svg
Data/system/app_icon/tactility.terminal.svg is excluded by !**/*.svg
Data/system/app_icon/tactility.terminal_mono.svg is excluded by !**/*.svg
Data/system/app_icon/tactility.timedatesettings.svg is excluded by !**/*.svg
Data/system/app_icon/tactility.timedatesettings_mono.svg is excluded by !**/*.svg
Data/system/app_icon/tactility.touchcalibration.svg is excluded by !**/*.svg
Data/system/app_icon/tactility.touchcalibration_mono.svg is excluded by !**/*.svg
Data/system/app_icon/tactility.trackballsettings.svg is excluded by !**/*.svg
Data/system/app_icon/tactility.trackballsettings_mono.svg is excluded by !**/*.svg
Data/system/app_icon/tactility.usbsettings.svg is excluded by !**/*.svg
Data/system/app_icon/tactility.usbsettings_mono.svg is excluded by !**/*.svg
Data/system/app_icon/tactility.vibrationsettings.svg is excluded by !**/*.svg
Data/system/app_icon/tactility.vibrationsettings_mono.svg is excluded by !**/*.svg
Data/system/app_icon/tactility.wifimanage.svg is excluded by !**/*.svg
Data/system/app_icon/tactility.wifimanage_mono.svg is excluded by !**/*.svg
Data/system/file_icon/app.svg is excluded by !**/*.svg
Data/system/file_icon/app_mono.svg is excluded by !**/*.svg
Data/system/file_icon/executable.svg is excluded by !**/*.svg
Data/system/file_icon/executable_mono.svg is excluded by !**/*.svg
Data/system/file_icon/file.svg is excluded by !**/*.svg
Data/system/file_icon/file_mono.svg is excluded by !**/*.svg
Data/system/file_icon/folder.svg is excluded by !**/*.svg
Data/system/file_icon/folder_mono.svg is excluded by !**/*.svg
Data/system/file_icon/image.svg is excluded by !**/*.svg
Data/system/file_icon/image_mono.svg is excluded by !**/*.svg
Data/system/file_icon/link.svg is excluded by !**/*.svg
Data/system/file_icon/link_mono.svg is excluded by !**/*.svg
Data/system/file_icon/sd_card.svg is excluded by !**/*.svg
Data/system/file_icon/sd_card_mono.svg is excluded by !**/*.svg
Data/system/file_icon/text.svg is excluded by !**/*.svg
Data/system/file_icon/text_mono.svg is excluded by !**/*.svg
Data/system/file_icon/usb.svg is excluded by !**/*.svg
Data/system/file_icon/usb_mono.svg is excluded by !**/*.svg
Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 2 remain after this review.
This callback removes a different function/context pair. The test therefore passes even if removing the active listener does nothing. Keep this unmatched-removal case, but add a callback that removes its own registered pair. Trigger another package notification and check that the callback does not run again.
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: cd12c987-1ed6-4919-9f67-68a061e2dcf3
Commits
Reviewing files that changed from the base of the PR and between e530408 and 631d67f.
Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 2 remain after this review.
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to GitHub limitations.
⚠️ Outside diff range comments (1)
Minor · Move calibration persistence out of the LVGL callback. · TouchCalibration.cpp:131
Tactility/Source/app/touchcalibration/TouchCalibration.cpp:131 Minor · Stability & Availability · Quick win
Move calibration persistence out of the LVGL callback.
On the fourth press, onPress calls finishCalibration, which saves settings before the callback returns. save creates the settings directory and writes a properties file. Slow storage therefore stalls the UI during calibration completion. Signal the app task to save the fitted ranges, then update the widgets under the LVGL lock. (raw.githubusercontent.com)
As per coding guidelines, “Code that runs on the LVGL thread must not do file operations (including reading or writing settings).”
Source: Coding guidelines
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: 030a0bca-46b5-41df-9213-a0ad594d1991
Commits
Reviewing files that changed from the base of the PR and between 631d67f and 2d81b72.
Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 3 remain after this review.
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to GitHub limitations.
⚠️ Outside diff range comments (2)
Major · Propagate color-submission failures before waiting. · ili9488.cpp:240
Drivers/ili9488-module/source/ili9488.cpp:240 Major · Stability & Availability · Heavy lift
Propagate color-submission failures before waiting.
The pinned ILI9488 component discards the result of esp_lcd_panel_io_tx_color() and returns ESP_OK. If a strip cannot be queued, this error branch is skipped and the subsequent semaphore wait can block indefinitely while holding the SPI lock. Make the panel driver propagate submission failures and handle any partially queued transfer safely. (raw.githubusercontent.com)
Major · Drain the completion semaphore under the SPI bus lock. · ili9488.cpp:228
Drivers/ili9488-module/source/ili9488.cpp:228 Major · Stability & Availability · Quick win
Drain the completion semaphore under the SPI bus lock.
The drain at line 228 runs before spi_controller_lock(). A concurrent draw can consume the completion signal of a strip that is still in flight. The first draw then blocks at xSemaphoreTake(..., portMAX_DELAY) while it holds the SPI bus lock. No later transfer signals the semaphore again, so the wait does not end, and the shared SPI bus can stay locked until reset.
The driver does not serialize draws before line 228, and display_draw_bitmap() forwards calls without a lock. The LVGL flush, the terminal renderer, and the boot screen can all reach this path. Move the drain under the bus lock so that a draw clears only signals that no other draw owns.
Suggested fix
static error_t ili9488_draw_bitmap(Device* device, int32_t x_start, int32_t y_start, int32_t x_end, int32_t y_end, const void* color_data) {
auto* internal = static_cast<Ili9488Internal*>(device_get_driver_data(device));
- // Drain any stale signal left over from a prior non-draw transaction (bring-up commands like- // reset/gap also complete through on_color_trans_done), so the take() below can only be- // satisfied by this draw's own transfer completing.- xSemaphoreTake(internal->draw_done_semaphore, 0);-
// The panel driver converts a whole area through its conversion buffer and doesn't check bounds,
// so a larger area is sent in strips of rows that fit in that buffer. The conversion buffer is
// reused by the next strip, which is why each strip is waited for.
const int32_t width = x_end - x_start;
const int32_t strip_rows = std::max<int32_t>(1, static_cast<int32_t>(GET_CONFIG(device)->color_conversion_buffer_size) / width);
const auto* pixels = static_cast<const uint16_t*>(color_data);
spi_controller_lock(internal->spi_controller);
+ // Drain under the bus lock so that no other draw can consume this draw's completion signal.+ // This also clears stale signals from prior non-draw transactions (bring-up commands like+ // reset/gap also complete through on_color_trans_done), so the take() below can only be+ // satisfied by this draw's own transfer completing.+ xSemaphoreTake(internal->draw_done_semaphore, 0);
for (int32_t y = y_start; y < y_end; y += strip_rows) {
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: 784cd6f6-2f32-44cf-80b6-f8ce4f0c0e28
Commits
Reviewing files that changed from the base of the PR and between 2d81b72 and bf3b07f.
Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 2 remain after this review.
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
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: 7f7c3375-b29c-4c52-a112-ae3e7085e601
Commits
Reviewing files that changed from the base of the PR and between bf3b07f and 28240e8.
Files selected for processing: 3
Drivers/ili9488-module/source/ili9488.cpp
Modules/lvgl-module/source/devices/keyboard.cpp
Modules/lvgl-module/source/widgets/toolbar.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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
KenVanHoeylandt
changed the title
Summary by CodeRabbit