Fix: Climate page crash and screensaver ignoring user settings#81
Merged
Fix: Climate page crash and screensaver ignoring user settings#81
Conversation
It was causing noise without adding much value. And it was never going to the end from user point of view.
It isn't released yet
Contributor
📝 WalkthroughWalkthroughRemoved several nspanel_easy sdkconfig options and the utilities blueprint flag; refactored boot and display initialization; moved screensaver setup to timer-driven apply logic; added indoor temperature visibility and screensaver globals across HMI variants; renumbered boot page component IDs; tightened weather-page theming and bumped blueprint/version thresholds. Changes
Sequence Diagram(s)(No sequence diagrams generated.) Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
components/nspanel_easy/base.h (1)
80-85: Update stale bit-range/count comments after reducing active flags to five.Lines 80–85 and 89–99 still describe six active flags (bits 0–5), but code now tracks five active flags. Please align the comments to prevent confusion.
📝 Suggested doc/comment alignment
- * `@brief` Check if all active flags (bits 0-5) are set - * `@return` true if all active flags (bits 0-5) are set, false otherwise + * `@brief` Check if all active flags (bits 0-4) are set + * `@return` true if all active flags (bits 0-4) are set, false otherwise @@ - // All 6 active flags must be set + // All 5 active flags must be set @@ - * `@brief` Count active flags (bits 0-5) set + * `@brief` Count active flags (bits 0-4) set @@ - * `@return` Percentage (0.0-100.0) of active flags set (bits 0-5) + * `@return` Percentage (0.0-100.0) of active flags set (bits 0-4)Also applies to: 89-90, 97-99
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/nspanel_easy/base.h` around lines 80 - 85, The comments describing "six active flags (bits 0-5)" are stale—update them to reflect there are five active flags (bits 0-4) and adjust any phrasing that mentions "6" or "bits 0-5"; specifically update the docblock for all_active_flags_set() and any nearby comment blocks that reference the old bit range so they state five active flags (bits 0-4) and list the tracked flags (page_home, page_qrcode, relay_settings, version, hw_buttons_settings) to match the boolean return logic.esphome/nspanel_esphome_hw_display.yaml (1)
637-638: Consider guarding runtimewakeup_page_idsend for consistency.Line 638 always sends the value, while boot init only sends when
wakeup_page_id > 0(Line 177). Mirroring that guard here would make behavior consistent and safer.♻️ Suggested consistency patch
- lambda: |- wakeup_page_id = get_page_id(x.c_str()); - disp1->send_command_printf("wakeup_page_id=%" PRIu8, wakeup_page_id); + if (wakeup_page_id > 0) { + disp1->send_command_printf("wakeup_page_id=%" PRIu8, wakeup_page_id); + } wait_to_be_ready->execute();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@esphome/nspanel_esphome_hw_display.yaml` around lines 637 - 638, The code always calls disp1->send_command_printf("wakeup_page_id=%" PRIu8, wakeup_page_id) after computing wakeup_page_id via get_page_id(x.c_str()); make this consistent with the boot/init path by only sending the command when wakeup_page_id > 0; locate the get_page_id(...) call and wrap the disp1->send_command_printf(...) call in an if (wakeup_page_id > 0) { ... } so the runtime behavior matches the existing boot-time guard.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@esphome/nspanel_esphome_page_boot.yaml`:
- Line 11: PAGE_BOOT_COMPONENT_ID_BT_REBOOT was changed to 20 but
nspanel_easy_landscape/boot.txt still defines bt_reboot as 21, causing a
mismatch; update the bt_reboot ID in nspanel_easy_landscape/boot.txt to 20 (or
revert PAGE_BOOT_COMPONENT_ID_BT_REBOOT to 21) so all boot.txt variants use the
same ID, and verify the constant PAGE_BOOT_COMPONENT_ID_BT_REBOOT and the
bt_reboot entries across all boot.txt files are consistent.
In `@esphome/nspanel_esphome_version.yaml`:
- Around line 14-15: The min_tft_version (currently set to 15) conflicts with
the landscape variant which still declares version: 13 in
hmi/dev/nspanel_easy_landscape/boot.txt; fix by updating the landscape
boot.txt's version field from 13 to 15 so it matches the other device variants
and satisfies min_tft_version, or alternatively adjust min_tft_version to 13 if
you intend to support the older landscape firmware—modify either the
min_tft_version value or the version line in
hmi/dev/nspanel_easy_landscape/boot.txt accordingly.
In `@nspanel_easy_blueprint.yaml`:
- Around line 8330-8344: The current conditional only sets component and icon
color when repeat.item.label_color_rgb != [200, 204, 200], so when
label_color_rgb returns to the default [200, 204, 200] the previous tint
remains; update the templated block around the conditional that calls esphome.{{
nspanel_name }}_component_color (for id '{{ repeat.item.component }}' and '{{
repeat.item.component }}_icon') to include an else branch that explicitly sets
both component and icon back to the default palette color (i.e., send the
default RGB value) and keep continue_on_error/delays consistent with the
existing then branch so the UI always resets when label_color_rgb equals [200,
204, 200].
---
Nitpick comments:
In `@components/nspanel_easy/base.h`:
- Around line 80-85: The comments describing "six active flags (bits 0-5)" are
stale—update them to reflect there are five active flags (bits 0-4) and adjust
any phrasing that mentions "6" or "bits 0-5"; specifically update the docblock
for all_active_flags_set() and any nearby comment blocks that reference the old
bit range so they state five active flags (bits 0-4) and list the tracked flags
(page_home, page_qrcode, relay_settings, version, hw_buttons_settings) to match
the boolean return logic.
In `@esphome/nspanel_esphome_hw_display.yaml`:
- Around line 637-638: The code always calls
disp1->send_command_printf("wakeup_page_id=%" PRIu8, wakeup_page_id) after
computing wakeup_page_id via get_page_id(x.c_str()); make this consistent with
the boot/init path by only sending the command when wakeup_page_id > 0; locate
the get_page_id(...) call and wrap the disp1->send_command_printf(...) call in
an if (wakeup_page_id > 0) { ... } so the runtime behavior matches the existing
boot-time guard.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 3a8f6c94-bd9b-4ea0-b57f-1b921f359e14
📒 Files selected for processing (82)
components/nspanel_easy/__init__.pycomponents/nspanel_easy/base.hesphome/nspanel_esphome_addon_climate_base.yamlesphome/nspanel_esphome_addon_upload_tft.yamlesphome/nspanel_esphome_api.yamlesphome/nspanel_esphome_base.yamlesphome/nspanel_esphome_boot.yamlesphome/nspanel_esphome_core.yamlesphome/nspanel_esphome_hw_display.yamlesphome/nspanel_esphome_hw_memory.yamlesphome/nspanel_esphome_page_boot.yamlesphome/nspanel_esphome_page_home.yamlesphome/nspanel_esphome_page_screensaver.yamlesphome/nspanel_esphome_page_utilities.yamlesphome/nspanel_esphome_version.yamlhmi/dev/nspanel_CJK_eu_code/Program.s.txthmi/dev/nspanel_CJK_eu_code/boot.txthmi/dev/nspanel_CJK_eu_code/home.txthmi/dev/nspanel_CJK_eu_code/screensaver.txthmi/dev/nspanel_CJK_eu_code/weather01.txthmi/dev/nspanel_CJK_eu_code/weather02.txthmi/dev/nspanel_CJK_eu_code/weather03.txthmi/dev/nspanel_CJK_eu_code/weather04.txthmi/dev/nspanel_CJK_eu_code/weather05.txthmi/dev/nspanel_CJK_us_code/Program.s.txthmi/dev/nspanel_CJK_us_code/boot.txthmi/dev/nspanel_CJK_us_code/home.txthmi/dev/nspanel_CJK_us_code/screensaver.txthmi/dev/nspanel_CJK_us_code/weather01.txthmi/dev/nspanel_CJK_us_code/weather02.txthmi/dev/nspanel_CJK_us_code/weather03.txthmi/dev/nspanel_CJK_us_code/weather04.txthmi/dev/nspanel_CJK_us_code/weather05.txthmi/dev/nspanel_CJK_us_land_code/Program.s.txthmi/dev/nspanel_CJK_us_land_code/boot.txthmi/dev/nspanel_CJK_us_land_code/home.txthmi/dev/nspanel_CJK_us_land_code/screensaver.txthmi/dev/nspanel_CJK_us_land_code/weather01.txthmi/dev/nspanel_CJK_us_land_code/weather02.txthmi/dev/nspanel_CJK_us_land_code/weather03.txthmi/dev/nspanel_CJK_us_land_code/weather04.txthmi/dev/nspanel_CJK_us_land_code/weather05.txthmi/dev/nspanel_eu_code/Program.s.txthmi/dev/nspanel_eu_code/boot.txthmi/dev/nspanel_eu_code/home.txthmi/dev/nspanel_eu_code/screensaver.txthmi/dev/nspanel_eu_code/weather01.txthmi/dev/nspanel_eu_code/weather02.txthmi/dev/nspanel_eu_code/weather03.txthmi/dev/nspanel_eu_code/weather04.txthmi/dev/nspanel_eu_code/weather05.txthmi/dev/nspanel_us_code/Program.s.txthmi/dev/nspanel_us_code/boot.txthmi/dev/nspanel_us_code/home.txthmi/dev/nspanel_us_code/screensaver.txthmi/dev/nspanel_us_code/weather01.txthmi/dev/nspanel_us_code/weather02.txthmi/dev/nspanel_us_code/weather03.txthmi/dev/nspanel_us_code/weather04.txthmi/dev/nspanel_us_code/weather05.txthmi/dev/nspanel_us_land_code/Program.s.txthmi/dev/nspanel_us_land_code/boot.txthmi/dev/nspanel_us_land_code/home.txthmi/dev/nspanel_us_land_code/screensaver.txthmi/dev/nspanel_us_land_code/weather01.txthmi/dev/nspanel_us_land_code/weather02.txthmi/dev/nspanel_us_land_code/weather03.txthmi/dev/nspanel_us_land_code/weather04.txthmi/dev/nspanel_us_land_code/weather05.txthmi/nspanel_CJK_eu.HMIhmi/nspanel_CJK_eu.tfthmi/nspanel_CJK_us.HMIhmi/nspanel_CJK_us.tfthmi/nspanel_CJK_us_land.HMIhmi/nspanel_CJK_us_land.tfthmi/nspanel_eu.HMIhmi/nspanel_eu.tfthmi/nspanel_us.HMIhmi/nspanel_us.tfthmi/nspanel_us_land.HMIhmi/nspanel_us_land.tftnspanel_easy_blueprint.yaml
💤 Files with no reviewable changes (2)
- esphome/nspanel_esphome_addon_upload_tft.yaml
- esphome/nspanel_esphome_core.yaml
edwardtfn
changed the title
This was referenced Apr 5, 2026
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Two fixes in this release: the climate page was crashing due to a regression introduced in the previous release, and the screensaver was not respecting the user's configured behavior (turn off or show time).
Summary by CodeRabbit
Summary by CodeRabbit
New Features
Bug Fixes & Improvements
UI Updates