Skip to content

fix(color): memory leak when creating a blank model#7541

Open
philmoz wants to merge 1 commit into
mainfrom
philmoz/fix-memory-leak
Open

fix(color): memory leak when creating a blank model#7541
philmoz wants to merge 1 commit into
mainfrom
philmoz/fix-memory-leak

Conversation

@philmoz

@philmoz philmoz commented Jul 9, 2026

Copy link
Copy Markdown
Collaborator

When creating a blank model an extra view screen is created.

Summary by CodeRabbit

  • Bug Fixes
    • Improved custom-screen lifecycle: screens reload more consistently during startup and after removing/updating screens, avoiding stale or lingering layouts.
    • Fixed model/template initialization so the default UI layout reliably loads when an empty template name is used.
    • Improved SD storage reset/format flow (including first-run behavior) so the full reset and save sequence completes consistently, reducing partial or inconsistent restores.

@philmoz philmoz added this to the 2.12.3 milestone Jul 9, 2026
@philmoz philmoz added bug 🪲 Something isn't working color Related generally to color LCD radios UX-UI Related to user experience (UX) or user interface (UI) behaviour backport/2.12 To be backported to a 2.12 release also. labels Jul 9, 2026
@coderabbitai

coderabbitai Bot commented Jul 9, 2026

Copy link
Copy Markdown

Review Change Stack

Note

Reviews paused

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.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Custom-screen deletion is centralized in LayoutFactory::loadCustomScreens(), with startup, screen removal, hardware-input, and model flows updated. Storage formatting, erasure, and immediate-save behavior are consolidated into internal helpers and their call sites are updated.

Changes

Custom Screen Lifecycle Changes

Layer / File(s) Summary
Centralize screen deletion
radio/src/gui/colorlcd/mainview/layout.cpp
loadCustomScreens() now clears existing custom screens before loading configured layouts.
Update custom-screen callers
radio/src/edgetx.cpp, radio/src/gui/colorlcd/mainview/screen_setup.cpp, radio/src/gui/colorlcd/radio/hw_inputs.cpp, radio/src/gui/colorlcd/model/model_select.cpp, radio/src/model_init.cpp
Startup, screen removal, hardware-input, model creation, and default-template flows use the revised screen lifecycle.

Storage Save/Erase Helper Refactor

Layer / File(s) Summary
Introduce storage helpers
radio/src/storage/sdcard_common.cpp, radio/src/storage/storage.h
forceSave() and storageFormat() are added; storageEraseAll() becomes static and argument-less; obsolete header declarations are removed.
Update storage call sites
radio/src/storage/sdcard_common.cpp
Model creation and storage initialization use forceSave(), while storageReadAll() calls the new storageEraseAll() signature.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related PRs

  • EdgeTX/edgetx#7537: Touches the same ColorLCD custom-screen lifecycle and reload call paths.
🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Description check ⚠️ Warning The description is too sparse and omits the required template sections, including the Fixes # reference and Summary of changes heading. Add the required template sections: a Fixes # issue reference and a Summary of changes section describing the blank-model screen fix.
Docstring Coverage ⚠️ Warning Docstring coverage is 9.09% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly matches the main change: fixing the blank-model screen leak issue.
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.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch philmoz/fix-memory-leak

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.

@philmoz
philmoz force-pushed the philmoz/fix-memory-leak branch 4 times, most recently from 2109662 to 6a57298 Compare July 16, 2026 05:01
@philmoz
philmoz force-pushed the philmoz/fix-memory-leak branch from 6a57298 to ecd0c79 Compare July 21, 2026 07:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport/2.12 To be backported to a 2.12 release also. bug 🪲 Something isn't working color Related generally to color LCD radios UX-UI Related to user experience (UX) or user interface (UI) behaviour

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant