improve: Unified TFT files and 921600 bps default baud rate#103
Merged
improve: Unified TFT files and 921600 bps default baud rate#103
Conversation
Contributor
|
Caution Review failedPull request was closed or merged during review 📝 WalkthroughWalkthroughRefactors display state from a single Changes
Sequence Diagram(s)sequenceDiagram
participant Nextion as Nextion HMI
participant EEPROM as Nextion EEPROM
participant ESP as ESP32/ESPHome
Note over Nextion,EEPROM: Nextion boot-time EEPROM flow
Nextion->>EEPROM: read magic (addr 0-3)
EEPROM-->>Nextion: magic value
alt magic == "Easy"
Nextion->>EEPROM: read display_mode (addr 4-7)
EEPROM-->>Nextion: persisted mode
Nextion->>Nextion: clamp/validate mode (1..7)
alt valid
Nextion->>ESP: emit boot params (contract v2 incl. mode, magic_flag)
else invalid
Nextion->>EEPROM: write hard_coded_display_mode
Nextion->>ESP: emit boot params (hard-coded mode, magic_flag=0)
end
else magic != "Easy"
Nextion->>EEPROM: write magic + hard_coded_display_mode
Nextion->>ESP: emit boot params (hard-coded mode, magic_flag=0)
end
Note over ESP,Nextion: ESPHome boot-side handling
ESP->>ESP: parse boot params (contract/version/mode)
ESP->>ESP: set display_mode_eeprom, display_easy, display_portrait, display_valid
ESP->>ESP: continue page_change and TFT-upload logic (guards use display_valid/display_easy)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
🚥 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: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/addon_upload_tft.md`:
- Around line 109-110: The documented default for nextion_update_base_url is
incorrect; update the docs to show the actual default value used by the addon
(https://raw.githubusercontent.com/edwardtfn/NSPanel-Easy/v${version}/hmi)
instead of the repository root so generated TFT URLs are valid, and keep the
nextion_update_url description as-is noting it overrides the base URL; edit the
entry for nextion_update_base_url in the addon_upload_tft documentation to match
the real default used by esphome/nspanel_esphome_addon_upload_tft.yaml
(nextion_update_base_url) and ensure the explanatory text clarifies that the
version placeholder (${version}) and /hmi suffix are part of the default.
In `@esphome/nspanel_esphome_addon_upload_tft.yaml`:
- Around line 99-103: The current guard in the upload block allows the "blank"
model to be treated as uploadable; change it so uploading only proceeds for a
non-blank hardware model (i.e., require the selected
display_model->active_index().value_or(UINT8_MAX) to be > 0 rather than !=
UINT8_MAX) and/or explicitly check that the resolved TFT filename is not
"nspanel_blank.tft" before attempting upload; update the condition around
display_model/active_index() in the upload logic that uses TAG_UPLOAD_TFT and
respect upload_tft_automatically so the blank model (NSPanel Blank ->
nspanel_blank.tft) is skipped and the panel does not auto-flash the blank TFT on
first install.
In `@esphome/nspanel_esphome_hw_buttons.yaml`:
- Line 173: display_model->active_index() returns a 0-based index but the code
treats display_mode_idx as 1-based, causing mis-mapped coordinates; locate the
uses of display_mode_idx (the declaration const uint8_t display_mode_idx =
display_model->active_index().value_or(UINT8_MAX); and the switch/case blocks
that follow around the ranges you noted) and either adjust display_mode_idx to
be 1-based (e.g., add 1 only when valid) or, preferably, change the switch/case
labels to 0/1/2 (instead of 1/2/3) so the 0-based active_index maps correctly;
apply the same correction to the other two blocks referenced (around the 185-198
and 212-225 regions) so all button-bar placement switches use consistent 0-based
indices.
🪄 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: 57b967db-28fd-4356-b856-2449e134ad88
⛔ Files ignored due to path filters (162)
hmi/dev/ui/landscape-white/0.pngis excluded by!**/*.pnghmi/dev/ui/landscape-white/1.pngis excluded by!**/*.pnghmi/dev/ui/landscape-white/10.pngis excluded by!**/*.pnghmi/dev/ui/landscape-white/11.pngis excluded by!**/*.pnghmi/dev/ui/landscape-white/12.pngis excluded by!**/*.pnghmi/dev/ui/landscape-white/13.pngis excluded by!**/*.pnghmi/dev/ui/landscape-white/14.pngis excluded by!**/*.pnghmi/dev/ui/landscape-white/15.pngis excluded by!**/*.pnghmi/dev/ui/landscape-white/16.pngis excluded by!**/*.pnghmi/dev/ui/landscape-white/17.pngis excluded by!**/*.pnghmi/dev/ui/landscape-white/18.pngis excluded by!**/*.pnghmi/dev/ui/landscape-white/19.pngis excluded by!**/*.pnghmi/dev/ui/landscape-white/2.pngis excluded by!**/*.pnghmi/dev/ui/landscape-white/20.pngis excluded by!**/*.pnghmi/dev/ui/landscape-white/21.pngis excluded by!**/*.pnghmi/dev/ui/landscape-white/22.pngis excluded by!**/*.pnghmi/dev/ui/landscape-white/23.pngis excluded by!**/*.pnghmi/dev/ui/landscape-white/24.pngis excluded by!**/*.pnghmi/dev/ui/landscape-white/25.pngis excluded by!**/*.pnghmi/dev/ui/landscape-white/26.pngis excluded by!**/*.pnghmi/dev/ui/landscape-white/27.pngis excluded by!**/*.pnghmi/dev/ui/landscape-white/28.pngis excluded by!**/*.pnghmi/dev/ui/landscape-white/29.pngis excluded by!**/*.pnghmi/dev/ui/landscape-white/3.pngis excluded by!**/*.pnghmi/dev/ui/landscape-white/30.pngis excluded by!**/*.pnghmi/dev/ui/landscape-white/31.pngis excluded by!**/*.pnghmi/dev/ui/landscape-white/32.pngis excluded by!**/*.pnghmi/dev/ui/landscape-white/33.pngis excluded by!**/*.pnghmi/dev/ui/landscape-white/34.pngis excluded by!**/*.pnghmi/dev/ui/landscape-white/35.pngis excluded by!**/*.pnghmi/dev/ui/landscape-white/36.pngis excluded by!**/*.pnghmi/dev/ui/landscape-white/37.pngis excluded by!**/*.pnghmi/dev/ui/landscape-white/38.pngis excluded by!**/*.pnghmi/dev/ui/landscape-white/39.pngis excluded by!**/*.pnghmi/dev/ui/landscape-white/4.pngis excluded by!**/*.pnghmi/dev/ui/landscape-white/40.pngis excluded by!**/*.pnghmi/dev/ui/landscape-white/41.pngis excluded by!**/*.pnghmi/dev/ui/landscape-white/42.pngis excluded by!**/*.pnghmi/dev/ui/landscape-white/43.pngis excluded by!**/*.pnghmi/dev/ui/landscape-white/44.pngis excluded by!**/*.pnghmi/dev/ui/landscape-white/45.pngis excluded by!**/*.pnghmi/dev/ui/landscape-white/46.pngis excluded by!**/*.pnghmi/dev/ui/landscape-white/47.pngis excluded by!**/*.pnghmi/dev/ui/landscape-white/5.pngis excluded by!**/*.pnghmi/dev/ui/landscape-white/6.pngis excluded by!**/*.pnghmi/dev/ui/landscape-white/7.pngis excluded by!**/*.pnghmi/dev/ui/landscape-white/8.pngis excluded by!**/*.pnghmi/dev/ui/landscape-white/9.pngis excluded by!**/*.pnghmi/dev/ui/landscape/0.pngis excluded by!**/*.pnghmi/dev/ui/landscape/1.pngis excluded by!**/*.pnghmi/dev/ui/landscape/10.pngis excluded by!**/*.pnghmi/dev/ui/landscape/11.pngis excluded by!**/*.pnghmi/dev/ui/landscape/12.pngis excluded by!**/*.pnghmi/dev/ui/landscape/13.pngis excluded by!**/*.pnghmi/dev/ui/landscape/14.pngis excluded by!**/*.pnghmi/dev/ui/landscape/15.pngis excluded by!**/*.pnghmi/dev/ui/landscape/16.pngis excluded by!**/*.pnghmi/dev/ui/landscape/17.pngis excluded by!**/*.pnghmi/dev/ui/landscape/18.pngis excluded by!**/*.pnghmi/dev/ui/landscape/19.pngis excluded by!**/*.pnghmi/dev/ui/landscape/2.pngis excluded by!**/*.pnghmi/dev/ui/landscape/20.pngis excluded by!**/*.pnghmi/dev/ui/landscape/21.pngis excluded by!**/*.pnghmi/dev/ui/landscape/22.pngis excluded by!**/*.pnghmi/dev/ui/landscape/23.pngis excluded by!**/*.pnghmi/dev/ui/landscape/24.pngis excluded by!**/*.pnghmi/dev/ui/landscape/25.pngis excluded by!**/*.pnghmi/dev/ui/landscape/26.pngis excluded by!**/*.pnghmi/dev/ui/landscape/27.pngis excluded by!**/*.pnghmi/dev/ui/landscape/28.pngis excluded by!**/*.pnghmi/dev/ui/landscape/29.pngis excluded by!**/*.pnghmi/dev/ui/landscape/3.pngis excluded by!**/*.pnghmi/dev/ui/landscape/30.pngis excluded by!**/*.pnghmi/dev/ui/landscape/31.pngis excluded by!**/*.pnghmi/dev/ui/landscape/32.pngis excluded by!**/*.pnghmi/dev/ui/landscape/33.pngis excluded by!**/*.pnghmi/dev/ui/landscape/34.pngis excluded by!**/*.pnghmi/dev/ui/landscape/35.pngis excluded by!**/*.pnghmi/dev/ui/landscape/36.pngis excluded by!**/*.pnghmi/dev/ui/landscape/37.pngis excluded by!**/*.pnghmi/dev/ui/landscape/38.pngis excluded by!**/*.pnghmi/dev/ui/landscape/39.pngis excluded by!**/*.pnghmi/dev/ui/landscape/4.pngis excluded by!**/*.pnghmi/dev/ui/landscape/40.pngis excluded by!**/*.pnghmi/dev/ui/landscape/41.pngis excluded by!**/*.pnghmi/dev/ui/landscape/42.pngis excluded by!**/*.pnghmi/dev/ui/landscape/43.pngis excluded by!**/*.pnghmi/dev/ui/landscape/44.pngis excluded by!**/*.pnghmi/dev/ui/landscape/45.pngis excluded by!**/*.pnghmi/dev/ui/landscape/46.pngis excluded by!**/*.pnghmi/dev/ui/landscape/47.pngis excluded by!**/*.pnghmi/dev/ui/landscape/5.pngis excluded by!**/*.pnghmi/dev/ui/landscape/6.pngis excluded by!**/*.pnghmi/dev/ui/landscape/7.pngis excluded by!**/*.pnghmi/dev/ui/landscape/8.pngis excluded by!**/*.pnghmi/dev/ui/landscape/9.pngis excluded by!**/*.pnghmi/dev/ui/landscape/NSPanel - EU.pptxis excluded by!**/*.pptxhmi/dev/ui/portrait-white/0.pngis excluded by!**/*.pnghmi/dev/ui/portrait-white/1.pngis excluded by!**/*.pnghmi/dev/ui/portrait-white/10.pngis excluded by!**/*.pnghmi/dev/ui/portrait-white/11.pngis excluded by!**/*.pnghmi/dev/ui/portrait-white/12.pngis excluded by!**/*.pnghmi/dev/ui/portrait-white/13.pngis excluded by!**/*.pnghmi/dev/ui/portrait-white/14.pngis excluded by!**/*.pnghmi/dev/ui/portrait-white/15.pngis excluded by!**/*.pnghmi/dev/ui/portrait-white/16.pngis excluded by!**/*.pnghmi/dev/ui/portrait-white/17.pngis excluded by!**/*.pnghmi/dev/ui/portrait-white/18.pngis excluded by!**/*.pnghmi/dev/ui/portrait-white/19.pngis excluded by!**/*.pnghmi/dev/ui/portrait-white/2.pngis excluded by!**/*.pnghmi/dev/ui/portrait-white/20.pngis excluded by!**/*.pnghmi/dev/ui/portrait-white/21.pngis excluded by!**/*.pnghmi/dev/ui/portrait-white/22.pngis excluded by!**/*.pnghmi/dev/ui/portrait-white/23.pngis excluded by!**/*.pnghmi/dev/ui/portrait-white/24.pngis excluded by!**/*.pnghmi/dev/ui/portrait-white/25.pngis excluded by!**/*.pnghmi/dev/ui/portrait-white/26.pngis excluded by!**/*.pnghmi/dev/ui/portrait-white/27.pngis excluded by!**/*.pnghmi/dev/ui/portrait-white/28.pngis excluded by!**/*.pnghmi/dev/ui/portrait-white/29.pngis excluded by!**/*.pnghmi/dev/ui/portrait-white/3.pngis excluded by!**/*.pnghmi/dev/ui/portrait-white/30.pngis excluded by!**/*.pnghmi/dev/ui/portrait-white/31.pngis excluded by!**/*.pnghmi/dev/ui/portrait-white/32.pngis excluded by!**/*.pnghmi/dev/ui/portrait-white/33.pngis excluded by!**/*.pnghmi/dev/ui/portrait-white/34.pngis excluded by!**/*.pnghmi/dev/ui/portrait-white/35.pngis excluded by!**/*.pnghmi/dev/ui/portrait-white/36.pngis excluded by!**/*.pnghmi/dev/ui/portrait-white/37.pngis excluded by!**/*.pnghmi/dev/ui/portrait-white/38.pngis excluded by!**/*.pnghmi/dev/ui/portrait-white/39.pngis excluded by!**/*.pnghmi/dev/ui/portrait-white/4.pngis excluded by!**/*.pnghmi/dev/ui/portrait-white/40.pngis excluded by!**/*.pnghmi/dev/ui/portrait-white/41.pngis excluded by!**/*.pnghmi/dev/ui/portrait-white/42.pngis excluded by!**/*.pnghmi/dev/ui/portrait-white/43.pngis excluded by!**/*.pnghmi/dev/ui/portrait-white/44.pngis excluded by!**/*.pnghmi/dev/ui/portrait-white/45.pngis excluded by!**/*.pnghmi/dev/ui/portrait-white/46.pngis excluded by!**/*.pnghmi/dev/ui/portrait-white/47.pngis excluded by!**/*.pnghmi/dev/ui/portrait-white/5.pngis excluded by!**/*.pnghmi/dev/ui/portrait-white/6.pngis excluded by!**/*.pnghmi/dev/ui/portrait-white/7.pngis excluded by!**/*.pnghmi/dev/ui/portrait-white/8.pngis excluded by!**/*.pnghmi/dev/ui/portrait-white/9.pngis excluded by!**/*.pnghmi/dev/ui/portrait/0.pngis excluded by!**/*.pnghmi/dev/ui/portrait/1.pngis excluded by!**/*.pnghmi/dev/ui/portrait/10.pngis excluded by!**/*.pnghmi/dev/ui/portrait/11.pngis excluded by!**/*.pnghmi/dev/ui/portrait/12.pngis excluded by!**/*.pnghmi/dev/ui/portrait/13.pngis excluded by!**/*.pnghmi/dev/ui/portrait/14.pngis excluded by!**/*.pnghmi/dev/ui/portrait/15.pngis excluded by!**/*.pnghmi/dev/ui/portrait/16.pngis excluded by!**/*.pnghmi/dev/ui/portrait/17.pngis excluded by!**/*.pnghmi/dev/ui/portrait/18.pngis excluded by!**/*.pnghmi/dev/ui/portrait/19.pngis excluded by!**/*.pnghmi/dev/ui/portrait/2.pngis excluded by!**/*.pnghmi/dev/ui/portrait/20.pngis excluded by!**/*.pnghmi/dev/ui/portrait/21.pngis excluded by!**/*.pnghmi/dev/ui/portrait/22.pngis excluded by!**/*.pnghmi/dev/ui/portrait/23.pngis excluded by!**/*.png
📒 Files selected for processing (138)
components/nspanel_easy/hw_display.cppcomponents/nspanel_easy/hw_display.hdocs/addon_upload_tft.mddocs/migration_from_blackymas.mdesphome/nspanel_esphome_addon_climate_cool.yamlesphome/nspanel_esphome_addon_climate_dual.yamlesphome/nspanel_esphome_addon_climate_heat.yamlesphome/nspanel_esphome_addon_upload_tft.yamlesphome/nspanel_esphome_api.yamlesphome/nspanel_esphome_core.yamlesphome/nspanel_esphome_hw_buttons.yamlesphome/nspanel_esphome_hw_display.yamlesphome/nspanel_esphome_page_boot.yamlesphome/nspanel_esphome_page_home.yamlesphome/nspanel_esphome_page_media_player.yamlesphome/nspanel_esphome_page_notification.yamlesphome/nspanel_esphome_standard.yamlhmi/dev/create_us_land_and_CJK_hmi_from_original.mdhmi/dev/nspanel_blank/Program.s.txthmi/dev/nspanel_blank/blank.txthmi/dev/nspanel_easy_landscape/Program.s.txthmi/dev/nspanel_easy_landscape/boot.txthmi/dev/nspanel_landscape_code/Program.s.txthmi/dev/nspanel_landscape_code/alarm.txthmi/dev/nspanel_landscape_code/boot.txthmi/dev/nspanel_landscape_code/button.txthmi/dev/nspanel_landscape_code/buttonpage01.txthmi/dev/nspanel_landscape_code/buttonpage02.txthmi/dev/nspanel_landscape_code/buttonpage03.txthmi/dev/nspanel_landscape_code/buttonpage04.txthmi/dev/nspanel_landscape_code/climate.txthmi/dev/nspanel_landscape_code/confirm.txthmi/dev/nspanel_landscape_code/cover.txthmi/dev/nspanel_landscape_code/debug.txthmi/dev/nspanel_landscape_code/entitypage01.txthmi/dev/nspanel_landscape_code/entitypage02.txthmi/dev/nspanel_landscape_code/entitypage03.txthmi/dev/nspanel_landscape_code/entitypage04.txthmi/dev/nspanel_landscape_code/fan.txthmi/dev/nspanel_landscape_code/home.txthmi/dev/nspanel_landscape_code/home_new.txthmi/dev/nspanel_landscape_code/home_smpl.txthmi/dev/nspanel_landscape_code/keyb_num.txthmi/dev/nspanel_landscape_code/light.txthmi/dev/nspanel_landscape_code/media_player.txthmi/dev/nspanel_landscape_code/notification.txthmi/dev/nspanel_landscape_code/qrcode.txthmi/dev/nspanel_landscape_code/screensaver.txthmi/dev/nspanel_landscape_code/settings.txthmi/dev/nspanel_landscape_code/switch.txthmi/dev/nspanel_landscape_code/theme_apply.txthmi/dev/nspanel_landscape_code/utilities.txthmi/dev/nspanel_landscape_code/water_heater.txthmi/dev/nspanel_landscape_code/weather01.txthmi/dev/nspanel_landscape_code/weather02.txthmi/dev/nspanel_landscape_code/weather03.txthmi/dev/nspanel_landscape_code/weather04.txthmi/dev/nspanel_landscape_code/weather05.txthmi/dev/nspanel_portrait_code/Program.s.txthmi/dev/nspanel_portrait_code/alarm.txthmi/dev/nspanel_portrait_code/boot.txthmi/dev/nspanel_portrait_code/button.txthmi/dev/nspanel_portrait_code/buttonpage01.txthmi/dev/nspanel_portrait_code/buttonpage02.txthmi/dev/nspanel_portrait_code/buttonpage03.txthmi/dev/nspanel_portrait_code/buttonpage04.txthmi/dev/nspanel_portrait_code/climate.txthmi/dev/nspanel_portrait_code/confirm.txthmi/dev/nspanel_portrait_code/cover.txthmi/dev/nspanel_portrait_code/debug.txthmi/dev/nspanel_portrait_code/entitypage01.txthmi/dev/nspanel_portrait_code/entitypage02.txthmi/dev/nspanel_portrait_code/entitypage03.txthmi/dev/nspanel_portrait_code/entitypage04.txthmi/dev/nspanel_portrait_code/fan.txthmi/dev/nspanel_portrait_code/home.txthmi/dev/nspanel_portrait_code/home_smpl.txthmi/dev/nspanel_portrait_code/keyb_num.txthmi/dev/nspanel_portrait_code/light.txthmi/dev/nspanel_portrait_code/media_player.txthmi/dev/nspanel_portrait_code/notification.txthmi/dev/nspanel_portrait_code/qrcode.txthmi/dev/nspanel_portrait_code/screensaver.txthmi/dev/nspanel_portrait_code/settings.txthmi/dev/nspanel_portrait_code/switch.txthmi/dev/nspanel_portrait_code/theme_apply.txthmi/dev/nspanel_portrait_code/utilities.txthmi/dev/nspanel_portrait_code/water_heater.txthmi/dev/nspanel_portrait_code/weather01.txthmi/dev/nspanel_portrait_code/weather02.txthmi/dev/nspanel_portrait_code/weather03.txthmi/dev/nspanel_portrait_code/weather04.txthmi/dev/nspanel_portrait_code/weather05.txthmi/dev/nspanel_us_code/Program.s.txthmi/dev/nspanel_us_land_code/alarm.txthmi/dev/nspanel_us_land_code/boot.txthmi/dev/nspanel_us_land_code/button.txthmi/dev/nspanel_us_land_code/buttonpage01.txthmi/dev/nspanel_us_land_code/buttonpage02.txthmi/dev/nspanel_us_land_code/buttonpage03.txthmi/dev/nspanel_us_land_code/buttonpage04.txthmi/dev/nspanel_us_land_code/climate.txthmi/dev/nspanel_us_land_code/confirm.txthmi/dev/nspanel_us_land_code/cover.txthmi/dev/nspanel_us_land_code/debug.txthmi/dev/nspanel_us_land_code/entitypage01.txthmi/dev/nspanel_us_land_code/entitypage02.txthmi/dev/nspanel_us_land_code/entitypage03.txthmi/dev/nspanel_us_land_code/entitypage04.txthmi/dev/nspanel_us_land_code/fan.txthmi/dev/nspanel_us_land_code/home.txthmi/dev/nspanel_us_land_code/home_new.txthmi/dev/nspanel_us_land_code/home_smpl.txthmi/dev/nspanel_us_land_code/keyb_num.txthmi/dev/nspanel_us_land_code/light.txthmi/dev/nspanel_us_land_code/media_player.txthmi/dev/nspanel_us_land_code/notification.txthmi/dev/nspanel_us_land_code/qrcode.txthmi/dev/nspanel_us_land_code/screensaver.txthmi/dev/nspanel_us_land_code/settings.txthmi/dev/nspanel_us_land_code/switch.txthmi/dev/nspanel_us_land_code/theme_apply.txthmi/dev/nspanel_us_land_code/utilities.txthmi/dev/nspanel_us_land_code/water_heater.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/dev/scripts/generate_images.shhmi/dev/scripts/generate_nextion_txt.shhmi/dev/scripts/update_nextion_version.shhmi/dev/ui/generate_pics_all.cmdhmi/dev/ui/landscape/draft/home_light.pagehmi/dev/ui/landscape/draft/home_smpl.pagehmi/dev/ui/landscape/draft/new_boot.pagehmi/dev/ui/landscape/draft/power.pagehmi/dev/ui/landscape/draft/power2.page
💤 Files with no reviewable changes (24)
- hmi/dev/create_us_land_and_CJK_hmi_from_original.md
- hmi/dev/nspanel_us_code/Program.s.txt
- hmi/dev/nspanel_us_land_code/entitypage03.txt
- hmi/dev/nspanel_us_land_code/home.txt
- hmi/dev/nspanel_us_land_code/confirm.txt
- hmi/dev/nspanel_us_land_code/home_new.txt
- hmi/dev/nspanel_us_land_code/alarm.txt
- hmi/dev/nspanel_us_land_code/buttonpage02.txt
- hmi/dev/nspanel_us_land_code/climate.txt
- hmi/dev/nspanel_us_land_code/boot.txt
- hmi/dev/nspanel_us_land_code/entitypage04.txt
- hmi/dev/nspanel_us_land_code/home_smpl.txt
- hmi/dev/nspanel_us_land_code/buttonpage01.txt
- hmi/dev/nspanel_us_land_code/buttonpage03.txt
- hmi/dev/nspanel_us_land_code/button.txt
- hmi/dev/nspanel_us_land_code/fan.txt
- hmi/dev/nspanel_us_land_code/light.txt
- hmi/dev/nspanel_us_land_code/entitypage01.txt
- hmi/dev/nspanel_us_land_code/cover.txt
- hmi/dev/nspanel_us_land_code/media_player.txt
- hmi/dev/nspanel_us_land_code/debug.txt
- hmi/dev/nspanel_us_land_code/buttonpage04.txt
- hmi/dev/nspanel_us_land_code/keyb_num.txt
- hmi/dev/nspanel_us_land_code/entitypage02.txt
Experimental
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
hmi/dev/nspanel_landscape_code/canvas.txt (1)
1-27:⚠️ Potential issue | 🔴 CriticalCritical: landscape page rename breaks existing QR-code navigation contract.
Line [1] changes the page name to
canvas, but firmware paths still targetqrcode(components/nspanel_easy/page_qrcode.hLine [31],esphome/nspanel_esphome_page_qrcode.yamlLine [81]-[87]). On landscape this can make QR-code navigation fail at runtime.💡 Minimal compatibility fix
-Page canvas +Page qrcode ... - canvas.pic=pic_background_main + qrcode.pic=pic_background_mainIf the intended migration is
qrcode→canvas, then all firmware/blueprint references must be switched consistently in the same PR.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@hmi/dev/nspanel_landscape_code/canvas.txt` around lines 1 - 27, The page was renamed to "canvas" but other firmware/blueprint references still point to "qrcode", breaking QR navigation; update all references to the page name so they match the rename (change all occurrences of "qrcode" to "canvas" in the page/component references used by the system such as the page identifier used by page_qrcode (and any blueprint/ESPhome entries like nspanel_esphome_page_qrcode.yaml) or instead revert the page name back to "qrcode" if you intended no API change), ensuring the Preinitialize Event and any component imports/IDs use the same identifier so runtime navigation stays consistent.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@hmi/dev/nspanel_landscape_code/canvas.txt`:
- Around line 1-27: The page was renamed to "canvas" but other
firmware/blueprint references still point to "qrcode", breaking QR navigation;
update all references to the page name so they match the rename (change all
occurrences of "qrcode" to "canvas" in the page/component references used by the
system such as the page identifier used by page_qrcode (and any
blueprint/ESPhome entries like nspanel_esphome_page_qrcode.yaml) or instead
revert the page name back to "qrcode" if you intended no API change), ensuring
the Preinitialize Event and any component imports/IDs use the same identifier so
runtime navigation stays consistent.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 5bf4e12f-72e5-4a4d-b5c8-b43bb1003769
📒 Files selected for processing (3)
components/nspanel_easy/pages.hhmi/dev/nspanel_landscape_code/canvas.txthmi/dev/nspanel_portrait_code/canvas.txt
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
esphome/nspanel_esphome_addon_upload_tft.yaml (1)
92-103:⚠️ Potential issue | 🟠 MajorApply
nextion_update_urlbefore the auto-update gates.The PR notes describe this as a full override, but this block still returns early on both the version check and the blank/invalid-model guard before
upload_tftis ever called. With a custom/local override configured, automatic uploads can still be skipped.💡 Suggested fix
`#if` NSPANEL_EASY_ADDON_UPLOAD_TFT_AUTOMATICALLY + static const char* const NEXTION_UPDATE_URL = "${nextion_update_url}"; + const bool has_url_override = NEXTION_UPDATE_URL[0] != '\0'; // Return if sensor has value and versions match (no update needed) - if (tft_ver_gte(version_tft->state, "${min_tft_version}")) { + if (!has_url_override && tft_ver_gte(version_tft->state, "${min_tft_version}")) { ESP_LOGV("${TAG_UPLOAD_TFT}", "Supported version"); return; } // Only proceed when a standard TFT is selected (indices 1+) - if (!display_valid || display_model->current_option() == "${DISPLAY_MODES.BLANK.TEXT}") { + if (!has_url_override && + (!display_valid || display_model->current_option() == "${DISPLAY_MODES.BLANK.TEXT}")) { ESP_LOGD("${TAG_UPLOAD_TFT}", "No hardware TFT selected"); return; }🤖 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 92 - 103, The auto-update gate returns early on version and model checks before honoring a configured override; detect if a custom nextion_update_url (or equivalent override variable) is set and, if so, call upload_tft (or bypass the early-return logic) before the tft_ver_gte(version_tft->state, "${min_tft_version}") and display_valid/display_model->current_option() checks. In practice modify the block around tft_ver_gte, display_valid and DISPLAY_MODES.BLANK.TEXT to first check nextion_update_url and invoke upload_tft (or skip the early returns) so a configured override always triggers the upload regardless of version or model.
🤖 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 194-197: The branch currently treats any non-empty url literally,
causing the string "default" to be downloaded; change the condition that sets
resolved_url so that it only uses url when it is non-empty and not equal to the
legacy sentinel "default" (i.e., check url != "default" in addition to
!url.empty()), so that when url is empty or "default" the code falls through to
the model-resolved file path; update the branch around resolved_url and url to
implement this check (referencing the variables resolved_url and url in this
block).
- Around line 340-344: Remove the unnecessary initial_option override in the
display_model extension: delete the line setting initial_option:
${DISPLAY_MODES.BLANK.TEXT} so the selector inherits the base initial_option
(${DISPLAY_MODES.EU.TEXT}) defined in hw_display.yaml; keep the id: !extend
display_model block and its options intact (do not alter restore_value or the
display_model on_value handler) since the existing guard already prevents
uploads when BLANK is selected.
- Around line 210-217: The error branch currently uses a bare return inside the
lambda which only exits the lambda and not the upload_tft script, leaving
system_flags.tft_upload_active true and nextion_init false and allowing
subsequent steps to run; replace the bare return with an explicit script abort
(call id(upload_tft).stop() or script.stop equivalent) and perform cleanup
there: set system_flags.tft_upload_active to false and restore nextion_init
(publish true) before stopping the script, ensuring disp1->set_tft_url() remains
skipped and no further upload/delay/BLE/reboot steps execute.
In `@esphome/nspanel_esphome_page_canvas.yaml`:
- Around line 24-27: The page_canvas script currently declares `then:` with no
value (which YAML parses as null), causing ESPHome to reject it; update the `id:
page_canvas` script so that `then:` is an explicit action list (use `then: []`
for an empty list) so the automation has a valid (possibly empty) actions array.
---
Outside diff comments:
In `@esphome/nspanel_esphome_addon_upload_tft.yaml`:
- Around line 92-103: The auto-update gate returns early on version and model
checks before honoring a configured override; detect if a custom
nextion_update_url (or equivalent override variable) is set and, if so, call
upload_tft (or bypass the early-return logic) before the
tft_ver_gte(version_tft->state, "${min_tft_version}") and
display_valid/display_model->current_option() checks. In practice modify the
block around tft_ver_gte, display_valid and DISPLAY_MODES.BLANK.TEXT to first
check nextion_update_url and invoke upload_tft (or skip the early returns) so a
configured override always triggers the upload regardless of version or model.
🪄 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: e8a80a47-d458-460d-b76d-50cf462d41d0
📒 Files selected for processing (5)
docs/addon_upload_tft.mdesphome/nspanel_esphome_addon_upload_tft.yamlesphome/nspanel_esphome_hw_buttons.yamlesphome/nspanel_esphome_page_canvas.yamlesphome/nspanel_esphome_standard.yaml
🚧 Files skipped from review as they are similar to previous changes (3)
- esphome/nspanel_esphome_hw_buttons.yaml
- esphome/nspanel_esphome_standard.yaml
- docs/addon_upload_tft.md
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
esphome/nspanel_esphome_addon_upload_tft.yaml (2)
195-197:⚠️ Potential issue | 🟠 MajorKeep
url: "default"backward-compatible.This branch treats any non-empty
urlliterally. Existing service calls usingurl: "default"(as documented in the action comments at lines 36, 44) will try to download a URL named "default" instead of falling back to model-resolved behavior.💡 Suggested fix
- } else if (!url.empty()) { + } else if (!url.empty() && url != "default") { // URL explicitly provided to this script resolved_url = url;🤖 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 195 - 197, The branch currently treats any non-empty url literally, breaking backward compatibility for the sentinel value "default"; update the condition that assigns resolved_url (the else if (!url.empty()) branch) to ignore the string "default" (e.g., use if (!url.empty() && url != "default") or treat url == "default" as empty) so model-resolved behavior still occurs when callers pass url: "default".
217-219:⚠️ Potential issue | 🔴 CriticalThe error
returnonly exits the lambda, not the upload_tft script.When no valid display model is selected,
return;exits only the lambda at line 218. The script continues executing subsequent steps: delay at line 225,stop_allat line 228, BLE shutdown, upload attempts, and reboot. Sincedisp1->set_tft_url()is skipped, the upload proceeds with the default URL (nspanel_blank.tft).Additionally:
system_flags.tft_upload_active(settrueat line 184) is never reset tofalsenextion_init(publishedfalseat line 179) is never restored totrueUse ESPHome's script control to abort properly and clean up state.
💡 Suggested fix
if (idx < FILENAMES_COUNT) { resolved_url = std::string("${nextion_update_base_url}/nspanel_") + FILENAMES[idx] + ".tft"; } else { ESP_LOGE("${TAG_UPLOAD_TFT}", "No valid display model selected"); + system_flags.tft_upload_active = false; + nextion_init->publish_state(true); + id(upload_tft).stop(); return; }🤖 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 217 - 219, When no valid display model is selected, replace the lambda-only `return;` with a scripted abort flow that resets state and stops the upload script: set `system_flags.tft_upload_active` to false, publish `nextion_init` true, log the error (keeping the ESP_LOGE line), and then stop/abort the upload script (use ESPHome script control to stop the running `upload_tft` script, e.g. `script.stop: upload_tft` or the equivalent `script.abort` action), so execution does not continue to delays, BLE shutdown, upload attempts, or reboot.
🤖 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 200-219: The FILENAMES array is misordered relative to
display_model->active_index() causing wrong file selection; reorder and/or
replace the entries in the static const char* const FILENAMES[] so its element 0
maps to the BLANK file and element 1 maps to the EASY_EU file (and ensure the
rest of the entries follow the same order as the display_model enum/options),
keeping FILENAMES_COUNT as-is and leaving the resolved_url construction and
error path unchanged (references: FILENAMES, FILENAMES_COUNT,
display_model->active_index(), resolved_url, TAG_UPLOAD_TFT).
- Around line 341-345: The addon breaks the mapping between the restricted
selector options and the FILENAMES array: when you read
display_model->active_index() it must index FILENAMES, so either make the addon
options carry the original selector indices (e.g., use values matching FILENAMES
positions for BLANK and EASY_EU instead of simple 0/1) or add an explicit
mapping layer that converts the addon index to the correct FILENAMES index
before lookup; also stop resetting the selector on boot by enabling persistence
(set restore_value: true on the display_model selector or remove the addon
override that forces initial_option), and update any code using
display_model->active_index() to use the mapped index or persistent value so
BLANK/EASY_EU resolve to the correct FILENAMES entries.
---
Duplicate comments:
In `@esphome/nspanel_esphome_addon_upload_tft.yaml`:
- Around line 195-197: The branch currently treats any non-empty url literally,
breaking backward compatibility for the sentinel value "default"; update the
condition that assigns resolved_url (the else if (!url.empty()) branch) to
ignore the string "default" (e.g., use if (!url.empty() && url != "default") or
treat url == "default" as empty) so model-resolved behavior still occurs when
callers pass url: "default".
- Around line 217-219: When no valid display model is selected, replace the
lambda-only `return;` with a scripted abort flow that resets state and stops the
upload script: set `system_flags.tft_upload_active` to false, publish
`nextion_init` true, log the error (keeping the ESP_LOGE line), and then
stop/abort the upload script (use ESPHome script control to stop the running
`upload_tft` script, e.g. `script.stop: upload_tft` or the equivalent
`script.abort` action), so execution does not continue to delays, BLE shutdown,
upload attempts, or reboot.
🪄 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: 92eeda9a-c445-4951-803f-2c58662deb8d
📒 Files selected for processing (2)
esphome/nspanel_esphome_addon_upload_tft.yamlesphome/nspanel_esphome_hw_buttons.yaml
🚧 Files skipped from review as they are similar to previous changes (1)
- esphome/nspanel_esphome_hw_buttons.yaml
I don't know why this is failing to resolve.
Owner
Author
|
@codex review |
![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: 4439f3bc80
ℹ️ 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: 3
🧹 Nitpick comments (2)
esphome/nspanel_esphome_hw_uart.yaml (1)
283-292: Minor logic issue in wait_for_uart_available.The script stops itself when
tf_uart->available() < 1(queue is empty), which is the desired behavior. However, the condition at line 290 stops on "less than 1" (i.e.,== 0), which is correct but could be more clearly written astf_uart->available() == 0.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@esphome/nspanel_esphome_hw_uart.yaml` around lines 283 - 292, Replace the ambiguous less-than check in the wait_for_uart_available lambda with an explicit zero-check: change the condition from tf_uart->available() < 1 to tf_uart->available() == 0 so that the self-stop logic in the wait_for_uart_available task clearly triggers when the UART queue is empty; keep the surrounding logic (iteration % 5 logging via boot_log->execute("Boot", "Waiting for UART empty queue") and the 1s delay) unchanged.esphome/nspanel_esphome_page_boot.yaml (1)
107-156: Contract version handling is comprehensive.Each protocol version correctly:
- v0/v1: Initializes EEPROM state from TFT-reported
display_mode_from_tft- v2: Distinguishes between uninitialized EEPROM (
magic_flag_found_at_boot == 0→ use hard-coded default) and valid EEPROM (usedisplay_mode_from_tft)The duplicate validation logic between v0 and v1 cases (lines 110-117 and 121-128) could be extracted to a helper, but this is acceptable for clarity given the different log messages.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@esphome/nspanel_esphome_page_boot.yaml` around lines 107 - 156, The v0 and v1 branches duplicate the same validation/apply sequence for display_mode_from_tft (checking range, setting display_mode_eeprom, calling display_model->make_call(), or logging invalid value); refactor that repeated logic into a helper like apply_display_mode_from_tft(uint8_t mode) (or validate_and_apply_display_mode) that performs the range check, sets display_mode_eeprom, obtains and performs the call via display_model->make_call(), and logs errors with ESP_LOGW so both case 0 and case 1 simply call this helper while preserving their distinct log messages.
🤖 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_hw_uart.yaml`:
- Around line 168-171: The current else-branch calls baud_upgrade->execute()
when BAUD_RATE == BAUD_RATE_FALLBACK, causing infinite recursion; change the
call to baud_upgrade->stop() so the upgrade script halts instead of re-invoking
itself (locate the else block using the symbol baud_upgrade and replace the
execute() invocation with stop()).
- Around line 155-157: The script entry for baud_upgrade is not indented under
the existing script: mapping; move the "- id: baud_upgrade" block (and its child
keys like mode: and then:) two spaces to the right so it nests as an item of the
script: list (ensure the entire baud_upgrade block uses consistent indentation
with the other scripts in the file and that keys like id, mode, and then remain
aligned under that list item).
- Around line 69-72: The else branch currently calls baud_negotiate->execute(),
causing infinite recursion when BAUD_RATE == BAUD_RATE_FALLBACK; change that
call to baud_negotiate->stop() so the negotiation script exits gracefully
(update the branch that logs "Baud negotiate: called with equal rates -
stopping" to invoke baud_negotiate->stop() instead of execute()).
---
Nitpick comments:
In `@esphome/nspanel_esphome_hw_uart.yaml`:
- Around line 283-292: Replace the ambiguous less-than check in the
wait_for_uart_available lambda with an explicit zero-check: change the condition
from tf_uart->available() < 1 to tf_uart->available() == 0 so that the self-stop
logic in the wait_for_uart_available task clearly triggers when the UART queue
is empty; keep the surrounding logic (iteration % 5 logging via
boot_log->execute("Boot", "Waiting for UART empty queue") and the 1s delay)
unchanged.
In `@esphome/nspanel_esphome_page_boot.yaml`:
- Around line 107-156: The v0 and v1 branches duplicate the same
validation/apply sequence for display_mode_from_tft (checking range, setting
display_mode_eeprom, calling display_model->make_call(), or logging invalid
value); refactor that repeated logic into a helper like
apply_display_mode_from_tft(uint8_t mode) (or validate_and_apply_display_mode)
that performs the range check, sets display_mode_eeprom, obtains and performs
the call via display_model->make_call(), and logs errors with ESP_LOGW so both
case 0 and case 1 simply call this helper while preserving their distinct log
messages.
🪄 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: c39048fb-f95f-4bb3-ba34-ec194950a1ac
📒 Files selected for processing (7)
.github/workflows/versioning.ymlcomponents/nspanel_easy/base.hesphome/nspanel_esphome_addon_upload_tft.yamlesphome/nspanel_esphome_core.yamlesphome/nspanel_esphome_hw_display.yamlesphome/nspanel_esphome_hw_uart.yamlesphome/nspanel_esphome_page_boot.yaml
✅ Files skipped from review due to trivial changes (2)
- .github/workflows/versioning.yml
- esphome/nspanel_esphome_addon_upload_tft.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 `@hmi/dev/nspanel_easy_landscape/Program.s.txt`:
- Around line 36-39: The startup baud setting "bauds=921600" is only written
when the EEPROM magic flag indicates a fresh device, causing existing panels to
keep old persisted startup baud rates; move the "bauds=921600" assignment out of
the first-init guard so it is always written on firmware upgrade (leaving
"baud=921600" as-is), ensuring the Nextion startup BAUD_RATE is updated for all
devices and avoiding repeated rate-negotiation loops tied to existing persisted
values used by baud_negotiate and baud_upgrade.
🪄 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: beef7384-8bc4-4376-a28d-745178af8eba
📒 Files selected for processing (4)
hmi/dev/nspanel_blank/Program.s.txthmi/dev/nspanel_easy_landscape/Program.s.txthmi/dev/nspanel_landscape_code/Program.s.txthmi/dev/nspanel_portrait_code/Program.s.txt
🚧 Files skipped from review as they are similar to previous changes (1)
- hmi/dev/nspanel_portrait_code/Program.s.txt
edwardtfn
changed the title
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_hw_uart.yaml`:
- Around line 276-301: The code duplicates a post-probe success path causing
false success logs and clearing system_flags.baud_fallback_active after we've
already decided outcome earlier; remove the redundant branch that checks
tf_uart->available() and its success-path (the block that flushes tf_uart,
clears system_flags.baud_fallback_active, logs "Baud upgrade: success..." and
calls boot_log->execute("Baud", "Upgraded to primary rate")), leaving only the
prior outcome handling (including the fallback revert that calls
tf_uart->set_baud_rate(${BAUD_RATE_FALLBACK}), tf_uart->load_settings(),
baud_rate_active->update(), and disp1->send_command_printf(...)); ensure no
extra delay/probe timeout is introduced and references to TAG_HW_UART,
BAUD_RATE, BAUD_RATE_FALLBACK, tf_uart, system_flags, boot_log,
baud_rate_active, and disp1 remain consistent.
🪄 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: 098a9788-a653-4a29-924c-ca91021f0942
📒 Files selected for processing (2)
esphome/nspanel_esphome_hw_display.yamlesphome/nspanel_esphome_hw_uart.yaml
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
esphome/nspanel_esphome_hw_uart.yaml (1)
322-331: Minor naming clarity observation.The script name
wait_for_uart_availablecould be clearer - it actually waits for the UART RX buffer to be empty/drained (line 329:available() == 0), not for bytes to become available. The log message on line 330 correctly says "empty queue."A name like
wait_for_uart_drainwould be more self-documenting, but this is purely a readability nit - the logic is correct.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@esphome/nspanel_esphome_hw_uart.yaml` around lines 322 - 331, The action id wait_for_uart_available is misleading because the logic checks tf_uart->available() == 0 (waiting for the RX buffer to be empty/drained) and logs "Waiting for UART empty queue"; rename the id to a clearer name such as wait_for_uart_drain (or similar) and update any references to that id so they match, keeping the existing logic (tf_uart->available(), boot_log->execute) unchanged to preserve behavior and messages.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@esphome/nspanel_esphome_hw_uart.yaml`:
- Around line 322-331: The action id wait_for_uart_available is misleading
because the logic checks tf_uart->available() == 0 (waiting for the RX buffer to
be empty/drained) and logs "Waiting for UART empty queue"; rename the id to a
clearer name such as wait_for_uart_drain (or similar) and update any references
to that id so they match, keeping the existing logic (tf_uart->available(),
boot_log->execute) unchanged to preserve behavior and messages.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 85f8f0e5-7df4-4fa8-be99-154a9e09098b
📒 Files selected for processing (1)
esphome/nspanel_esphome_hw_uart.yaml
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.
TFT files are now unified — CJK language support (Chinese, Japanese, Korean) is built into the standard files, and EU and US Landscape share a single file. There are no longer separate CJK variants to choose from.
The default UART baud rate between the ESP32 and the Nextion display has been raised from 115200 to 921600 bps, improving display responsiveness and command throughput.
What changed
A new Display model selector appears on your device's page in Home Assistant under Configuration. This replaces the previous model selection that was tied to the TFT upload add-on, and now drives all model-specific runtime behavior — not just TFT uploads. The selected model is persisted in the Nextion display's EEPROM and survives power cycles
independently of Home Assistant or ESPHome connectivity.
The firmware now negotiates the baud rate automatically on boot. If your panel is running an older TFT (NSPanel Easy, Blackymas, or Sonoff stock), it will fall back to 115200 bps, connect, and either upgrade the baud rate mid-session or trigger a TFT upload as appropriate. No manual intervention is required.
Breaking changes
Baud rate
The default UART baud rate is now 921600 bps. If you have
BAUD_RATEexplicitly set to115200in your substitutions, you can remove it to adopt the new default, or keep it to preserve the old behavior. The negotiation system is active only whenBAUD_RATEandBAUD_RATE_FALLBACKdiffer — setting both to the same value disables it entirely.nextion_update_urlbehaviournextion_update_urlnow acts as a full override — when set, it bypasses the model selector and version logic entirely, and the specified URL is used as-is for every upload.If you have
nextion_update_urldefined but are not actively using it to host a custom or local TFT file, remove it from your substitutions. Leaving it set will prevent automatic model-based URL resolution and stop your panel from receiving automatic TFT updates.Display model selector reset
The Display model selector has been restructured and will not carry over its previous value after updating. After flashing the new firmware, go to your device page and verify the Display model selector is set correctly for your hardware before triggering a TFT update.
Upgrading
nextion_update_urldefined without a specific reason, remove it from your substitutions and reflashNotes for US Landscape users
If this is your first install with this version and your panel has never stored a display model in EEPROM, the selector will default to NSPanel EU on first boot. US Landscape users should correct this manually — the setting will then persist correctly from that point forward.
Notes for users migrating from older TFT versions
On first boot after updating the firmware, the panel may take up to two minutes to connect while the baud rate negotiation runs. This is a one-time cost — once the new TFT is installed, all subsequent boots connect immediately at 921600 bps.
Summary by CodeRabbit
New Features
Improvements
Documentation