MUI: portduino SDL support for Linux/Windows - #11121
Conversation
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughPortduino display handling now supports runtime SDL selection, thread-local screen initialization, and YAML-configured zoom. Native TFT and Windows environments receive SDL build dependencies and flags, while the device UI dependency is updated to a newer archived revision. ChangesPortduino display integration
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant tftSetup
participant DisplayThread
participant deviceScreen
participant PacketClient
participant tft_task_handler
tftSetup->>DisplayThread: start tft_thread_entry()
DisplayThread->>deviceScreen: init(new PacketClient)
deviceScreen->>PacketClient: create packet client
DisplayThread->>tft_task_handler: run display task handler
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 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 |
⚡ Try this PR in the Web Flasher
Warning This is an automated, unreviewed CI test build. Back up your device configuration Supported boards built by this PR (31)
Build artifacts expire on 2026-08-24. Updated for |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/graphics/tftSetup.cpp (1)
42-47: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse
camelCasefor the new thread entry point.Rename
tft_thread_entrytotftThreadEntryand update its call site. As per coding guidelines, C++ functions and members usecamelCase.Suggested rename
-void tft_thread_entry(void) +void tftThreadEntry(void)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/graphics/tftSetup.cpp` around lines 42 - 47, Rename the thread entry function tft_thread_entry to tftThreadEntry and update every call site to use the new camelCase name, preserving its existing behavior.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/graphics/tftSetup.cpp`:
- Around line 59-60: Handle the HUB75/hub75 panel selection before the fallback
lookup into panels, since its enum value is outside the panels array range.
Update the panel-selection logic around the panels array to return or process
the HUB75-specific path directly, while preserving indexed lookup for the
existing panel values.
In `@variants/native/portduino/platformio.ini`:
- Around line 310-327: Update the Windows build environment configuration around
HAS_TFT and USE_SDL so it no longer simultaneously defines HAS_SCREEN=0 or
MESHTASTIC_EXCLUDE_SCREEN=1 and no longer excludes Panel_sdl.cpp or
TFTDisplay.cpp. Preserve the intended SDL/MUI display build behavior and clarify
the configuration only if the environment is meant to remain headless.
---
Nitpick comments:
In `@src/graphics/tftSetup.cpp`:
- Around line 42-47: Rename the thread entry function tft_thread_entry to
tftThreadEntry and update every call site to use the new camelCase name,
preserving its existing behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: ad520ca7-be95-49a4-8510-82ad05d35089
📒 Files selected for processing (6)
platformio.inisrc/graphics/tftSetup.cppsrc/main.cppsrc/platform/portduino/PortduinoGlue.cppsrc/platform/portduino/PortduinoGlue.hvariants/native/portduino/platformio.ini
| static char *panels[] = {"NOSCREEN", "X11", "SDL", "FB", "ST7789", "ST7735", "ST7735S", | ||
| "ST7796", "ILI9341", "ILI9342", "ILI9486", "ILI9488", "HX8357D"}; |
There was a problem hiding this comment.
🩺 Stability & Availability | 🔴 Critical | ⚡ Quick win
Handle hub75 before indexing panels.
hub75 is enum index 13, but panels[] ends at index 12. A valid Display.Panel: HUB75 configuration therefore performs an out-of-bounds lookup in the fallback branch.
Suggested fix
- "ST7796", "ILI9341", "ILI9342", "ILI9486", "ILI9488", "HX8357D"};
+ "ST7796", "ILI9341", "ILI9342", "ILI9486", "ILI9488", "HX8357D", "HUB75"};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| static char *panels[] = {"NOSCREEN", "X11", "SDL", "FB", "ST7789", "ST7735", "ST7735S", | |
| "ST7796", "ILI9341", "ILI9342", "ILI9486", "ILI9488", "HX8357D"}; | |
| static char *panels[] = {"NOSCREEN", "X11", "SDL", "FB", "ST7789", "ST7735", "ST7735S", | |
| "ST7796", "ILI9341", "ILI9342", "ILI9486", "ILI9488", "HX8357D", "HUB75"}; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/graphics/tftSetup.cpp` around lines 59 - 60, Handle the HUB75/hub75 panel
selection before the fallback lookup into panels, since its enum value is
outside the panels array range. Update the panel-selection logic around the
panels array to return or process the HUB75-specific path directly, while
preserving indexed lookup for the existing panel values.
|
Converted to draft as lvgl 9.5.0 crashes (needs someone with a smarter AI model to throw at lv_draw_sdl.c) |
|
1 participant
This PR adds support for SDL for Linux+Windows (replaces #11090)
Note: requires device-ui PR#227 and patched lvgl 9.5.0
🤝 Attestations
Summary by CodeRabbit
New Features
Bug Fixes