Decoupled OTA and Wi-Fi Passwords + New Boot Page#22
Conversation
… into update-pics
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThis PR localizes doc image assets, introduces an Changes
Sequence Diagram(s)(omitted) Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![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: 009d53c4b1
ℹ️ 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".
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
docs/error_initializing.md (1)
82-86:⚠️ Potential issue | 🟠 MajorAdd the missing image asset
pics/ha_device_sensors_duplicated.png.
The documentation at line 86 references an image that doesn't exist in the repository, which will break the doc rendering. This file must be added topics/before merging.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/error_initializing.md` around lines 82 - 86, The docs reference a missing image named ha_device_sensors_duplicated.png which breaks rendering; add the actual image file (exactly named ha_device_sensors_duplicated.png) into the pics/ assets used by docs/error_initializing.md, commit it, and verify the image renders in the built docs (and optionally optimize or resize the image to match other pics and ensure casing matches the markdown reference).hmi/dev/nspanel_CJK_us_code/boot.txt (1)
143-581:⚠️ Potential issue | 🔴 CriticalFix critical component ID mismatch:
bt_rebootHMI ID is 21 but ESPHome expects 24.The reboot button's touch event handler in
esphome/nspanel_esphome_page_boot.yaml(line 26) checks forcomponent_id == ${PAGE_BOOT_COMPONENT_ID_BT_REBOOT}which is set to 24, but all HMI boot.txt files (across all variants) definebt_rebootwithID: 21. This mismatch prevents the touch event from being recognized, breaking the reboot functionality.The timers (
log_scrollandtm_esphome) are unaffected as they are referenced by component name in ESPHome scripts, not by numeric ID.Update the HMI component ID from 21 to 24 to match the ESPHome configuration.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@hmi/dev/nspanel_CJK_us_code/boot.txt` around lines 143 - 581, The bt_reboot component in the HMI definition has ID: 21 but ESPHome expects PAGE_BOOT_COMPONENT_ID_BT_REBOOT == 24, so the touch event never matches; change the bt_reboot Attributes ID from 21 to 24 (update the "Dual-state Button bt_reboot" block's ID attribute) so the HMI numeric component ID matches the ESPHome constant used in esphome/nspanel_esphome_page_boot.yaml (PAGE_BOOT_COMPONENT_ID_BT_REBOOT).hmi/dev/nspanel_eu_code/boot.txt (1)
143-581:⚠️ Potential issue | 🔴 CriticalID mismatch breaks boot page reboot button handler.
The
bt_rebootcomponent has ID 21 in all boot.txt files, but the handler inesphome/nspanel_esphome_page_boot.yamlexpects ID 24 (PAGE_BOOT_COMPONENT_ID_BT_REBOOT: 24). When users press the reboot button on the boot page, the display sends component_id 21, which does not match the handler's check, so the reboot function will never execute. UpdatePAGE_BOOT_COMPONENT_ID_BT_REBOOTto 21.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@hmi/dev/nspanel_eu_code/boot.txt` around lines 143 - 581, The reboot button on the boot page is defined as component bt_reboot with ID 21 but the handler constant PAGE_BOOT_COMPONENT_ID_BT_REBOOT is set to 24, so component_id checks never match; update PAGE_BOOT_COMPONENT_ID_BT_REBOOT to 21 in esphome/nspanel_esphome_page_boot.yaml (or wherever the constant is defined) so the handler compares against the actual bt_reboot ID and the reboot action will execute when component_id 21 is received.hmi/dev/nspanel_CJK_eu_code/boot.txt (1)
499-544:⚠️ Potential issue | 🟠 MajorFix substr length for
\rcharacter detection in log_scroll timerAt line 518 in all boot.txt files,
substr log_body.txt,log_temp.txt,log_pos.val,2extracts a 2-character substring, but line 519 compares it against the single-character string"\r". String comparison in Nextion is exact, so this will fail to match in all cases except when\rhappens to be the final character in the log buffer with a null terminator immediately following—effectively missing the vast majority of carriage return occurrences.This causes the line-counting logic to fail, allowing the log buffer to grow unbounded instead of trimming at
log_max_lines. The comment indicates the intent is "Extract each character to find \r", confirming the substr length should be1.This issue exists identically across all six boot.txt file variants (nspanel_CJK_eu_code, nspanel_CJK_us_code, nspanel_CJK_us_land_code, nspanel_eu_code, nspanel_us_code, nspanel_us_land_code).
Proposed fix
- substr log_body.txt,log_temp.txt,log_pos.val,2 + substr log_body.txt,log_temp.txt,log_pos.val,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/boot.txt` around lines 499 - 544, The Timer "log_scroll" handler is extracting two characters when checking for a carriage return using substr log_body.txt,log_temp.txt,log_pos.val,2 and then comparing log_temp.txt to "\r", which never matches; change that substr call to extract a single character (length 1) so the comparison to "\r" succeeds. Update the substr call in the log_scroll event (the loop that uses log_pos.val, log_temp.txt and compares to "\r") across all boot.txt variants (nspanel_CJK_eu_code, nspanel_CJK_us_code, nspanel_CJK_us_land_code, nspanel_eu_code, nspanel_us_code, nspanel_us_land_code) so line counting/trimming works as intended.
🧹 Nitpick comments (1)
hmi/dev/nspanel_CJK_eu_code/boot.txt (1)
31-56: Consider batching the 24linedraws withref_stop/ref_starto eliminate incremental-draw flickerOn slower panels, 24 sequential
linecommands can produce a visible one-by-one drawing effect. Wrapping the entire block inref_stop/ref_starcollapses all draws into a single screen refresh.✨ Proposed refactor
Postinitialize Event + ref_stop line 216,55,450,55,WHITE line 450,55,450,301,WHITE ... line 16,299,200,299,WHITE vis sys_title,1 + ref_star🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@hmi/dev/nspanel_CJK_eu_code/boot.txt` around lines 31 - 56, Multiple sequential `line` draws cause visible incremental rendering on slow panels; wrap the whole block of `line` commands with `ref_stop` before the first `line` and `ref_star` after the last `line` so the panel performs a single refresh. Specifically, insert `ref_stop` immediately before the first `line 216,55,450,55,WHITE` and `ref_star` immediately after the final `line 16,299,200,299,WHITE` (the block that includes the `vis log_title,1` / `vis ver_title,1` adjacent lines) to collapse the 24 draws into one atomic update. Ensure the start/end tokens are balanced and keep the `vis` calls in place.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/howto.md`:
- Around line 93-100: The docs reference four missing image assets
(pics/ha_blueprint_dashboard_reimport.png,
pics/ha_blueprint_dashboard_reimport_confirm.png,
pics/esphome_dashboard_install_wirelessly.png,
pics/esphome_dashboard_install_completed.png) in docs/howto.md; either add those
exact files into the pics/ directory with matching filenames and correct
screenshots, or remove the corresponding markdown image references from
docs/howto.md (the two occurrences around the "NSPanel Configuration" blueprint
steps and the two occurrences around the ESPHome dashboard install steps) so the
doc no longer points to non-existent images.
In `@hmi/dev/manuals_screenshots.md`:
- Around line 19-25: Two log entries appended to log_body (the statements using
log_body.txt+=) are missing a space after the colon; update the two strings
"Boot:Waiting for Wi-Fi" and "Boot:Waiting for TFT version" to "Boot: Waiting
for Wi-Fi" and "Boot: Waiting for TFT version" so they match the other "Boot:
..." labels and maintain consistent formatting.
In `@hmi/dev/nspanel_CJK_eu_code/boot.txt`:
- Around line 30-58: The three title components (log_title, ver_title,
sys_title) are missing a Visible: 0 attribute so they render briefly before
Postinitialize runs; edit each component's attribute block to add "Visible: 0"
so they start hidden and the existing Postinitialize "vis <name>,1" calls will
reveal them after the border lines are drawn.
---
Outside diff comments:
In `@docs/error_initializing.md`:
- Around line 82-86: The docs reference a missing image named
ha_device_sensors_duplicated.png which breaks rendering; add the actual image
file (exactly named ha_device_sensors_duplicated.png) into the pics/ assets used
by docs/error_initializing.md, commit it, and verify the image renders in the
built docs (and optionally optimize or resize the image to match other pics and
ensure casing matches the markdown reference).
In `@hmi/dev/nspanel_CJK_eu_code/boot.txt`:
- Around line 499-544: The Timer "log_scroll" handler is extracting two
characters when checking for a carriage return using substr
log_body.txt,log_temp.txt,log_pos.val,2 and then comparing log_temp.txt to "\r",
which never matches; change that substr call to extract a single character
(length 1) so the comparison to "\r" succeeds. Update the substr call in the
log_scroll event (the loop that uses log_pos.val, log_temp.txt and compares to
"\r") across all boot.txt variants (nspanel_CJK_eu_code, nspanel_CJK_us_code,
nspanel_CJK_us_land_code, nspanel_eu_code, nspanel_us_code,
nspanel_us_land_code) so line counting/trimming works as intended.
In `@hmi/dev/nspanel_CJK_us_code/boot.txt`:
- Around line 143-581: The bt_reboot component in the HMI definition has ID: 21
but ESPHome expects PAGE_BOOT_COMPONENT_ID_BT_REBOOT == 24, so the touch event
never matches; change the bt_reboot Attributes ID from 21 to 24 (update the
"Dual-state Button bt_reboot" block's ID attribute) so the HMI numeric component
ID matches the ESPHome constant used in esphome/nspanel_esphome_page_boot.yaml
(PAGE_BOOT_COMPONENT_ID_BT_REBOOT).
In `@hmi/dev/nspanel_eu_code/boot.txt`:
- Around line 143-581: The reboot button on the boot page is defined as
component bt_reboot with ID 21 but the handler constant
PAGE_BOOT_COMPONENT_ID_BT_REBOOT is set to 24, so component_id checks never
match; update PAGE_BOOT_COMPONENT_ID_BT_REBOOT to 21 in
esphome/nspanel_esphome_page_boot.yaml (or wherever the constant is defined) so
the handler compares against the actual bt_reboot ID and the reboot action will
execute when component_id 21 is received.
---
Nitpick comments:
In `@hmi/dev/nspanel_CJK_eu_code/boot.txt`:
- Around line 31-56: Multiple sequential `line` draws cause visible incremental
rendering on slow panels; wrap the whole block of `line` commands with
`ref_stop` before the first `line` and `ref_star` after the last `line` so the
panel performs a single refresh. Specifically, insert `ref_stop` immediately
before the first `line 216,55,450,55,WHITE` and `ref_star` immediately after the
final `line 16,299,200,299,WHITE` (the block that includes the `vis log_title,1`
/ `vis ver_title,1` adjacent lines) to collapse the 24 draws into one atomic
update. Ensure the start/end tokens are balanced and keep the `vis` calls in
place.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (41)
docs/pics/287632422-afb433e8-f29c-4b3f-8d6b-16a12b181422.pngis excluded by!**/*.pngdocs/pics/GitHub_repo_select_tag_tft_files.pngis excluded by!**/*.pngdocs/pics/alarm-shield-airplane-outline.svgis excluded by!**/*.svgdocs/pics/alarm-shield-alert-outline-white.svgis excluded by!**/*.svgdocs/pics/alarm-shield-alert-outline.svgis excluded by!**/*.svgdocs/pics/alarm-shield-half-full.svgis excluded by!**/*.svgdocs/pics/alarm-shield-home-outline.svgis excluded by!**/*.svgdocs/pics/alarm-shield-lock-outline.svgis excluded by!**/*.svgdocs/pics/alarm-shield-moon-outline.svgis excluded by!**/*.svgdocs/pics/alarm-shield-off-outline-amber.svgis excluded by!**/*.svgdocs/pics/alarm-shield-off-outline.svgis excluded by!**/*.svgdocs/pics/alarm-shield-outline.svgis excluded by!**/*.svgdocs/pics/alarm_home.pngis excluded by!**/*.pngdocs/pics/boot_eu.pngis excluded by!**/*.pngdocs/pics/boot_us.pngis excluded by!**/*.pngdocs/pics/esphome_dashboard_install_completed.pngis excluded by!**/*.pngdocs/pics/esphome_dashboard_install_download_project.jpgis excluded by!**/*.jpgdocs/pics/esphome_dashboard_install_wirelessly.pngis excluded by!**/*.pngdocs/pics/esphome_dashboard_yaml_ref_main.pngis excluded by!**/*.pngdocs/pics/esphome_web_connect.jpgis excluded by!**/*.jpgdocs/pics/esphome_web_install.jpgis excluded by!**/*.jpgdocs/pics/eu_boot.pngis excluded by!**/*.pngdocs/pics/eu_boot_initializing.pngis excluded by!**/*.pngdocs/pics/eu_boot_with_ip_address.pngis excluded by!**/*.pngdocs/pics/ha_blueprint_01.pngis excluded by!**/*.pngdocs/pics/ha_blueprint_dashboard_reimport.pngis excluded by!**/*.pngdocs/pics/ha_blueprint_dashboard_reimport_confirm.pngis excluded by!**/*.pngdocs/pics/ha_device_sensors_duplicated.pngis excluded by!**/*.pngdocs/pics/ha_esphome_dashboard_new_device_06.pngis excluded by!**/*.pngdocs/pics/ha_esphome_dashboard_new_device_07.pngis excluded by!**/*.pngdocs/pics/ha_file_editor_blueprint_source_url_v414.pngis excluded by!**/*.pngdocs/pics/home_page_chips.jpgis excluded by!**/*.jpgdocs/pics/home_page_custom_buttons.jpgis excluded by!**/*.jpgdocs/pics/home_page_entity_state_01_04.jpgis excluded by!**/*.jpgdocs/pics/image-20230317162851693.pngis excluded by!**/*.pngdocs/pics/moving_the_panels_board_away_from_the_metal_backing.jpgis excluded by!**/*.jpgdocs/pics/us_boot.pngis excluded by!**/*.pnghmi/dev/ui/eu/pics/32.pngis excluded by!**/*.pnghmi/dev/ui/eu/pics/32_big.pngis excluded by!**/*.pnghmi/dev/ui/us/pics/32.pngis excluded by!**/*.pnghmi/dev/ui/us/pics/32_big.pngis excluded by!**/*.png
📒 Files selected for processing (29)
docs/addon_climate.mddocs/alarm.mddocs/blueprint.mddocs/customization.mddocs/error_initializing.mddocs/howto.mddocs/install.mddocs/migration_from_blackymas.mdesphome/nspanel_esphome_base.yamlesphome/nspanel_esphome_version.yamlhmi/dev/manuals_screenshots.mdhmi/dev/nspanel_CJK_eu_code/boot.txthmi/dev/nspanel_CJK_us_code/boot.txthmi/dev/nspanel_CJK_us_land_code/boot.txthmi/dev/nspanel_eu_code/boot.txthmi/dev/nspanel_us_code/boot.txthmi/dev/nspanel_us_land_code/boot.txthmi/nspanel_CJK_eu.HMIhmi/nspanel_CJK_eu.tfthmi/nspanel_CJK_us.HMIhmi/nspanel_CJK_us.tfthmi/nspanel_CJK_us_land.HMIhmi/nspanel_CJK_us_land.tfthmi/nspanel_eu.HMIhmi/nspanel_eu.tfthmi/nspanel_us.HMIhmi/nspanel_us.tfthmi/nspanel_us_land.HMIhmi/nspanel_us_land.tft
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)
.github/workflows/validate_markdown.yml (1)
58-65:⚠️ Potential issue | 🟡 MinorRemove
check-modified-files-only: yesor change trigger topull_request.The migration from
gaurav-nelson/github-action-markdown-link-check(deprecated) to the maintainedtcort/github-action-markdown-link-check@v1.1.2fork is correct, and the v1.1.2 tag is valid. However,check-modified-files-onlyis designed forpull_requestevents only — it compares git refs between the PR head and base branch. Themarkdown-linksjob is triggered onpush(notpull_request), where this logic produces unpredictable results since there is no PR context. Either removecheck-modified-files-only: yesor change the trigger topull_request.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/validate_markdown.yml around lines 58 - 65, The markdown-links job using tcort/github-action-markdown-link-check@v1.1.2 currently sets check-modified-files-only: yes but is triggered on push, which yields unpredictable results; either remove the check-modified-files-only: yes input from the markdown-links job or change the workflow trigger from push to pull_request so the action can compare PR refs correctly—update the markdown-links job configuration (the step using tcort/github-action-markdown-link-check@v1.1.2 and the check-modified-files-only input) or adjust the workflow trigger to pull_request accordingly.
🤖 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 @.github/workflows/validate_markdown.yml:
- Around line 58-65: The markdown-links job using
tcort/github-action-markdown-link-check@v1.1.2 currently sets
check-modified-files-only: yes but is triggered on push, which yields
unpredictable results; either remove the check-modified-files-only: yes input
from the markdown-links job or change the workflow trigger from push to
pull_request so the action can compare PR refs correctly—update the
markdown-links job configuration (the step using
tcort/github-action-markdown-link-check@v1.1.2 and the check-modified-files-only
input) or adjust the workflow trigger to pull_request accordingly.
1 participant
Summary by CodeRabbit
Documentation
Configuration Updates
UI Improvements
Chores