Conversation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughBumps minimum TFT version from 15 to 16, updates HMI version text values, refactors button-page initialization to theme-gated styling with fixed component ranges (removing Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant ESPHome as ESPhome/Node
participant Script as action_button (queued)
participant Display as Nextion_Display
participant State as current_page/state
User->>ESPHome: press button (api action)
ESPHome->>ESPHome: compute page_id, icon_color_rgb565
alt page differs
ESPHome->>Script: queue action_button(page_id, button_id, ...)
Script->>State: check current_page_id == page_id?
opt still same page
Script->>Display: set background/picc/pco, set icon/font/text, set brightness
Display-->>Script: ack
Script->>Display: vis components (show button parts)
end
else same page
ESPHome-->>User: no-op (skip rendering)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (11)
hmi/dev/nspanel_us_land_code/buttonpage04.txt (1)
29-48:⚠️ Potential issue | 🟠 MajorReset
page_index.txtbefore appending dots.Line 30 onward appends with
+=but never clears existing text. On repeated page opens, dots can accumulate and produce exactly the kind of intermittent visual artifacts this PR is fixing.🩹 Proposed fix
// Build page index dots if more than one button page is active + page_index.txt="" sys0=is_buttonpage01+is_buttonpage02+is_buttonpage03+is_buttonpage04 if(sys0>1) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@hmi/dev/nspanel_us_land_code/buttonpage04.txt` around lines 29 - 48, The page index string is never cleared before appending dots, causing dots to accumulate; in the block that runs when sys0>1 (which computes sys0 from is_buttonpage01..04), reset page_index.txt to an empty string (e.g., set page_index.txt = "" or equivalent) before any "+=" operations so the filled/empty dot logic (references: page_index.txt, is_buttonpage01/is_buttonpage02/is_buttonpage03/is_buttonpage04, sys0, vis page_index,1) builds a fresh indicator each time.hmi/dev/nspanel_us_code/buttonpage01.txt (1)
29-48:⚠️ Potential issue | 🟠 MajorReset
page_index.txtbefore appending dots.
page_index.txtis appended on every preinitialize run. On repeated navigation, dots can accumulate and reintroduce rendering artifacts.Suggested fix
// Hide all components initially vis 255,0 + page_index.txt="" // Build page index dots if more than one button page is active sys0=is_buttonpage01+is_buttonpage02+is_buttonpage03+is_buttonpage04🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@hmi/dev/nspanel_us_code/buttonpage01.txt` around lines 29 - 48, Reset page_index.txt to an empty string at the start of the page-index construction so dots don't accumulate on repeated preinitialize runs: inside the block that computes sys0 and builds the dots (the logic referencing sys0, is_buttonpage01..04, page_index.txt and vis page_index,1) set page_index.txt="" before appending the filled/unfilled dot characters so each build recreates the indicator from scratch.hmi/dev/nspanel_us_code/buttonpage04.txt (1)
29-48:⚠️ Potential issue | 🟠 MajorInitialize
page_index.txtbefore constructing the indicator.At Line 36-Line 46,
page_index.txtis only appended to. Because this page starts with prefilled indicator text (Line 152), repeated navigation can produce duplicated dots and unstable UI state.Suggested fix
// Build page index dots if more than one button page is active + page_index.txt="" sys0=is_buttonpage01+is_buttonpage02+is_buttonpage03+is_buttonpage04 if(sys0>1) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@hmi/dev/nspanel_us_code/buttonpage04.txt` around lines 29 - 48, Reset page_index.txt before appending dots so repeated navigation doesn't accumulate duplicates: at the start of the indicator build block (before computing/using sys0 or appending any "\xe2\x97\x8b"/"\xe2\x97\x8f"), explicitly set page_index.txt to an empty string, then proceed to check is_buttonpage01/is_buttonpage02/is_buttonpage03/is_buttonpage04 and append the filled/empty dot characters and call vis page_index,1; this ensures page_index.txt is reinitialized each time the indicator is rebuilt.hmi/dev/nspanel_us_land_code/buttonpage02.txt (1)
29-48:⚠️ Potential issue | 🟠 MajorClear
page_index.txtbefore appending page dots.At Line 36-Line 45, the indicator string is incrementally appended every initialization. Without a reset, repeated opens can accumulate stale dots and misrender the current page indicator.
Suggested fix
// Build page index dots if more than one button page is active + page_index.txt="" sys0=is_buttonpage01+is_buttonpage02+is_buttonpage03+is_buttonpage04 if(sys0>1) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@hmi/dev/nspanel_us_land_code/buttonpage02.txt` around lines 29 - 48, The page_index.txt string is never cleared before appending page indicator dots, causing dots to accumulate across initializations; update the block that builds the index (referencing page_index.txt and flags is_buttonpage01/is_buttonpage02/is_buttonpage03/is_buttonpage04 and variable sys0) to reset/clear page_index.txt (e.g., set it to an empty string) before appending the filled/empty dot characters, then proceed with the existing conditional appends and vis page_index,1 so each build starts fresh.hmi/dev/nspanel_eu_code/buttonpage02.txt (1)
29-48:⚠️ Potential issue | 🟠 MajorAdd explicit
page_index.txtreset before dot generation.At Line 36-Line 45, the dot string is built via
+=without clearing previous content. Repeated page entries can therefore accumulate extra dots and show incorrect navigation state.Suggested fix
// Build page index dots if more than one button page is active + page_index.txt="" sys0=is_buttonpage01+is_buttonpage02+is_buttonpage03+is_buttonpage04 if(sys0>1) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@hmi/dev/nspanel_eu_code/buttonpage02.txt` around lines 29 - 48, The page index dot string is appended with += every time without clearing previous content causing accumulated dots; before building the dots for page_index.txt in the block that checks sys0>1, explicitly reset page_index.txt to an empty string (e.g., set page_index.txt = "" ) then append the filled/empty dot characters based on is_buttonpage01, is_buttonpage02, is_buttonpage03, is_buttonpage04 and finally call vis page_index,1 to ensure a fresh indicator each update.hmi/dev/nspanel_us_land_code/buttonpage03.txt (1)
29-48:⚠️ Potential issue | 🟠 MajorReset
page_index.txtbefore appending dots.At Line 36-Line 45, dots are appended with
+=but the string is never cleared first. On repeated page opens, this can accumulate stale dots and recreate the intermittent rendering/log issues this PR is fixing.Suggested fix
// Build page index dots if more than one button page is active + page_index.txt="" sys0=is_buttonpage01+is_buttonpage02+is_buttonpage03+is_buttonpage04 if(sys0>1) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@hmi/dev/nspanel_us_land_code/buttonpage03.txt` around lines 29 - 48, The page index string is never cleared before appending dots, causing accumulation on repeated opens; in the page-building block that checks sys0>1, reset page_index.txt to an empty string (e.g., set page_index.txt = "" or equivalent) before any "+=" operations, then append the filled/empty dot characters based on is_buttonpage01, is_buttonpage02, is_buttonpage03, is_buttonpage04 and finally call vis page_index,1 so the indicator is built fresh each time.hmi/dev/nspanel_CJK_eu_code/buttonpage03.txt (1)
29-48:⚠️ Potential issue | 🟠 MajorPrevent indicator accumulation by resetting
page_index.txt.At Line 36-Line 45, the index is built with
+=only. Add a reset before the block so repeated visits don’t retain old dots and produce incorrect indicators.Suggested fix
// Build page index dots if more than one button page is active + page_index.txt="" sys0=is_buttonpage01+is_buttonpage02+is_buttonpage03+is_buttonpage04 if(sys0>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/buttonpage03.txt` around lines 29 - 48, Reset page_index.txt before building the page dots to avoid accumulation: inside the sys0>1 block (where you compute sys0 and append to page_index.txt using += and check is_buttonpage01/02/03/04) set page_index.txt to an empty string first, then append the filled/empty dot characters as currently done so repeated visits produce correct indicators.hmi/dev/nspanel_eu_code/buttonpage03.txt (1)
29-48:⚠️ Potential issue | 🟠 MajorClear
page_index.txtbefore rebuilding index dots.The indicator string is appended each init cycle without reset. That can leave stale/duplicated dots after repeated opens.
💡 Proposed fix
// Hide all components initially vis 255,0 + page_index.txt="" // Build page index dots if more than one button page is active sys0=is_buttonpage01+is_buttonpage02+is_buttonpage03+is_buttonpage04🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@hmi/dev/nspanel_eu_code/buttonpage03.txt` around lines 29 - 48, The page index string is being appended each init cycle causing duplicated dots; before building the dots in the block that checks sys0>1, reset the indicator by clearing page_index.txt (e.g., set page_index.txt to an empty string) then proceed to append the filled/empty dot characters based on is_buttonpage01, is_buttonpage02, is_buttonpage03, is_buttonpage04 so page_index is rebuilt cleanly each time.hmi/dev/nspanel_us_code/buttonpage02.txt (1)
29-48:⚠️ Potential issue | 🟠 MajorInitialize
page_index.txtbefore appending indicator glyphs.Without resetting, the dot string can persist and grow between navigations, causing incorrect page index rendering.
💡 Proposed fix
// Hide all components initially vis 255,0 + page_index.txt="" // Build page index dots if more than one button page is active sys0=is_buttonpage01+is_buttonpage02+is_buttonpage03+is_buttonpage04🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@hmi/dev/nspanel_us_code/buttonpage02.txt` around lines 29 - 48, The page index string isn't reset before appending glyphs, so page_index.txt can grow across navigations; modify the block that builds the dots (the if(sys0>1) branch) to initialize/clear page_index.txt (e.g., set page_index.txt to an empty string) before appending any indicator glyphs using is_buttonpage01/is_buttonpage02/is_buttonpage03/is_buttonpage04, then proceed with the existing logic and call vis page_index,1.hmi/dev/nspanel_CJK_us_code/buttonpage02.txt (1)
29-48:⚠️ Potential issue | 🟠 MajorReset
page_index.txtbefore appending dots.
page_index.txtis built with+=but never cleared inPreinitialize. On repeated page opens, this can accumulate stale dots and reintroduce visual artifacts.💡 Proposed fix
// Hide all components initially vis 255,0 + page_index.txt="" // Build page index dots if more than one button page is active sys0=is_buttonpage01+is_buttonpage02+is_buttonpage03+is_buttonpage04🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@hmi/dev/nspanel_CJK_us_code/buttonpage02.txt` around lines 29 - 48, Reset page_index.txt before appending page dots to avoid accumulation; in Preinitialize (or immediately before the dots construction block) set page_index.txt to an empty string (or explicitly clear it) so the subsequent logic that builds the indicator using page_index.txt+="\xe2\x97\x8b"/"\xe2\x97\x8f" produces only the current page dots; update the code around the page-index build (the block referencing page_index.txt and is_buttonpage01..is_buttonpage04) to clear page_index.txt first.hmi/dev/nspanel_CJK_us_code/buttonpage03.txt (1)
29-48:⚠️ Potential issue | 🟠 MajorPrevent dot indicator accumulation across page re-entry.
page_index.txtuses+=but is not reset during preinitialize. This can accumulate stale dots over repeated navigations.💡 Proposed fix
// Hide all components initially vis 255,0 + page_index.txt="" // Build page index dots if more than one button page is active sys0=is_buttonpage01+is_buttonpage02+is_buttonpage03+is_buttonpage04🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@hmi/dev/nspanel_CJK_us_code/buttonpage03.txt` around lines 29 - 48, The page index string page_index.txt is appended to with += in the page-building block (using is_buttonpage01/02/03/04) and is never cleared, causing dots to accumulate on repeated entries; fix by resetting page_index.txt to an empty string before you build the dots (for example in the same preinitialize/entry code that runs this block or immediately before summing sys0) so the code that checks is_buttonpage01/is_buttonpage02/is_buttonpage03/is_buttonpage04 and appends "◉"/"○" always builds from a clean page_index state and then calls vis page_index,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_version.yaml`:
- Line 21: The repo has an inconsistent version contract:
nspanel_esphome_version.yaml sets min_tft_version: 16 while
hmi/dev/nspanel_CJK_us_land_code/boot.txt still reports "Text : 15", causing
version-gate failures; update the variant's boot metadata so the reported Text /
TFT version matches the new minimum (e.g., change the "Text : 15" entry in
hmi/dev/nspanel_CJK_us_land_code/boot.txt to 16) or instead lower
min_tft_version in nspanel_esphome_version.yaml—edit the file containing the
mismatched string (boot.txt) or the min_tft_version symbol to make both values
identical.
---
Outside diff comments:
In `@hmi/dev/nspanel_CJK_eu_code/buttonpage03.txt`:
- Around line 29-48: Reset page_index.txt before building the page dots to avoid
accumulation: inside the sys0>1 block (where you compute sys0 and append to
page_index.txt using += and check is_buttonpage01/02/03/04) set page_index.txt
to an empty string first, then append the filled/empty dot characters as
currently done so repeated visits produce correct indicators.
In `@hmi/dev/nspanel_CJK_us_code/buttonpage02.txt`:
- Around line 29-48: Reset page_index.txt before appending page dots to avoid
accumulation; in Preinitialize (or immediately before the dots construction
block) set page_index.txt to an empty string (or explicitly clear it) so the
subsequent logic that builds the indicator using
page_index.txt+="\xe2\x97\x8b"/"\xe2\x97\x8f" produces only the current page
dots; update the code around the page-index build (the block referencing
page_index.txt and is_buttonpage01..is_buttonpage04) to clear page_index.txt
first.
In `@hmi/dev/nspanel_CJK_us_code/buttonpage03.txt`:
- Around line 29-48: The page index string page_index.txt is appended to with +=
in the page-building block (using is_buttonpage01/02/03/04) and is never
cleared, causing dots to accumulate on repeated entries; fix by resetting
page_index.txt to an empty string before you build the dots (for example in the
same preinitialize/entry code that runs this block or immediately before summing
sys0) so the code that checks
is_buttonpage01/is_buttonpage02/is_buttonpage03/is_buttonpage04 and appends
"◉"/"○" always builds from a clean page_index state and then calls vis
page_index,1.
In `@hmi/dev/nspanel_eu_code/buttonpage02.txt`:
- Around line 29-48: The page index dot string is appended with += every time
without clearing previous content causing accumulated dots; before building the
dots for page_index.txt in the block that checks sys0>1, explicitly reset
page_index.txt to an empty string (e.g., set page_index.txt = "" ) then append
the filled/empty dot characters based on is_buttonpage01, is_buttonpage02,
is_buttonpage03, is_buttonpage04 and finally call vis page_index,1 to ensure a
fresh indicator each update.
In `@hmi/dev/nspanel_eu_code/buttonpage03.txt`:
- Around line 29-48: The page index string is being appended each init cycle
causing duplicated dots; before building the dots in the block that checks
sys0>1, reset the indicator by clearing page_index.txt (e.g., set page_index.txt
to an empty string) then proceed to append the filled/empty dot characters based
on is_buttonpage01, is_buttonpage02, is_buttonpage03, is_buttonpage04 so
page_index is rebuilt cleanly each time.
In `@hmi/dev/nspanel_us_code/buttonpage01.txt`:
- Around line 29-48: Reset page_index.txt to an empty string at the start of the
page-index construction so dots don't accumulate on repeated preinitialize runs:
inside the block that computes sys0 and builds the dots (the logic referencing
sys0, is_buttonpage01..04, page_index.txt and vis page_index,1) set
page_index.txt="" before appending the filled/unfilled dot characters so each
build recreates the indicator from scratch.
In `@hmi/dev/nspanel_us_code/buttonpage02.txt`:
- Around line 29-48: The page index string isn't reset before appending glyphs,
so page_index.txt can grow across navigations; modify the block that builds the
dots (the if(sys0>1) branch) to initialize/clear page_index.txt (e.g., set
page_index.txt to an empty string) before appending any indicator glyphs using
is_buttonpage01/is_buttonpage02/is_buttonpage03/is_buttonpage04, then proceed
with the existing logic and call vis page_index,1.
In `@hmi/dev/nspanel_us_code/buttonpage04.txt`:
- Around line 29-48: Reset page_index.txt before appending dots so repeated
navigation doesn't accumulate duplicates: at the start of the indicator build
block (before computing/using sys0 or appending any
"\xe2\x97\x8b"/"\xe2\x97\x8f"), explicitly set page_index.txt to an empty
string, then proceed to check
is_buttonpage01/is_buttonpage02/is_buttonpage03/is_buttonpage04 and append the
filled/empty dot characters and call vis page_index,1; this ensures
page_index.txt is reinitialized each time the indicator is rebuilt.
In `@hmi/dev/nspanel_us_land_code/buttonpage02.txt`:
- Around line 29-48: The page_index.txt string is never cleared before appending
page indicator dots, causing dots to accumulate across initializations; update
the block that builds the index (referencing page_index.txt and flags
is_buttonpage01/is_buttonpage02/is_buttonpage03/is_buttonpage04 and variable
sys0) to reset/clear page_index.txt (e.g., set it to an empty string) before
appending the filled/empty dot characters, then proceed with the existing
conditional appends and vis page_index,1 so each build starts fresh.
In `@hmi/dev/nspanel_us_land_code/buttonpage03.txt`:
- Around line 29-48: The page index string is never cleared before appending
dots, causing accumulation on repeated opens; in the page-building block that
checks sys0>1, reset page_index.txt to an empty string (e.g., set page_index.txt
= "" or equivalent) before any "+=" operations, then append the filled/empty dot
characters based on is_buttonpage01, is_buttonpage02, is_buttonpage03,
is_buttonpage04 and finally call vis page_index,1 so the indicator is built
fresh each time.
In `@hmi/dev/nspanel_us_land_code/buttonpage04.txt`:
- Around line 29-48: The page index string is never cleared before appending
dots, causing dots to accumulate; in the block that runs when sys0>1 (which
computes sys0 from is_buttonpage01..04), reset page_index.txt to an empty string
(e.g., set page_index.txt = "" or equivalent) before any "+=" operations so the
filled/empty dot logic (references: page_index.txt,
is_buttonpage01/is_buttonpage02/is_buttonpage03/is_buttonpage04, sys0, vis
page_index,1) builds a fresh indicator each time.
🪄 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: 53dbb52e-b373-4967-b934-858b664b9db2
📒 Files selected for processing (39)
esphome/nspanel_esphome_version.yamlhmi/dev/nspanel_CJK_eu_code/boot.txthmi/dev/nspanel_CJK_eu_code/buttonpage01.txthmi/dev/nspanel_CJK_eu_code/buttonpage02.txthmi/dev/nspanel_CJK_eu_code/buttonpage03.txthmi/dev/nspanel_CJK_eu_code/buttonpage04.txthmi/dev/nspanel_CJK_us_code/boot.txthmi/dev/nspanel_CJK_us_code/buttonpage01.txthmi/dev/nspanel_CJK_us_code/buttonpage02.txthmi/dev/nspanel_CJK_us_code/buttonpage03.txthmi/dev/nspanel_CJK_us_code/buttonpage04.txthmi/dev/nspanel_easy_landscape/boot.txthmi/dev/nspanel_eu_code/boot.txthmi/dev/nspanel_eu_code/buttonpage01.txthmi/dev/nspanel_eu_code/buttonpage02.txthmi/dev/nspanel_eu_code/buttonpage03.txthmi/dev/nspanel_eu_code/buttonpage04.txthmi/dev/nspanel_us_code/boot.txthmi/dev/nspanel_us_code/buttonpage01.txthmi/dev/nspanel_us_code/buttonpage02.txthmi/dev/nspanel_us_code/buttonpage03.txthmi/dev/nspanel_us_code/buttonpage04.txthmi/dev/nspanel_us_land_code/boot.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/nspanel_CJK_eu.HMIhmi/nspanel_CJK_eu.tfthmi/nspanel_CJK_us.HMIhmi/nspanel_CJK_us.tfthmi/nspanel_easy_landscape.hmihmi/nspanel_easy_landscape.tfthmi/nspanel_eu.HMIhmi/nspanel_eu.tfthmi/nspanel_us.HMIhmi/nspanel_us.tfthmi/nspanel_us_land.HMIhmi/nspanel_us_land.tft
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
hmi/dev/nspanel_CJK_us_land_code/buttonpage04.txt (1)
29-48:⚠️ Potential issue | 🟠 MajorInitialize
page_index.txtbefore composing indicators.Because the code only appends to
page_index.txt, repeated opens can leave residual dots and cause visual glitches.💡 Suggested fix
// Build page index dots if more than one button page is active sys0=is_buttonpage01+is_buttonpage02+is_buttonpage03+is_buttonpage04 + page_index.txt="" if(sys0>1) { // Current page indicator (filled dot)🤖 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/buttonpage04.txt` around lines 29 - 48, The page index string page_index.txt is never reset before appending, causing residual dots across opens; in the block that runs when sys0 (sum of is_buttonpage01..is_buttonpage04) > 1, initialize page_index.txt to an empty string (e.g., set page_index.txt = "" or equivalent) before adding the filled/empty dot characters, then proceed to check is_buttonpage01/02/03 and append the dot bytes and call vis page_index,1 so the indicator is freshly composed each time.hmi/dev/nspanel_CJK_us_land_code/buttonpage03.txt (1)
29-48:⚠️ Potential issue | 🟠 MajorClear
page_index.txtbefore rebuilding dot markers.The current
+=construction can retain old markers between visits, causing duplicated/incorrect page-index rendering.💡 Suggested fix
// Build page index dots if more than one button page is active sys0=is_buttonpage01+is_buttonpage02+is_buttonpage03+is_buttonpage04 + page_index.txt="" if(sys0>1) { // Current page indicator (filled dot)🤖 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/buttonpage03.txt` around lines 29 - 48, The page_index.txt string is currently appended to (using +=) and may retain old markers; before building the dots reset page_index.txt to an empty string (e.g., set page_index.txt="") when entering the block that computes sys0 and renders dots; then proceed to append the filled/empty markers based on is_buttonpage01, is_buttonpage02, is_buttonpage03, is_buttonpage04 (and the intermediate fixed "center" marker currently added) and finally call vis page_index,1 so the indicator is rebuilt cleanly each time.hmi/dev/nspanel_CJK_us_land_code/buttonpage01.txt (1)
29-48:⚠️ Potential issue | 🟠 MajorReset
page_index.txtbefore appending dots.
page_index.txtis only appended to (+=) and never cleared inPreinitialize. On repeated visits, dots can accumulate and reintroduce the rendering artifacts this PR is trying to eliminate.💡 Suggested fix
// Build page index dots if more than one button page is active sys0=is_buttonpage01+is_buttonpage02+is_buttonpage03+is_buttonpage04 + page_index.txt="" if(sys0>1) { // Current page indicator (filled dot) page_index.txt+="\xe2\x97\x8f"🤖 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/buttonpage01.txt` around lines 29 - 48, Reset page_index.txt before appending dots: in the Preinitialize block that builds the page index (the code that uses sys0 and the is_buttonpage01..04 flags and updates page_index.txt), set page_index.txt to an empty string before any "+=" operations so each visit recreates the dots instead of accumulating them; keep the existing logic that conditionally appends "●" ("\xe2\x97\x8f") and "◯" ("\xe2\x97\x8b") based on is_buttonpage02/is_buttonpage03/is_buttonpage04, and ensure vis page_index,1 remains unchanged.hmi/dev/nspanel_CJK_us_land_code/buttonpage02.txt (1)
29-48:⚠️ Potential issue | 🟠 MajorPrevent page-index dot accumulation across navigations.
page_index.txtis concatenated each init but never reset. This can produce stale/duplicated indicators after multiple page opens.💡 Suggested fix
// Build page index dots if more than one button page is active sys0=is_buttonpage01+is_buttonpage02+is_buttonpage03+is_buttonpage04 + page_index.txt="" if(sys0>1) { // Current page indicator (filled dot)🤖 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/buttonpage02.txt` around lines 29 - 48, Reset the page index text before building indicators to avoid accumulation: clear page_index.txt (e.g., set to empty) at the start of the block that builds dots (the code manipulating page_index.txt and checking is_buttonpage01/is_buttonpage02/is_buttonpage03/is_buttonpage04), then append the appropriate filled/hollow characters as currently implemented and call vis page_index,1; this ensures page_index.txt is not concatenated across navigations.
🤖 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`:
- Line 16: The default high baud of 921600 (upload_tft_baud_rate) should not be
the only path: update the config to either revert upload_tft_baud_rate to 115200
or add staged retry logic inside upload_tft_attempt so it tries nextion_upload
at 921600, then 460800, then 115200 (or another fallback list) before calling
App.safe_reboot(); ensure the auto-upload default behavior (auto_upload) won’t
immediately trigger a reboot loop by performing retries and only calling
App.safe_reboot() after all baud rates in the fallback sequence fail.
---
Outside diff comments:
In `@hmi/dev/nspanel_CJK_us_land_code/buttonpage01.txt`:
- Around line 29-48: Reset page_index.txt before appending dots: in the
Preinitialize block that builds the page index (the code that uses sys0 and the
is_buttonpage01..04 flags and updates page_index.txt), set page_index.txt to an
empty string before any "+=" operations so each visit recreates the dots instead
of accumulating them; keep the existing logic that conditionally appends "●"
("\xe2\x97\x8f") and "◯" ("\xe2\x97\x8b") based on
is_buttonpage02/is_buttonpage03/is_buttonpage04, and ensure vis page_index,1
remains unchanged.
In `@hmi/dev/nspanel_CJK_us_land_code/buttonpage02.txt`:
- Around line 29-48: Reset the page index text before building indicators to
avoid accumulation: clear page_index.txt (e.g., set to empty) at the start of
the block that builds dots (the code manipulating page_index.txt and checking
is_buttonpage01/is_buttonpage02/is_buttonpage03/is_buttonpage04), then append
the appropriate filled/hollow characters as currently implemented and call vis
page_index,1; this ensures page_index.txt is not concatenated across
navigations.
In `@hmi/dev/nspanel_CJK_us_land_code/buttonpage03.txt`:
- Around line 29-48: The page_index.txt string is currently appended to (using
+=) and may retain old markers; before building the dots reset page_index.txt to
an empty string (e.g., set page_index.txt="") when entering the block that
computes sys0 and renders dots; then proceed to append the filled/empty markers
based on is_buttonpage01, is_buttonpage02, is_buttonpage03, is_buttonpage04 (and
the intermediate fixed "center" marker currently added) and finally call vis
page_index,1 so the indicator is rebuilt cleanly each time.
In `@hmi/dev/nspanel_CJK_us_land_code/buttonpage04.txt`:
- Around line 29-48: The page index string page_index.txt is never reset before
appending, causing residual dots across opens; in the block that runs when sys0
(sum of is_buttonpage01..is_buttonpage04) > 1, initialize page_index.txt to an
empty string (e.g., set page_index.txt = "" or equivalent) before adding the
filled/empty dot characters, then proceed to check is_buttonpage01/02/03 and
append the dot bytes and call vis page_index,1 so the indicator is freshly
composed each time.
🪄 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: 3f81063a-9677-43c3-93ad-6bb976a19b34
📒 Files selected for processing (9)
esphome/nspanel_esphome_addon_upload_tft.yamlhmi/dev/nspanel_CJK_us_land_code/boot.txthmi/dev/nspanel_CJK_us_land_code/buttonpage01.txthmi/dev/nspanel_CJK_us_land_code/buttonpage02.txthmi/dev/nspanel_CJK_us_land_code/buttonpage03.txthmi/dev/nspanel_CJK_us_land_code/buttonpage04.txthmi/nspanel_CJK_eu.HMIhmi/nspanel_CJK_us_land.HMIhmi/nspanel_CJK_us_land.tft
I will implement a fallback engine later
To allow blueprint sending to the queue faster without losing data
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
.github/workflows/versioning.yml (1)
263-267: sed silently no-ops ifblueprint_versionfield is missing.If
blueprint_version:doesn't exist at the expected indentation level, sed will silently succeed without making changes. Consider adding a verification step or usinggrep -qto confirm the field exists before the replacement.🔧 Optional: Add verification after sed
sed -i \ "s/^ blueprint_version: .*/ blueprint_version: ${NEW_VERSION}/" \ "$BLUEPRINT" + + # Verify the replacement succeeded + if ! grep -q "^ blueprint_version: ${NEW_VERSION}$" "$BLUEPRINT"; then + echo "ERROR: Failed to update blueprint_version in $BLUEPRINT" + exit 1 + fi🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/versioning.yml around lines 263 - 267, The sed replacement for "blueprint_version" can silently no-op if the field is absent; add a verification step that checks the BLUEPRINT for a line matching the expected indentation and key (e.g., grep -q "^ blueprint_version:" "$BLUEPRINT") before running sed and fail the job with a clear error if not found, or alternatively run the sed and then verify the NEW_VERSION exists in the file afterwards (e.g., grep -q "${NEW_VERSION}" "$BLUEPRINT") so the workflow explicitly detects and errors when the blueprint_version field is missing or unchanged.esphome/nspanel_esphome_page_buttons.yaml (1)
97-108: Buffer sizing is safe but relies on controlledbutton_idvalues.The 15-char buffers accommodate the documented maximum ("button07" + "icon" = 12 chars + NUL). The
snprintfusage ensures no overflow, but if a longerbutton_idwere ever passed, the component name would be silently truncated, causing commands to target non-existent components.Since
button_idcomes from the blueprint (not user input), this is acceptable. Consider adding a debug-level log or assertion if defensive robustness is desired.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@esphome/nspanel_esphome_page_buttons.yaml` around lines 97 - 108, The current snprintf calls (using btn derived from button_id) can silently truncate component names if button_id is unexpectedly long; before formatting comp_pic/comp_bri/comp_text/comp_icon validate that button_id.size() + max_suffix_length < sizeof(comp_pic) and handle the error (e.g., log a debug/error via ESP logging or assert) so you fail fast instead of producing truncated component names; specifically, check button_id.length() against the remaining space required for the longest suffix (e.g., "icon"/"text") and, if it would overflow, emit a diagnostic or abort, then proceed to snprintf for comp_pic/comp_bri/comp_text/comp_icon.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@esphome/nspanel_esphome_page_buttons.yaml`:
- Around line 71-74: The script with id action_button uses mode: queued and
max_runs: 24 which can overflow if many button events cascade across up to 32
button entities; increase max_runs to a safer value (for example 64 or 128) or
implement monitoring to detect dropped executions, by updating the script block
(script id: action_button) to a higher max_runs and/or add logging/telemetry
around queued executions to detect overflows so queued button updates are not
silently dropped.
---
Nitpick comments:
In @.github/workflows/versioning.yml:
- Around line 263-267: The sed replacement for "blueprint_version" can silently
no-op if the field is absent; add a verification step that checks the BLUEPRINT
for a line matching the expected indentation and key (e.g., grep -q "^
blueprint_version:" "$BLUEPRINT") before running sed and fail the job with a
clear error if not found, or alternatively run the sed and then verify the
NEW_VERSION exists in the file afterwards (e.g., grep -q "${NEW_VERSION}"
"$BLUEPRINT") so the workflow explicitly detects and errors when the
blueprint_version field is missing or unchanged.
In `@esphome/nspanel_esphome_page_buttons.yaml`:
- Around line 97-108: The current snprintf calls (using btn derived from
button_id) can silently truncate component names if button_id is unexpectedly
long; before formatting comp_pic/comp_bri/comp_text/comp_icon validate that
button_id.size() + max_suffix_length < sizeof(comp_pic) and handle the error
(e.g., log a debug/error via ESP logging or assert) so you fail fast instead of
producing truncated component names; specifically, check button_id.length()
against the remaining space required for the longest suffix (e.g.,
"icon"/"text") and, if it would overflow, emit a diagnostic or abort, then
proceed to snprintf for comp_pic/comp_bri/comp_text/comp_icon.
🪄 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: 488b6c32-f2ec-464a-8b65-1461082a71b9
📒 Files selected for processing (3)
.github/workflows/versioning.ymlesphome/nspanel_esphome_page_buttons.yamlnspanel_easy_blueprint.yaml
To avoid using the code during release workflow running
Closed
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: 84369657f4
ℹ️ 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: 4
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_eu_code/buttonpage02.txt (1)
29-48:⚠️ Potential issue | 🟠 MajorReset
page_index.txtbefore appending dotsAt Line 30 onward, dot markers are appended with
+=but never cleared first. On repeated visits, this can accumulate stale dots and reintroduce intermittent render artifacts.Suggested fix
// Build page index dots if more than one button page is active + page_index.txt="" sys0=is_buttonpage01+is_buttonpage02+is_buttonpage03+is_buttonpage04 if(sys0>1) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@hmi/dev/nspanel_eu_code/buttonpage02.txt` around lines 29 - 48, The page_index.txt string is never cleared before appending dot markers, causing accumulation on repeated visits; in the block that builds the page index (variables sys0, is_buttonpage01..04 and function/command vis page_index,1), reset page_index.txt to an empty string before any "+=" operations and then append the filled/empty dot characters based on is_buttonpage01..is_buttonpage04 so the indicator is rebuilt fresh each time.
🤖 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_cover.md`:
- Line 57: The documentation currently references a non-existent Git ref "ref:
latest" in docs/addon_cover.md; update the docs to point to an existing ref
(e.g., replace "ref: latest" with "ref: main" or another valid branch/tag) or
create the "latest" ref in the repo (as a branch or tag) to match the doc;
ensure the string "ref: latest" is the exact token you change so
examples/install snippets and any CI/embed links that reference that token are
consistent.
In `@esphome/nspanel_esphome_page_buttons.yaml`:
- Around line 48-53: The warning log in the page visibility guard (the block
using get_page_id(page.c_str()), current_page_id, page_names and TAG_API) should
be demoted to a debug log because hidden-page updates are expected when
action_button is queued; change the ESP_LOGW call to ESP_LOGD (or equivalent
debug-level logger) and keep the message content but adjust severity so normal
hidden-page skips do not produce warnings.
In `@hmi/dev/nspanel_us_code/buttonpage01.txt`:
- Around line 49-69: Add an else branch to the existing if(theme==1) block that
restores dark-mode defaults for the same controls you set for light mode: reset
b[0].pic and its picc/pco counterparts on page_label, page_index, button_back
(including button_back.picc2) and looped button sub-components b[sys0].picc
(sys0 11..42); in that else set pic/picc back to the dark/default values (e.g.,
pic=0 and picc=0) and restore pco to the dark/default color constants (the dark
equivalents of color_pco_main and color_pco_navigation or the original
defaults), and apply the same else-block change to buttonpage01/02/03/04 in both
nspanel_us_code and nspanel_us_land_code so behavior is symmetric when switching
themes at runtime.
In `@hmi/dev/nspanel_us_code/buttonpage04.txt`:
- Around line 60-62: Page 04 is missing the Preinitialize assignment for the
back-button foreground color: add button_back.pco=color_pco_navigation to the
Preinitialize Event in buttonpage04.txt so it matches buttonpage01/02/03 and
ensures the back-arrow foreground contrasts with pic_background_main; locate the
Preinitialize Event block and add the button_back.pco assignment alongside
button_back.picc and button_back.picc2.
---
Outside diff comments:
In `@hmi/dev/nspanel_eu_code/buttonpage02.txt`:
- Around line 29-48: The page_index.txt string is never cleared before appending
dot markers, causing accumulation on repeated visits; in the block that builds
the page index (variables sys0, is_buttonpage01..04 and function/command vis
page_index,1), reset page_index.txt to an empty string before any "+="
operations and then append the filled/empty dot characters based on
is_buttonpage01..is_buttonpage04 so the indicator is rebuilt fresh each time.
🪄 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: 783a83bf-b0ec-433d-866f-ec31c3000ad3
📒 Files selected for processing (22)
.github/workflows/versioning.ymldocs/addon_cover.mdesphome/nspanel_esphome_page_buttons.yamlhmi/dev/nspanel_eu_code/buttonpage01.txthmi/dev/nspanel_eu_code/buttonpage02.txthmi/dev/nspanel_eu_code/buttonpage03.txthmi/dev/nspanel_eu_code/buttonpage04.txthmi/dev/nspanel_us_code/buttonpage01.txthmi/dev/nspanel_us_code/buttonpage02.txthmi/dev/nspanel_us_code/buttonpage03.txthmi/dev/nspanel_us_code/buttonpage04.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/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 due to trivial changes (2)
- hmi/dev/nspanel_us_code/buttonpage02.txt
- nspanel_easy_blueprint.yaml
🚧 Files skipped from review as they are similar to previous changes (6)
- hmi/dev/nspanel_eu_code/buttonpage01.txt
- hmi/dev/nspanel_us_land_code/buttonpage01.txt
- hmi/dev/nspanel_us_land_code/buttonpage02.txt
- hmi/dev/nspanel_us_land_code/buttonpage04.txt
- hmi/dev/nspanel_eu_code/buttonpage04.txt
- hmi/dev/nspanel_eu_code/buttonpage03.txt
edwardtfn
changed the title
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.
Fixes render issues when navigating to custom button pages, which could cause visual glitches and log warnings on page load. The root cause was the page initialization code iterating over all components indiscriminately — including components that don't support background images, and applying light mode theming unconditionally even on dark mode.
Closes #90
Summary by CodeRabbit
New Features
Improvements