Skip to content

Fix: Empty notification page #92

Merged
edwardtfn merged 18 commits intomainfrom
v9999.99.9
Apr 5, 2026
Merged

Fix: Empty notification page #92
edwardtfn merged 18 commits intomainfrom
v9999.99.9

Conversation

@edwardtfn
Copy link
Copy Markdown
Owner

@edwardtfn edwardtfn commented Apr 5, 2026

Notification page is now rendered accordingly.

Revives the pre-built firmware feature, allowing users to flash NSPanel Easy without needing to compile the firmware themselves.

Two pre-built variants are now built and published automatically on every release:

  • NSPanel Easy (prebuilt/nspanel_esphome_prebuilt.yaml) — the standard pre-built firmware
  • Wall Display (prebuilt/wall_display.yaml) — the climate heat variant by @MichaelHeimann

The release workflow now builds both binaries in parallel after the version bump, commits them to prebuilt/ along with their checksums and update manifests, and only then creates the version tag — ensuring the tag always points to a commit that includes the binaries.

Build tests for both variants (against ESPHome latest and dev) are also added to the CI workflow, including daily scheduled runs.

Summary by CodeRabbit

  • Chores

    • Improved CI firmware build and verification workflow.
    • Added a script to update version placeholders in the bug report template.
    • Modernized internal C++ namespace syntax for consistency.
  • Bug Fixes

    • Removed extraneous blank lines from the ESPHome Logs textarea description.
    • Fixed notification handling: rejects empty messages, reliably shows/hides UI, separates sound playback, and improves clear/navigation behavior.
  • Documentation

    • Updated blueprint display/version metadata to v9999.99.9.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Apr 5, 2026

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Refactors many C++ headers/sources to C++17 nested-namespace syntax, adds a Python script to update bug-template placeholders, introduces new prebuilt ESPHome CI build jobs and a staged versioning/release workflow, updates blueprint/version YAMLs, and changes the notification page API/scripts.

Changes

Cohort / File(s) Summary
Issue template & helper
/.github/ISSUE_TEMPLATE/bug.yml, /.github/scripts/update_bug_template.py
Removed extra blank lines from the "ESPHome Logs" textarea and added an executable Python script that updates three placeholder version values (TFT Version, Firmware Version, Blueprint Version) via targeted regex substitutions with single-match enforcement.
Workflows: build & release
/.github/workflows/esphome_build.yml, /.github/workflows/versioning.yml
Added build_prebuilt_latest and build_prebuilt_dev build jobs; split previous release job into bump, per-device prebuilt builds, and commit-and-release; introduced artifact staging, binary/firmware-factory verification, MD5/manifest generation, stricter rebase/commit failure handling, and updated job outputs/needs wiring.
C++ namespace refactor (components/nspanel_easy)
components/nspanel_easy/... (e.g., addon_climate.*, addon_upload_tft.*, base.*, ha_components.*, hw_display.*, nextion_*.{h,cpp}, page_*.{h,cpp}, pages.*, text.*, versioning.*, weather.*)
Converted many files from nested namespace esphome { namespace nspanel_easy { ... } } to C++17 namespace esphome::nspanel_easy { ... } (and condensed hmi::* nests where present). Only namespace syntax and closing comments changed; no API/signature or logic changes.
YAML: versioning & blueprint
esphome/nspanel_esphome_version.yaml, nspanel_easy_blueprint.yaml
Replaced !!merge with << include for substitutions and minor whitespace/comment edits; bumped blueprint CalVer metadata from 2026.4.29999.99.9 in display name and variables.blueprint_version.
Page notification & scripts
esphome/nspanel_esphome_page_notification.yaml
Added empty-message validation to notification_show, changed state assignments to store .c_str(), moved sound playback to a standalone script, switched visibility calls to send_command, and refactored notification clear/playback/page-change scripts and API actions.
Build helper (Windows)
hmi/dev/ui/generate_pics_all.cmd
Added conditional pauses on image-generation errors and an unconditional final timeout to reduce noisy failures during Windows runs.

Sequence Diagram(s)

sequenceDiagram
  participant Bump as "Bump Job"
  participant Build as "Build Jobs (prebuilt devices)"
  participant Staging as "Staging Area / Artifacts"
  participant Commit as "Commit-and-Release"
  participant GH as "GitHub (Repo & Release)"
  participant Notify as "Notify-Discord"

  Bump->>Build: trigger parallel prebuilt builds
  Build->>Staging: produce firmware.bin, firmware-factory.bin, MD5, manifests
  Staging->>Commit: download artifacts for verification
  Commit->>Staging: verify binaries, checksums, manifests
  Commit->>GH: commit prebuilt/, push tags, create Release
  GH-->>Commit: return release_url & status
  Commit->>Notify: output released/version/release_url for notification
  Notify->>GH: post release notification
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

Poem

🐰 I hopped through braces, made namespaces neat and bright,

A tiny script now swaps placeholders overnight.
Builds hum in parallel, manifests in a line,
Notifications tidy, firmware checks align.
A cheerful rabbit nibbles CI carrots, sweet and light.

🚥 Pre-merge checks | ✅ 1 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Title check ⚠️ Warning The PR title 'Fix: Empty notification page' is vague and does not accurately reflect the actual primary changes, which focus on reviving prebuilt firmware features, automating CI builds, and refactoring workflows. Update the title to reflect the main objective, such as 'Add prebuilt firmware builds and CI automation for NSPanel Easy' or 'Automate prebuilt firmware generation and release workflow'.
Docstring Coverage ⚠️ Warning Docstring coverage is 44.74% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (1 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch v9999.99.9

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🧹 Nitpick comments (2)
components/nspanel_easy/page_utilities.h (1)

25-138: Consider applying consistent namespace syntax throughout the file.

The outer namespace uses the new C++17 combined syntax (esphome::nspanel_easy), but lines 61-62 still use the nested form for namespace hmi { namespace utilities {. For consistency, you could unify to namespace hmi::utilities {.

♻️ Optional: Unify namespace syntax
-namespace hmi {
-namespace utilities {
+namespace hmi::utilities {
 
 /**
  * `@namespace` utilities
-}  // namespace utilities
-}  // namespace hmi
+}  // namespace hmi::utilities
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@components/nspanel_easy/page_utilities.h` around lines 25 - 138, The file
mixes namespace syntax; change the nested declaration "namespace hmi { namespace
utilities {" to the unified C++17 form "namespace hmi::utilities {" and update
the matching closing comments/brace lines (currently " }  // namespace
utilities" and " }  // namespace hmi") to a single closing " }  // namespace
hmi::utilities" so the namespace for the utilities symbols (e.g., PAGE, TITLE,
GROUP01_LABEL) uses a consistent combined namespace style.
.github/workflows/versioning.yml (1)

290-313: Heredoc produces JSON with leading whitespace.

The heredoc content inherits the YAML indentation, so the manifest JSON will have leading spaces on each line (e.g., {). Most JSON parsers handle whitespace gracefully, but for cleaner output consider using <<-EOF with tabs, or piping through a formatter.

♻️ Optional: Use `jq` to generate clean JSON
       - name: Generate update manifest
         env:
           NEW_VERSION: ${{ needs.bump.outputs.version }}
         run: |
           MD5=$(cat staging/nspanel_esphome_prebuilt.bin.md5)
-          cat > staging/nspanel_esphome_prebuilt.manifest.json <<EOF
-          {
-            "name": "NSPanel Easy Pre-built",
-            "version": "${NEW_VERSION}",
-            "home_assistant_domain": "esphome",
-            "new_install_prompt_erase": false,
-            "builds": [
-              {
-                "chipFamily": "ESP32",
-                "ota": {
-                  "path": "nspanel_esphome_prebuilt.bin",
-                  "md5": "${MD5}",
-                  "summary": "NSPanel Easy Pre-built v${NEW_VERSION}",
-                  "release_url": "https://github.com/edwardtfn/NSPanel-Easy/releases/tag/v${NEW_VERSION}"
-                }
-              }
-            ]
-          }
-          EOF
+          jq -n \
+            --arg name "NSPanel Easy Pre-built" \
+            --arg version "$NEW_VERSION" \
+            --arg md5 "$MD5" \
+            '{
+              name: $name,
+              version: $version,
+              home_assistant_domain: "esphome",
+              new_install_prompt_erase: false,
+              builds: [{
+                chipFamily: "ESP32",
+                ota: {
+                  path: "nspanel_esphome_prebuilt.bin",
+                  md5: $md5,
+                  summary: ("NSPanel Easy Pre-built v" + $version),
+                  release_url: ("https://github.com/edwardtfn/NSPanel-Easy/releases/tag/v" + $version)
+                }
+              }]
+            }' > staging/nspanel_esphome_prebuilt.manifest.json

This also avoids potential issues if PR titles/versions ever contain characters that could break the heredoc or JSON syntax.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.github/workflows/versioning.yml around lines 290 - 313, The heredoc used to
create staging/nspanel_esphome_prebuilt.manifest.json inherits YAML indentation
producing leading spaces; change the heredoc so it strips indentation (use a
dedenting heredoc like <<-EOF) or replace the heredoc with a safe JSON generator
(for example build the object with jq or echo key/value subtitution piped into
jq) to ensure the JSON written for "name"/"version"/"builds" (where NEW_VERSION
and MD5 are injected) has no leading whitespace and is robust against special
characters.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In @.github/scripts/update_bug_template.py:
- Around line 62-64: The current calls to update_placeholder (used for 'TFT
Version', 'Firmware Version', 'Blueprint Version') silently allow
missing/renamed placeholders which can leave stale template hints; change the
behavior so the process fails fast by having update_placeholder raise/throw an
explicit error when the target placeholder is not found (or return a falsy
sentinel that the caller checks), and update the calling code (the three calls
that assign to content) to catch that condition and raise a clear exception
(include the placeholder name) so the workflow errors immediately if any
placeholder is missing.
- Around line 41-43: The regex in the pattern variable and its re.sub call is
too greedy and uses re.DOTALL, which causes it to swallow the rest of the file;
update the pattern to constrain matching to the single line containing the
placeholder (e.g. use '^' and '$' anchors and replace the trailing '.*' with
'[^\n\r]*') and change the flags argument from re.DOTALL to re.MULTILINE so only
the intended line is replaced; keep the existing replacement variable and
count=1 but ensure the modified pattern targets the placeholder text on one line
only (refer to the pattern, replacement, and the re.sub(...) invocation).

---

Nitpick comments:
In @.github/workflows/versioning.yml:
- Around line 290-313: The heredoc used to create
staging/nspanel_esphome_prebuilt.manifest.json inherits YAML indentation
producing leading spaces; change the heredoc so it strips indentation (use a
dedenting heredoc like <<-EOF) or replace the heredoc with a safe JSON generator
(for example build the object with jq or echo key/value subtitution piped into
jq) to ensure the JSON written for "name"/"version"/"builds" (where NEW_VERSION
and MD5 are injected) has no leading whitespace and is robust against special
characters.

In `@components/nspanel_easy/page_utilities.h`:
- Around line 25-138: The file mixes namespace syntax; change the nested
declaration "namespace hmi { namespace utilities {" to the unified C++17 form
"namespace hmi::utilities {" and update the matching closing comments/brace
lines (currently " }  // namespace utilities" and " }  // namespace hmi") to a
single closing " }  // namespace hmi::utilities" so the namespace for the
utilities symbols (e.g., PAGE, TITLE, GROUP01_LABEL) uses a consistent combined
namespace style.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 08de2200-fef2-4757-bb5e-83d2720e8ca4

📥 Commits

Reviewing files that changed from the base of the PR and between 1c7e540 and 05f33aa.

📒 Files selected for processing (44)
  • .github/ISSUE_TEMPLATE/bug.yml
  • .github/scripts/update_bug_template.py
  • .github/workflows/esphome_build.yml
  • .github/workflows/versioning.yml
  • components/nspanel_easy/addon_climate.cpp
  • components/nspanel_easy/addon_climate.h
  • components/nspanel_easy/addon_upload_tft.cpp
  • components/nspanel_easy/addon_upload_tft.h
  • components/nspanel_easy/base.cpp
  • components/nspanel_easy/base.h
  • components/nspanel_easy/ha_components.cpp
  • components/nspanel_easy/ha_components.h
  • components/nspanel_easy/hardware.h
  • components/nspanel_easy/hw_display.cpp
  • components/nspanel_easy/hw_display.h
  • components/nspanel_easy/nextion_components.cpp
  • components/nspanel_easy/nextion_components.h
  • components/nspanel_easy/nextion_constants.h
  • components/nspanel_easy/nspanel_easy.h
  • components/nspanel_easy/page_alarm.h
  • components/nspanel_easy/page_climate.cpp
  • components/nspanel_easy/page_climate.h
  • components/nspanel_easy/page_entities.cpp
  • components/nspanel_easy/page_entities.h
  • components/nspanel_easy/page_home.h
  • components/nspanel_easy/page_media_player.cpp
  • components/nspanel_easy/page_media_player.h
  • components/nspanel_easy/page_notification.cpp
  • components/nspanel_easy/page_notification.h
  • components/nspanel_easy/page_qrcode.h
  • components/nspanel_easy/page_screensaver.cpp
  • components/nspanel_easy/page_screensaver.h
  • components/nspanel_easy/page_utilities.cpp
  • components/nspanel_easy/page_utilities.h
  • components/nspanel_easy/pages.cpp
  • components/nspanel_easy/pages.h
  • components/nspanel_easy/text.cpp
  • components/nspanel_easy/text.h
  • components/nspanel_easy/versioning.cpp
  • components/nspanel_easy/versioning.h
  • components/nspanel_easy/weather.cpp
  • components/nspanel_easy/weather.h
  • esphome/nspanel_esphome_version.yaml
  • nspanel_easy_blueprint.yaml
💤 Files with no reviewable changes (1)
  • .github/ISSUE_TEMPLATE/bug.yml

Comment thread .github/scripts/update_bug_template.py Outdated
Comment thread .github/scripts/update_bug_template.py Outdated
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
.github/workflows/versioning.yml (1)

186-200: 🛠️ Refactor suggestion | 🟠 Major

Hardcoded array indices are fragile and error-prone.

Using positional indices (body[2], body[3], body[4]) to update the bug template will silently update wrong fields if the template structure changes. According to the relevant code snippets, there's a Python script at .github/scripts/update_bug_template.py that uses label matching ('TFT Version', 'Firmware Version', 'Blueprint Version') which is more robust.

Consider using the Python script instead, or at minimum, add validation that the expected labels exist at those indices.

♻️ Suggested fix: Use the existing Python script
      - name: Update bug report template with current versions
        if: steps.skip_check.outputs.skip == 'false'
        env:
          VERSION: ${{ steps.versions.outputs.version }}
          MIN_BLUEPRINT_VERSION: ${{ steps.versions.outputs.min_blueprint_version }}
          MIN_TFT_VERSION: ${{ steps.versions.outputs.min_tft_version }}
        run: |
          TEMPLATE=".github/ISSUE_TEMPLATE/bug.yml"
-
-          # Update TFT Version placeholder (index 2)
-          yq eval \
-            '.body[2].attributes.placeholder = ("e.g., " + strenv(MIN_TFT_VERSION))' \
-            -i "$TEMPLATE"
-
-          # Update Firmware Version placeholder (index 3)
-          yq eval \
-            '.body[3].attributes.placeholder = ("e.g., " + strenv(VERSION))' \
-            -i "$TEMPLATE"
-
-          # Update Blueprint Version placeholder (index 4)
-          yq eval \
-            '.body[4].attributes.placeholder = ("e.g., " + strenv(MIN_BLUEPRINT_VERSION))' \
-            -i "$TEMPLATE"
+          python .github/scripts/update_bug_template.py \
+            "$TEMPLATE" \
+            "$MIN_TFT_VERSION" \
+            "$VERSION" \
+            "$MIN_BLUEPRINT_VERSION"
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.github/workflows/versioning.yml around lines 186 - 200, The workflow uses
fragile hardcoded indices (body[2], body[3], body[4]) to set placeholders;
replace this with the robust updater in .github/scripts/update_bug_template.py
(or call its functionality) so updates are done by label ('TFT Version',
'Firmware Version', 'Blueprint Version') rather than position, or if you must
keep yq ensure you validate the labels at those indices before writing (fail the
job with a clear error if labels don't match); target the step that runs the yq
commands and switch it to invoke the Python script (or add validation logic) to
prevent silent mis-updates.
🧹 Nitpick comments (2)
.github/workflows/versioning.yml (2)

113-117: Unnecessary push before version calculation.

The rebase ensures the local branch is up-to-date, but the subsequent git push origin HEAD:main at line 117 pushes unchanged code (no commits have been made yet). This is either a no-op or could fail if there's nothing to push. Consider removing this push and only pushing after commits are made.

♻️ Suggested fix: Remove premature push
       git fetch origin main
       git rebase origin/main || {
         echo "Rebase failed - main has diverged"
         exit 1
       }
-      git push origin HEAD:main
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.github/workflows/versioning.yml around lines 113 - 117, The workflow
contains an unnecessary premature push: after running "git rebase origin/main"
the script runs "git push origin HEAD:main" before any commits are created;
remove the "git push origin HEAD:main" step (or gate it behind a condition that
there are new commits) so pushing only occurs after version changes/commits are
made rather than immediately after the rebase.

258-268: Consider adding debug output to help troubleshoot build failures.

When the build output structure changes, debugging why firmware.bin wasn't found can be difficult. Consider listing the build directory contents on failure.

🔧 Suggested improvement
       - name: Locate compiled binaries
         id: locate
         run: |
           BIN=$(find prebuilt/.esphome/build -name "firmware.bin" | head -1)
           BIN_FACTORY=$(find prebuilt/.esphome/build -name "firmware-factory.bin" | head -1)
           if [ -z "$BIN" ]; then
             echo "ERROR: firmware.bin not found after build"
+            echo "Build directory contents:"
+            find prebuilt/.esphome/build -type f 2>/dev/null || echo "Build directory does not exist"
             exit 1
           fi
           echo "bin=${BIN}" >> "$GITHUB_OUTPUT"
           echo "bin_factory=${BIN_FACTORY}" >> "$GITHUB_OUTPUT"
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.github/workflows/versioning.yml around lines 258 - 268, The Locate compiled
binaries step exits with a generic error when firmware.bin isn't found; update
the step that defines BIN and BIN_FACTORY to emit diagnostic output on failure
by printing the contents of prebuilt/.esphome/build (and subdirs) and the raw
find results for BIN and BIN_FACTORY before exiting so you can see why find
returned nothing; specifically augment the shell block that sets BIN/BIN_FACTORY
and the error branch so process shows the output of ls -la and/or find
prebuilt/.esphome/build -maxdepth 3 -type f and the values of BIN and
BIN_FACTORY to aid debugging.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In @.github/workflows/versioning.yml:
- Around line 414-417: The commit-and-release job lacks concurrency protection
and can race with bump; add the same concurrency group used by bump
(concurrency: group: version-and-release) to the commit-and-release job (job
name: commit-and-release) or move a concurrency: group: version-and-release
declaration to the workflow level so both bump and commit-and-release share the
same concurrency group and prevent parallel runs.
- Around line 295-313: The heredoc used to create
staging/nspanel_esphome_prebuilt.manifest.json is indented, causing leading
spaces to be written into the JSON; change the heredoc to a stripped form (use
<<-EOF with tabs or remove indentation so the JSON is left-aligned) in the cat >
staging/nspanel_esphome_prebuilt.manifest.json block so the produced file
contains valid, non-indented JSON.
- Around line 383-401: The heredoc that writes
staging/wall_display.manifest.json contains leading spaces which produce
malformed JSON; fix by removing the indentation so the JSON keys start at column
0 (apply the same left-aligned heredoc change used for the nspanel manifest),
keeping the same variables (NEW_VERSION, MD5, wall_display.bin and the
release_url) intact in the manifest content.
- Around line 253-256: Replace the branch reference for the GitHub Action usage
of edwardtfn/build-action (currently referenced as `@add-substitutions-support`)
with a pinned full commit SHA to ensure reproducible and secure builds; update
both occurrences of the action in the workflow (the two uses of
edwardtfn/build-action) to the provided commit SHA
11cba210daa0fc52ac434eb9ea862eaad9e9ea8f.

---

Outside diff comments:
In @.github/workflows/versioning.yml:
- Around line 186-200: The workflow uses fragile hardcoded indices (body[2],
body[3], body[4]) to set placeholders; replace this with the robust updater in
.github/scripts/update_bug_template.py (or call its functionality) so updates
are done by label ('TFT Version', 'Firmware Version', 'Blueprint Version')
rather than position, or if you must keep yq ensure you validate the labels at
those indices before writing (fail the job with a clear error if labels don't
match); target the step that runs the yq commands and switch it to invoke the
Python script (or add validation logic) to prevent silent mis-updates.

---

Nitpick comments:
In @.github/workflows/versioning.yml:
- Around line 113-117: The workflow contains an unnecessary premature push:
after running "git rebase origin/main" the script runs "git push origin
HEAD:main" before any commits are created; remove the "git push origin
HEAD:main" step (or gate it behind a condition that there are new commits) so
pushing only occurs after version changes/commits are made rather than
immediately after the rebase.
- Around line 258-268: The Locate compiled binaries step exits with a generic
error when firmware.bin isn't found; update the step that defines BIN and
BIN_FACTORY to emit diagnostic output on failure by printing the contents of
prebuilt/.esphome/build (and subdirs) and the raw find results for BIN and
BIN_FACTORY before exiting so you can see why find returned nothing;
specifically augment the shell block that sets BIN/BIN_FACTORY and the error
branch so process shows the output of ls -la and/or find prebuilt/.esphome/build
-maxdepth 3 -type f and the values of BIN and BIN_FACTORY to aid debugging.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: deec77a6-6e8f-4fb6-b200-7510e8df9d18

📥 Commits

Reviewing files that changed from the base of the PR and between 05f33aa and c311fc0.

📒 Files selected for processing (1)
  • .github/workflows/versioning.yml

Comment thread .github/workflows/versioning.yml
Comment thread .github/workflows/versioning.yml Outdated
Comment thread .github/workflows/versioning.yml Outdated
Comment thread .github/workflows/versioning.yml
@edwardtfn edwardtfn changed the title feat: Pre-built firmware for NSPanel Easy and Wall Display Fix: Empty notification page Apr 5, 2026
@edwardtfn edwardtfn linked an issue Apr 5, 2026 that may be closed by this pull request
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🧹 Nitpick comments (1)
hmi/dev/ui/generate_pics_all.cmd (1)

10-10: Make the 30s trailing delay opt-in.

At Line 10, timeout /t 30 always adds latency, even on successful runs. Consider gating it behind a debug flag (or removing it) to keep normal runs fast.

Example adjustment
-timeout /t 30
+if defined NSPANEL_UI_DEBUG timeout /t 30 >nul
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@hmi/dev/ui/generate_pics_all.cmd` at line 10, The script currently always
executes "timeout /t 30" which adds unwanted latency; change it to run only when
an opt-in flag or env var is set (e.g., check %PAUSE_ON_SUCCESS% or
%DEBUG_TIMEOUT% equals "1" or a passed-in parameter) so the default behavior has
no delay. Update the top of the script to parse a CLI arg or read an environment
variable (default off), then replace the unconditional "timeout /t 30" with a
conditional branch that executes the same "timeout /t 30" only when that flag is
enabled; keep the timeout command and its value unchanged so behavior is
identical when opted-in.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In @.github/workflows/versioning.yml:
- Around line 435-445: Before the "Download nspanel-easy artifacts" and
"Download wall-display artifacts" steps, add a preparatory step that removes
existing generated files from the prebuilt/ directory (e.g., run a shell step to
rm -rf prebuilt/* or otherwise ensure prebuilt/ is empty) so stale artifacts are
not preserved; then, where the commit is created (the commit step that stages
changes), replace git add prebuilt/ (or similar) with git add -A prebuilt/ or
git add -A so deletions are staged too (ensure the step that runs git commit/git
push uses the updated git add command). Update the steps named "Download
nspanel-easy artifacts" and "Download wall-display artifacts" and the subsequent
commit/staging steps to reflect these changes.
- Around line 44-48: The pipeline currently reads `main` in downstream jobs
causing builds/tags to potentially use a newer commit than the calculated
version; update the `bump` job to capture and expose the immutable post-bump
commit SHA (e.g., set an output like bumped_sha) and then change all downstream
checkouts and the release/rebase steps to use that output (replace places that
checkout or rebase `main` with ${{ needs.bump.outputs.bumped_sha }}), ensuring
all jobs that reference `needs.bump.outputs.version` also reference the
corresponding `needs.bump.outputs.bumped_sha` for checkout/tag operations so the
build and tag use the exact bumped commit.

In `@hmi/dev/ui/generate_pics_all.cmd`:
- Around line 2-8: The current generate_pics_all.cmd script swallows failures by
only running timeout when a bash ../scripts/generate_images.sh call fails (the
if errorlevel 1 lines), which lets the batch continue; change each failure
handler to propagate the error and stop execution (e.g., after each bash
../scripts/generate_images.sh invocation, test the exit status and call exit /b
%ERRORLEVEL% or a non‑zero exit code) so generator failures from
generate_images.sh are preserved and returned to callers instead of just
pausing.

---

Nitpick comments:
In `@hmi/dev/ui/generate_pics_all.cmd`:
- Line 10: The script currently always executes "timeout /t 30" which adds
unwanted latency; change it to run only when an opt-in flag or env var is set
(e.g., check %PAUSE_ON_SUCCESS% or %DEBUG_TIMEOUT% equals "1" or a passed-in
parameter) so the default behavior has no delay. Update the top of the script to
parse a CLI arg or read an environment variable (default off), then replace the
unconditional "timeout /t 30" with a conditional branch that executes the same
"timeout /t 30" only when that flag is enabled; keep the timeout command and its
value unchanged so behavior is identical when opted-in.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 09dd2d78-0b01-4bfa-b750-3f9866fccb65

📥 Commits

Reviewing files that changed from the base of the PR and between c311fc0 and 5a98b32.

⛔ Files ignored due to path filters (153)
  • hmi/dev/scripts/ui/eu-white/0.png is excluded by !**/*.png
  • hmi/dev/scripts/ui/eu/0.png is excluded by !**/*.png
  • hmi/dev/scripts/ui/us-white/0.png is excluded by !**/*.png
  • hmi/dev/scripts/ui/us/0.png is excluded by !**/*.png
  • hmi/dev/ui/eu-white/1.png is excluded by !**/*.png
  • hmi/dev/ui/eu-white/10.png is excluded by !**/*.png
  • hmi/dev/ui/eu-white/11.png is excluded by !**/*.png
  • hmi/dev/ui/eu-white/12.png is excluded by !**/*.png
  • hmi/dev/ui/eu-white/13.png is excluded by !**/*.png
  • hmi/dev/ui/eu-white/14.png is excluded by !**/*.png
  • hmi/dev/ui/eu-white/15.png is excluded by !**/*.png
  • hmi/dev/ui/eu-white/17.png is excluded by !**/*.png
  • hmi/dev/ui/eu-white/19.png is excluded by !**/*.png
  • hmi/dev/ui/eu-white/2.png is excluded by !**/*.png
  • hmi/dev/ui/eu-white/20.png is excluded by !**/*.png
  • hmi/dev/ui/eu-white/21.png is excluded by !**/*.png
  • hmi/dev/ui/eu-white/22.png is excluded by !**/*.png
  • hmi/dev/ui/eu-white/23.png is excluded by !**/*.png
  • hmi/dev/ui/eu-white/24.png is excluded by !**/*.png
  • hmi/dev/ui/eu-white/25.png is excluded by !**/*.png
  • hmi/dev/ui/eu-white/26.png is excluded by !**/*.png
  • hmi/dev/ui/eu-white/27.png is excluded by !**/*.png
  • hmi/dev/ui/eu-white/28.png is excluded by !**/*.png
  • hmi/dev/ui/eu-white/29.png is excluded by !**/*.png
  • hmi/dev/ui/eu-white/3.png is excluded by !**/*.png
  • hmi/dev/ui/eu-white/32.png is excluded by !**/*.png
  • hmi/dev/ui/eu-white/33.png is excluded by !**/*.png
  • hmi/dev/ui/eu-white/34.png is excluded by !**/*.png
  • hmi/dev/ui/eu-white/37.png is excluded by !**/*.png
  • hmi/dev/ui/eu-white/39.png is excluded by !**/*.png
  • hmi/dev/ui/eu-white/4.png is excluded by !**/*.png
  • hmi/dev/ui/eu-white/40.png is excluded by !**/*.png
  • hmi/dev/ui/eu-white/46.png is excluded by !**/*.png
  • hmi/dev/ui/eu-white/47.png is excluded by !**/*.png
  • hmi/dev/ui/eu-white/5.png is excluded by !**/*.png
  • hmi/dev/ui/eu-white/6.png is excluded by !**/*.png
  • hmi/dev/ui/eu-white/7.png is excluded by !**/*.png
  • hmi/dev/ui/eu-white/8.png is excluded by !**/*.png
  • hmi/dev/ui/eu-white/9.png is excluded by !**/*.png
  • hmi/dev/ui/eu/1.png is excluded by !**/*.png
  • hmi/dev/ui/eu/10.png is excluded by !**/*.png
  • hmi/dev/ui/eu/11.png is excluded by !**/*.png
  • hmi/dev/ui/eu/12.png is excluded by !**/*.png
  • hmi/dev/ui/eu/13.png is excluded by !**/*.png
  • hmi/dev/ui/eu/14.png is excluded by !**/*.png
  • hmi/dev/ui/eu/15.png is excluded by !**/*.png
  • hmi/dev/ui/eu/17.png is excluded by !**/*.png
  • hmi/dev/ui/eu/19.png is excluded by !**/*.png
  • hmi/dev/ui/eu/2.png is excluded by !**/*.png
  • hmi/dev/ui/eu/20.png is excluded by !**/*.png
  • hmi/dev/ui/eu/21.png is excluded by !**/*.png
  • hmi/dev/ui/eu/22.png is excluded by !**/*.png
  • hmi/dev/ui/eu/23.png is excluded by !**/*.png
  • hmi/dev/ui/eu/24.png is excluded by !**/*.png
  • hmi/dev/ui/eu/25.png is excluded by !**/*.png
  • hmi/dev/ui/eu/26.png is excluded by !**/*.png
  • hmi/dev/ui/eu/27.png is excluded by !**/*.png
  • hmi/dev/ui/eu/28.png is excluded by !**/*.png
  • hmi/dev/ui/eu/29.png is excluded by !**/*.png
  • hmi/dev/ui/eu/3.png is excluded by !**/*.png
  • hmi/dev/ui/eu/32.png is excluded by !**/*.png
  • hmi/dev/ui/eu/33.png is excluded by !**/*.png
  • hmi/dev/ui/eu/34.png is excluded by !**/*.png
  • hmi/dev/ui/eu/37.png is excluded by !**/*.png
  • hmi/dev/ui/eu/39.png is excluded by !**/*.png
  • hmi/dev/ui/eu/4.png is excluded by !**/*.png
  • hmi/dev/ui/eu/40.png is excluded by !**/*.png
  • hmi/dev/ui/eu/46.png is excluded by !**/*.png
  • hmi/dev/ui/eu/47.png is excluded by !**/*.png
  • hmi/dev/ui/eu/5.png is excluded by !**/*.png
  • hmi/dev/ui/eu/6.png is excluded by !**/*.png
  • hmi/dev/ui/eu/7.png is excluded by !**/*.png
  • hmi/dev/ui/eu/8.png is excluded by !**/*.png
  • hmi/dev/ui/eu/9.png is excluded by !**/*.png
  • hmi/dev/ui/eu/NSPanel - EU.pptx is excluded by !**/*.pptx
  • hmi/dev/ui/pics/15.png is excluded by !**/*.png
  • hmi/dev/ui/pics/15_new_black.png is excluded by !**/*.png
  • hmi/dev/ui/pics/15_new_white.png is excluded by !**/*.png
  • hmi/dev/ui/pics/settings_brightness_bar_blue_landscape.png is excluded by !**/*.png
  • hmi/dev/ui/pics/settings_brightness_bar_blue_portrait.png is excluded by !**/*.png
  • hmi/dev/ui/pics/settings_brightness_bar_gray_landscape.png is excluded by !**/*.png
  • hmi/dev/ui/pics/settings_brightness_bar_gray_portrait.png is excluded by !**/*.png
  • hmi/dev/ui/us-white/1.png is excluded by !**/*.png
  • hmi/dev/ui/us-white/10.png is excluded by !**/*.png
  • hmi/dev/ui/us-white/11.png is excluded by !**/*.png
  • hmi/dev/ui/us-white/12.png is excluded by !**/*.png
  • hmi/dev/ui/us-white/13.png is excluded by !**/*.png
  • hmi/dev/ui/us-white/14.png is excluded by !**/*.png
  • hmi/dev/ui/us-white/15.png is excluded by !**/*.png
  • hmi/dev/ui/us-white/17.png is excluded by !**/*.png
  • hmi/dev/ui/us-white/19.png is excluded by !**/*.png
  • hmi/dev/ui/us-white/2.png is excluded by !**/*.png
  • hmi/dev/ui/us-white/20.png is excluded by !**/*.png
  • hmi/dev/ui/us-white/21.png is excluded by !**/*.png
  • hmi/dev/ui/us-white/22.png is excluded by !**/*.png
  • hmi/dev/ui/us-white/23.png is excluded by !**/*.png
  • hmi/dev/ui/us-white/24.png is excluded by !**/*.png
  • hmi/dev/ui/us-white/25.png is excluded by !**/*.png
  • hmi/dev/ui/us-white/26.png is excluded by !**/*.png
  • hmi/dev/ui/us-white/27.png is excluded by !**/*.png
  • hmi/dev/ui/us-white/28.png is excluded by !**/*.png
  • hmi/dev/ui/us-white/29.png is excluded by !**/*.png
  • hmi/dev/ui/us-white/3.png is excluded by !**/*.png
  • hmi/dev/ui/us-white/32.png is excluded by !**/*.png
  • hmi/dev/ui/us-white/33.png is excluded by !**/*.png
  • hmi/dev/ui/us-white/34.png is excluded by !**/*.png
  • hmi/dev/ui/us-white/37.png is excluded by !**/*.png
  • hmi/dev/ui/us-white/39.png is excluded by !**/*.png
  • hmi/dev/ui/us-white/4.png is excluded by !**/*.png
  • hmi/dev/ui/us-white/40.png is excluded by !**/*.png
  • hmi/dev/ui/us-white/46.png is excluded by !**/*.png
  • hmi/dev/ui/us-white/47.png is excluded by !**/*.png
  • hmi/dev/ui/us-white/5.png is excluded by !**/*.png
  • hmi/dev/ui/us-white/6.png is excluded by !**/*.png
  • hmi/dev/ui/us-white/7.png is excluded by !**/*.png
  • hmi/dev/ui/us-white/8.png is excluded by !**/*.png
  • hmi/dev/ui/us-white/9.png is excluded by !**/*.png
  • hmi/dev/ui/us/1.png is excluded by !**/*.png
  • hmi/dev/ui/us/10.png is excluded by !**/*.png
  • hmi/dev/ui/us/11.png is excluded by !**/*.png
  • hmi/dev/ui/us/12.png is excluded by !**/*.png
  • hmi/dev/ui/us/13.png is excluded by !**/*.png
  • hmi/dev/ui/us/14.png is excluded by !**/*.png
  • hmi/dev/ui/us/15.png is excluded by !**/*.png
  • hmi/dev/ui/us/17.png is excluded by !**/*.png
  • hmi/dev/ui/us/19.png is excluded by !**/*.png
  • hmi/dev/ui/us/2.png is excluded by !**/*.png
  • hmi/dev/ui/us/20.png is excluded by !**/*.png
  • hmi/dev/ui/us/21.png is excluded by !**/*.png
  • hmi/dev/ui/us/22.png is excluded by !**/*.png
  • hmi/dev/ui/us/23.png is excluded by !**/*.png
  • hmi/dev/ui/us/24.png is excluded by !**/*.png
  • hmi/dev/ui/us/25.png is excluded by !**/*.png
  • hmi/dev/ui/us/26.png is excluded by !**/*.png
  • hmi/dev/ui/us/27.png is excluded by !**/*.png
  • hmi/dev/ui/us/28.png is excluded by !**/*.png
  • hmi/dev/ui/us/29.png is excluded by !**/*.png
  • hmi/dev/ui/us/3.png is excluded by !**/*.png
  • hmi/dev/ui/us/32.png is excluded by !**/*.png
  • hmi/dev/ui/us/33.png is excluded by !**/*.png
  • hmi/dev/ui/us/34.png is excluded by !**/*.png
  • hmi/dev/ui/us/37.png is excluded by !**/*.png
  • hmi/dev/ui/us/39.png is excluded by !**/*.png
  • hmi/dev/ui/us/4.png is excluded by !**/*.png
  • hmi/dev/ui/us/40.png is excluded by !**/*.png
  • hmi/dev/ui/us/46.png is excluded by !**/*.png
  • hmi/dev/ui/us/47.png is excluded by !**/*.png
  • hmi/dev/ui/us/5.png is excluded by !**/*.png
  • hmi/dev/ui/us/6.png is excluded by !**/*.png
  • hmi/dev/ui/us/7.png is excluded by !**/*.png
  • hmi/dev/ui/us/8.png is excluded by !**/*.png
  • hmi/dev/ui/us/9.png is excluded by !**/*.png
  • hmi/dev/ui/us/NSPanel - US.pptx is excluded by !**/*.pptx
📒 Files selected for processing (10)
  • .github/scripts/update_bug_template.py
  • .github/workflows/versioning.yml
  • esphome/nspanel_esphome_page_notification.yaml
  • hmi/dev/ui/generate_pics_all.cmd
  • hmi/nspanel_eu.HMI
  • hmi/nspanel_eu.tft
  • hmi/nspanel_us.HMI
  • hmi/nspanel_us.tft
  • hmi/nspanel_us_land.HMI
  • hmi/nspanel_us_land.tft
✅ Files skipped from review due to trivial changes (1)
  • .github/scripts/update_bug_template.py

Comment thread .github/workflows/versioning.yml
Comment thread .github/workflows/versioning.yml
Comment thread hmi/dev/ui/generate_pics_all.cmd
@edwardtfn edwardtfn merged commit dbc95c0 into main Apr 5, 2026
35 checks passed
@edwardtfn edwardtfn deleted the v9999.99.9 branch April 5, 2026 13:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bug Empty notification message

1 participant