Climate chip visibility control & inverted Kelvin color temperature fix#36
Merged
Climate chip visibility control & inverted Kelvin color temperature fix#36
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Contributor
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughMinimum blueprint/TFT versions bumped to 5; HMI boot files' version Text updated to 5; tempslider bounds swapped (inverted) across light configs; blueprint adds Changes
Sequence Diagram(s)sequenceDiagram
participant Nextion as NSPanel UI
participant Blueprint as Blueprint (nspanel_easy)
participant ESPHome as ESPhome addon (addon_climate)
participant Storage as Global Restore
Note over Blueprint,ESPHome: Boot / initialization flow for climate visibility
Blueprint->>ESPHome: send action_component_val(page:"mem", key:"climate_chip_visibility", val)
ESPHome->>ESPHome: parse value -> set global climate_chip_visibility
ESPHome->>Storage: store climate_chip_visibility (restore)
Nextion->>ESPHome: request update_climate_icon / render
ESPHome->>ESPHome: evaluate climate_chip_visibility (NEVER/ALWAYS/ACTIVE_ONLY)
alt Visible
ESPHome->>Nextion: send icon/color update
else Hidden
ESPHome->>Nextion: send NONE / BLACK (no icon)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![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: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@hmi/dev/nspanel_CJK_us_code/light.txt`:
- Around line 687-688: The tempslider bounds are inverted: swap the two numeric
values so the Upper range limit is 6535 and the Lower range limit is 2000;
update the "Upper range limit" and "Lower range limit" entries for the
tempslider block (currently showing 2000 and 6535) to the corrected values to
restore proper slider/Kelvin behavior.
In `@hmi/dev/nspanel_CJK_us_land_code/light.txt`:
- Around line 687-688: The tempslider bounds are inverted: the "Upper range
limit" and "Lower range limit" lines currently put Upper at 2000 and Lower at
6535; update the tempslider bounds so the upper bound is 6535 and the lower
bound is 2000 by swapping the values on the existing "Upper range limit" and
"Lower range limit" entries (look for the tempslider/kelvin slider block
containing those two lines) so the slider domain is valid and interactive.
In `@hmi/dev/nspanel_eu_code/light.txt`:
- Around line 687-688: The tempslider's Kelvin bounds are inverted (Upper range
limit is 2000 and Lower range limit is 6535) which breaks color_temp_kelvin
forwarded to esphome/nspanel_esphome_page_light.yaml; fix by swapping the values
so the Upper range limit is 6535 and the Lower range limit is 2000 (restore
normal min/max for tempslider to match the color_temp_kelvin domain).
In `@hmi/dev/nspanel_us_land_code/light.txt`:
- Around line 687-688: The tempslider range was inverted (Upper and Lower values
swapped), making the slider invalid; locate the tempslider configuration where
"Upper range limit" and "Lower range limit" are set and restore them to
Upper=6535 and Lower=2000 so the min→max ordering is correct (i.e., swap the
current values back to Upper 6535, Lower 2000).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: d6a0958c-1ca4-4cf4-832c-338f2ab872dc
📒 Files selected for processing (26)
esphome/nspanel_esphome_version.yamlhmi/dev/nspanel_CJK_eu_code/boot.txthmi/dev/nspanel_CJK_eu_code/light.txthmi/dev/nspanel_CJK_us_code/boot.txthmi/dev/nspanel_CJK_us_code/light.txthmi/dev/nspanel_CJK_us_land_code/boot.txthmi/dev/nspanel_CJK_us_land_code/light.txthmi/dev/nspanel_eu_code/boot.txthmi/dev/nspanel_eu_code/light.txthmi/dev/nspanel_us_code/boot.txthmi/dev/nspanel_us_code/light.txthmi/dev/nspanel_us_land_code/boot.txthmi/dev/nspanel_us_land_code/light.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
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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_addon_climate_base.yaml`:
- Around line 394-397: The visibility check erroneously treats
CLIMATE_ACTION_IDLE as "active"; update the active-range logic used with
id(climate_chip_visibility) and the is_active boolean so
CLIMATE_CHIP_ACTIVE_ONLY excludes CLIMATE_ACTION_IDLE (value
CLIMATE_ACTION_IDLE) — i.e., consider only heating/cooling/drying/fan actions
between CLIMATE_ACTION_COOLING and CLIMATE_ACTION_FAN but skip the idle action
when computing is_active; make the same change for the other occurrence handling
visibility (the block around the second check). Ensure you reference
nspanel_easy::ClimateChipVisibility, CLIMATE_CHIP_ACTIVE_ONLY, is_active,
CLIMATE_ACTION_COOLING, CLIMATE_ACTION_IDLE, and CLIMATE_ACTION_FAN when
updating the conditions.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: ed5b64c4-3c12-4d1d-a626-0e9d39b746c7
📒 Files selected for processing (3)
components/nspanel_easy/addon_climate.hesphome/nspanel_esphome_addon_climate_base.yamlnspanel_easy_blueprint.yaml
edwardtfn
changed the title
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
esphome/nspanel_esphome_addon_climate_base.yaml (1)
103-105: Sanitize incomingclimate_chip_visibilitybefore storing.Line 104 currently trusts
valand casts directly touint8_t. Clamping to known enum range avoids invalid persisted/runtime states.Proposed refactor
if (component == "climate_chip_visibility") { - id(climate_chip_visibility) = static_cast<uint8_t>(val); + const bool valid = (val >= static_cast<int>(nspanel_easy::CLIMATE_CHIP_ACTIVE_ONLY) && + val <= static_cast<int>(nspanel_easy::CLIMATE_CHIP_NEVER)); + id(climate_chip_visibility) = valid + ? static_cast<uint8_t>(val) + : static_cast<uint8_t>(nspanel_easy::CLIMATE_CHIP_ACTIVE_ONLY); + if (!valid) { + ESP_LOGW("${TAG_ADDON_CLIMATE}", "Invalid climate_chip_visibility: %d. Falling back to ACTIVE_ONLY.", val); + } ESP_LOGV("${TAG_ADDON_CLIMATE}", "climate_chip_visibility: %" PRIu8, id(climate_chip_visibility)); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@esphome/nspanel_esphome_addon_climate_base.yaml` around lines 103 - 105, The code directly casts val into id(climate_chip_visibility) without validation; validate and clamp val to the allowed enum range before assigning to id(climate_chip_visibility) to avoid invalid persisted/runtime states — e.g., compute a sanitized_value = std::max(min_enum_value, std::min(max_enum_value, val)) using the known enum bounds for climate_chip_visibility and then assign id(climate_chip_visibility) = static_cast<uint8_t>(sanitized_value), updating the ESP_LOGV call to log the sanitized_value; locate this logic where component == "climate_chip_visibility" and replace the direct static_cast with the clamping step.
🤖 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_addon_climate_base.yaml`:
- Around line 415-426: The code only sets icon/color when action ==
nspanel_easy::CLIMATE_ACTION_OFF or is_active, so when visible == ALWAYS and
action is IDLE the chip is left as Icons::NONE; update the branch in the visible
handling to also assign icon/color for idle (or any non-active non-OFF action)
by using climate_action_icons[action].icon and .color (or specifically handle
nspanel_easy::CLIMATE_ACTION_IDLE) so IDLE renders correctly; look for the
visible check and the action/is_active logic to add this assignment.
---
Nitpick comments:
In `@esphome/nspanel_esphome_addon_climate_base.yaml`:
- Around line 103-105: The code directly casts val into
id(climate_chip_visibility) without validation; validate and clamp val to the
allowed enum range before assigning to id(climate_chip_visibility) to avoid
invalid persisted/runtime states — e.g., compute a sanitized_value =
std::max(min_enum_value, std::min(max_enum_value, val)) using the known enum
bounds for climate_chip_visibility and then assign id(climate_chip_visibility) =
static_cast<uint8_t>(sanitized_value), updating the ESP_LOGV call to log the
sanitized_value; locate this logic where component == "climate_chip_visibility"
and replace the direct static_cast with the clamping step.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: ce5d98dc-e237-453e-bfe9-ce82e4906ce0
📒 Files selected for processing (1)
esphome/nspanel_esphome_addon_climate_base.yaml
This was referenced Mar 24, 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.
Improvements
Bug Fixes
Other Changes
The
climate_chip_always_visibleboolean input has been removed. Please update your blueprint automation:climate_chip_always_visible: false(default)climate_chip_visibility: active_onlyclimate_chip_always_visible: trueclimate_chip_visibility: alwaysIf you haven't customized this setting, no action is needed — the new default (
active_only) preserves the previous behaviour.Summary by CodeRabbit
New Features
Bug Fixes
Chores