Water heater entity support & critical blueprint fix#39
Merged
Conversation
Adjust device_name to nspanel_name in blueprint
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)
📝 WalkthroughWalkthroughAdds water_heater UI pages and ESPHome wiring, increments blueprint/TFT minimums 6→7, streamlines BLE teardown for TFT uploads, removes a global C++ temperature_unit symbol, and changes climatesetting Page Exit payload keys from "page,..." to "climate,...". Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant NSPanel as NSPanel UI
participant ESPHome as ESPHome (nspanel_easy)
participant HA as Home Assistant
User->>NSPanel: touch slider or press mode button
NSPanel->>ESPHome: emit localevent (temperature CSV or operation_mode)
ESPHome->>HA: call service (water_heater.set_temperature / set_operation_mode)
HA-->>ESPHome: state update / confirmation
ESPHome-->>NSPanel: publish updated page values
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 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 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: 13
🤖 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_eu_code/water_heater.txt`:
- Around line 452-709: The water_heater page incorrectly uses HVAC mode buttons
(Text button01..button07) and sets lastclick.txt to "water_heater,hvac_mode,..."
which is invalid for water heater entities; update the Touch Release Event
strings for the affected components (Text button01 through Text button07) to use
water heater operation modes (e.g., change
lastclick.txt="water_heater,hvac_mode,eco" to
lastclick.txt="water_heater,operation_mode,eco", and map buttons as recommended:
button01->eco, button02->electric, button03->performance, button04->heat_pump,
button05->gas, button07->off), ensuring the event payloads (prints/ printh
blocks) remain the same except for the updated operation_mode token.
- Around line 965-975: The Timer Event for click_timer builds lastclick.txt with
the wrong component name; replace the literal "climate" in the assignment
lastclick.txt="climate,long_click,"+click_comp.txt with "water_heater" so the
entry becomes lastclick.txt="water_heater,long_click,"+click_comp.txt; update
only that string in the click_timer Timer Event so other fields (printh, prints,
click_timer.en) remain unchanged.
- Around line 738-755: The Touch Release Event for button08 builds lastclick.txt
with the wrong entity type ("climate"); update the string construction in that
event so it uses "water_heater" instead of "climate" (i.e., change the literal
in the assignment to lastclick.txt="water_heater,short_click,"+click_comp.txt)
while leaving the surrounding logic (click_timer.en check, resetting
click_timer.en, and the subsequent printh/prints calls) unchanged; locate this
in the Touch Release Event for button08 where lastclick.txt is assigned.
- Around line 51-63: The Page Exit Event constructs climatesetting.txt using a
period separator ("water_heater.temperature,") which is inconsistent with the
timer0 Timer Event format ("water_heater,temperature,"); update the Page Exit
Event string construction (where climatesetting.txt is set, referencing va1.txt
and timer0.en) to use the comma-separated event identifier
"water_heater,temperature," so both timer0 and the Page Exit Event emit the same
event format.
In `@hmi/dev/nspanel_us_code/water_heater.txt`:
- Around line 965-975: The Timer Event handler click_timer constructs
lastclick.txt with the wrong namespace; change the string literal "climate" to
"water_heater" so lastclick.txt is built as
"water_heater,long_click,"+click_comp.txt; locate the click_timer Timer Event
block and update the assignment to lastclick.txt accordingly, leaving
click_comp.txt and the rest of the prints unchanged.
- Around line 452-709: The page's Text buttons (Text button01..Text button07 /
IDs 16–22) send HVAC mode payloads that water_heater entities don't support;
update each Touch Release Event to use water_heater,operation_mode,<mode> with
valid water heater modes instead of HVAC modes — e.g. change lastclick.txt for
ID 16 (Text button01) to "water_heater,operation_mode,eco", ID 17 to
"water_heater,operation_mode,electric", ID 18 to
"water_heater,operation_mode,performance", ID 19 to
"water_heater,operation_mode,heat_pump", ID 20 to
"water_heater,operation_mode,gas", ID 21 to a valid mode such as
"water_heater,operation_mode,electric" (or another appropriate water_heater
mode), and ID 22 to "water_heater,operation_mode,off"; ensure the
prints/sequence around lastclick.txt remain the same but reference the new
operation_mode strings.
- Around line 738-755: The Touch Release Event builds lastclick.txt using the
wrong entity type; update the string construction in the Touch Release Event so
lastclick.txt uses "water_heater" instead of "climate" (locate the line that
sets lastclick.txt="climate,short_click,"+click_comp.txt in the Touch Release
Event and change it to "water_heater,short_click,"+click_comp.txt); ensure you
only modify that literal while leaving click_comp.txt, click_timer.en, and the
surrounding prints/printh sequence unchanged.
- Around line 51-63: The Page Exit Event emits an inconsistent event string
("water_heater.temperature," with a period) that will be misparsed compared to
the timer0 Timer Event ("water_heater,temperature,"), so update the string
construction in the Page Exit Event (where climatesetting.txt is set using
va1.txt) to use the comma-separated format "water_heater,temperature," to match
timer0; search for other occurrences of "water_heater.temperature" and
standardize them to "water_heater,temperature" to keep event formatting
consistent across timer0 and the Page Exit Event.
- Around line 92-96: The local integer variable temp_step is set to 0 in the EU
and US landscape variants which is invalid per the API (temp_step encodes step
size*10); update the temp_step initialization for those variants to 1 to match
the US variant (representing 0.1°C steps) unless the difference is
intentional—if intentional, add a clear comment/documentation explaining why
temp_step remains 0; search for the temp_step symbol in each variant block and
change the Value from 0 to 1 (or add the explanatory comment).
In `@hmi/dev/nspanel_us_land_code/water_heater.txt`:
- Around line 965-975: The Timer Event for click_timer builds lastclick.txt with
the wrong page identifier; update the string construction
lastclick.txt="climate,long_click,"+click_comp.txt to use "water_heater" instead
of "climate" so it reads
lastclick.txt="water_heater,long_click,"+click_comp.txt; modify the click_timer
Timer Event where lastclick.txt is assigned (and verify click_comp.txt is the
intended suffix) to ensure page context consistency.
- Around line 452-709: The page is using HVAC modes for water_heater buttons;
update each Text button's Touch Release Event to use operation_mode instead of
hvac_mode and set water-heater-appropriate values: change Text button01 (ID 16)
lastclick.txt to "water_heater,operation_mode,eco", Text button02 (ID 17) to
"water_heater,operation_mode,electric", Text button03 (ID 18) to
"water_heater,operation_mode,performance", Text button04 (ID 19) to
"water_heater,operation_mode,heat_pump", Text button05 (ID 20) to
"water_heater,operation_mode,gas", and Text button07 (ID 22) to
"water_heater,operation_mode,off"; for Text button06 (ID 21) either replace the
hvac_mode value with a valid water heater operation_mode or remove/repurpose the
button if no matching mode exists.
- Around line 51-63: The Page Exit Event builds climatesetting.txt using
"water_heater.temperature," which is inconsistent with the timer0 Timer Event
format "water_heater,temperature,"; update the concatenation in the Page Exit
Event (the block that checks timer0.en and sets climatesetting.txt using
va1.txt) to use a comma separator so it becomes
"water_heater,temperature,"+va1.txt (i.e., ensure climatesetting.txt uses the
same "water_heater,temperature," token as timer0) to fix parsing mismatch.
- Around line 738-755: The Touch Release Event builds lastclick.txt using the
wrong entity type string ("climate"); update the construction of lastclick.txt
in the Touch Release Event to use "water_heater" instead of "climate" (i.e.,
change the literal in the expression that sets lastclick.txt =
"climate,short_click," + click_comp.txt to "water_heater,short_click," +
click_comp.txt) so events are sent to the correct entity; verify there are no
other occurrences of the "climate,short_click" literal on this page and keep the
surrounding timer reset (click_timer.en=0) and print/send sequence intact.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 7b2feb93-ae93-4a9d-81fe-43221f7ea823
📒 Files selected for processing (13)
hmi/dev/nspanel_eu_code/boot.txthmi/dev/nspanel_eu_code/water_heater.txthmi/dev/nspanel_us_code/boot.txthmi/dev/nspanel_us_code/water_heater.txthmi/dev/nspanel_us_land_code/boot.txthmi/dev/nspanel_us_land_code/water_heater.txthmi/nspanel_eu.HMIhmi/nspanel_eu.tfthmi/nspanel_us.HMIhmi/nspanel_us.tfthmi/nspanel_us_land.HMIhmi/nspanel_us_land.tftnspanel_easy_blueprint.yaml
Owner
Author
|
@CodeRabbit review |
Contributor
✅ Actions performedReview triggered.
|
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
♻️ Duplicate comments (5)
hmi/dev/nspanel_CJK_us_land_code/water_heater.txt (1)
51-56:⚠️ Potential issue | 🔴 CriticalSend the exit event with commas, not a dotted token.
Line 56 uses
water_heater.temperature,..., while the timer path and ESPHome handler usewater_heater,temperature,.... With the current string, leaving the page beforetimer0fires loses the pending setpoint change.🔧 Proposed fix
- climatesetting.txt="water_heater.temperature,"+va1.txt + climatesetting.txt="water_heater,temperature,"+va1.txt🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@hmi/dev/nspanel_CJK_us_land_code/water_heater.txt` around lines 51 - 56, The exit-event string uses a dotted token and must use comma-separated fields so the ESPHome handler and timer path match; update the assignment to climatesetting.txt in the Page Exit Event (the block handling timer0.en and covx temp_number0.val) to build "water_heater,temperature," + va1.txt instead of "water_heater.temperature,...", ensuring the event sent when timer0.en is cleared uses commas to preserve the pending setpoint change.hmi/dev/nspanel_CJK_us_code/water_heater.txt (1)
51-56:⚠️ Potential issue | 🔴 CriticalMatch the page-exit payload to the water-heater parser.
Line 56 builds
water_heater.temperature,..., but the receiver expectswater_heater,temperature,.... That breaks the page-exit fallback and can silently drop the target-temperature update.🔧 Proposed fix
- climatesetting.txt="water_heater.temperature,"+va1.txt + climatesetting.txt="water_heater,temperature,"+va1.txt🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@hmi/dev/nspanel_CJK_us_code/water_heater.txt` around lines 51 - 56, The page-exit payload string built in the branch that runs when timer0.en==1 currently sets climatesetting.txt to "water_heater.temperature,"+va1.txt which doesn't match the parser's expected "water_heater,temperature,..."; update the assignment in that block so climatesetting.txt uses the comma-separated prefix "water_heater,temperature," + va1.txt (locate the code around timer0.en, covx temp_number0.val and va1.txt) to restore the correct page-exit fallback payload format.hmi/dev/nspanel_us_land_code/water_heater.txt (1)
51-56:⚠️ Potential issue | 🔴 CriticalFix the page-exit temperature event token.
Line 56 uses
water_heater.temperature,...instead of the comma-separated token used everywhere else. That makes the page-exit fallback emit a 2-field payload, so a quick back-navigation can discard the pending temperature change.🔧 Proposed fix
- climatesetting.txt="water_heater.temperature,"+va1.txt + climatesetting.txt="water_heater,temperature,"+va1.txt🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@hmi/dev/nspanel_us_land_code/water_heater.txt` around lines 51 - 56, The page-exit event builds a wrong token string: replace the dotted token "water_heater.temperature,"+va1.txt with the comma-separated token used elsewhere so the fallback emits the correct multi-field payload; update the assignment to climatesetting.txt (in the Page Exit Event block that checks timer0.en) to use "water_heater,temperature," + va1.txt so it matches other token formatting.hmi/dev/nspanel_eu_code/water_heater.txt (1)
51-56:⚠️ Potential issue | 🔴 CriticalUse the same CSV event token on page exit.
Line 56 sends
water_heater.temperature,..., but the timer path useswater_heater,temperature,...and the ESPHome handler only acceptswater_heater,key,value. If the user leaves beforetimer0fires, that pending setpoint update is dropped.🔧 Proposed fix
- climatesetting.txt="water_heater.temperature,"+va1.txt + climatesetting.txt="water_heater,temperature,"+va1.txt🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@hmi/dev/nspanel_eu_code/water_heater.txt` around lines 51 - 56, The page-exit action is emitting a different CSV token ("water_heater.temperature,...") than the timer/event path expects ("water_heater,temperature,..."), which causes the ESPHome handler (expects water_heater,key,value) to drop updates; change the page-exit assignment that sets climatesetting.txt so it uses the same comma-separated token format (e.g., "water_heater,temperature," + va1.txt) to match the timer0 path and the ESPHome handler (refer to timer0, covx temp_number0.val, va1.txt, and climatesetting.txt in the diff).hmi/dev/nspanel_us_code/water_heater.txt (1)
51-56:⚠️ Potential issue | 🔴 CriticalKeep the page-exit localevent in CSV format.
Line 56 still uses
water_heater.temperature,.... That does not match thewater_heater,key,valueformat consumed byesphome/nspanel_esphome_page_water_heater.yaml, so backing out of the page beforetimer0fires drops the new target temperature.🔧 Proposed fix
- climatesetting.txt="water_heater.temperature,"+va1.txt + climatesetting.txt="water_heater,temperature,"+va1.txt🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@hmi/dev/nspanel_us_code/water_heater.txt` around lines 51 - 56, The page-exit localevent must be in CSV format; change the climatesetting.txt assignment so it emits three comma-separated fields instead of a dotted prefix. In the Page Exit Event block where timer0.en is cleared and covx temp_number0.val,va1.txt is called, replace climatesetting.txt="water_heater.temperature,"+va1.txt with climatesetting.txt="water_heater,temperature,"+va1.txt (i.e., use "water_heater,<key>,<value>" CSV format) so esphome/nspanel_esphome_page_water_heater.yaml can consume the event.
🤖 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_upload_tft.yaml`:
- Around line 267-281: The esp_bt_controller_mem_release(ESP_BT_MODE_BTDM) call
is currently guarded by USE_ESP32_BLE which is also defined for NimBLE and other
BLE stacks and can break builds/runtimes; update the guard so the ESP-IDF
Bluetooth controller release runs only when the Bluetooth Proxy controller is in
use (e.g., switch the preprocessor check to USE_BLUETOOTH_PROXY or add an
additional `#if` defined(USE_BLUETOOTH_PROXY) around the
esp_bt_controller_mem_release block), or alternatively add a runtime check that
the BT controller was initialized before calling
esp_bt_controller_mem_release(ESP_BT_MODE_BTDM) (referencing
esp_bt_controller_mem_release, ESP_BT_MODE_BTDM and the surrounding
lambda/App.feed_wdt block).
In `@esphome/nspanel_esphome_page_climate.yaml`:
- Around line 106-125: The addon extension's change_climate_state lambda
currently only checks embedded before making thermostat calls and can run when
the device isn't ready; add a guard that calls is_device_ready_for_tasks() at
the start of that lambda (alongside the existing embedded check) so the function
returns early if the device isn't ready, preventing calls like
thermostat_embedded->make_call() from running on an unready device.
In `@esphome/nspanel_esphome_page_water_heater.yaml`:
- Around line 9-10: The PAGE_WATER_HEATER_ID substitution is wrong (7) and must
match the central registry where water_heater is registered as 30; update the
substitution value PAGE_WATER_HEATER_ID to 30 and adjust any occurrences that
reference it (the hook that currently fires for the climate page) so they use
the corrected substitution, ensuring the page hook for water_heater now targets
index 30 as declared in components/nspanel_easy/pages.h.
In `@hmi/dev/nspanel_CJK_eu_code/water_heater.txt`:
- Around line 957-967: The long_click event string is using the wrong entity
type ("climate,long_click,") on the water_heater page; update the emitted event
to use "water_heater,long_click," instead. Locate the Timer Event block where
lastclick.txt is built (references: lastclick.txt, click_comp.txt,
click_timer.en) and replace "climate,long_click," with
"water_heater,long_click," so the click_timer emission matches the page's entity
naming convention.
In `@nspanel_easy_blueprint.yaml`:
- Line 11091: The UI only enables matching mode buttons but doesn't first
hide/reset buttons from a previously-used water heater; update the block that
uses operation_list and water_heater_entity (and the similar block at the other
occurrence) to first reset/hide all mode buttons, then iterate operation_list to
enable/show only supported modes, and compute the active button styling using
the resolved operation_mode (derived from the current water_heater_entity's
state or state_attr) rather than raw states(...), ensuring unsupported buttons
are not visible or clickable when switching entities.
- Around line 11203-11207: The total_steps calculation should use an aligned min
and an aligned max to avoid overshooting or hiding valid steps: change the
max_temp computation (currently using rounding: max_temp = '(max_temp_raw /
temp_step) | round * temp_step') to align down to the nearest step (i.e.,
subtract max_temp_raw % temp_step when not divisible) so it never exceeds the
entity max, keep temp_offset as the aligned minimum (temp_offset_raw adjusted up
as currently implemented), and compute total_steps as '( (aligned_max -
temp_offset) / temp_step ) | round(0) | int' (use the aligned_max symbol in
place of max_temp_raw) so the step count is derived from the aligned range
between temp_offset and the floored max_temp.
---
Duplicate comments:
In `@hmi/dev/nspanel_CJK_us_code/water_heater.txt`:
- Around line 51-56: The page-exit payload string built in the branch that runs
when timer0.en==1 currently sets climatesetting.txt to
"water_heater.temperature,"+va1.txt which doesn't match the parser's expected
"water_heater,temperature,..."; update the assignment in that block so
climatesetting.txt uses the comma-separated prefix "water_heater,temperature," +
va1.txt (locate the code around timer0.en, covx temp_number0.val and va1.txt) to
restore the correct page-exit fallback payload format.
In `@hmi/dev/nspanel_CJK_us_land_code/water_heater.txt`:
- Around line 51-56: The exit-event string uses a dotted token and must use
comma-separated fields so the ESPHome handler and timer path match; update the
assignment to climatesetting.txt in the Page Exit Event (the block handling
timer0.en and covx temp_number0.val) to build "water_heater,temperature," +
va1.txt instead of "water_heater.temperature,...", ensuring the event sent when
timer0.en is cleared uses commas to preserve the pending setpoint change.
In `@hmi/dev/nspanel_eu_code/water_heater.txt`:
- Around line 51-56: The page-exit action is emitting a different CSV token
("water_heater.temperature,...") than the timer/event path expects
("water_heater,temperature,..."), which causes the ESPHome handler (expects
water_heater,key,value) to drop updates; change the page-exit assignment that
sets climatesetting.txt so it uses the same comma-separated token format (e.g.,
"water_heater,temperature," + va1.txt) to match the timer0 path and the ESPHome
handler (refer to timer0, covx temp_number0.val, va1.txt, and climatesetting.txt
in the diff).
In `@hmi/dev/nspanel_us_code/water_heater.txt`:
- Around line 51-56: The page-exit localevent must be in CSV format; change the
climatesetting.txt assignment so it emits three comma-separated fields instead
of a dotted prefix. In the Page Exit Event block where timer0.en is cleared and
covx temp_number0.val,va1.txt is called, replace
climatesetting.txt="water_heater.temperature,"+va1.txt with
climatesetting.txt="water_heater,temperature,"+va1.txt (i.e., use
"water_heater,<key>,<value>" CSV format) so
esphome/nspanel_esphome_page_water_heater.yaml can consume the event.
In `@hmi/dev/nspanel_us_land_code/water_heater.txt`:
- Around line 51-56: The page-exit event builds a wrong token string: replace
the dotted token "water_heater.temperature,"+va1.txt with the comma-separated
token used elsewhere so the fallback emits the correct multi-field payload;
update the assignment to climatesetting.txt (in the Page Exit Event block that
checks timer0.en) to use "water_heater,temperature," + va1.txt so it matches
other token formatting.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 3afcea44-5a63-4b87-b197-534fc3ad5541
📒 Files selected for processing (32)
components/nspanel_easy/page_climate.cppcomponents/nspanel_easy/page_climate.hcomponents/nspanel_easy/pages.hesphome/nspanel_esphome_addon_upload_tft.yamlesphome/nspanel_esphome_api.yamlesphome/nspanel_esphome_hw_temperature.yamlesphome/nspanel_esphome_page_climate.yamlesphome/nspanel_esphome_page_water_heater.yamlesphome/nspanel_esphome_standard.yamlesphome/nspanel_esphome_version.yamlhmi/dev/nspanel_CJK_eu_code/boot.txthmi/dev/nspanel_CJK_eu_code/water_heater.txthmi/dev/nspanel_CJK_us_code/boot.txthmi/dev/nspanel_CJK_us_code/water_heater.txthmi/dev/nspanel_CJK_us_land_code/boot.txthmi/dev/nspanel_CJK_us_land_code/water_heater.txthmi/dev/nspanel_eu_code/water_heater.txthmi/dev/nspanel_us_code/water_heater.txthmi/dev/nspanel_us_land_code/water_heater.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)
- components/nspanel_easy/page_climate.h
- components/nspanel_easy/page_climate.cpp
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (7)
esphome/nspanel_esphome_addon_upload_tft.yaml (1)
247-281:⚠️ Potential issue | 🟠 MajorGuard the BLE teardown with
USE_BLUETOOTH_PROXY.This stop/wait/release sequence is still compiled under
USE_ESP32_BLE, but this repo already models BLE server, BLE client, and Bluetooth proxy as separate modes. That keepsesp32_ble::global_ble->disable()andesp_bt_controller_mem_release()reachable on non-proxy BLE builds, which is the same compatibility problem previously raised for tracker/server/NimBLE-style configs.Please verify this path on a config that enables BLE server or BLE client without Bluetooth Proxy.
🛠️ Suggested fix
- `#ifdef` USE_ESP32_BLE + `#ifdef` USE_BLUETOOTH_PROXY ESP_LOGI("${TAG_UPLOAD_TFT}", "Stopping Bluetooth stack to free memory for TFT upload"); ESP_LOGI("${TAG_UPLOAD_TFT}", "Free heap before: %u bytes, largest block: %u bytes", esp_get_free_heap_size(), heap_caps_get_largest_free_block(MALLOC_CAP_8BIT)); App.feed_wdt(); if (esp32_ble::global_ble != nullptr) { ESP_LOGI("${TAG_UPLOAD_TFT}", "Disabling BLE stack"); esp32_ble::global_ble->disable(); } - `#endif` // USE_ESP32_BLE + `#endif` // USE_BLUETOOTH_PROXY ... - `#ifdef` USE_ESP32_BLE + `#ifdef` USE_BLUETOOTH_PROXY return (esp32_ble::global_ble == nullptr || !esp32_ble::global_ble->is_active()); `#else` return true; - `#endif` // USE_ESP32_BLE + `#endif` // USE_BLUETOOTH_PROXY ... - `#ifdef` USE_ESP32_BLE + `#ifdef` USE_BLUETOOTH_PROXY App.feed_wdt(); ESP_LOGD("${TAG_UPLOAD_TFT}", "Releasing Bluetooth memory (BTDM)"); esp_err_t err = esp_bt_controller_mem_release(ESP_BT_MODE_BTDM); if (err == ESP_OK) { ESP_LOGI("${TAG_UPLOAD_TFT}", "Bluetooth memory released successfully"); } else { ESP_LOGW("${TAG_UPLOAD_TFT}", "Bluetooth memory release failed: %s", esp_err_to_name(err)); } ESP_LOGI("${TAG_UPLOAD_TFT}", "Free heap after: %u bytes, largest block: %u bytes", esp_get_free_heap_size(), heap_caps_get_largest_free_block(MALLOC_CAP_8BIT)); - `#endif` // USE_ESP32_BLE + `#endif` // USE_BLUETOOTH_PROXYBased on learnings: "In ESPHome YAML configurations, avoid calling ESP-IDF Bluetooth deinitialization functions (esp_bt_controller_disable, esp_bt_controller_deinit, esp_bt_controller_mem_release) unless USE_BLUETOOTH_PROXY is defined."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@esphome/nspanel_esphome_addon_upload_tft.yaml` around lines 247 - 281, The BLE teardown sequence should only run for Bluetooth Proxy builds: wrap the disable/wait/release logic with `#ifdef` USE_BLUETOOTH_PROXY so that calls like esp32_ble::global_ble->disable(), checks of esp32_ble::global_ble->is_active(), and esp_bt_controller_mem_release(ESP_BT_MODE_BTDM) are compiled only when USE_BLUETOOTH_PROXY is defined; update the wait_until lambda and the subsequent lambda that calls App.feed_wdt(), logs, and calls esp_bt_controller_mem_release to be guarded by the same USE_BLUETOOTH_PROXY macro to avoid invoking ESP-IDF BT deinit on non-proxy BLE server/client builds.hmi/dev/nspanel_us_land_code/water_heater.txt (1)
51-63:⚠️ Potential issue | 🔴 CriticalUse the same CSV payload format in the page-exit fallback.
This path sends
water_heater.temperature,..., but the ESPHome handler for this page only acceptswater_heater,key,value. Leaving the page beforetimer0fires will drop the pending temperature update.🔧 Proposed fix
- climatesetting.txt="water_heater.temperature,"+va1.txt + climatesetting.txt="water_heater,temperature,"+va1.txt🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@hmi/dev/nspanel_us_land_code/water_heater.txt` around lines 51 - 63, The page-exit fallback currently builds a payload like "water_heater.temperature,<value>" which the ESPHome handler does not accept; change the payload construction in the Page Exit Event (where timer0.en is checked, covx temp_number0.val reads into va1.txt and climatesetting.txt is set) to use the CSV format "water_heater,temperature,<value>" so the prints/printh sequence sends "water_heater,key,value" (ensure climatesetting.txt is updated before prints climatesetting.txt is emitted).hmi/dev/nspanel_eu_code/water_heater.txt (1)
51-63:⚠️ Potential issue | 🔴 CriticalUse the same CSV payload format in the page-exit fallback.
This path sends
water_heater.temperature,..., but the ESPHome handler for this page only acceptswater_heater,key,value. Leaving the page beforetimer0fires will drop the pending temperature update.🔧 Proposed fix
- climatesetting.txt="water_heater.temperature,"+va1.txt + climatesetting.txt="water_heater,temperature,"+va1.txt🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@hmi/dev/nspanel_eu_code/water_heater.txt` around lines 51 - 63, The page-exit fallback currently emits "water_heater.temperature,...", which doesn't match the ESPHome handler; update the payload construction in the Page Exit Event (inside the timer0.en check) to emit a 3-field CSV "water_heater,temperature,<value>" by changing how climatesetting.txt is built (use va1.txt as the value), leaving the existing printh/prints sequence intact so the same sends occur; ensure you update the concatenation that currently produces "water_heater.temperature,"+va1.txt to instead produce "water_heater,temperature,"+va1.txt (references: timer0, temp_number0.val, va1.txt, climatesetting.txt, printh/prints).nspanel_easy_blueprint.yaml (2)
11091-11091:⚠️ Potential issue | 🟠 MajorReset unsupported water-heater mode buttons before rendering the supported set.
This still only turns matching buttons on. When the page is reused for another entity, unsupported buttons from the previous water heater can stay visible/clickable, and
repeat.item.mode in operation_listis unsafe whenoperation_listis missing. Normalizeoperation_listto[], hide all mode buttons first, then re-enable the supported ones and style the active button from the resolved mode variable instead of rawstates(...).Suggested fix
- operation_list: '{{ state_attr(water_heater_entity, "operation_list") if settings_entity_domain == "water_heater" }}' + operation_list: '{{ state_attr(water_heater_entity, "operation_list") | default([], true) if settings_entity_domain == "water_heater" else [] }}' ... - &water_heater_update_buttons alias: water_heater_update_buttons sequence: + - *delay_default + - action: 'esphome.{{ nspanel_name }}_components_visibility' + data: + page: water_heater + ids: '{{ page_water_heater.buttons.operation_mode | map(attribute="component") | list }}' + visible: false + continue_on_error: true - repeat: for_each: '{{ page_water_heater.buttons.operation_mode }}' sequence: - condition: '{{ repeat.item.mode in operation_list }}' ... - if states(water_heater_entity) == repeat.item.mode + if operation_mode == repeat.item.mode else nextion.color.disabledAlso applies to: 11315-11350
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@nspanel_easy_blueprint.yaml` at line 11091, Normalize operation_list to an empty array when absent (e.g., ensure operation_list = state_attr(water_heater_entity, "operation_list") || []) and, before rendering, reset/hide all mode buttons so leftover buttons from a previous entity are not visible; then iterate only the supported modes to re-enable/show those buttons (use repeat.item.mode in operation_list after normalizing). Also stop reading the active mode directly from states(...) in the template rendering and instead use the resolved mode variable (the computed/templated "mode" value) to apply the active styling so styling reflects the current entity rather than stale raw states.
11203-11207:⚠️ Potential issue | 🟠 MajorAlign
max_tempdown before derivingtotal_steps.
round(max_temp_raw / temp_step)can push the slider one step past the entity’s actual max, so the top position can send an unsupported temperature. Floor the max to the nearest valid step and computetotal_stepsfrom that aligned range.Based on learnings, the `temp_step` value is an encoded device granularity, so the slider range needs to stay aligned to supported steps.Suggested fix
- max_temp: '{{ (max_temp_raw / temp_step) | round * temp_step }}' - total_steps: '{{ ((max_temp - temp_offset) / temp_step) | round(0) | int }}' + max_temp: '{{ max_temp_raw - (max_temp_raw % temp_step) }}' + total_steps: > + {{ + (((max_temp - temp_offset) / temp_step) | round(0) | int) + if max_temp >= temp_offset else 0 + }}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@nspanel_easy_blueprint.yaml` around lines 11203 - 11207, The computed max_temp should be floored to the nearest supported step before deriving total_steps: replace the current rounding logic for max_temp (which uses round(max_temp_raw / temp_step) * temp_step) with a floor-based alignment (e.g., floor(max_temp_raw / temp_step) * temp_step) so the slider top cannot exceed the entity's actual max; then compute total_steps from the aligned max_temp and temp_offset using integer division or a floor to ensure total_steps = ((max_temp - temp_offset) / temp_step) yields an integer. Reference the variables temp_offset_raw, temp_offset, max_temp_raw, max_temp, total_steps, temp_step and the water_heater_entity attribute lookups when making this change.esphome/nspanel_esphome_page_water_heater.yaml (1)
9-10:⚠️ Potential issue | 🔴 CriticalCritical: Page ID collision with climate page.
PAGE_WATER_HEATER_ID: 7conflicts withPAGE_CLIMATE_ID: 7defined inesphome/nspanel_esphome_page_climate.yaml. According tocomponents/nspanel_easy/pages.h,water_heateris registered at index 30, not 7.This causes the
page_changehook at Line 56 to trigger for the climate page instead of the water heater page.🔧 Proposed fix
substitutions: - PAGE_WATER_HEATER_ID: 7 + PAGE_WATER_HEATER_ID: 30 TAG_PAGE_WATER_HEATER: nspanel.page.water_heater🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@esphome/nspanel_esphome_page_water_heater.yaml` around lines 9 - 10, PAGE_WATER_HEATER_ID is set to 7 which collides with PAGE_CLIMATE_ID; change the substitution PAGE_WATER_HEATER_ID to the correct registered index 30 (as per the pages registry) so the page_change hook and water heater handlers trigger for the water heater page rather than the climate page; update the value in nspanel_esphome_page_water_heater.yaml (substitution PAGE_WATER_HEATER_ID) to 30 and verify no other pages share that ID.hmi/dev/nspanel_CJK_eu_code/water_heater.txt (1)
957-966:⚠️ Potential issue | 🟡 MinorInconsistent entity type in long_click event.
The
click_timerevent emits"climate,long_click,"instead of"water_heater,long_click,". This is a copy-paste artifact from the climate page.🔧 Proposed fix
Events Timer Event - lastclick.txt="climate,long_click,"+click_comp.txt + lastclick.txt="water_heater,long_click,"+click_comp.txt printh 92🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@hmi/dev/nspanel_CJK_eu_code/water_heater.txt` around lines 957 - 966, The long click event currently emits the wrong entity type due to a copy-paste: change the string assignment that builds lastclick.txt from "climate,long_click," to "water_heater,long_click," so the Timer Event emits the correct entity type; update the line that sets lastclick.txt (and any identical occurrences used by click_timer.en) to use "water_heater,long_click," instead of "climate,long_click,".
🧹 Nitpick comments (2)
esphome/nspanel_esphome_api.yaml (1)
313-315: Keepmemhandling out of the base API package.This adds a
page == "mem"branch directly toaction_component_text, which breaks the project’s!extendpattern for memory variables. Please move themui_unavailableassignment into a small!extend action_component_textblock next to the new global instead of teaching the base API script about a specific mem key.Based on learnings: In the esphome directory, do not implement a page == "mem" case in action_component_color, action_component_text, or action_component_val within nspanel_esphome_api.yaml. This is by design; other files extend these scripts with !extend to add page == "mem" handling for their memory variables (e.g., nspanel_esphome_version.yaml, nspanel_esphome_page_utilities.yaml, etc.).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@esphome/nspanel_esphome_api.yaml` around lines 313 - 315, The base action_component_text function currently handles a page == "mem" case by assigning id(mui_unavailable) = txt; remove that mem-specific branch from action_component_text so the base API stays generic, and instead add a small !extend action_component_text block next to the new global definition that performs the mui_unavailable assignment for the mem page; in short, delete the page == "mem" handling from action_component_text and implement the mem-specific id(mui_unavailable) = txt logic in an adjacent !extend action_component_text block to preserve the !extend pattern.hmi/dev/nspanel_CJK_eu_code/water_heater.txt (1)
92-96: Consider aligning temp_step default with other variants.The
temp_stepinitial value is0here, while the CJK US variant uses1. Withtemp_step=0, the slider has no effect on temperature display until the blueprint sets the value. This is not a bug (the blueprint should initialize it), but consistency across variants would be preferable.♻️ Suggested change
Variable (int32) temp_step Attributes ID : 28 Scope: local - Value: 0 + Value: 1🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@hmi/dev/nspanel_CJK_eu_code/water_heater.txt` around lines 92 - 96, The temp_step variable currently has a default Value of 0 which makes the slider inert until a blueprint initializes it; update the temp_step (int32) variable's default Value from 0 to 1 to match the CJK US variant and ensure consistent behavior across variants (locate the temp_step variable definition to change its Value attribute).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@components/nspanel_easy/pages.h`:
- Around line 55-56: The PAGE_WATER_HEATER_ID constant is out of sync with the
pages array (you appended "water_heater" making its runtime index 30), so update
the hard-coded PAGE_WATER_HEATER_ID (currently 7 in
esphome/nspanel_esphome_page_water_heater.yaml) to match the new index (30) so
that page_change can dispatch page_water_heater->execute() correctly; locate the
PAGE_WATER_HEATER_ID definition and change its value to the runtime index that
corresponds to the "water_heater" entry.
In `@hmi/dev/nspanel_CJK_eu_code/water_heater.txt`:
- Around line 51-63: The Page Exit Event builds climatesetting.txt using dot
notation ("water_heater.temperature,") which is incompatible with the ESPHome
comma-separated event format; update the string concatenation in that block (the
code manipulating timer0.en, temp_number0.val, va1.txt, and climatesetting.txt)
to use a comma-separated event name like "water_heater,temperature," + va1.txt
so the event emitted by this Page Exit Event matches ESPHome's expected format.
In `@hmi/dev/nspanel_CJK_us_code/water_heater.txt`:
- Around line 51-63: The Page Exit Event is building the localevent string with
dot notation "water_heater.temperature," which is inconsistent with the expected
comma-separated format used by timer0 and the ESPHome handler; update the string
construction in that block (where climatesetting.txt is set, and related uses of
va1.txt and covx temp_number0.val) to use "water_heater,temperature," + va1.txt
so the emitted event matches the timer0 Timer Event and ESPHome event handler
formats.
- Around line 957-966: The long_click timer event is emitting
"climate,long_click," which is wrong for the water_heater page; update the Timer
Event that sets lastclick.txt (the block referencing Timer Event / lastclick.txt
and click_timer.en) to emit "water_heater,long_click," instead of
"climate,long_click," and scan/update any other occurrences of that string or
related click identifiers (lastclick.txt, click_timer) in this module to ensure
consistency with the water_heater page.
In `@hmi/dev/nspanel_CJK_us_land_code/water_heater.txt`:
- Around line 51-63: On the Page Exit Event where timer0 fallback sends an
update, change the CSV payload format from "water_heater.temperature,<value>" to
the handler's expected "water_heater,<key>,<value>" form: build
climatesetting.txt using the key "temperature" and the value in va1.txt (the
same pattern used elsewhere), then send that via the existing prints/printh
sequence so the ESPHome page handler (which expects water_heater,key,value)
receives the update; locate the code in the Page Exit Event referencing timer0,
va1.txt, and climatesetting.txt and adjust the payload composition accordingly.
---
Duplicate comments:
In `@esphome/nspanel_esphome_addon_upload_tft.yaml`:
- Around line 247-281: The BLE teardown sequence should only run for Bluetooth
Proxy builds: wrap the disable/wait/release logic with `#ifdef`
USE_BLUETOOTH_PROXY so that calls like esp32_ble::global_ble->disable(), checks
of esp32_ble::global_ble->is_active(), and
esp_bt_controller_mem_release(ESP_BT_MODE_BTDM) are compiled only when
USE_BLUETOOTH_PROXY is defined; update the wait_until lambda and the subsequent
lambda that calls App.feed_wdt(), logs, and calls esp_bt_controller_mem_release
to be guarded by the same USE_BLUETOOTH_PROXY macro to avoid invoking ESP-IDF BT
deinit on non-proxy BLE server/client builds.
In `@esphome/nspanel_esphome_page_water_heater.yaml`:
- Around line 9-10: PAGE_WATER_HEATER_ID is set to 7 which collides with
PAGE_CLIMATE_ID; change the substitution PAGE_WATER_HEATER_ID to the correct
registered index 30 (as per the pages registry) so the page_change hook and
water heater handlers trigger for the water heater page rather than the climate
page; update the value in nspanel_esphome_page_water_heater.yaml (substitution
PAGE_WATER_HEATER_ID) to 30 and verify no other pages share that ID.
In `@hmi/dev/nspanel_CJK_eu_code/water_heater.txt`:
- Around line 957-966: The long click event currently emits the wrong entity
type due to a copy-paste: change the string assignment that builds lastclick.txt
from "climate,long_click," to "water_heater,long_click," so the Timer Event
emits the correct entity type; update the line that sets lastclick.txt (and any
identical occurrences used by click_timer.en) to use "water_heater,long_click,"
instead of "climate,long_click,".
In `@hmi/dev/nspanel_eu_code/water_heater.txt`:
- Around line 51-63: The page-exit fallback currently emits
"water_heater.temperature,...", which doesn't match the ESPHome handler; update
the payload construction in the Page Exit Event (inside the timer0.en check) to
emit a 3-field CSV "water_heater,temperature,<value>" by changing how
climatesetting.txt is built (use va1.txt as the value), leaving the existing
printh/prints sequence intact so the same sends occur; ensure you update the
concatenation that currently produces "water_heater.temperature,"+va1.txt to
instead produce "water_heater,temperature,"+va1.txt (references: timer0,
temp_number0.val, va1.txt, climatesetting.txt, printh/prints).
In `@hmi/dev/nspanel_us_land_code/water_heater.txt`:
- Around line 51-63: The page-exit fallback currently builds a payload like
"water_heater.temperature,<value>" which the ESPHome handler does not accept;
change the payload construction in the Page Exit Event (where timer0.en is
checked, covx temp_number0.val reads into va1.txt and climatesetting.txt is set)
to use the CSV format "water_heater,temperature,<value>" so the prints/printh
sequence sends "water_heater,key,value" (ensure climatesetting.txt is updated
before prints climatesetting.txt is emitted).
In `@nspanel_easy_blueprint.yaml`:
- Line 11091: Normalize operation_list to an empty array when absent (e.g.,
ensure operation_list = state_attr(water_heater_entity, "operation_list") || [])
and, before rendering, reset/hide all mode buttons so leftover buttons from a
previous entity are not visible; then iterate only the supported modes to
re-enable/show those buttons (use repeat.item.mode in operation_list after
normalizing). Also stop reading the active mode directly from states(...) in the
template rendering and instead use the resolved mode variable (the
computed/templated "mode" value) to apply the active styling so styling reflects
the current entity rather than stale raw states.
- Around line 11203-11207: The computed max_temp should be floored to the
nearest supported step before deriving total_steps: replace the current rounding
logic for max_temp (which uses round(max_temp_raw / temp_step) * temp_step) with
a floor-based alignment (e.g., floor(max_temp_raw / temp_step) * temp_step) so
the slider top cannot exceed the entity's actual max; then compute total_steps
from the aligned max_temp and temp_offset using integer division or a floor to
ensure total_steps = ((max_temp - temp_offset) / temp_step) yields an integer.
Reference the variables temp_offset_raw, temp_offset, max_temp_raw, max_temp,
total_steps, temp_step and the water_heater_entity attribute lookups when making
this change.
---
Nitpick comments:
In `@esphome/nspanel_esphome_api.yaml`:
- Around line 313-315: The base action_component_text function currently handles
a page == "mem" case by assigning id(mui_unavailable) = txt; remove that
mem-specific branch from action_component_text so the base API stays generic,
and instead add a small !extend action_component_text block next to the new
global definition that performs the mui_unavailable assignment for the mem page;
in short, delete the page == "mem" handling from action_component_text and
implement the mem-specific id(mui_unavailable) = txt logic in an adjacent
!extend action_component_text block to preserve the !extend pattern.
In `@hmi/dev/nspanel_CJK_eu_code/water_heater.txt`:
- Around line 92-96: The temp_step variable currently has a default Value of 0
which makes the slider inert until a blueprint initializes it; update the
temp_step (int32) variable's default Value from 0 to 1 to match the CJK US
variant and ensure consistent behavior across variants (locate the temp_step
variable definition to change its Value attribute).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 793a70ff-2551-4a7e-a48f-835cad2ba14c
📒 Files selected for processing (32)
components/nspanel_easy/page_climate.cppcomponents/nspanel_easy/page_climate.hcomponents/nspanel_easy/pages.hesphome/nspanel_esphome_addon_upload_tft.yamlesphome/nspanel_esphome_api.yamlesphome/nspanel_esphome_hw_temperature.yamlesphome/nspanel_esphome_page_climate.yamlesphome/nspanel_esphome_page_water_heater.yamlesphome/nspanel_esphome_standard.yamlesphome/nspanel_esphome_version.yamlhmi/dev/nspanel_CJK_eu_code/boot.txthmi/dev/nspanel_CJK_eu_code/water_heater.txthmi/dev/nspanel_CJK_us_code/boot.txthmi/dev/nspanel_CJK_us_code/water_heater.txthmi/dev/nspanel_CJK_us_land_code/boot.txthmi/dev/nspanel_CJK_us_land_code/water_heater.txthmi/dev/nspanel_eu_code/water_heater.txthmi/dev/nspanel_us_code/water_heater.txthmi/dev/nspanel_us_land_code/water_heater.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)
- components/nspanel_easy/page_climate.cpp
- components/nspanel_easy/page_climate.h
✅ Files skipped from review due to trivial changes (2)
- hmi/dev/nspanel_CJK_us_code/boot.txt
- hmi/dev/nspanel_us_code/water_heater.txt
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
nspanel_easy_blueprint.yaml (1)
11342-11344: Remove duplicate delay.There are two consecutive
*delay_defaultanchors (lines 11342 and 11344), which introduces unnecessary latency in the button update loop.Suggested fix
- condition: '{{ repeat.item.mode in operation_list }}' - - *delay_default ### ICON Font Color ### - *delay_default🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@nspanel_easy_blueprint.yaml` around lines 11342 - 11344, There are two consecutive uses of the YAML anchor *delay_default around the "### ICON Font Color ###" section causing duplicated delay; edit the nspanel_easy_blueprint.yaml template to remove the extra *delay_default so that only a single *delay_default remains in that list (locate the block containing "### ICON Font Color ###" and the adjacent - *delay_default entries and delete the redundant one).hmi/dev/nspanel_CJK_us_code/water_heater.txt (1)
92-96: Minor inconsistency:temp_stepdefault differs from other variants.This file initializes
temp_stepto1(Line 96), while the other water_heater variants (CJK_eu, CJK_us_land) initialize it to0. This is likely overwritten by the blueprint before use, but worth noting for consistency.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@hmi/dev/nspanel_CJK_us_code/water_heater.txt` around lines 92 - 96, The temp_step local variable is initialized to 1 here (Variable int32 temp_step, ID 28) which is inconsistent with other variants (CJK_eu, CJK_us_land) that use 0; change the default Value from 1 to 0 for temp_step to match those variants so the blueprint receives a consistent initial value.
🤖 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_us_land_code/water_heater.txt`:
- Around line 21-49: The Preinitialize Event hides the slider but leaves the
decrease_temp and increase_temp hotspots active, allowing inadvertent
temperature=0 updates because temp_step defaults to 0; update the code paths
that make slider visible to also explicitly vis decrease_temp,1 and vis
increase_temp,1 (and set/ensure temp_step is initialized appropriately) so both
hotspots are re-enabled whenever slider is shown—apply the same change in the
corresponding Preinitialize/visibility paths for decrease_temp and increase_temp
in this file (symbols: slider, decrease_temp, increase_temp, temp_step).
---
Nitpick comments:
In `@hmi/dev/nspanel_CJK_us_code/water_heater.txt`:
- Around line 92-96: The temp_step local variable is initialized to 1 here
(Variable int32 temp_step, ID 28) which is inconsistent with other variants
(CJK_eu, CJK_us_land) that use 0; change the default Value from 1 to 0 for
temp_step to match those variants so the blueprint receives a consistent initial
value.
In `@nspanel_easy_blueprint.yaml`:
- Around line 11342-11344: There are two consecutive uses of the YAML anchor
*delay_default around the "### ICON Font Color ###" section causing duplicated
delay; edit the nspanel_easy_blueprint.yaml template to remove the extra
*delay_default so that only a single *delay_default remains in that list (locate
the block containing "### ICON Font Color ###" and the adjacent - *delay_default
entries and delete the redundant one).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: da2a70c4-b00d-4097-87c2-38c5c1630554
📒 Files selected for processing (26)
esphome/nspanel_esphome_page_water_heater.yamlhmi/dev/nspanel_CJK_eu_code/climate.txthmi/dev/nspanel_CJK_eu_code/water_heater.txthmi/dev/nspanel_CJK_us_code/climate.txthmi/dev/nspanel_CJK_us_code/water_heater.txthmi/dev/nspanel_CJK_us_land_code/climate.txthmi/dev/nspanel_CJK_us_land_code/water_heater.txthmi/dev/nspanel_eu_code/climate.txthmi/dev/nspanel_eu_code/water_heater.txthmi/dev/nspanel_us_code/climate.txthmi/dev/nspanel_us_code/water_heater.txthmi/dev/nspanel_us_land_code/climate.txthmi/dev/nspanel_us_land_code/water_heater.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 skipped from review as they are similar to previous changes (1)
- hmi/dev/nspanel_us_code/water_heater.txt
Owner
Author
|
@codex review |
edwardtfn
changed the title
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1c12632920
ℹ️ 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
- 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 address that feedback".
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
nspanel_easy_blueprint.yaml (1)
11195-11249: Unusedmax_tempvariable can be removed.Line 11215 defines
max_tempbuttotal_stepsat line 11216-11220 usesmax_temp_rawdirectly. This was noted in past reviews as housekeeping — safe to remove for tidiness, but not a functional issue.♻️ Optional cleanup
temp_offset: '{{ temp_offset_raw if (temp_offset_raw % temp_step) == 0 else temp_offset_raw + temp_step - (temp_offset_raw % temp_step) }}' max_temp_raw: '{{ (state_attr(water_heater_entity, "max_temp") | float(35) * 10) | round(0) | int }}' - max_temp: '{{ (max_temp_raw / temp_step) | round * temp_step }}' total_steps: >🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@nspanel_easy_blueprint.yaml` around lines 11195 - 11249, Remove the unused variable max_temp declared in the water_heater_update_slider sequence (the variables block that sets max_temp_raw and max_temp); it isn’t referenced elsewhere (total_steps uses max_temp_raw), so delete the max_temp assignment to tidy the variables list and avoid dead code.hmi/dev/nspanel_us_code/water_heater.txt (1)
793-910: Consider centralizing the target-formatting/update routine.The same temperature-render block is copied into slider release, slider move, decrement, and increment handlers. That already makes small fixes easy to miss, and the same duplication exists across the other panel variants too. A shared generator/template for this routine would make future behavior changes much safer.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@hmi/dev/nspanel_us_code/water_heater.txt` around lines 793 - 910, Extract the repeated temperature-formatting block (the logic that sets temp_number0 from slider.val and temp_step.val/ temp_offset.val, computes va0/va1, writes target.txt with dec_separator.txt and temp_unit.txt) into a single reusable routine named e.g. update_target_text() and call it from all locations that currently duplicate the code (the slider Touch Release Event, Touch Move Event, hotspot decrease_temp Touch Press Event, hotspot increase_temp Touch Press Event); keep the timer0.en=1 lines in the Touch Release handlers as-is and replace the duplicated assignment blocks with a single call to update_target_text() so future changes are made in one place.
🤖 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_upload_tft.yaml`:
- Around line 257-279: The wait_until timeout can fall through causing
esp_bt_controller_mem_release(ESP_BT_MODE_BTDM) to run even if BLE didn't shut
down; after the wait_until block re-check esp32_ble::global_ble == nullptr ||
!esp32_ble::global_ble->is_active() and only proceed to call App.feed_wdt() and
esp_bt_controller_mem_release(ESP_BT_MODE_BTDM) (and the ESP_LOG* lines using
TAG_UPLOAD_TFT) when that condition is true; if the condition is false, abort
the upload flow (e.g., log an error and reboot/return) to avoid releasing BT
memory while BLE is still active.
In `@hmi/dev/nspanel_us_land_code/water_heater.txt`:
- Around line 21-39: In the Preinitialize Event, also hide and clear any
remaining dynamic fields so stale data cannot flash: explicitly set vis=0 for
page_label and target and for each value/icon pair (clear their text/content as
well) in addition to the existing vis calls (e.g., target_icon, current_temp,
slider, decrease_temp, increase_temp, button01..button08); update the
Preinitialize Event block to both hide (vis ...,0) and reset/clear the
text/content for page_label, target, and the four value/icon pairs so the page
starts blank before the blueprint repaints.
---
Nitpick comments:
In `@hmi/dev/nspanel_us_code/water_heater.txt`:
- Around line 793-910: Extract the repeated temperature-formatting block (the
logic that sets temp_number0 from slider.val and temp_step.val/ temp_offset.val,
computes va0/va1, writes target.txt with dec_separator.txt and temp_unit.txt)
into a single reusable routine named e.g. update_target_text() and call it from
all locations that currently duplicate the code (the slider Touch Release Event,
Touch Move Event, hotspot decrease_temp Touch Press Event, hotspot increase_temp
Touch Press Event); keep the timer0.en=1 lines in the Touch Release handlers
as-is and replace the duplicated assignment blocks with a single call to
update_target_text() so future changes are made in one place.
In `@nspanel_easy_blueprint.yaml`:
- Around line 11195-11249: Remove the unused variable max_temp declared in the
water_heater_update_slider sequence (the variables block that sets max_temp_raw
and max_temp); it isn’t referenced elsewhere (total_steps uses max_temp_raw), so
delete the max_temp assignment to tidy the variables list and avoid dead code.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 13c9562a-c105-415b-97a9-0f4c42f2620e
📒 Files selected for processing (20)
esphome/nspanel_esphome_addon_upload_tft.yamlhmi/dev/nspanel_CJK_eu_code/water_heater.txthmi/dev/nspanel_CJK_us_code/water_heater.txthmi/dev/nspanel_CJK_us_land_code/water_heater.txthmi/dev/nspanel_eu_code/water_heater.txthmi/dev/nspanel_us_code/water_heater.txthmi/dev/nspanel_us_land_code/water_heater.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
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 878cb31d32
ℹ️ 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
- 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 address that feedback".
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@nspanel_easy_blueprint.yaml`:
- Around line 12958-12974: The live-update branch only calls the generic
*display_value sequence, losing slot-specific overrides
(water_heater_value0x_icon and water_heater_value0x_label_color); extract the
full slot-render sequence used for initial render (the sequence that applies
those two overrides) into a shared anchor (e.g., water_heater_slot_render) and
replace both the initial-render usage and the repeat sequence under the Update
water heater page values path (which currently uses *variables_water_heater_page
and *display_value) to call the new shared anchor so live updates reapplies the
slot-specific icon and label_color overrides.
- Around line 11313-11353: The current code updates current_temp but never makes
it visible and leaves target populated when is_target_temperature is false;
update the actions so (1) the esphome.{{ nspanel_name }}_components_visibility
call includes "current_temp" as visible when displaying the heater page (or add
a separate visibility action to set current_temp visible unconditionally), and
(2) in the branch where is_target_temperature is false explicitly clear or
overwrite the target text via esphome.{{ nspanel_name }}_component_text (id:
target) with an empty string and ensure
target_icon/slider/decrease_temp/increase_temp are hidden via esphome.{{
nspanel_name }}_components_visibility; use the existing component_text,
component_val, and components_visibility actions and the component ids
current_temp, target, target_icon, slider, decrease_temp, and increase_temp to
locate and modify the block accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 40dd4e05-351c-4d77-a35e-4a53a61259ed
📒 Files selected for processing (1)
nspanel_easy_blueprint.yaml
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 98395c06a9
ℹ️ 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
- 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 address that feedback".
Owner
Author
|
@CodeRabbit resume |
Contributor
✅ Actions performedReviews resumed. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 83512d46d9
ℹ️ 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
- 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 address that feedback".
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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_eu_code/water_heater.txt`:
- Around line 21-48: Preinitialize Event is not hiding the conditional component
current_temp; update the Preinitialize block to set current_temp visibility to 0
alongside target_icon, slider, decrease_temp, increase_temp, and
button01–button08 so the blueprint can later show/repaint it; locate the
Preinitialize Event in this file and add the line vis current_temp,0 (and ensure
any font or other init for current_temp is handled consistently with
button01..button08 and button_back if needed).
In `@hmi/dev/nspanel_CJK_us_land_code/water_heater.txt`:
- Around line 21-48: In the Preinitialize Event block add the hide instruction
for the dynamic component by setting vis current_temp,0 so it is hidden on page
entry (like the other vis ... lines); update the same Preinitialize sequence
(where button01..button08, slider, target_icon, etc. are set) to include
current_temp so the blueprint can later show/repaint it as intended.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 2c51bd9e-02ce-4c4c-ba68-927a9bb21eba
📒 Files selected for processing (19)
hmi/dev/nspanel_CJK_eu_code/water_heater.txthmi/dev/nspanel_CJK_us_code/water_heater.txthmi/dev/nspanel_CJK_us_land_code/water_heater.txthmi/dev/nspanel_eu_code/water_heater.txthmi/dev/nspanel_us_code/water_heater.txthmi/dev/nspanel_us_land_code/water_heater.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
This was referenced Mar 23, 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
2 participants
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.
What's New
Water heater entity support
Added
water_heateras a supported entity type across the project. You can now assign water heater entities to buttons, chips, entity pages, and hardware buttons — just like you would with lights, covers, fans, or climate entities.Features:
operation_listSupported inputs:
Bug Fixes
Fixed blueprint action naming (#38)
Fixed a critical issue where the blueprint generated incorrect action names when the device name didn't match the expected format. The blueprint was using
device_nameto construct action calls (e.g.,esphome.{{ device_name }}_component_val), but this didn't always match the actual action name registered by ESPHome. Changed to usenspanel_namewhich correctly references the device's ESPHome name, ensuring action calls always work regardless of the device's friendly name in Home Assistant.Thanks to @vithurshanselvarajah for identifying and fixing this issue.
Summary by CodeRabbit
New Features
Improvements
Bug Fixes