feat: add settings controller for launcher settings page#84
Conversation
There was a problem hiding this comment.
Codex Review: Here are some suggestions.
Reply with @codex fix comments to fix any unresolved comments.
About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you open a pull request for review, mark a draft as ready, or comment "@codex review". If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex fix this CI failure" or "@codex address that feedback".
| void SettingsController::Impl::SetBrightness(uint8_t percent) | ||
| { | ||
| enqueue_task( | ||
| [this, percent]() | ||
| { | ||
| uint8_t clamped = std::clamp<uint8_t>(percent, kMinBrightness, kMaxBrightness); | ||
| config_.ui.brightness = clamped; | ||
| GetHAL()->setDisplayBrightness(clamped); | ||
| persist_config(); | ||
| ui_page_settings_set_brightness(clamped); |
There was a problem hiding this comment.
[P1] Avoid persisting config on every brightness tick
The brightness handler saves the full configuration to persistent storage for every call to SetBrightness. The settings page’s slider invokes set_brightness on each LV_EVENT_VALUE_CHANGED step, so dragging from 5% to 100% will enqueue dozens of persist_config() → app_cfg_save() writes in quick succession. app_cfg_save writes the struct through the registered backend (flash), so this will generate heavy flash wear and can stall the UI while the worker thread flushes each write. Consider applying the brightness change live but deferring the save until interaction ends or batching updates.
Useful? React with 👍 / 👎.
Summary
Testing
https://chatgpt.com/codex/tasks/task_e_68cd2dd842e08324bf88b02ad988d79c