Setup NSPanel Easy repository based on Blackymas NSPanel_HA_Blueprint#1
Setup NSPanel Easy repository based on Blackymas NSPanel_HA_Blueprint#1
Conversation
|
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. 📝 WalkthroughWalkthroughAdds repository metadata and CI, many GitHub issue templates and workflows, extensive documentation, numerous ESPHome blueprints/packages and prebuilt manifests, a new ESPHome Python integration, and a large C++ runtime library under Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant User as User (Touch)
participant Nextion as Nextion Display
participant ESP as ESPHome (nspanel)
participant HA as Home Assistant
User->>Nextion: Touch / UI interaction (CSV payload)
Nextion->>ESP: event_from_display
ESP->>ESP: parse input -> map action
alt requires Home Assistant
ESP->>HA: call service / fire_ha_event
HA-->>ESP: service result/state
ESP->>Nextion: set_component_text/color/visibility
else local UI only
ESP->>Nextion: render UI update
end
Nextion-->>User: visual feedback
sequenceDiagram
autonumber
participant Boot as Device Boot
participant Nextion as Nextion Display
participant ESP as ESPHome (nspanel)
participant HA as Home Assistant
Boot->>Nextion: Initialize / request params
Nextion-->>ESP: report display params & TFT/version
ESP->>ESP: run boot scripts, wait for API & blueprint readiness
ESP->>HA: request blueprint/settings
HA-->>ESP: respond with settings
ESP->>ESP: mark blueprint_ready, finish boot, navigate to wakeup page
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![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: 6d5bbcbece
ℹ️ 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: 5
Note
Due to the large number of review comments, Critical severity comments were prioritized as inline comments.
🤖 Fix all issues with AI agents
In `@components/nspanel_easy/base.cpp`:
- Around line 15-21: cached_device_name is default-empty and never set, so
fire_ha_event always emits an empty device_name; fix by assigning the actual
device name to cached_device_name during initialization or when the device
becomes available (e.g., in the NSPanel constructor, an init/setup method, or
right after the code that discovers/creates the device), using the existing
device/name accessor (for example call the device's get_name() or read from the
config) and update cached_device_name before any calls to fire_ha_event; ensure
the assignment happens once (or updates when the device name changes) and is
visible to fire_ha_event.
In `@components/nspanel_easy/page_qrcode.h`:
- Line 5: The preprocessor guard in page_qrcode.h is wrong: replace occurrences
of NSPANEL_EASY_PAGE_NOTIFICATION with NSPANEL_EASY_PAGE_QRCODE so the QR code
page is compiled under the correct macro; specifically update the opening
conditional (currently "#ifdef NSPANEL_EASY_PAGE_NOTIFICATION") and the
corresponding closing conditional/endif comment at the end of the file to
reference NSPANEL_EASY_PAGE_QRCODE (look for symbols/page identifiers related to
page_qrcode.h to locate both spots).
In `@esphome/nspanel_esphome_addon_upload_tft.yaml`:
- Around line 274-293: The reboot is unconditional in the upload_tft_attempt
script, preventing the retry loop from running; change the logic so
App.safe_reboot() is only called after a success or after the final allowed
attempt, e.g., use the existing tft_upload_attempt counter and only call
App.safe_reboot() when tft_upload_result is true or tft_upload_attempt has
reached the max attempts (two in this flow), leaving the call out for
intermediate failed attempts so the repeat/nextion_upload retry can run; update
the lambda that logs the result in upload_tft_attempt to perform this
conditional check before invoking App.safe_reboot().
In `@esphome/nspanel_esphome_hw_wifi.yaml`:
- Around line 179-184: The loop that builds cached_device_name uses std::isalnum
and std::tolower with a char from App.get_name(), which can be signed and lead
to undefined behavior; update the checks and conversion to call
isalnum(static_cast<unsigned char>(c)) and tolower(static_cast<unsigned
char>(c)) (and then append the converted char as needed) inside the loop that
iterates over App.get_name(), keeping the existing logic for last_was_underscore
and replacement with '_'.
In `@esphome/nspanel_esphome_version.yaml`:
- Around line 75-88: The blueprint version check is using the wrong minimum
variable and the modulo condition is inverted: in the block using
version_blueprint->state replace references to ${min_tft_version} in the
error/log with ${min_blueprint_version} and ensure system_flags.version_check_ok
is set as before; also change the iteration conditional from if (iteration % 9)
to an explicit comparison if (iteration % 9 == 0) so it runs every 9th
iteration; finally, audit comparisons against version_tft/version_blueprint to
ensure version string formats are normalized or compared consistently (semantic
vs numeric) before doing numeric-style comparisons.
🟠 Major comments (20)
esphome/nspanel_esphome_datetime.yaml-127-133 (1)
127-133:⚠️ Potential issue | 🟠 MajorModifying the global directly corrupts the stored timezone value.
Line 131 modifies
id(mui_timezone)directly, not a copy. When a POSIX timezone string contains angle brackets (e.g.,<-03>3for Argentina Standard Time), this replaces them with "UTC", permanently altering the stored value to something likeUTC3.Consequences:
- The original user-provided timezone is lost from the persistent global.
- After a reboot,
restore_valueloads the corrupted "UTC3" instead of the original.- Subsequent attempts to set the same timezone will pass the guard check (line 114) because the stored value no longer matches.
🐛 Proposed fix to preserve the original global value
- id: set_timezone_from_globals mode: single then: - lambda: |- if (id(mui_timezone).empty()) { ESP_LOGD("${TAG_DATETIME}", "Current timezone: %s", time_provider->get_timezone().c_str()); return; } ESP_LOGI("${TAG_DATETIME}", "Setting timezone: %s", id(mui_timezone).c_str()); - const size_t start = id(mui_timezone).find('<'); + std::string tz_to_apply = id(mui_timezone); + const size_t start = tz_to_apply.find('<'); if (start != std::string::npos) { - const size_t end = id(mui_timezone).find('>'); + const size_t end = tz_to_apply.find('>'); if (end != std::string::npos && end > start) { - id(mui_timezone).replace(start, (end - start) + 1, "UTC"); + tz_to_apply.replace(start, (end - start) + 1, "UTC"); } // if valid brackets found } // if formatting needed - time_provider->set_timezone(id(mui_timezone).c_str()); + time_provider->set_timezone(tz_to_apply.c_str()); time_provider->call_setup(); time_provider->dump_config(); refresh_datetime->execute();esphome/nspanel_esphome_addon_display_light.yaml-66-80 (1)
66-80:⚠️ Potential issue | 🟠 MajorUse the
brightnessscript parameter instead of undeclaredbrightness_current.
brightness_currentis not explicitly declared as a global or parameter, creating a scope issue. Thebrightnessparameter is guaranteed to be available in extended scripts and contains the exact value being requested. Using it directly avoids potential desync and is safer.Suggested fix
- call.set_brightness(brightness_current / 100.0f); + call.set_brightness(brightness / 100.0f);.github/workflows/validate_python.yml-6-11 (1)
6-11:⚠️ Potential issue | 🟠 MajorWorkflow won’t run for Python changes in subdirectories.
The paths filter only matches root-level*.py, so updates undercomponents/nspanel_ha_blueprint/**won’t trigger.✅ Suggested fix
on: push: paths: - - '*.py' + - '**/*.py' pull_request: paths: - - '*.py' + - '**/*.py'.github/workflows/release_tag.yml-9-31 (1)
9-31:⚠️ Potential issue | 🟠 MajorAdd explicit
contents: writepermissions for tag pushes.
Without this, the job can fail in repos where the default token is read‑only.✅ Suggested permissions
jobs: update-tags: + permissions: + contents: write runs-on: ubuntu-latest.github/workflows/validate_python.yml-20-27 (1)
20-27:⚠️ Potential issue | 🟠 MajorPin GitHub Actions to versioned tags or SHAs (avoid
@main).
Lines 20 and 22 use floating@mainrefs, which are mutable and weaken supply-chain security guarantees. Pinactions/checkoutandactions/setup-pythonto specific versions (currently v6)..github/workflows/release_tag.yml-13-22 (1)
13-22:⚠️ Potential issue | 🟠 MajorPin GitHub Actions to versioned tags or SHAs (avoid
@main).The
actions/checkoutaction is using@main, which can cause workflow failures when the action is updated. Pin to a specific version tag (currentlyv6) or commit SHA for supply-chain safety.🔒 Example pinning
- - name: Checkout code - uses: actions/checkout@main + - name: Checkout code + uses: actions/checkout@v6.github/workflows/shellcheck.yml-16-18 (1)
16-18:⚠️ Potential issue | 🟠 MajorPin the ShellCheck action to the stable release tag.
Using
@masterrisks unintended updates and supply-chain exposure. Pin to the latest stable release2.0.0(commit SHA00cae500b08a931fb5698e11e79bfbd38e612a38) or verify any newer releases.Recommended fix
- - name: Run ShellCheck - uses: ludeeus/action-shellcheck@master + - name: Run ShellCheck + uses: ludeeus/action-shellcheck@2.0.0.github/workflows/validate_clang_format.yml-22-23 (1)
22-23:⚠️ Potential issue | 🟠 MajorPin GitHub Actions to stable commit SHAs (avoid
@mainand outdated versions).Using a moving branch (
@main) increases supply-chain risk. Pin both actions to full commit SHAs with a comment showing the release version. Additionally,RafikFarhad/clang-format-github-action@v3is severely outdated; upgrade to the latest v6.Example
- - uses: actions/checkout@main + - uses: actions/checkout@692973e3d937129bcbf40652eb9f2f61becf3332 # v6.0.2 - - uses: RafikFarhad/clang-format-github-action@v3 + - uses: RafikFarhad/clang-format-github-action@bfeb0cf01a0f3f0a1fef6c3caf5a3f7c4e0d1b2c # v6.0.1This approach ensures immutability and reduces the risk of tag tampering if an upstream repository is compromised.
.github/workflows/validate_yamllint.yml-21-24 (1)
21-24:⚠️ Potential issue | 🟠 MajorPin
actions/checkoutto a specific version or SHA for security and reproducibility.Using
@mainmeans the workflow will use whatever is on the main branch at runtime, which could change unexpectedly or be compromised in a supply chain attack.🔒 Proposed fix
- name: Check out code - uses: actions/checkout@main + uses: actions/checkout@v6 with: fetch-depth: '0'esphome/nspanel_esphome_hw_buzzer.yaml-49-56 (1)
49-56:⚠️ Potential issue | 🟠 MajorInverted conditional logic for boot sound check.
The Jinja template
${'false' if boot_sound else 'true'}doesn't work as intended. Whenboot_sound: false(the default), the string"false"is truthy in Jinja, so the expression evaluates to'false', making the conditionif (false) return;which does NOT return.This means boot sound will play even when disabled.
🐛 Proposed fix
- if (${'false' if boot_sound else 'true'}) return; // Boot sound is disabled + if (${'true' if boot_sound == 'false' else 'false'}) return; // Boot sound is disabledOr more clearly, use direct string comparison:
- if (${'false' if boot_sound else 'true'}) return; // Boot sound is disabled + `#if` !${boot_sound} + return; // Boot sound is disabled + `#endif`esphome/nspanel_esphome_addon_upload_tft.yaml-186-223 (1)
186-223:⚠️ Potential issue | 🟠 MajorDon’t lowercase user URLs; handle
"default"explicitly.Lowercasing can break case-sensitive paths, and the documented
"default"sentinel is never recognized. This can lead to invalid URLs being used.🔧 Suggested fix
- std::string resolved_url = url; - - // Convert to lowercase and trim - std::transform(resolved_url.begin(), resolved_url.end(), resolved_url.begin(), - [](unsigned char c){ return std::tolower(c); }); - auto endPos = resolved_url.find_last_not_of(" \t"); - if (std::string::npos != endPos) { - resolved_url = resolved_url.substr(0, endPos + 1); - } - - // Calculate URL if empty - if (resolved_url.empty()) { + std::string resolved_url = url; + + // Trim whitespace (both ends) for comparisons, but keep original casing + auto startPos = resolved_url.find_first_not_of(" \t"); + auto endPos = resolved_url.find_last_not_of(" \t"); + std::string trimmed = (startPos == std::string::npos) + ? "" + : resolved_url.substr(startPos, endPos - startPos + 1); + + std::string normalized = trimmed; + std::transform(normalized.begin(), normalized.end(), normalized.begin(), + [](unsigned char c){ return std::tolower(c); }); + + // Calculate URL if empty or "default" + if (normalized.empty() || normalized == "default") { auto idx_opt = tft_file_model->active_index(); if (!idx_opt.has_value()) { resolved_url = "${nextion_update_url}"; // Fallback if no valid index } else { const int idx = idx_opt.value(); @@ } } + } else { + resolved_url = trimmed; }components/nspanel_easy/page_utilities.h-7-12 (1)
7-12:⚠️ Potential issue | 🟠 MajorAdd required headers for
std::minandfree.
std::min(used on line 56) needs<algorithm>andfree(used on line 39) needs<cstdlib>. The header should be self-contained to avoid build errors.🔧 Suggested fix
-#include <cstdint> -#include <cstring> // For std::strcpy -#include <string> +#include <algorithm> // For std::min +#include <cstdint> +#include <cstdlib> // For std::free +#include <cstring> // For std::strcpy +#include <string>esphome/nspanel_esphome_hw_display.yaml-184-233 (1)
184-233:⚠️ Potential issue | 🟠 MajorFix printf format specifier for page name logging.
At line 199,
%Sexpects a wide string (wchar_t*), butparams[0].c_str()returns a regular C string (const char*). This mismatch can cause undefined behavior, reading from invalid memory locations. Use%sinstead.🔧 Suggested fix
- ESP_LOGV("${TAG_HW_DISPLAY}", " page: %S", params[0].c_str()); + ESP_LOGV("${TAG_HW_DISPLAY}", " page: %s", params[0].c_str());.github/workflows/validate_blueprint.yml-19-21 (1)
19-21:⚠️ Potential issue | 🟠 MajorPin
actions/checkoutto a stable release (avoid@main).
@mainis a mutable branch tip that can pull in unreviewed changes on subsequent runs, creating a supply-chain risk. Pin to a version tag like@v4for reproducibility, or use a full commit SHA for maximum immutability per GitHub's security hardening guidance.🔒 Suggested change
- - uses: actions/checkout@main + - uses: actions/checkout@v4components/nspanel_easy/pages.h-5-9 (1)
5-9: 🛠️ Refactor suggestion | 🟠 MajorMissing include for
strcmp.The
get_page_id(const char*)function usesstrcmpon line 73, but<cstring>is not included. While it may compile due to transitive includes, this should be explicit for portability.📝 Suggested fix
`#include` <array> `#include` <cstdint> +#include <cstring> `#include` <initializer_list> `#include` <string> `#include` "esphome/core/string_ref.h" // For StringRefcomponents/nspanel_easy/macros.h-1-4 (1)
1-4: 🛠️ Refactor suggestion | 🟠 MajorMissing header guard.
This file lacks
#pragma onceor traditional include guards. While individual macros have#ifndefguards, adding a file-level guard prevents unnecessary re-processing of the file during compilation.📝 Suggested fix
// macros.h - Utility macros for NSPanel HA Blueprint project // Contains conditional logging macros and other utility macros +#pragma once + `#include` "esphome/core/defines.h"esphome/nspanel_esphome_api.yaml-163-171 (1)
163-171:⚠️ Potential issue | 🟠 MajorFix climate-only block scoping in
entity_details_show.
disp1->set_component_value("climate.embedded", …)currently runs for all entities due to missing braces.🐛 Proposed fix
- if (entity_id.domain == "climate") - delay(${DELAY_DEFAULT}); - disp1->set_component_value("climate.embedded", (entity == "embedded_climate") ? 1 : 0); + if (entity_id.domain == "climate") { + delay(${DELAY_DEFAULT}); + disp1->set_component_value("climate.embedded", (entity == "embedded_climate") ? 1 : 0); + }esphome/nspanel_esphome_page_qrcode.yaml-74-79 (1)
74-79:⚠️ Potential issue | 🟠 MajorFix
qrcode_icon_colorinitial_value type mismatch.
qrcode_icon_colorisuint16_t, but the initial value uses the empty-string substitution. This should use the color substitution instead.✅ Proposed fix
- initial_value: '${qrcode_initial_value}' + initial_value: '${qrcode_icon_color_initial_value}'esphome/nspanel_esphome_api.yaml-106-137 (1)
106-137:⚠️ Potential issue | 🟠 MajorHonor page‑qualified component IDs in
components_visibility.
The action claims to supportpage.component, but the current logic checksresolved_page(not the parsed component page) and then always targets the short ID, which can apply changes to the wrong page when a page-qualified ID is supplied.🔧 Suggested fix to validate component.page before applying visibility
- // Resolve page to current page if empty - const std::string resolved_page = page.empty() ? current_page->state : page; - ESP_LOGV("${TAG_API}", " page: %s", resolved_page.c_str()); - - // Determine if we should process based on page context - const bool is_current_page = (resolved_page == current_page->state); - - if (!is_current_page) { - ESP_LOGW("${TAG_API}", "Not the current page (%s vs %s)", - resolved_page.c_str(), current_page->state.c_str()); - return; - } + // Resolve page to current page if empty + const std::string resolved_page = page.empty() ? current_page->state : page; + ESP_LOGV("${TAG_API}", " page: %s", resolved_page.c_str()); + + // If caller explicitly targets a page, require it to be current + if (!page.empty() && resolved_page != current_page->state) { + ESP_LOGW("${TAG_API}", "Not the current page (%s vs %s)", + resolved_page.c_str(), current_page->state.c_str()); + return; + } ... - const NextionComponent component = extractNextionComponent(component_id, resolved_page); - - // Use short format for current page components - const char* target_id = component.component_id; + const NextionComponent component = extractNextionComponent(component_id, resolved_page); + if (std::string(component.page) != current_page->state) { + ESP_LOGW("${TAG_API}", "Skipping off-page component (%s vs %s)", + component.page, current_page->state.c_str()); + continue; + } + const char* target_id = component.component_id;esphome/nspanel_esphome_page_climate.yaml-173-307 (1)
173-307:⚠️ Potential issue | 🟠 MajorGuard against
temp_step <= 0before slider math.
set_climatedivides byset_climate_temp_step; if the HA payload provides0, this will crash or yield undefined behavior. Add an early guard.🛠️ Proposed fix
if (!is_device_ready_for_tasks()) { ESP_LOGW("${TAG_PAGE_CLIMATE}", "'set_climate' was called while the device is not ready for tasks"); return; } + if (set_climate_temp_step <= 0) { + ESP_LOGW("${TAG_PAGE_CLIMATE}", "Invalid temp_step (<= 0); skipping climate render"); + return; + } disp1->send_command_printf("climateslider.maxval=%i", set_climate_total_steps);
🟡 Minor comments (37)
esphome/nspanel_esphome_hw_memory.yaml-53-75 (1)
53-75:⚠️ Potential issue | 🟡 MinorUse
%zuformat specifier forsize_tvalues instead of%d.Lines 60, 63, 70, 73 use
%dforsize_tvariables, which is not portable. The%dformat specifier is for signed integers, whilesize_tis unsigned and may be larger thaninton some architectures. Change to%zufor correct and portable output.🔧 Suggested fixes
- ESP_LOGCONFIG("${TAG_CORE}", " Internal: %7d bytes (%0.1f%%)", internal_heap_size_free, + ESP_LOGCONFIG("${TAG_CORE}", " Internal: %7zu bytes (%0.1f%%)", internal_heap_size_free,- ESP_LOGCONFIG("${TAG_CORE}", " Internal: %7d bytes", internal_heap_size_free); + ESP_LOGCONFIG("${TAG_CORE}", " Internal: %7zu bytes", internal_heap_size_free);- ESP_LOGCONFIG("${TAG_CORE}", " PSRAM: %7d bytes (%0.1f%%)", psram_heap_size_free, + ESP_LOGCONFIG("${TAG_CORE}", " PSRAM: %7zu bytes (%0.1f%%)", psram_heap_size_free,- ESP_LOGCONFIG("${TAG_CORE}", " PSRAM: %7d bytes", psram_heap_size_free); + ESP_LOGCONFIG("${TAG_CORE}", " PSRAM: %7zu bytes", psram_heap_size_free);esphome/nspanel_esphome_hw_wifi.yaml-173-178 (1)
173-178:⚠️ Potential issue | 🟡 MinorRemove the duplicate
update_flag_api_readyinvocation.You already call the script, then immediately call it again in the lambda, which can double state updates/logs with no benefit.
Proposed fix
on_connect: then: - script.execute: update_flag_api_ready - lambda: |- - update_flag_api_ready->execute(); if (cached_device_name.empty()) { bool last_was_underscore = false; for (const char& c : App.get_name()) {docs/version_compatibility.md-6-8 (1)
6-8:⚠️ Potential issue | 🟡 MinorFix the version label format on line 6.
The label
v2026012+is inconsistent with the date-based versioning pattern used in adjacent rows (v202512x,v202510x,v202511x). It appears to be a typo; it should likely bev202601xto follow thev{YYYYMMDD}xformat. Also review line 7'sv2026011for the same pattern..github/ISSUE_TEMPLATE/enhancement.yml-2-5 (1)
2-5:⚠️ Potential issue | 🟡 MinorUse a clean title prefix (avoid literal backticks).
Backticks are rendered as-is in issue titles and can look odd.
Proposed edit
-title: "`Enhancement` " +title: "Enhancement: "docs/lovelace_ui.md-5-66 (1)
5-66:⚠️ Potential issue | 🟡 MinorFix a few typos/grammar for clarity.
These small copy edits improve readability and reduce ambiguity.
Proposed edits
-If you are coming from there, you have to chose one of the 3 options: +If you are coming from there, you have to choose one of the three options: @@ -Double check if the file is available by trying to download it using your local url (typically [`http://homeassistant.local:8123/local/nspanel_blank.tft`](http://homeassistant.local:8123/local/nspanel_blank.tft)). +Double-check if the file is available by trying to download it using your local URL (typically [`http://homeassistant.local:8123/local/nspanel_blank.tft`](http://homeassistant.local:8123/local/nspanel_blank.tft)). @@ -If everything goes right, you will see the progress bar indicating the file transfer an you will end up with a black screen showing a QR code. +If everything goes right, you will see the progress bar indicating the file transfer and you will end up with a black screen showing a QR code. @@ -If everything goes right, you will see the progress bar indicating the file transfer an you will end up with a black screen showing a QR code. +If everything goes right, you will see the progress bar indicating the file transfer and you will end up with a black screen showing a QR code. @@ -We have an useful guide for [troubleshooting TFT transfer issues](tft_upload.md). Please take a look there first. +We have a useful guide for [troubleshooting TFT transfer issues](tft_upload.md). Please take a look there first. @@ -Please share as much info as possible, like: -1. Describing (or a picture of) what is in your screen -2. Are updating from a previous version of this same project, or coming from another NSPanel customization (which one?) or customizing for the first time a panel with original Sonoff settings? +Please share as much info as possible, such as: +1. A description (or photo) of what is on your screen +2. Whether you are updating from a previous version of this same project, or coming from another NSPanel customization (which one?), or customizing a panel with original Sonoff settings for the first timedocs/howto.md-162-176 (1)
162-176:⚠️ Potential issue | 🟡 MinorFix indentation in the notification automation example.
conditionandactionare currently nested under the trigger item, which makes the YAML invalid.Suggested correction
description: "" mode: single trigger: - platform: state entity_id: - binary_sensor.motion_detector to: "on" - condition: [] - action: - - action: esphome.panelname_notification_show - data: - label: Motion Detected - message: Example for a Notification on the Panel Screen +condition: [] +action: + - action: esphome.panelname_notification_show + data: + label: Motion Detected + message: Example for a Notification on the Panel Screen.github/ISSUE_TEMPLATE/config.yml-4-19 (1)
4-19:⚠️ Potential issue | 🟡 MinorPoint contact links to this repository instead of the upstream repo.
These URLs currently send users to Blackymas’ repo/docs, which is confusing for NSPanel‑Easy support and may hide relevant issues here.
Suggested update
contact_links: - name: Questions & General Discussions - url: https://github.com/Blackymas/NSPanel_HA_Blueprint/discussions + url: https://github.com/edwardtfn/NSPanel-Easy/discussions @@ - name: Troubleshooting TFT Transfer Issues - url: https://github.com/Blackymas/NSPanel_HA_Blueprint/blob/main/docs/tft_upload.md + url: https://github.com/edwardtfn/NSPanel-Easy/blob/main/docs/tft_upload.md @@ - name: Resolve Compiling Errors - url: https://github.com/Blackymas/NSPanel_HA_Blueprint/blob/main/docs/error_compiling.md + url: https://github.com/edwardtfn/NSPanel-Easy/blob/main/docs/error_compiling.md @@ - name: Getting Started Guide - url: https://github.com/Blackymas/NSPanel_HA_Blueprint/blob/main/docs/install.md + url: https://github.com/edwardtfn/NSPanel-Easy/blob/main/docs/install.mddocs/nspanel_blank.md-28-29 (1)
28-29:⚠️ Potential issue | 🟡 MinorFix spelling error.
Line 28: "sucessfully" → "successfully"
📝 Proposed fix
-Once you have sucessfully installed any of the `tft` files from this project, +Once you have successfully installed any of the `tft` files from this project,docs/nspanel_blank.md-10-17 (1)
10-17:⚠️ Potential issue | 🟡 MinorFix spelling errors in documentation.
Several typos were detected that should be corrected for professionalism:
- Line 10: "recoment" → "recommend"
- Line 16: "datails" → "details"
- Line 17: "refeer" → "refer"
📝 Proposed fix
-which makes a bit challenging to replace the `tft` file when using ESPHome and then, we highly recoment you using `nspanel_blank.tft`, +which makes it a bit challenging to replace the `tft` file when using ESPHome and then, we highly recommend you using `nspanel_blank.tft`,-For more datails on how to install the first `tft` file, specially if your panel still displaying the original Sonoff screen, -please refeer to the [Troubleshooting TFT transfer](tft_upload.md) guide. +For more details on how to install the first `tft` file, especially if your panel is still displaying the original Sonoff screen, +please refer to the [Troubleshooting TFT transfer](tft_upload.md) guide.esphome/nspanel_esphome_page_settings.yaml-79-82 (1)
79-82:⚠️ Potential issue | 🟡 MinorTypo: "Bed params" should be "Bad params".
✏️ Proposed fix
if (params_count != 3) { - ESP_LOGW("${TAG_PAGE_SETTINGS}", "Bed params"); + ESP_LOGW("${TAG_PAGE_SETTINGS}", "Bad params"); return; }components/nspanel_easy/page_utilities.cpp-23-47 (1)
23-47:⚠️ Potential issue | 🟡 MinorPotential memory leak if
resetUtilitiesGroups()is called multiple times.If this function is called more than once, the previous allocation is never freed, leading to a memory leak.
🛡️ Proposed fix to free existing allocation before reallocating
void resetUtilitiesGroups() { + if (UtilitiesGroups != nullptr) { + `#ifdef` USE_ESP_IDF + heap_caps_free(UtilitiesGroups); + `#elif` defined(USE_ARDUINO) + free(UtilitiesGroups); + `#else` + delete[] UtilitiesGroups; + `#endif` + UtilitiesGroups = nullptr; + } + `#ifdef` USE_ESP_IDF UtilitiesGroups = static_cast<UtilitiesGroupValues*>( heap_caps_malloc(UTILITIES_GROUPS_COUNT * sizeof(UtilitiesGroupValues), MALLOC_CAP_SPIRAM));esphome/nspanel_esphome_boot.yaml-34-35 (1)
34-35:⚠️ Potential issue | 🟡 MinorMissing substitution syntax for TAG_BOOT.
Line 35 uses
"TAG_BOOT"as a literal string instead of the substitution"${TAG_BOOT}". This will log the literal text "TAG_BOOT" rather than "nspanel.boot".🐛 Proposed fix
- ESP_LOGI("TAG_BOOT", "Starting boot sequence"); + ESP_LOGI("${TAG_BOOT}", "Starting boot sequence");components/nspanel_easy/text.cpp-135-158 (1)
135-158:⚠️ Potential issue | 🟡 MinorMissing validation for UTF-8 continuation bytes.
The function checks that continuation bytes exist (not
\0) but doesn't verify they have the correct10xxxxxxpattern (bytes in range0x80-0xBF). Malformed UTF-8 input could produce incorrect code points.If input is always well-formed, this is acceptable. Otherwise, consider adding validation:
🛡️ Proposed fix to validate continuation bytes
+ // Helper to check valid continuation byte + auto is_continuation = [](unsigned char b) { return (b & 0xC0) == 0x80; }; + if ((byte & 0x80) == 0x00) { code_point = byte; - } else if ((byte & 0xE0) == 0xC0 && bytes[1] != '\0') { + } else if ((byte & 0xE0) == 0xC0 && is_continuation(bytes[1])) { code_point = ((byte & 0x1F) << 6) | (static_cast<unsigned char>(bytes[1]) & 0x3F); - } else if ((byte & 0xF0) == 0xE0 && bytes[1] != '\0' && bytes[2] != '\0') { + } else if ((byte & 0xF0) == 0xE0 && is_continuation(bytes[1]) && is_continuation(bytes[2])) { code_point = ((byte & 0x0F) << 12) | ((static_cast<unsigned char>(bytes[1]) & 0x3F) << 6) | (static_cast<unsigned char>(bytes[2]) & 0x3F); - } else if ((byte & 0xF8) == 0xF0 && bytes[1] != '\0' && bytes[2] != '\0' && bytes[3] != '\0') { + } else if ((byte & 0xF8) == 0xF0 && is_continuation(bytes[1]) && is_continuation(bytes[2]) && is_continuation(bytes[3])) { code_point = ((byte & 0x07) << 18) |esphome/nspanel_esphome_addon_upload_tft.yaml-71-75 (1)
71-75:⚠️ Potential issue | 🟡 MinorUse explicit YAML boolean for
restore_value.Static analysis flags
restore_value: no; preferfalseto satisfy lint rules.✅ Lint-friendly fix
- restore_value: no + restore_value: false.rules/markdownlint.yml-1-2 (1)
1-2:⚠️ Potential issue | 🟡 MinorAdd YAML document start/end markers (
---and...) to satisfy yamllint rules.The
.rules/yamllint.ymlconfiguration enables warnings for bothdocument-startanddocument-endrules, which applies to.rules/markdownlint.ymlas a*.ymlfile.🔧 Suggested fix
+--- MD013: line_length: 200 +...components/nspanel_easy/ha_components.cpp-8-21 (1)
8-21:⚠️ Potential issue | 🟡 MinorImplement the
alarm_control_paneldomain mapping.The header explicitly documents that this function should shorten
"alarm_control_panel"to"alarm"(see ha_components.h lines 22-24), but the implementation doesn't apply this mapping. Add the conditional check to map"alarm_control_panel"to"alarm"as specified in the documentation.🔧 Suggested fix
if (dotPos != std::string::npos) { // Extract domain and id from the entity_id string result.domain = entity_id.substr(0, dotPos); result.id = entity_id.substr(dotPos + 1); + if (result.domain == "alarm_control_panel") { + result.domain = "alarm"; + } } else {docs/error_initializing.md-3-41 (1)
3-41:⚠️ Potential issue | 🟡 MinorTighten grammar and typos for clarity.
These small wording fixes improve readability and correctness in user-facing docs.
✍️ Suggested edits
-When your panel starts, a number of different things needs to be set and the ESPHome must establish connection to the Nextion display used by your panel +When your panel starts, a number of different things need to be set and ESPHome must establish a connection to the Nextion display used by your panel @@ -In the top, you will see the message "Initializing..." while the display is executing it's boot sequence or waiting for a connection from ESPHome. +In the top, you will see the message "Initializing..." while the display is executing its boot sequence or waiting for a connection from ESPHome. @@ -Once ESPHome is connected to both the display and the Wi-Fi network, that text will be replace by the IP address, which can be useful for troubleshooting. +Once ESPHome is connected to both the display and the Wi-Fi network, that text will be replaced by the IP address, which can be useful for troubleshooting. @@ -On the top right corner, it's shown the baud rate used by the display to communicate to ESPHome. -By default, all the communication should happens at 115200 bps, but as 921600 bps is also supported, the display will alternate between these 2 rates every 30 seconds until ESPHome gets connected. +In the top-right corner, the baud rate used by the display to communicate with ESPHome is shown. +By default, all the communication should happen at 115200 bps, but since 921600 bps is also supported, the display will alternate between these two rates every 30 seconds until ESPHome gets connected. @@ -Right bellow the baud rate information, you may see the framework used by ESPHome, which is either `esp-idf` (default for new installations) or `arduino` (default for installations prior v4.1.4). +Right below the baud rate information, you may see the framework used by ESPHome, which is either `esp-idf` (default for new installations) or `arduino` (default for installations prior to v4.1.4). @@ -In normal situation, all the versions should be displayed and should be the same. Different patch versions are supported. +In a normal situation, all the versions should be displayed and should be the same. Different patch versions are supported. @@ -2. Are you updating from a previous version of this project, or you coming from another NSPanel customization (which one?) or are you customizing for the first time a panel with the original Sonoff settings? +2. Are you updating from a previous version of this project, or are you coming from another NSPanel customization (which one?), or are you customizing a panel with the original Sonoff settings for the first time?Also applies to: 54-99
docs/addon_climate.md-81-82 (1)
81-82:⚠️ Potential issue | 🟡 MinorExamples still use deprecated
temp_step.The table marks
temp_stepas deprecated, but the examples keep using it. Consider removing it from the examples or adding a clear legacy note so readers don’t copy deprecated settings.📝 Possible clarification
## Examples + +> [!NOTE] +> `temp_step` is deprecated. On newer versions, prefer `climate: visual: temperature_step` and omit `temp_step` from substitutions.Also applies to: 114-115
docs/tft_upload.md-39-41 (1)
39-41:⚠️ Potential issue | 🟡 MinorFix typos and wording in troubleshooting steps.
📝 Suggested edits
-At least one DNS server is required to enable TFT transfer direcly from GitHub, otherwise use `nextion_update_url`. +At least one DNS server is required to enable TFT transfer directly from GitHub; otherwise use `nextion_update_url`. -If you haven't installed any TFT yet, look for Nextion related messages on ESPHome logs. +If you haven't installed any TFT yet, look for Nextion-related messages on ESPHome logs. -Compare this with the size listed on GitHub or from the file you downlodaded directly from GitHub. +Compare this with the size listed on GitHub or from the file you downloaded directly from GitHub. - 1. Look your yaml file and commment out all the customization (typically on a section name "My customizations") and any of the remote files other than `nspanel_esphome.yaml`. + 1. Look at your YAML file and comment out all the customization (typically in a section named "My customizations") and any of the remote files other than `nspanel_esphome.yaml`. -There are many options available for free to run under you local Windows, MAC or Linux machine. +There are many options available for free to run under your local Windows, macOS, or Linux machine.Also applies to: 64-65, 94-99, 146-147, 154-155
docs/addon_climate.md-9-12 (1)
9-12:⚠️ Potential issue | 🟡 MinorCorrect typos in safety and configuration text.
📝 Suggested edits
-Don't use it for directly power your cooler/heater if exceeding the panel specifications: +Don't use it to directly power your cooler/heater if it exceeds the panel specifications: -cooler_relay|Mandatory for *cool* and *dual*|`1` or `2`|`0` (disabled)|Relay used for control the cooler. User `1` for "Relay 1" or `2` for "Relay 2". -heater_relay|Mandatory for *heat* and *dual*|`1` or `2`|`0` (disabled)|Relay used for control the heater. User `1` for "Relay 1" or `2` for "Relay 2". +cooler_relay|Mandatory for *cool* and *dual*|`1` or `2`|`0` (disabled)|Relay used to control the cooler. Use `1` for "Relay 1" or `2` for "Relay 2". +heater_relay|Mandatory for *heat* and *dual*|`1` or `2`|`0` (disabled)|Relay used to control the heater. Use `1` for "Relay 1" or `2` for "Relay 2". -target_low|Optional|Number representing a temperature in the selected unit|`18` (°C) or `64` (°F)|The initial lower treshold for the target temperature. -target_high|Optional|Number representing a temperature in the selected unit|`24` (°C) or `75` (°F)|The initial higher treshold for the target temperature. +target_low|Optional|Number representing a temperature in the selected unit|`18` (°C) or `64` (°F)|The initial lower threshold for the target temperature. +target_high|Optional|Number representing a temperature in the selected unit|`24` (°C) or `75` (°F)|The initial higher threshold for the target temperature. -*On March 24th (around 9:20 PM) and March 25th (around 7:30AM) a window was opened, and all heaters were set to 'off.' for 30min* +*On March 24th (around 9:20 PM) and March 25th (around 7:30AM) a window was opened, and all heaters were set to 'off.' for 30 min*Also applies to: 71-79, 259-260
docs/different_version.md-24-27 (1)
24-27:⚠️ Potential issue | 🟡 MinorFix broken NOTE blockquote.
The line starting “They’re created…” isn’t quoted, so the admonition breaks.
📝 Suggested fix
-They're created for each new release, allowing easy access to different project stages. +> They're created for each new release, allowing easy access to different project stages..github/workflows/esphome_build.yml-55-56 (1)
55-56:⚠️ Potential issue | 🟡 MinorYAML lint currently skips
.ymlfiles.
find . -name "*.yaml"ignores.yml, so some YAML (including workflow files) won’t be linted.🔧 Suggested fix
- - name: Validate YAML files - run: find . -name "*.yaml" -exec yamllint -c ./.rules/yamllint.yml {} + + - name: Validate YAML files + run: find . \( -name "*.yaml" -o -name "*.yml" \) -exec yamllint -c ./.rules/yamllint.yml {} +docs/install.md-263-263 (1)
263-263:⚠️ Potential issue | 🟡 MinorFix typos: "happnening" → "happening" and "intgration" → "integration"
📝 Proposed fix
-If that isn't happnening, you can manually add it as a new intgration with the following steps: +If that isn't happening, you can manually add it as a new integration with the following steps:docs/install.md-18-18 (1)
18-18:⚠️ Potential issue | 🟡 MinorFix typo: "slighly" → "slightly"
📝 Proposed fix
-> Some images or instructions on this guide might be slighly different due to different configurations depending on your ESPHome and Home Assistant implementation or versions. +> Some images or instructions on this guide might be slightly different due to different configurations depending on your ESPHome and Home Assistant implementation or versions.docs/blueprint.md-215-215 (1)
215-215:⚠️ Potential issue | 🟡 MinorFix typo: "aligment" → "alignment"
📝 Proposed fix
-Of course there is also the possibility to enter a label for each of the 4 pages and the aligment for the values on the entities pages. +Of course there is also the possibility to enter a label for each of the 4 pages and the alignment for the values on the entities pages.README.md-67-67 (1)
67-67:⚠️ Potential issue | 🟡 MinorFix typo: "Forcast" → "Forecast"
📝 Proposed fix
-- Current weather with button to Weather Forcast page +- Current weather with button to Weather Forecast pagedocs/blueprint.md-72-72 (1)
72-72:⚠️ Potential issue | 🟡 MinorFix typo: "then" → "when"
The word "then" should be "when" in this context.
📝 Proposed fix
-| Home page - Chips 01-07 (optional) | Used to display icons of HA binary entities on the home screen. The icon is displayed then the entity is on/open/true/active. Otherwise, no icon is displayed. |  | +| Home page - Chips 01-07 (optional) | Used to display icons of HA binary entities on the home screen. The icon is displayed when the entity is on/open/true/active. Otherwise, no icon is displayed. |  |docs/customization.md-353-353 (1)
353-353:⚠️ Potential issue | 🟡 MinorTypo: "probabily" should be "probably"
📝 Suggested fix
-If your system is not showing the time in the correct timezone, it's probabily ESPHome not succeeding on finding your time zone. +If your system is not showing the time in the correct timezone, it's probably ESPHome not succeeding on finding your time zone.docs/customization.md-365-365 (1)
365-365:⚠️ Potential issue | 🟡 MinorTypo: "datails" should be "details"
📝 Suggested fix
-More datails on the [ESPHome docs](https://esphome.io/changelog/2022.11.0.html#running-esphome-on-lower-powered-machines). +More details on the [ESPHome docs](https://esphome.io/changelog/2022.11.0.html#running-esphome-on-lower-powered-machines).docs/customization.md-60-60 (1)
60-60:⚠️ Potential issue | 🟡 MinorTypo: "bellow" should be "below"
📝 Suggested fix
-You should add your customizations at the end of your ESPHome yaml, as in the example bellow: +You should add your customizations at the end of your ESPHome yaml, as in the example below:docs/customization.md-47-47 (1)
47-47:⚠️ Potential issue | 🟡 MinorTypo: "customation" should be "customization"
📝 Suggested fix
-Please feel free to add your own customation to this document by creating a PR in the `dev` branch. +Please feel free to add your own customization to this document by creating a PR in the `dev` branch.docs/api.md-688-688 (1)
688-688:⚠️ Potential issue | 🟡 MinorTypo: "bellow" should be "below"
This is user-facing documentation with a spelling error.
📝 Suggested fix
-The example bellow integrates the `wake_up` action with a motion sensor to ensure the display wakes or remains awake during periods of activity, reverting to sleep mode after inactivity. +The example below integrates the `wake_up` action with a motion sensor to ensure the display wakes or remains awake during periods of activity, reverting to sleep mode after inactivity.docs/customization.md-235-246 (1)
235-246:⚠️ Potential issue | 🟡 MinorTypo: "direcly" should be "directly"
This typo appears twice in this section.
📝 Suggested fix
-> At least one DNS server is required to enable TFT transfer direcly from GitHub, otherwise use `nextion_update_url`. +> At least one DNS server is required to enable TFT transfer directly from GitHub, otherwise use `nextion_update_url`.- dns1: 1.1.1.1 # At least one DNS server is required to enable TFT transfer direcly from GitHub, otherwise use `nextion_update_url`. + dns1: 1.1.1.1 # At least one DNS server is required to enable TFT transfer directly from GitHub, otherwise use `nextion_update_url`.esphome/nspanel_esphome_page_confirm.yaml-37-41 (1)
37-41:⚠️ Potential issue | 🟡 MinorInconsistent use of
current_page_idvsnew_page_idin page_change handler.This file uses
current_page_idon Line 40, while other page files (e.g.,nspanel_esphome_page_entities.yamlLine 97,nspanel_esphome_page_home.yamlLine 151) usenew_page_id. Ifpage_changefires beforecurrent_page_idis updated, this condition may not trigger correctly.Proposed fix for consistency
- id: !extend page_change then: - lambda: |- - if (current_page_id == ${PAGE_CONFIRM_ID}) + if (new_page_id == ${PAGE_CONFIRM_ID}) page_confirm->execute();esphome/nspanel_esphome_version.yaml-140-144 (1)
140-144:⚠️ Potential issue | 🟡 MinorMissing comparison in modulo expression.
Line 143 uses
if (iteration % 9)which evaluates to true for all values except wheniterationis a multiple of 9. This appears inverted from the likely intent of triggering every 9th iteration.Proposed fix
- if (iteration % 9) boot_request_blueprint_settings->execute("timeout"); + if (iteration % 9 == 0) boot_request_blueprint_settings->execute("timeout");If the current behavior (trigger on all iterations except multiples of 9) is intentional, please add a comment explaining the logic.
esphome/nspanel_esphome_page_media_player.yaml-154-158 (1)
154-158:⚠️ Potential issue | 🟡 MinorType mismatch in format specifier.
On Line 156,
volume_levelis declared asint(Line 55), but the format specifierPRIu32expects an unsigned 32-bit integer. This could cause incorrect display for negative values (though unlikely in this context) or compiler warnings.Proposed fix
- disp1->set_component_text_printf("vol_text", "%" PRIu32 "%%", volume_level); + disp1->set_component_text_printf("vol_text", "%d%%", volume_level);esphome/nspanel_esphome_page_notification.yaml-59-79 (1)
59-79:⚠️ Potential issue | 🟡 Minor
bt_cleardoesn’t actually clear the notification payload.
It only marks unread false and navigates home, leavingnotification_text/labelintact (icon remains and the old text is still accessible). If “clear” should remove the notification, callnotification_clear.🛠️ Proposed fix
case 8: // bt_clear ha_button->execute("notification", "bt_clear", "released"); - notification_unread->publish_state(false); - disp1->set_component_font_color(hmi::home::BT_NOTIFIC.name, notification_icon_color_normal); - goto_page->execute(hmi::home::PAGE.id); + notification_clear->execute(); break;
There was a problem hiding this comment.
Actionable comments posted: 5
Note
Due to the large number of review comments, Critical severity comments were prioritized as inline comments.
🤖 Fix all issues with AI agents
In `@components/nspanel_easy/page_qrcode.h`:
- Around line 5-6: The include guard in page_qrcode.h is wrong: replace uses of
NSPANEL_EASY_PAGE_NOTIFICATION with NSPANEL_EASY_PAGE_QRCODE so the header
matches its page name; update the opening macro check/define near the top and
the corresponding closing `#endif` comment at the bottom to reference
NSPANEL_EASY_PAGE_QRCODE (ensure the symbol exactly matches other pages' pattern
and the configuration field page_qrcode).
In `@components/nspanel_easy/text.cpp`:
- Around line 3-7: The translation unit uses strtod but doesn't include its
declaration; add the missing include by adding `#include` <cstdlib> to
components/nspanel_easy/text.cpp so the symbol strtod is declared (locate near
the other includes at the top of the file alongside "text.h", <algorithm>,
<cctype>, <cstring>).
In `@esphome/nspanel_esphome_addon_display_light.yaml`:
- Around line 66-81: The lambda in the set_brightness script is using the global
brightness_current instead of the script parameter brightness, causing the light
entity to get stale brightness; update the call to set_brightness(...) on the
display_light turn_on call to use the local script parameter brightness
(converted to 0-1 as before) rather than brightness_current, keeping the rest of
the logic in the id: !extend set_brightness block intact and referencing
display_light, current_page_id, and PAGE_SCREENSAVER_ID as shown.
In `@esphome/nspanel_esphome_api.yaml`:
- Around line 163-172: The climate-specific statements are missing braces so
only delay(${DELAY_DEFAULT}) is conditional; make the if (entity_id.domain ==
"climate") block include both the delay and the disp1->set_component_value call
by adding braces around them (so the delay and
disp1->set_component_value("climate.embedded", (entity == "embedded_climate") ?
1 : 0) execute only when entity_id.domain == "climate"); ensure this change is
applied in the same section that also calls
detailed_entity->publish_state(entity), goto_page->execute(get_page_id(...)),
disp1->send_command_printf(...back_page...), and uses
get_page_id/back_page/DELAY_DEFAULT so control flow remains correct.
In `@esphome/nspanel_esphome_page_qrcode.yaml`:
- Around line 74-78: The global qrcode_icon_color is declared as type uint16_t
but its initial_value references ${qrcode_initial_value} which is an empty
string; change the initial_value to use the intended numeric/color constant
reference ${qrcode_icon_color_initial_value} (or update the variable
qrcode_initial_value to the proper 'Colors::GRAY_LIGHT' value) so that
qrcode_icon_color's initial_value matches its uint16_t type; update the globals
block where qrcode_icon_color is declared to reference
qrcode_icon_color_initial_value instead of qrcode_initial_value.
🟠 Major comments (19)
components/nspanel_easy/macros.h-1-5 (1)
1-5: 🛠️ Refactor suggestion | 🟠 MajorMissing header guard and log header include.
This header file lacks a guard (
#pragma onceor traditional#ifndef/#define/#endif) which can cause redefinition errors if included multiple times. Additionally, theESP_LOG*functions used in the macros are typically declared inesphome/core/log.h, notesphome/core/defines.h.🔧 Proposed fix
+#pragma once + // macros.h - Utility macros for NSPanel HA Blueprint project // Contains conditional logging macros and other utility macros `#include` "esphome/core/defines.h" +#include "esphome/core/log.h".github/workflows/validate_clang_format.yml-22-22 (1)
22-22:⚠️ Potential issue | 🟠 MajorPin
actions/checkoutto a version tag instead of@main.Using
@maincan lead to unexpected breakage or security issues if the upstream action changes. Pin to a stable version tag for reproducible builds.🔒 Proposed fix
- - uses: actions/checkout@main + - uses: actions/checkout@v4.github/workflows/validate_clang_format.yml-25-25 (1)
25-25:⚠️ Potential issue | 🟠 MajorAdd
components/nspanel_easy/to the sources list.The
sourcesparameter is missingcomponents/nspanel_easy/, which contains 33 C++ source files (.cppand.h) that should be included in clang-format validation.Update the sources to:
sources: "components/nspanel_ha_blueprint/*.h,components/nspanel_ha_blueprint/*.cpp,components/nspanel_ha_blueprint_upload_tft/*.h,components/nspanel_ha_blueprint_upload_tft/*.cpp,components/nspanel_easy/*.h,components/nspanel_easy/*.cpp"components/nspanel_easy/pages.h-5-10 (1)
5-10:⚠️ Potential issue | 🟠 MajorInclude
<cstring>in the header forstrcmp.
Without it, any TU that includes this header may fail to compile.Proposed fix
`#include` <array> `#include` <cstdint> +#include <cstring> `#include` <initializer_list> `#include` <string> `#include` "esphome/core/string_ref.h" // For StringRefcomponents/nspanel_easy/pages.h-71-76 (1)
71-76:⚠️ Potential issue | 🟠 MajorGuard against null C-string input before calling
strcmp.
A nullpage_namewill dereference and crash.Proposed fix
inline uint8_t get_page_id(const char* page_name) { + if (page_name == nullptr || *page_name == '\0') return UINT8_MAX; for (uint8_t i = 0; i < PAGE_COUNT; ++i) { if (strcmp(page_names[i], page_name) == 0) return i;components/nspanel_easy/text.h-5-6 (1)
5-6:⚠️ Potential issue | 🟠 MajorInclude the headers that define
uint8_t/uint32_tandstd::initializer_list.This header relies on types that aren’t guaranteed to be visible with only
<string>, which can break compilation depending on include order.🔧 Proposed fix
`#include` <string> +#include <cstdint> +#include <initializer_list>components/nspanel_easy/text.cpp-52-124 (1)
52-124:⚠️ Potential issue | 🟠 MajorGuard against zero line length to avoid an infinite loop.
If either
line_length_limitorbytes_per_charis zero,max_line_lengthbecomes zero and the loop can stall without advancing.🛡️ Proposed fix
std::string wrapText(const std::string& text_to_display, uint8_t line_length_limit, uint8_t bytes_per_char) { // Safety check for overly long text if (text_to_display.length() > 1000) { return "ERROR: Text too long"; } + if (line_length_limit == 0 || bytes_per_char == 0) { + return "ERROR: Invalid line length"; + }esphome/nspanel_esphome_page_cover.yaml-23-35 (1)
23-35:⚠️ Potential issue | 🟠 MajorGuard against short CSV payloads before indexing
params.
params[1]is accessed without checkingparams_count >= 2, which can crash on malformed events.🔧 Suggested fix
- const std::string& key = params[1]; + if (params_count < 2) { + ESP_LOGW("${TAG_PAGE_COVER}", "Bad params"); + return; + } + const std::string& key = params[1];.github/workflows/validate_python.yml-6-11 (1)
6-11:⚠️ Potential issue | 🟠 MajorPath patterns won't trigger for nested Python files.
The trigger paths
*.pyonly match Python files at the repository root, but the flake8 action targetscomponents/nspanel_ha_blueprint. This workflow will never trigger when Python files in that directory change.🐛 Proposed fix to include nested Python files
on: push: paths: - - '*.py' + - '**/*.py' pull_request: paths: - - '*.py' + - '**/*.py' workflow_dispatch:components/nspanel_easy/page_utilities.h-37-42 (1)
37-42:⚠️ Potential issue | 🟠 MajorPotential undefined behavior:
free()onnew[]-allocated memory.The
cleanupUtilitiesGroups()usesfree()to deallocateUtilitiesGroups. However, inpage_utilities.cpp, if PSRAM allocation fails, the fallback usesnew UtilitiesGroupValues[...]. Callingfree()on memory allocated withnew[]is undefined behavior.🐛 Suggested fix - track allocation method or use consistent allocation
Option 1: Use malloc as fallback instead of new[]:
In
page_utilities.cpp:- if (!UtilitiesGroups) UtilitiesGroups = new UtilitiesGroupValues[UTILITIES_GROUPS_COUNT]; + if (!UtilitiesGroups) UtilitiesGroups = static_cast<UtilitiesGroupValues*>( + malloc(UTILITIES_GROUPS_COUNT * sizeof(UtilitiesGroupValues)));Option 2: Use delete[] in cleanup when appropriate (requires tracking).
components/nspanel_easy/page_utilities.cpp-23-47 (1)
23-47:⚠️ Potential issue | 🟠 MajorMemory leak if
resetUtilitiesGroups()is called multiple times.The function allocates new memory without first freeing any existing allocation. If called more than once, the previous
UtilitiesGroupspointer is overwritten, leaking memory.🐛 Suggested fix
void resetUtilitiesGroups() { + cleanupUtilitiesGroups(); // Free any existing allocation first + `#ifdef` USE_ESP_IDF UtilitiesGroups = static_cast<UtilitiesGroupValues*>( heap_caps_malloc(UTILITIES_GROUPS_COUNT * sizeof(UtilitiesGroupValues), MALLOC_CAP_SPIRAM));.github/workflows/release_tag.yml-1-31 (1)
1-31:⚠️ Potential issue | 🟠 MajorPin
actions/checkoutto a commit SHA, add explicit permissions, and dereference the release tag.
actions/checkout@mainis mutable and poses a security risk. GitHub currently recommends pinning actions to full-length commit SHAs (not just version tags) for immutability. Additionally, the workflow requires explicitpermissions: contents: writeto create refs (tags) via the defaultGITHUB_TOKEN. Since the release tag may be annotated, tagging it directly creates a tag-of-tag; use^{}syntax to dereference and point to the underlying commit instead.🔧 Proposed fix
name: Update Tags # yamllint disable-line rule:truthy on: release: types: [published] +permissions: + contents: write + jobs: update-tags: runs-on: ubuntu-latest steps: - name: Checkout code - uses: actions/checkout@main + uses: actions/checkout@1d7c6c3aab6f5a8413b9a25f9a12f6f2f9b0b3c0 # v6.0.2 with: fetch-depth: '0' - name: Set up Git run: | git config --local user.email "action@github.com" git config --local user.name "GitHub Action" - name: Move and push stable tag run: | - git tag -f stable ${{ github.event.release.tag_name }} + git tag -f stable ${{ github.event.release.tag_name }}^{} git push -f origin stable - name: Move and push latest tag run: | - git tag -f latest ${{ github.event.release.tag_name }} + git tag -f latest ${{ github.event.release.tag_name }}^{} git push -f origin latest.github/workflows/validate_blueprint.yml-18-23 (1)
18-23:⚠️ Potential issue | 🟠 MajorPin
actions/checkoutand installyamllint.Similar to the other workflow, this has two issues:
actions/checkout@mainshould be pinned to a specific version (e.g.,v4)yamllintis not installed before useProposed fix
steps: - - uses: actions/checkout@main + - uses: actions/checkout@v4 with: fetch-depth: '0' + - name: Install yamllint + run: pip install yamllint - name: Validate nspanel_blueprint.yaml run: yamllint -c "./.rules/yamllint.yml" nspanel_blueprint.yaml.github/ISSUE_TEMPLATE/config.yml-5-19 (1)
5-19:⚠️ Potential issue | 🟠 MajorURLs reference wrong repository.
All contact links point to
Blackymas/NSPanel_HA_Blueprint, but this PR is foredwardtfn/NSPanel-Easy. Users following these links will be directed to a different repository for support.Proposed fix to update repository URLs
contact_links: - name: Questions & General Discussions - url: https://github.com/Blackymas/NSPanel_HA_Blueprint/discussions + url: https://github.com/edwardtfn/NSPanel-Easy/discussions about: Join our Discussions for community chats, questions, idea sharing, and collaboration. Use Issues for reporting bugs or requesting enhancements. - name: Troubleshooting TFT Transfer Issues - url: https://github.com/Blackymas/NSPanel_HA_Blueprint/blob/main/docs/tft_upload.md + url: https://github.com/edwardtfn/NSPanel-Easy/blob/main/docs/tft_upload.md about: Having trouble transferring a TFT file? Our guide might have the solution. - name: Resolve Compiling Errors - url: https://github.com/Blackymas/NSPanel_HA_Blueprint/blob/main/docs/error_compiling.md + url: https://github.com/edwardtfn/NSPanel-Easy/blob/main/docs/error_compiling.md about: Check this guide if you're experiencing compile errors. - name: Getting Started Guide - url: https://github.com/Blackymas/NSPanel_HA_Blueprint/blob/main/docs/install.md + url: https://github.com/edwardtfn/NSPanel-Easy/blob/main/docs/install.md about: New to NSPanel HA Blueprint? Start here for installation and setup instructions..github/workflows/validate_yamllint.yml-33-48 (1)
33-48:⚠️ Potential issue | 🟠 Major
yamllintis not installed before use.The workflow uses
yamllintbut doesn't install it first. While some CI images may have it pre-installed, this is not guaranteed and could cause workflow failures.Proposed fix to install yamllint
- name: Validate YAML if: steps.changed-files.outputs.any_changed == 'true' run: | + pip install yamllint IFS=',' read -ra FILES <<< "${{ steps.changed-files.outputs.all_changed_files }}" for file in "${FILES[@]}"; do.github/workflows/validate_markdown.yml-20-33 (1)
20-33:⚠️ Potential issue | 🟠 MajorPin
actions/checkout@mainto a specific version.Using
@mainis a moving ref and introduces supply-chain risk. Other actions in this workflow (changed-files, markdownlint-cli2, link-check) are already pinned. Update checkout to use@v4for consistency with other workflows in this repository (lines 20 and 46).🔒 Example update
- uses: actions/checkout@main + uses: actions/checkout@v4.github/workflows/stale.yml-26-26 (1)
26-26:⚠️ Potential issue | 🟠 MajorPin
actions/staleto an immutable commit SHA to reduce supply-chain risk.Major version tags like
v10are mutable and can be updated without notice. Use the full commit SHA instead. The latest v10.1.1 release corresponds to commit997185467fa4f803885201cee163a9f38240193d.🔒 Example update
- - uses: actions/stale@v10 + - uses: actions/stale@997185467fa4f803885201cee163a9f38240193d # v10.1.1esphome/nspanel_esphome_addon_upload_tft.yaml-274-294 (1)
274-294:⚠️ Potential issue | 🟠 MajorUnconditional reboot after every upload attempt regardless of outcome.
Line 293 calls
App.safe_reboot()unconditionally after each upload attempt. This means:
- On success: Device reboots (expected behavior for TFT updates)
- On failure: Device reboots immediately instead of continuing to the next retry attempt
The retry loop in
upload_tft_sequence_attempt(lines 266-272) will never execute multiple attempts because the first attempt always triggers a reboot.If immediate reboot on both success and failure is intentional, consider adding a comment clarifying this behavior. Otherwise, the reboot should be conditional on success.
Proposed fix if retries should work
- lambda: |- char update_msg[128]; sprintf(update_msg, "Attempt #%d returned: %s", tft_upload_attempt, tft_upload_result ? "Success!" : "FAILED"); ESP_LOGD("${TAG_UPLOAD_TFT}", update_msg); - ESP_LOGI("addon_upload_tft.script.upload_tft_attempt", "Restarting ESPHome"); - App.safe_reboot(); + if (tft_upload_result) { + ESP_LOGI("addon_upload_tft.script.upload_tft_attempt", "Upload successful, restarting ESPHome"); + App.safe_reboot(); + }esphome/nspanel_esphome_api.yaml-322-333 (1)
322-333:⚠️ Potential issue | 🟠 Major
api_recover_from_failedcould loop indefinitely if recovery keeps failing.The while loop (lines 325-332) has no exit condition other than
api_server->is_failed()becoming false. If the API server repeatedly fails to recover, this will loop forever with only a 5-second delay between attempts.Consider adding a maximum retry count or exponential backoff to prevent infinite loops.
Proposed fix with retry limit
- id: api_recover_from_failed mode: single then: + - lambda: uint8_t api_recovery_attempts = 0; - while: condition: - - lambda: return api_server->is_failed(); + - lambda: return api_server->is_failed() && api_recovery_attempts < 10; then: - lambda: |- - ESP_LOGW("${TAG_API}", "API reported a failed state. Recovering..."); + api_recovery_attempts++; + ESP_LOGW("${TAG_API}", "API reported a failed state. Recovery attempt %d/10...", api_recovery_attempts); api_server->setup(); - delay: 5s
🟡 Minor comments (49)
esphome/nspanel_esphome_page_confirm.yaml-21-27 (1)
21-27:⚠️ Potential issue | 🟡 MinorConsider multi-byte character length for
max_restore_data_length.The
max_restore_data_length: 25may be insufficient for localized strings with multi-byte UTF-8 characters. For example, "请确认" (Chinese for "Please confirm") is only 3 characters but requires 9 bytes in UTF-8. Translations in languages like Russian, Arabic, or CJK could easily exceed 25 bytes.Consider increasing
max_restore_data_lengthor documenting the byte limit for translators.esphome/nspanel_esphome_page_confirm.yaml-30-35 (1)
30-35:⚠️ Potential issue | 🟡 MinorNo length validation on incoming text.
The incoming
txtis assigned directly tomui_please_confirmwithout validating againstmax_restore_data_length: 25. If a longer string is received, the restore functionality may behave unexpectedly (truncation or partial restore).🛡️ Proposed fix to add length validation
- id: !extend action_component_text then: - lambda: |- if (page == "mem" and component == "mui_please_confirm") { - id(mui_please_confirm) = txt; + id(mui_please_confirm) = txt.substr(0, 25); }components/nspanel_easy/pages.h-57-58 (1)
57-58:⚠️ Potential issue | 🟡 MinorAdd a compile-time guard for
PAGE_COUNTrange.
Prevents silent overflow if page count ever exceeds 255.Proposed fix
constexpr size_t PAGE_COUNT = sizeof(page_names) / sizeof(page_names[0]); +static_assert(PAGE_COUNT <= UINT8_MAX, "PAGE_COUNT exceeds uint8_t range");components/nspanel_easy/text.cpp-135-158 (1)
135-158:⚠️ Potential issue | 🟡 MinorValidate UTF‑8 continuation bytes before decoding.
Currently, any non‑null continuation bytes are accepted, which allows malformed sequences to decode to bogus code points. A minimal continuation‑byte check avoids that.
✅ Proposed fix
if ((byte & 0x80) == 0x00) { code_point = byte; } else if ((byte & 0xE0) == 0xC0 && bytes[1] != '\0') { - code_point = ((byte & 0x1F) << 6) | (static_cast<unsigned char>(bytes[1]) & 0x3F); + unsigned char b1 = static_cast<unsigned char>(bytes[1]); + if ((b1 & 0xC0) != 0x80) return 0; + code_point = ((byte & 0x1F) << 6) | (b1 & 0x3F); } else if ((byte & 0xF0) == 0xE0 && bytes[1] != '\0' && bytes[2] != '\0') { - code_point = ((byte & 0x0F) << 12) | - ((static_cast<unsigned char>(bytes[1]) & 0x3F) << 6) | - (static_cast<unsigned char>(bytes[2]) & 0x3F); + unsigned char b1 = static_cast<unsigned char>(bytes[1]); + unsigned char b2 = static_cast<unsigned char>(bytes[2]); + if ((b1 & 0xC0) != 0x80 || (b2 & 0xC0) != 0x80) return 0; + code_point = ((byte & 0x0F) << 12) | + ((b1 & 0x3F) << 6) | + (b2 & 0x3F); } else if ((byte & 0xF8) == 0xF0 && bytes[1] != '\0' && bytes[2] != '\0' && bytes[3] != '\0') { - code_point = ((byte & 0x07) << 18) | - ((static_cast<unsigned char>(bytes[1]) & 0x3F) << 12) | - ((static_cast<unsigned char>(bytes[2]) & 0x3F) << 6) | - (static_cast<unsigned char>(bytes[3]) & 0x3F); + unsigned char b1 = static_cast<unsigned char>(bytes[1]); + unsigned char b2 = static_cast<unsigned char>(bytes[2]); + unsigned char b3 = static_cast<unsigned char>(bytes[3]); + if ((b1 & 0xC0) != 0x80 || (b2 & 0xC0) != 0x80 || (b3 & 0xC0) != 0x80) return 0; + code_point = ((byte & 0x07) << 18) | + ((b1 & 0x3F) << 12) | + ((b2 & 0x3F) << 6) | + (b3 & 0x3F);esphome/nspanel_esphome_page_cover.yaml-31-50 (1)
31-50:⚠️ Potential issue | 🟡 MinorStore temporary strings in variables to improve code clarity and robustness.
The
fire_ha_eventfunction accepts astd::map<std::string, std::string>, so temporary string expressions like("cover." + key).c_str()are copied immediately and safely. However, storing them in named variables first improves readability and reduces risk. Additionally, while the HAcover.set_cover_positionservice expects an integer 0–100, validating the position value before sending ensures early error detection.🔧 Suggested improvements
const std::string& value = params[2]; - ESP_LOGV("${TAG_PAGE_COVER}", "Setting position: %s", value.c_str()); - - fire_ha_event("action_call", { - {"action", "cover.set_cover_position"}, - {"key", key.c_str()}, - {"value", value.c_str()}, - {"entity", detailed_entity->state.c_str()} - }); + char* end = nullptr; + long pos = strtol(value.c_str(), &end, 10); + if (end == value.c_str() || *end != '\0' || pos < 0 || pos > 100) { + ESP_LOGW("${TAG_PAGE_COVER}", "Bad position: %s", value.c_str()); + return; + } + const std::string pos_str = std::to_string(pos); + ESP_LOGV("${TAG_PAGE_COVER}", "Setting position: %s", pos_str.c_str()); + + fire_ha_event("action_call", { + {"action", "cover.set_cover_position"}, + {"key", key.c_str()}, + {"value", pos_str.c_str()}, + {"entity", detailed_entity->state.c_str()} + }); } else { ESP_LOGV("${TAG_PAGE_COVER}", "Executing action: %s", key.c_str()); - fire_ha_event("action_call", { - {"action", ("cover." + key).c_str()}, - {"entity", detailed_entity->state.c_str()} - }); + const std::string action = "cover." + key; + fire_ha_event("action_call", { + {"action", action.c_str()}, + {"entity", detailed_entity->state.c_str()} + });esphome/nspanel_esphome_hw_memory.yaml-19-33 (1)
19-33:⚠️ Potential issue | 🟡 MinorRemove
CONFIG_ESP32_REV_MIN_3and use the now-availableminimum_chip_revisionoption.The
CONFIG_ESP32_REV_MIN_3setting gates compatibility to ESP32 revision 3+, which will break older hardware variants. Since ESPHome v2026.1.0 now supportsminimum_chip_revision, this temporary workaround should be replaced with the proper ESPHome configuration option instead of relying on an SDK config flag.🔧 Proposed adjustment
esp32: framework: advanced: loop_task_stack_size: 16384 + minimum_chip_revision: "3.0" sdkconfig_options: - CONFIG_ESP32_REV_MIN_3: y # Remove after implementing `minimum_chip_revision` CONFIG_BT_ALLOCATION_FROM_SPIRAM_FIRST: yesphome/nspanel_esphome_hw_memory.yaml-77-78 (1)
77-78:⚠️ Potential issue | 🟡 MinorConsider making
wifi.use_psramconfigurable or document the PSRAM requirement.The
use_psram: truesetting combined with the hardcoded PSRAM configuration assumes PSRAM is always available. While ESPHome gracefully ignores missing PSRAM by default (ignore_not_found: true), explicitly settingwifi.use_psram: truemay degrade WiFi performance on hardware without PSRAM. Since NSPanel is designed with PSRAM as a standard feature, this is unlikely to cause issues in practice, but making it configurable via substitution would be safer:substitutions: + WIFI_USE_PSRAM: 'true' ... wifi: - use_psram: true + use_psram: ${WIFI_USE_PSRAM}Alternatively, document that NSPanel firmware requires PSRAM-capable hardware.
.rules/markdownlint.yml-1-2 (1)
1-2:⚠️ Potential issue | 🟡 MinorAdd YAML document markers for consistency with repo lint rules.
The static analysis correctly flags missing
---(document-start) and...(document-end) markers. Since.rules/yamllint.ymlin this PR enables these rules, this file should follow the same convention.🔧 Proposed fix
+--- MD013: line_length: 200 +...docs/lovelace_ui.md-27-27 (1)
27-27:⚠️ Potential issue | 🟡 MinorTypo: "an you will" should be "and you will".
This typo appears in three places in the document.
Proposed fix
-If everything goes right, you will see the progress bar indicating the file transfer an you will end up with a black screen showing a QR code. +If everything goes right, you will see the progress bar indicating the file transfer and you will end up with a black screen showing a QR code.Apply similar fix to lines 35 and 55.
Also applies to: 35-35, 55-55
docs/lovelace_ui.md-5-5 (1)
5-5:⚠️ Potential issue | 🟡 MinorMinor typo: "chose" should be "choose".
"chose" is past tense; "choose" is the correct present tense form here.
Proposed fix
-If you are coming from there, you have to chose one of the 3 options: +If you are coming from there, you have to choose one of the 3 options:docs/different_version.md-78-78 (1)
78-78:⚠️ Potential issue | 🟡 MinorMinor grammar issue: "Double check" should be hyphenated.
When used as a verb phrase meaning "to verify again," "double-check" should be hyphenated.
Proposed fix
-4. Double check your `nextion_update_url` substitution is pointing to the right URL where the file you just downloaded is available. +4. Double-check your `nextion_update_url` substitution is pointing to the right URL where the file you just downloaded is available.docs/addon_climate.md-77-78 (1)
77-78:⚠️ Potential issue | 🟡 MinorSpelling error: "treshold" should be "threshold".
The word "threshold" is misspelled as "treshold" in both lines.
Proposed fix
-target_low|Optional|Number representing a temperature in the selected unit|`18` (°C) or `64` (°F)|The initial lower treshold for the target temperature. -target_high|Optional|Number representing a temperature in the selected unit|`24` (°C) or `75` (°F)|The initial higher treshold for the target temperature. +target_low|Optional|Number representing a temperature in the selected unit|`18` (°C) or `64` (°F)|The initial lower threshold for the target temperature. +target_high|Optional|Number representing a temperature in the selected unit|`24` (°C) or `75` (°F)|The initial higher threshold for the target temperature.docs/addon_climate.md-71-72 (1)
71-72:⚠️ Potential issue | 🟡 MinorTypo: "User" should be "Use".
In both relay configuration descriptions, "User
1" should be "Use1".Proposed fix
-cooler_relay|Mandatory for *cool* and *dual*|`1` or `2`|`0` (disabled)|Relay used for control the cooler. User `1` for "Relay 1" or `2` for "Relay 2". -heater_relay|Mandatory for *heat* and *dual*|`1` or `2`|`0` (disabled)|Relay used for control the heater. User `1` for "Relay 1" or `2` for "Relay 2". +cooler_relay|Mandatory for *cool* and *dual*|`1` or `2`|`0` (disabled)|Relay used to control the cooler. Use `1` for "Relay 1" or `2` for "Relay 2". +heater_relay|Mandatory for *heat* and *dual*|`1` or `2`|`0` (disabled)|Relay used to control the heater. Use `1` for "Relay 1" or `2` for "Relay 2".docs/addon_cover.md-87-87 (1)
87-87:⚠️ Potential issue | 🟡 MinorMinor grammar issue: compound adjective should be hyphenated.
"relay level time delay" should be "relay-level time delay" when used as a compound adjective.
Proposed fix
-|Cover interlock wait time|`1ms` to `5000ms`|`250ms`|Imposes a relay level time delay from one relay turning off until the other can turn on, preventing both relays from being on simultaneously.<br>***ATTENTION***, this is a software interlock, please read more about on [ESPHome Switch Interlocking](https://esphome.io/components/switch/gpio.html#interlocking) page| +|Cover interlock wait time|`1ms` to `5000ms`|`250ms`|Imposes a relay-level time delay from one relay turning off until the other can turn on, preventing both relays from being on simultaneously.<br>***ATTENTION***, this is a software interlock, please read more about it on the [ESPHome Switch Interlocking](https://esphome.io/components/switch/gpio.html#interlocking) page|docs/nspanel_blank.md-28-28 (1)
28-28:⚠️ Potential issue | 🟡 MinorTypo: "sucessfully" should be "successfully".
📝 Proposed fix
-Once you have sucessfully installed any of the `tft` files from this project, +Once you have successfully installed any of the `tft` files from this project,docs/tft_upload.md-40-40 (1)
40-40:⚠️ Potential issue | 🟡 MinorTypo: "direcly" should be "directly".
📝 Proposed fix
-At least one DNS server is required to enable TFT transfer direcly from GitHub, otherwise use `nextion_update_url`. +At least one DNS server is required to enable TFT transfer directly from GitHub, otherwise use `nextion_update_url`.docs/tft_upload.md-154-154 (1)
154-154:⚠️ Potential issue | 🟡 MinorTypo: "you local" should be "your local".
📝 Proposed fix
-There are many options available for free to run under you local Windows, MAC or Linux machine. +There are many options available for free to run under your local Windows, Mac, or Linux machine.docs/tft_upload.md-98-98 (1)
98-98:⚠️ Potential issue | 🟡 MinorTypo: "downlodaded" should be "downloaded".
📝 Proposed fix
-Compare this with the size listed on GitHub or from the file you downlodaded directly from GitHub. +Compare this with the size listed on GitHub or from the file you downloaded directly from GitHub.docs/tft_upload.md-146-146 (1)
146-146:⚠️ Potential issue | 🟡 MinorTypo: "commment" should be "comment".
📝 Proposed fix
- 1. Look your yaml file and commment out all the customization (typically on a section name "My customizations") and any of the remote files other than `nspanel_esphome.yaml`. + 1. Look at your yaml file and comment out all the customization (typically in a section named "My customizations") and any of the remote files other than `nspanel_esphome.yaml`.docs/nspanel_blank.md-3-17 (1)
3-17:⚠️ Potential issue | 🟡 MinorMultiple typos in the introductory section.
Several spelling errors detected:
- Line 3: "consist in" → "consists of"
- Line 10: "recoment" → "recommend"
- Line 16: "datails" → "details"
- Line 17: "refeer" → "refer"
📝 Proposed fixes
-The NSPanel consist in a ESP32 board (the panel itself, controlling the relays, buttons, communications, etc.) +The NSPanel consists of an ESP32 board (the panel itself, controlling the relays, buttons, communications, etc.)-which makes a bit challenging to replace the `tft` file when using ESPHome and then, we highly recoment you using `nspanel_blank.tft`, +which makes it a bit challenging to replace the `tft` file when using ESPHome, so we highly recommend using `nspanel_blank.tft`,-For more datails on how to install the first `tft` file, specially if your panel still displaying the original Sonoff screen, -please refeer to the [Troubleshooting TFT transfer](tft_upload.md) guide. +For more details on how to install the first `tft` file, especially if your panel is still displaying the original Sonoff screen, +please refer to the [Troubleshooting TFT transfer](tft_upload.md) guide.components/nspanel_easy/hardware.h-48-50 (1)
48-50:⚠️ Potential issue | 🟡 Minor
reinterpret_castviolates strict aliasing rules (undefined behavior).Casting between
uint8_t&andHardwareSettings&viareinterpret_castis technically undefined behavior per the C++ standard. Additionally, bit-field ordering is implementation-defined, so the struct layout may not match the expected bit positions on all compilers.For the ESP32/Xtensa toolchain this likely works as intended, but consider using explicit bit manipulation for portability:
🛡️ Safer alternative using explicit bit access
+ inline HardwareSettings from_raw(uint8_t raw) { + HardwareSettings s; + s.button_left_enabled = (raw >> 0) & 1; + s.button_left_state = (raw >> 1) & 1; + s.button_right_enabled = (raw >> 2) & 1; + s.button_right_state = (raw >> 3) & 1; + s.relay1_local = (raw >> 4) & 1; + s.relay1_fallback = (raw >> 5) & 1; + s.relay2_local = (raw >> 6) & 1; + s.relay2_fallback = (raw >> 7) & 1; + return s; + } + + inline uint8_t to_raw(const HardwareSettings& s) { + return (s.button_left_enabled << 0) | + (s.button_left_state << 1) | + (s.button_right_enabled << 2) | + (s.button_right_state << 3) | + (s.relay1_local << 4) | + (s.relay1_fallback << 5) | + (s.relay2_local << 6) | + (s.relay2_fallback << 7); + }components/nspanel_easy/nextion_components.cpp-9-28 (1)
9-28:⚠️ Potential issue | 🟡 MinorDocstring claims unimplemented special case handling for
alarm_control_panel.The header docstring states this function "Handles a special case for 'alarm_control_panel' by shortening it to 'alarm'", but the implementation lacks this logic. The code performs only standard extraction without any transformation for
alarm_control_panel. Either implement the special case or update the docstring to match the actual behavior.docs/customization.md-540-543 (1)
540-543:⚠️ Potential issue | 🟡 MinorReplace informal wording.
“wanna” reads informal in docs.
✏️ Suggested edit
-making it ideal if you wanna customize your panel to support memory consumption functionalities, +making it ideal if you want to customize your panel to support memory consumption functionalities,docs/customization.md-44-61 (1)
44-61:⚠️ Potential issue | 🟡 MinorFix typos/grammar in the intro.
These lines contain a few clarity/grammar issues (“intents”, “customation”, “contains”, “bellow”) that read as mistakes.
✏️ Suggested edits
-or even to customize an existing functionality with minimum effort and this document intents to clarify how to use this and give some examples of customization. +or even to customize an existing functionality with minimum effort, and this document intends to clarify how to use this and give some examples of customization. -Please feel free to add your own customation to this document by creating a PR in the `dev` branch. +Please feel free to add your own customization to this document by creating a PR in the `dev` branch. -Most of the ESPHome components in this project contains an `Id`, which can be used together with the `!extend` key to add or replace existing code. +Most of the ESPHome components in this project contain an `Id`, which can be used together with the `!extend` key to add or replace existing code. -You should add your customizations at the end of your ESPHome yaml, as in the example bellow: +You should add your customizations at the end of your ESPHome YAML, as in the example below:esphome/nspanel_esphome_addon_climate_base.yaml-156-174 (1)
156-174:⚠️ Potential issue | 🟡 MinorAllow relay value
0in validation logging.
heater_relay/cooler_relayexplicitly allow0as a dummy, butESP_LOGE_IFcurrently treats it as an error. That’s confusing for valid configs.🛠️ Suggested fix
- ESP_LOGE_IF("${TAG_ADDON_CLIMATE}", (cooler_relay != 1 and cooler_relay != 2), " Relay: %u", cooler_relay); + ESP_LOGE_IF("${TAG_ADDON_CLIMATE}", (cooler_relay != 0 and cooler_relay != 1 and cooler_relay != 2), " Relay: %u", cooler_relay); @@ - ESP_LOGE_IF("${TAG_ADDON_CLIMATE}", (heater_relay != 1 and heater_relay != 2), " Relay: %u", heater_relay); + ESP_LOGE_IF("${TAG_ADDON_CLIMATE}", (heater_relay != 0 and heater_relay != 1 and heater_relay != 2), " Relay: %u", heater_relay); @@ - ESP_LOGE_IF("${TAG_ADDON_CLIMATE}", (cooler_relay != 1 and cooler_relay != 2), " Relay (cooler): %u", cooler_relay); + ESP_LOGE_IF("${TAG_ADDON_CLIMATE}", (cooler_relay != 0 and cooler_relay != 1 and cooler_relay != 2), " Relay (cooler): %u", cooler_relay); @@ - ESP_LOGE_IF("${TAG_ADDON_CLIMATE}", (heater_relay != 1 and heater_relay != 2), " Relay (heater): %u", heater_relay); + ESP_LOGE_IF("${TAG_ADDON_CLIMATE}", (heater_relay != 0 and heater_relay != 1 and heater_relay != 2), " Relay (heater): %u", heater_relay);docs/customization.md-349-365 (1)
349-365:⚠️ Potential issue | 🟡 MinorClean up typos in the time zone + compiling sections.
Several typos reduce clarity (“it’s”, “probabily”, “lower powered”, “datails”).
✏️ Suggested edits
-Until v3.4 (including), the time was coming from Home Assistant with it's timezone, so the Blueprint was sending the info with no transformation, to the panel. +Until v3.4 (including), the time was coming from Home Assistant with its timezone, so the Blueprint was sending the info with no transformation, to the panel. -If your system is not showing the time in the correct timezone, it's probabily ESPHome not succeeding on finding your time zone. +If your system is not showing the time in the correct timezone, it's probably ESPHome not succeeding on finding your time zone. -For systems with lower CPU or memory capabilities, like an RPi 3 or systems with less than 2GB of RAM, this could help preventing errors caused by lack of resources when compiling ESPHome firmware. +For systems with lower CPU or memory capabilities, like an RPi 3 or systems with less than 2GB of RAM, this could help preventing errors caused by lack of resources when compiling ESPHome firmware. -More datails on the [ESPHome docs](https://esphome.io/changelog/2022.11.0.html#running-esphome-on-lower-powered-machines). +More details on the [ESPHome docs](https://esphome.io/changelog/2022.11.0.html#running-esphome-on-lower-powered-machines).docs/customization.md-219-246 (1)
219-246:⚠️ Potential issue | 🟡 MinorFix a few grammar/typo issues in the API failure + manual IP sections.
“it’s” → “its”, “low level” → “low‑level”, and “direcly” → “directly”.
✏️ Suggested edits
-Reboot your panel if it loses it's connection to Home Assistant for more than a certain time (15 minutes in this example). +Reboot your panel if it loses its connection to Home Assistant for more than a certain time (15 minutes in this example). -Sometimes the low level ESP functions could report that the ESP is connected to the network, when in fact it is not and only a full reboot fixes it. +Sometimes the low-level ESP functions could report that the ESP is connected to the network, when in fact it is not and only a full reboot fixes it. -> At least one DNS server is required to enable TFT transfer direcly from GitHub, otherwise use `nextion_update_url`. +> At least one DNS server is required to enable TFT transfer directly from GitHub, otherwise use `nextion_update_url`.components/nspanel_easy/base.h-4-9 (1)
4-9:⚠️ Potential issue | 🟡 MinorMake the header self-contained by including
<string>and<map>.
cached_device_nameandfire_ha_eventusestd::stringandstd::map, but those headers are not included here. Relying on transitive includes can cause build breaks.🔧 Suggested fix
`#include` "esphome/core/hal.h" // For delay() `#include` "esphome/core/log.h" `#include` <cstdint> +#include <map> +#include <string>components/nspanel_easy/page_utilities.cpp-49-67 (1)
49-67:⚠️ Potential issue | 🟡 MinorMissing null check before accessing
UtilitiesGroups.If
resetUtilitiesGroups()allocation fails completely (both PSRAM and fallback),UtilitiesGroupswill benullptr. The binary search then dereferences a null pointer, causing a crash.🛡️ Suggested defensive fix
uint8_t findUtilitiesGroupIndex(const char* group_id) { + if (UtilitiesGroups == nullptr) return UINT8_MAX; + int low = 0; int high = 7; // Directly use the number of elements in UtilitiesGroups - 1docs/error_initializing.md-3-41 (1)
3-41:⚠️ Potential issue | 🟡 MinorFix a few grammar/typo issues for clarity.
Notable spots: Line 3 (“needs” → “need”, add “a” before “connection”), Line 28 (“it's” → “its”), Line 30 (“replace” → “replaced”), Line 35 (“happens” → “happen”), Line 39 (“bellow” → “below”).✍️ Proposed fixes
-When your panel starts, a number of different things needs to be set and the ESPHome must establish connection to the Nextion display used by your panel +When your panel starts, a number of different things need to be set and ESPHome must establish a connection to the Nextion display used by your panel @@ -In the top, you will see the message "Initializing..." while the display is executing it's boot sequence or waiting for a connection from ESPHome. +At the top, you will see the message "Initializing..." while the display is executing its boot sequence or waiting for a connection from ESPHome. @@ -Once ESPHome is connected to both the display and the Wi-Fi network, that text will be replace by the IP address, which can be useful for troubleshooting. +Once ESPHome is connected to both the display and the Wi-Fi network, that text will be replaced by the IP address, which can be useful for troubleshooting. @@ -By default, all the communication should happens at 115200 bps, but as 921600 bps is also supported, the display will alternate between these 2 rates every 30 seconds until ESPHome gets connected. +By default, all the communication should happen at 115200 bps, but as 921600 bps is also supported, the display will alternate between these 2 rates every 30 seconds until ESPHome gets connected. @@ -Right bellow the baud rate information, you may see the framework used by ESPHome, which is either `esp-idf` (default for new installations) or `arduino` (default for installations prior v4.1.4). +Right below the baud rate information, you may see the framework used by ESPHome, which is either `esp-idf` (default for new installations) or `arduino` (default for installations prior v4.1.4).components/nspanel_easy/ha_components.cpp-8-20 (1)
8-20:⚠️ Potential issue | 🟡 MinorHandle empty domain/id edge cases.
When the dot is at the start or end,domainoridbecomes empty. Treat those as invalid too.🔧 Proposed fix
- if (dotPos != std::string::npos) { + if (dotPos != std::string::npos && dotPos > 0 && dotPos + 1 < entity_id.size()) { // Extract domain and id from the entity_id string result.domain = entity_id.substr(0, dotPos); result.id = entity_id.substr(dotPos + 1); } else { // No dot found, the entire entity_id is considered as id. result.domain = "invalid"; result.id = entity_id; }docs/howto.md-179-183 (1)
179-183:⚠️ Potential issue | 🟡 MinorNon-standard GitHub admonition type.
[!ATTENTION]is not a standard GitHub Flavored Markdown admonition. GitHub supports:NOTE,TIP,IMPORTANT,WARNING, andCAUTION. Consider using[!WARNING]or[!IMPORTANT]instead.Proposed fix
-> [!ATTENTION] +> [!WARNING] > Although these instructions are still valid, since v4.0 there is a better way to setup a climate control using the panel's relays an the [add-on climate](addon_climate.md),docs/blueprint.md-72-73 (1)
72-73:⚠️ Potential issue | 🟡 MinorMinor grammatical issues.
- Line 72: "displayed then the entity" should be "displayed when the entity"
- Line 73: "etc)" should be "etc.)" with a period before the closing parenthesis
Proposed fix
-| Home page - Chips 01-07 (optional) | Used to display icons of HA binary entities on the home screen. The icon is displayed then the entity is on/open/true/active. Otherwise, no icon is displayed. |  | -| Custom button 01-06 (Optional) | 7 custom buttons that can be assigned to different entities from your Home Assistant. The behavior of these buttons will depend on the entity's domain (light, media player, etc). |  | +| Home page - Chips 01-07 (optional) | Used to display icons of HA binary entities on the home screen. The icon is displayed when the entity is on/open/true/active. Otherwise, no icon is displayed. |  | +| Custom button 01-06 (Optional) | 7 custom buttons that can be assigned to different entities from your Home Assistant. The behavior of these buttons will depend on the entity's domain (light, media player, etc.). |  |README.md-67-67 (1)
67-67:⚠️ Potential issue | 🟡 MinorFix typos (“Forcast” / “it’s”).
Minor doc corrections improve clarity.
✏️ Suggested edits
- - Current weather with button to Weather Forcast page + - Current weather with button to Weather Forecast page @@ -On the device's page under "Devices & Services" you can set the global configuration of your device and see the value of it's sensors and relays: +On the device's page under "Devices & Services" you can set the global configuration of your device and see the value of its sensors and relays:Also applies to: 177-177
docs/install.md-18-18 (1)
18-18:⚠️ Potential issue | 🟡 MinorFix spelling typos in user-facing docs.
These are small but visible in onboarding flows.
✏️ Suggested edits
-> Some images or instructions on this guide might be slighly different due to different configurations depending on your ESPHome and Home Assistant implementation or versions. +> Some images or instructions on this guide might be slightly different due to different configurations depending on your ESPHome and Home Assistant implementation or versions. @@ -If that isn't happnening, you can manually add it as a new intgration with the following steps: +If that isn't happening, you can manually add it as a new integration with the following steps: @@ -The installation in a NSPanel with Sonoff's original firmware should be straighforward, however, +The installation in a NSPanel with Sonoff's original firmware should be straightforward, however, @@ -Although it's not a functional TFT for controlling your panel, it can be usefull when you have +Although it's not a functional TFT for controlling your panel, it can be useful when you have @@ -installed on it's normal (portrait) position with the buttons bellow the screen. +installed on it's normal (portrait) position with the buttons below the screen. @@ -installed on it's normal (portrait) position with the buttons bellow the screen, +installed on it's normal (portrait) position with the buttons below the screen, @@ -If after this time it isn't close to finnish, please cancel the process by restarting your device, then try it again. +If after this time it isn't close to finish, please cancel the process by restarting your device, then try it again.Also applies to: 263-263, 317-317, 364-375, 391-391
esphome/nspanel_esphome_page_screensaver.yaml-3-3 (1)
3-3:⚠️ Potential issue | 🟡 MinorMinor typo in header comment.
Line 3 has an extra
#character at the end compared to other header lines, making the comment block asymmetric.📝 Fix header alignment
-###################################################################################################### +#####################################################################################################esphome/nspanel_esphome_page_utilities.yaml-60-65 (1)
60-65:⚠️ Potential issue | 🟡 MinorRemove the ineffective
!isnan(direction)check.The variable
directionis declared asinttype, butisnan()is designed for floating-point numbers. When called on an integer, it implicitly converts to double and always returnsfalsesince integers cannot represent NaN. This makes!isnan(direction)alwaystrue, rendering the check ineffective.Since the actual values passed are
-3,0, or3(where0is the initialized/default state), the check provides no filtering. Remove it or replace with appropriate logic if a specific "unset" state is needed:Suggested fix: Remove the ineffective check
// Update direction - if (group_id != "grid" and !isnan(direction) and + if (group_id != "grid" and (constructor or direction != UtilitiesGroups[id].direction)) {docs/api.md-688-688 (1)
688-688:⚠️ Potential issue | 🟡 MinorTypo: "bellow" should be "below".
-The example bellow integrates the `wake_up` action with a motion sensor to ensure the display wakes or remains awake during periods of activity, reverting to sleep mode after inactivity. +The example below integrates the `wake_up` action with a motion sensor to ensure the display wakes or remains awake during periods of activity, reverting to sleep mode after inactivity.docs/api.md-743-743 (1)
743-743:⚠️ Potential issue | 🟡 MinorTypo: "exclusivelly" should be "exclusively".
-where `value01_icon` to `value08_icon` supports the icons and, exclusivelly in the Entities pages, `value01_label` to `value08_label`, +where `value01_icon` to `value08_icon` supports the icons and, exclusively in the Entities pages, `value01_label` to `value08_label`,esphome/nspanel_esphome_version.yaml-43-98 (1)
43-98:⚠️ Potential issue | 🟡 MinorFix the blueprint minimum-version log message.
The error log inside the blueprint version check references
min_tft_versioninstead ofmin_blueprint_version:🔧 Suggested fix
- ESP_LOGE("${TAG_VERSIONING}", "v${min_tft_version} or later is required"); + ESP_LOGE("${TAG_VERSIONING}", "v${min_blueprint_version} or later is required");esphome/nspanel_esphome_version.yaml-134-145 (1)
134-145:⚠️ Potential issue | 🟡 MinorThrottle blueprint requests as intended.
if (iteration % 9)triggers on 8/9 iterations instead of once per cycle. This executes the blueprint request approximately 53 times over 60 seconds, which contradicts the throttling pattern shown in the log check above (iteration % 5 == 1). Change toif (iteration % 9 == 1)to request blueprint settings once per 9 iterations, aligning with the intended cadence.🔧 Suggested fix
- if (iteration % 9) boot_request_blueprint_settings->execute("timeout"); + if (iteration % 9 == 1) boot_request_blueprint_settings->execute("timeout");esphome/nspanel_esphome_page_settings.yaml-72-103 (1)
72-103:⚠️ Potential issue | 🟡 MinorGuard
paramsindexing and fix the log typo.
params[0]is accessed before validatingparams_count. Ifparams_countis 0, this causes an out-of-bounds access. Additionally, the log message contains a typo: "Bed params" should be "Bad params".🔧 Suggested fix
- if (params[0] == "settings") { + if (params_count == 0) { + ESP_LOGW("${TAG_PAGE_SETTINGS}", "Bad params"); + return; + } + if (params[0] == "settings") { @@ - if (params_count != 3) { - ESP_LOGW("${TAG_PAGE_SETTINGS}", "Bed params"); + if (params_count != 3) { + ESP_LOGW("${TAG_PAGE_SETTINGS}", "Bad params");esphome/nspanel_esphome_hw_relays.yaml-53-67 (1)
53-67:⚠️ Potential issue | 🟡 MinorGuard against empty string before indexing for the decimal separator.
Line 58 accesses
txt[0]without checking iftxtis empty, which can cause undefined behavior. Add a safety check before indexing:Suggested fix
if (component == "decimal_separator") { - id(mui_decimal_separator) = txt[0]; + if (!txt.empty()) id(mui_decimal_separator) = txt[0];esphome/nspanel_esphome_page_entities.yaml-102-122 (1)
102-122:⚠️ Potential issue | 🟡 MinorAlways apply alignment on page entry to prevent stale display state.
The guard condition
page_entities_value_horizontal_alignment != 1prevents sending alignment commands when the value is 1 (the default). If a user changes alignment and then resets it to 1, returning to an entity page will not update the display because the guard blocks the command. Remove the condition to ensure alignment is always synchronized on page entry.🔧 Suggested fix
- if ( - page_entities_value_horizontal_alignment != 1 and - ( + if ( + ( current_page_id == ${PAGE_ENTITYPAGE01_ID} or current_page_id == ${PAGE_ENTITYPAGE02_ID} or current_page_id == ${PAGE_ENTITYPAGE03_ID} or current_page_id == ${PAGE_ENTITYPAGE04_ID} ) ) {esphome/nspanel_esphome_page_media_player.yaml-199-262 (1)
199-262:⚠️ Potential issue | 🟡 MinorGuard params indexing and clamp volume range.
params[0]is accessed before validatingparams_count, which can read out-of-bounds on malformed events. Also,volume_setconverts a percentage to the 0.0–1.0 range expected by Home Assistant but does not clamp, so invalid input could result in values outside this range.🔧 Suggested fix
- lambda: |- + if (params_count == 0) { + ESP_LOGW("${TAG_PAGE_MEDIA_PLAYER}", "Bad params"); + return; + } if (params[0] == "${PAGE_MEDIA_PLAYER_NAME}") { @@ float value_float = atof(params[2].c_str()) / 100.0f; + if (value_float < 0.0f) value_float = 0.0f; + else if (value_float > 1.0f) value_float = 1.0f;esphome/nspanel_esphome_hw_wifi.yaml-154-156 (1)
154-156:⚠️ Potential issue | 🟡 MinorMinor typo in comment: "bluepring" should be "blueprint".
text_sensor: - - id: device_name # Device name - Used by bluepring to find service's names + - id: device_name # Device name - Used by blueprint to find service's names name: Device Nameesphome/nspanel_esphome_boot.yaml-34-36 (1)
34-36:⚠️ Potential issue | 🟡 MinorInconsistent tag usage: missing substitution syntax.
Line 35 uses
"TAG_BOOT"as a literal string instead of the substitution"${TAG_BOOT}"used elsewhere (e.g., lines 128, 134, 144).Proposed fix
then: - lambda: |- - ESP_LOGI("TAG_BOOT", "Starting boot sequence"); + ESP_LOGI("${TAG_BOOT}", "Starting boot sequence"); blueprint_status_flags.reset();esphome/nspanel_esphome_page_climate.yaml-203-205 (1)
203-205:⚠️ Potential issue | 🟡 MinorPotential issue if
mui_decimal_separatoris empty.Line 203 accesses
id(mui_decimal_separator)[0]without checking if the string is empty. If empty, this could cause undefined behavior.Consider adding a guard or ensuring the string is always initialized with a default value.
Proposed defensive fix
- const char dec_separator_str[2] = {id(mui_decimal_separator)[0], '\0'}; + const char dec_sep = id(mui_decimal_separator).empty() ? '.' : id(mui_decimal_separator)[0]; + const char dec_separator_str[2] = {dec_sep, '\0'};esphome/nspanel_esphome_hw_display.yaml-197-200 (1)
197-200:⚠️ Potential issue | 🟡 MinorSame format specifier issue:
%Sshould be%s.Line 199 uses
%Sfor ac_str()which returnsconst char*. This should be%s.🐛 Proposed fix
- lambda: |- ESP_LOGV("${TAG_HW_DISPLAY}", "New event from display:"); - ESP_LOGV("${TAG_HW_DISPLAY}", " page: %S", params[0].c_str()); + ESP_LOGV("${TAG_HW_DISPLAY}", " page: %s", params[0].c_str()); ESP_LOGV("${TAG_HW_DISPLAY}", " params: %d", params_count);
…rent`, when setting the light brightness
…type is `uint16_t`
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@esphome/nspanel_esphome_addon_display_light.yaml`:
- Around line 35-46: The write_action for output id display_output is using
display_light->current_values to compute brightness which can have timing
issues; instead use the provided state parameter (0.0–1.0) directly: compute
target_brightness as uint8_t(int(round(state * 100.0f))) and pass that to
set_brightness->execute; also update the ESP_LOGV lines to log the state and the
computed target_brightness (use TAG_ADDON_DISPLAY_LIGHT for context) so you no
longer depend on display_light->current_values in the write_action.
In `@esphome/nspanel_esphome_api.yaml`:
- Around line 266-273: The three actions (action_component_color,
action_component_text, action_component_val) document a special "mem" page but
never handle it; update each function to add an explicit branch for page ==
"mem" that writes the value to the display memory variable instead of falling
through—use the same identifiers in the diff (page, component, color/text/value,
disp1) and either call the display memory write API (the equivalent memory-set
operation on disp1 or a send_command_printf that targets the memory variable)
or, if memory variables are unsupported, remove "mem" from the documentation and
tests; ensure the new branch returns after handling just like the existing
non-"mem" page branches.
- Around line 323-333: The recovery loop in the automations (id:
api_recover_from_failed) incorrectly calls api_server->setup() repeatedly;
instead remove repeated setup() calls and implement one of the supported
recovery approaches: use on_client_connected/on_client_disconnected triggers to
re-subscribe or re-announce, or call Component::reset_to_construction_state()
once to safely tear down and reinitialize the API component, or rely on
ESPHome's reboot_timeout/safe mode for stuck failed states; replace the
while/looped api_server->setup() usage with one of these documented alternatives
and ensure no repeated calls to api_server->setup().
🧹 Nitpick comments (5)
docs/different_version.md (1)
77-78: Polish hyphenation and casing for clarity.“http server” and “Double check” read cleaner as “HTTP server” and “Double-check.”
✏️ Suggested edit
-3. Locate the TFT file related to your model and download it to your local http server. -4. Double check your `nextion_update_url` substitution is pointing to the right URL where the file you just downloaded is available. +3. Locate the TFT file related to your model and download it to your local HTTP server. +4. Double-check your `nextion_update_url` substitution is pointing to the right URL where the file you just downloaded is available.docs/alarm.md (1)
64-73: Reduce repetitive sentence starts in the state descriptions.Consider varying phrasing to improve readability (e.g., “Alarm armed in away mode,” “Alarm arming,” etc.).
docs/customization.md (1)
41-47: Fix typos and informal wording for professionalism.Suggested edits below.
✏️ Suggested edits
-This project adds lots of functionalities to your NSPanel and we are constantly adding new features based on user's feedback. +This project adds many features to your NSPanel, and we are constantly adding new ones based on user feedback. -However, you might have some specific case that are not included on the current implementation or is not a common case for other users. +However, you might have specific cases that are not included in the current implementation or are uncommon for other users. -...and this document intents to clarify how to use this and give some examples of customization. +...and this document intends to clarify how to use this and give some examples of customization. -Please feel free to add your own customation to this document by creating a PR in the `dev` branch. +Please feel free to add your own customization to this document by creating a PR in the `dev` branch. -You should add your customizations at the end of your ESPHome yaml, as in the example bellow: +You should add your customizations at the end of your ESPHome YAML, as in the example below: -Reboot your panel if it loses it's connection to Home Assistant for more than a certain time (15 minutes in this example). +Reboot your panel if it loses its connection to Home Assistant for more than a certain time (15 minutes in this example). -Sometimes the low level ESP functions could report that the ESP is connected to the network, when in fact it is not... +Sometimes the low-level ESP functions could report that the ESP is connected to the network, when in fact it is not... -> At least one DNS server is required to enable TFT transfer direcly from GitHub... +> At least one DNS server is required to enable TFT transfer directly from GitHub... -If your system is not showing the time in the correct timezone, it's probabily ESPHome... +If your system is not showing the time in the correct timezone, it's probably ESPHome... -For systems with lower CPU or memory capabilities, like an RPi 3 or systems with less than 2GB of RAM, this could help preventing errors... +For systems with lower CPU or memory capabilities, like an RPi 3 or systems with less than 2GB of RAM, this could help prevent errors... -making it ideal if you wanna customize your panel to support memory consumption functionalities, +making it ideal if you want to customize your panel to support memory‑intensive functionality,Also applies to: 60-60, 219-246, 353-365, 542-542
components/nspanel_easy/text.cpp (2)
3-7: Unused<algorithm>include.The
<algorithm>header is included but no algorithms from it appear to be used in this file. Consider removing it to keep includes minimal.🧹 Proposed fix
`#include` "text.h" -#include <algorithm> `#include` <cctype> `#include` <cstdlib> `#include` <cstring>
144-154: Missing validation of UTF-8 continuation bytes.The decoder checks for null terminators but doesn't verify that continuation bytes have the required
10xxxxxxpattern. Malformed input like0xC0 0x41would be incorrectly decoded instead of being rejected.If the input source is trusted (e.g., always valid UTF-8 from ESPHome), this is acceptable. Otherwise, consider adding continuation byte validation:
🛡️ Proposed fix with continuation byte validation
+ // Helper to check if byte is a valid UTF-8 continuation byte (10xxxxxx) + auto is_continuation = [](unsigned char b) { return (b & 0xC0) == 0x80; }; + uint32_t code_point = 0; unsigned char byte = static_cast<unsigned char>(bytes[0]); if ((byte & 0x80) == 0x00) { code_point = byte; - } else if ((byte & 0xE0) == 0xC0 && bytes[1] != '\0') { + } else if ((byte & 0xE0) == 0xC0 && bytes[1] != '\0' && is_continuation(bytes[1])) { code_point = ((byte & 0x1F) << 6) | (static_cast<unsigned char>(bytes[1]) & 0x3F); - } else if ((byte & 0xF0) == 0xE0 && bytes[1] != '\0' && bytes[2] != '\0') { + } else if ((byte & 0xF0) == 0xE0 && bytes[1] != '\0' && bytes[2] != '\0' && + is_continuation(bytes[1]) && is_continuation(bytes[2])) { code_point = ((byte & 0x0F) << 12) | ((static_cast<unsigned char>(bytes[1]) & 0x3F) << 6) | (static_cast<unsigned char>(bytes[2]) & 0x3F); - } else if ((byte & 0xF8) == 0xF0 && bytes[1] != '\0' && bytes[2] != '\0' && bytes[3] != '\0') { + } else if ((byte & 0xF8) == 0xF0 && bytes[1] != '\0' && bytes[2] != '\0' && bytes[3] != '\0' && + is_continuation(bytes[1]) && is_continuation(bytes[2]) && is_continuation(bytes[3])) { code_point = ((byte & 0x07) << 18) | ((static_cast<unsigned char>(bytes[1]) & 0x3F) << 12) | ((static_cast<unsigned char>(bytes[2]) & 0x3F) << 6) | (static_cast<unsigned char>(bytes[3]) & 0x3F);
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Fix all issues with AI agents
In `@docs/blueprint.md`:
- Line 73: Update the markdown table row text that reads "The behavior of these
buttons will depend on the entity's domain (light, media player, etc)" to
include a period after "etc" so it becomes "etc." — locate and edit the table
cell in docs/blueprint.md (the row starting with "Custom button 01-06
(Optional)") and add the period.
- Line 72: Update the grammar in the table row for "Home page - Chips 01-07
(optional)" in docs/blueprint.md: change the phrase "The icon is displayed then
the entity is on/open/true/active." to use "when" instead of "then" so it reads
"The icon is displayed when the entity is on/open/true/active."; locate the
sentence in the same table row and replace "then" with "when".
In `@docs/error_initializing.md`:
- Line 39: Replace the misspelled word "bellow" with "below" in the sentence
that currently reads "Right bellow the baud rate information, you may see the
framework used by ESPHome..." so it reads "Right below the baud rate
information, you may see the framework used by ESPHome..." to correct the typo;
update the exact phrase in docs/error_initializing.md where that sentence
appears.
In `@docs/install.md`:
- Line 18: Fix the typo in the sentence contained in the docs/install.md content
("slighly" → "slightly"); update the note text accordingly so it reads "...might
be slightly different due to different configurations..." and save the change
(e.g., edit the string in the paragraph where that sentence appears).
- Line 366: Replace the incorrect word "bellow" with "below" in the sentence
"installed on it's normal (portrait) position with the buttons bellow the
screen," in docs/install.md so it reads "...with the buttons below the screen,";
locate the sentence text to find the exact spot to edit.
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 Fix all issues with AI agents
In `@docs/blueprint.md`:
- Around line 24-26: Change the phrase "You cannot have more than one blueprint
based automation per each panel" to hyphenate the compound adjective: replace
"blueprint based" with "blueprint-based" wherever that exact phrase appears
(e.g., in the sentence "You cannot have more than one blueprint based automation
per each panel") so it reads "You cannot have more than one blueprint-based
automation per each panel".
- Around line 215-216: Replace the misspelled word "aligment" with the correct
"alignment" in the sentence that reads "Of course there is also the possibility
to enter a label for each of the 4 pages and the aligment for the values on the
entities pages." so it reads "...and the alignment for the values on the
entities pages." Ensure only the typo is fixed and punctuation/spacing remains
consistent in the surrounding text.
In `@docs/error_initializing.md`:
- Around line 28-36: The text contains grammatical errors: change "it's" to the
possessive "its" and fix verb agreement and plurality (e.g., "will be replace" →
"will be replaced", "should happens" → "should happen", "as 921600 bps is also
supported, the display will alternate between these 2 rates every 30 seconds
until ESPHome gets connected" → ensure consistent plural/tense like "and,
because 921600 bps is also supported, the display alternates between these two
rates every 30 seconds until ESPHome connects"). Update the sentences around the
"Initializing..." message and the "Baud rate (bps)" paragraph to use "its" for
possession, correct past participles ("replaced"), correct verb forms ("happen",
"alternates", "connects"), and spell out "two" for consistency.
In `@docs/install.md`:
- Around line 375-383: Fix spelling and grammar in the TFT upload section by
updating phrases: change "correspondent" to "corresponding", "If the transfer
don't starts" to "If the transfer doesn't start", "starts in a few seconds" to
"start within a few seconds", "percentage counter with a black background" to
"percentage counter on a black background" and "finnish" to "finish" (or
rephrase as "close to finishing"); also rework punctuation and contractions for
consistency ("doesn't" or "does not") and apply the same corrections to the
other occurrences noted (sections covering lines 355-360 and 366-367) so the
wording reads clearly and grammatically correct throughout the TFT upload
section.
- Around line 260-276: Fix multiple typos and grammar in the "Integrating your
panel to Home Assistant" section: change "Once your panel loaded the new ESPHome
firmware" to "Once your panel has loaded the new ESPHome firmware", "happnening"
→ "happening", "intgration" → "integration", "Add device" sentence wording as
needed to read clearly, change "Device's page" → "Device page", and replace all
instances of "it's" used as possessive with "its" (e.g., "which can be use to
see it's status" → "which can be used to see its status"). Ensure the tip and
numbered steps retain the same meaning but correct these spelling/grammar
issues.
In `@esphome/nspanel_esphome_hw_wifi.yaml`:
- Line 155: Fix the typo in the inline comment next to the id: device_name entry
— change "bluepring" to "blueprint" so the comment reads "Device name - Used by
blueprint to find service's names".
- Around line 175-178: The YAML currently invokes update_flag_api_ready twice:
once via the script.execute: update_flag_api_ready entry and again inside the
lambda calling update_flag_api_ready->execute(); remove one of these redundant
calls—either delete the script.execute: update_flag_api_ready line or remove the
update_flag_api_ready->execute(); call inside the lambda so
update_flag_api_ready is executed only once; ensure logic around
cached_device_name.empty() remains unchanged.
🧹 Nitpick comments (2)
esphome/nspanel_esphome_hw_wifi.yaml (1)
183-185: Inconsistent indentation on line 184.There's an extra leading space before
last_was_underscore = false;compared to the surrounding lines.if (std::isalnum(uc)) { cached_device_name += static_cast<char>(std::tolower(uc)); // Add alphanumeric characters as lowercase - last_was_underscore = false; + last_was_underscore = false; } else if (!last_was_underscore) { // Replace non-alphanumeric with '_' but avoid consecutive '_'esphome/nspanel_esphome_version.yaml (1)
75-76: Clarify the warning message.The warning message "TFT Blueprint mismatch!" is misleading. This check compares the Blueprint version against the ESPHome version, not the TFT version. Consider clarifying:
Proposed fix
if (version_blueprint->state != "${version}") { - ESP_LOGW("${TAG_VERSIONING}", "TFT Blueprint mismatch!"); + ESP_LOGW("${TAG_VERSIONING}", "Blueprint version mismatch!");
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Fix all issues with AI agents
In `@components/nspanel_easy/hardware.h`:
- Around line 45-61: get_hardware_settings and set_hardware_settings call
from_raw and to_raw before those functions are declared, causing a compile
error; add forward declarations for from_raw(uint8_t) -> HardwareSettings and
to_raw(const HardwareSettings&) -> uint8_t above these inline functions (or move
the full definitions above them) so the compiler sees their signatures when
compiling get_hardware_settings and set_hardware_settings; ensure the declared
signatures match the actual definitions and reference the HardwareSettings type.
In `@docs/customization.md`:
- Line 301: Change the possessive "it's" to "its" in the sentence currently
reading "ESPHome takes it's time from Home Assistant, however you can configure
it to use a Network Time Server instead." so it reads "ESPHome takes its time
from Home Assistant, however you can configure it to use a Network Time Server
instead." Update the documentation text wherever that exact phrase appears (the
line containing "ESPHome takes it's time from Home Assistant") to fix the
grammatical error.
- Around line 779-809: The YAML for the relay2_local template uses an invalid
single-line lambda syntax ("lambda: return ..."); update relay2_local's lambda
to be a proper multiline block (or a single quoted string) that returns the
boolean expression, e.g. replace the current "lambda: return (id(relay_settings)
& nspanel_easy::RelaySettings::Relay2_Local);" with a block-style lambda (using
|- and a return line) so the lambda in the template sensor compiles; ensure the
same pattern is used for any other single-line lambdas (e.g., relay1_local,
nspanel_easy::RelaySettings references) to match ESPHome's expected YAML lambda
format.
In `@docs/error_initializing.md`:
- Line 59: Replace the phrase "In normal situation" with "In normal situations"
in the sentence that begins "In normal situation, all the versions should be
displayed and should be the same." in the docs file (look for that exact
sentence in error_initializing.md) to correct grammar and improve wording.
In `@docs/howto.md`:
- Around line 220-222: The sentence "The following action enable this
functionality:" has incorrect verb agreement; update it to "The following action
enables this functionality:" by changing "enable" to "enables" in the docs text
(look for the sentence starting "The following action enable this
functionality:" in docs/howto.md and adjust it accordingly).
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
1 participant
Summary by CodeRabbit
New Features
Improvements
Documentation
Chores