Skip to content

fix(color): model settings may not be saved when editing global variables#7478

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

fix(color): model settings may not be saved when editing global variables#7478
philmoz wants to merge 1 commit into
mainfrom
philmoz/fix-gvars

Conversation

@philmoz

@philmoz philmoz commented Jun 21, 2026

Copy link
Copy Markdown
Collaborator

Changes:

  • set dirty flag when the flight mode toggle switches are changed for a GV
  • add a 'Reset' menu option to set the GV values back to default

The 'Reset' option is added to clear the flight mode GV values back to the default setting.
The 'Clear' option sets the flight mode GV values to 0 instead of the default value.

2.12 & 3.0.

Summary by CodeRabbit

  • Refactor
    • Improved state tracking for global variable edits so modifications are reliably marked and reflected in the interface.
  • Bug Fixes
    • Updated flight-mode value toggles to properly refresh affected properties after changes.
    • Corrected model reset actions to ensure the intended global-variable values are cleared/reset and modification status updates consistently.

@philmoz philmoz added this to the 2.12.3 milestone Jun 21, 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 labels Jun 21, 2026
@coderabbitai

coderabbitai Bot commented Jun 21, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 351c3d81-bf87-4291-983b-d96216d9290c

📥 Commits

Reviewing files that changed from the base of the PR and between 657b095 and cbdab43.

📒 Files selected for processing (1)
  • radio/src/gui/colorlcd/model/model_gvars.cpp
🚧 Files skipped from review as they are similar to previous changes (1)
  • radio/src/gui/colorlcd/model/model_gvars.cpp

📝 Walkthrough

Walkthrough

Two fixes in model_gvars.cpp: the per-flight-mode ToggleSwitch handler now calls SET_DIRTY() and setProperties(flightMode) instead of manually invalidating the widget. The STR_CLEAR and STR_SF_RESET menu callbacks replace storageDirty(EE_MODEL) with SET_DIRTY(), and STR_SF_RESET now explicitly resets flightModeData[0].gvars[index] to 0 after bulk-initializing the other entries.

Changes

GVar UI and reset fixes

Layer / File(s) Summary
ToggleSwitch handler: dirty flag and property refresh
radio/src/gui/colorlcd/model/model_gvars.cpp
Removes lv_obj_invalidate(cb->getLvObj()) and adds SET_DIRTY() + setProperties(flightMode) in the per-flight-mode gvar toggle handler.
Menu clear/reset callbacks
radio/src/gui/colorlcd/model/model_gvars.cpp
Replaces storageDirty(EE_MODEL) with SET_DIRTY() in both menu actions; STR_SF_RESET loop now also explicitly sets flightModeData[0].gvars[index] = 0.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Description check ❓ Inconclusive The description is missing the 'Fixes #' section from the template and lacks clarity on the implementation details despite mentioning changes made. Add the issue reference (Fixes #) and clarify which changes correspond to the two key improvements described in the PR objectives.
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly identifies the main issue (model settings not saved when editing global variables) and is specific to the core problem addressed in the changeset.
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.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ 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 philmoz/fix-gvars

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-gvars branch from 657b095 to cbdab43 Compare June 24, 2026 01:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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