Improve: Smarter versioning makes updates easier and less disruptive#87
Improve: Smarter versioning makes updates easier and less disruptive#87
Conversation
This will make my life easier when testing things not yet released
📝 WalkthroughWalkthroughReplaces numeric global version storage with CalVer string/state handling, adds a C++ CalVer comparison helper, migrates ESPHome YAMLs to text/template sensors and state-based checks, tightens TFT parsing/publishing, adjusts boot/display script flows and SystemFlags bitfield, and extends CI to compute and inject an upstream minimum CalVer into YAMLs. Changes
Sequence Diagram(s)(omitted) Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
esphome/nspanel_esphome_addon_upload_tft.yaml (1)
94-105:⚠️ Potential issue | 🟡 MinorCast
version_tft->statetouint16_tbefore logging.Line 105 passes
version_tft->state(a float from the template sensor) to"%" PRIu8 "'", causing a varargs type mismatch. This is inconsistent with line 86 innspanel_esphome_version.yaml, which correctly usesPRIu16for the same sensor. Useuint16_tcast andPRIu16format instead:- ESP_LOGI("${TAG_UPLOAD_TFT}", "Auto updating TFT from '%" PRIu8 "' to '${min_tft_version}'", version_tft->state); + ESP_LOGI("${TAG_UPLOAD_TFT}", "Auto updating TFT from '%" PRIu16 "' to '${min_tft_version}'", + static_cast<uint16_t>(version_tft->state));🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@esphome/nspanel_esphome_addon_upload_tft.yaml` around lines 94 - 105, The log call using version_tft->state (in the block with ESP_LOGI("${TAG_UPLOAD_TFT}", "Auto updating TFT ...")) passes a float into a PRIu8 formatter causing a varargs mismatch; change the format to use PRIu16 and cast version_tft->state to (uint16_t) before passing it (i.e., update the format string to use PRIu16 and pass (uint16_t)version_tft->state), keeping the TAG_UPLOAD_TFT and tft_file_model->active_index() checks unchanged.nspanel_easy_blueprint.yaml (1)
10073-10100:⚠️ Potential issue | 🟠 MajorRestore the echoed-version equality check before declaring the panel healthy.
All versions OKnow only validates format and minimum compatibility. If ESPHome is echoing a different automation'sversions.blueprint, this path still passes and the communication/parallel-automation case disappears. That equality check needs to gate the success path and stay folded intoBlueprint version not detected.Minimal fix
- alias: All versions OK conditions: # Blueprint version is a valid CalVer string - >- {{ versions.blueprint | length >= 8 and not is_number(versions.blueprint) }} + - '{{ versions.blueprint == blueprint_version }}' # Blueprint version reported by ESPHome is compatible # with the minimum version required by the firmware - '{{ blueprint_compatible }}' # TFT version is valid and compatible - '{{ versions.tft > 0 and versions.tft_min > 0 }}' - '{{ versions.tft >= versions.tft_min }}' ... - alias: Blueprint version not detected conditions: # Blueprint version is missing or not a valid CalVer string, # meaning ESPHome could not receive or parse the blueprint version. - >- {{ versions.blueprint | length < 8 or is_number(versions.blueprint) + or versions.blueprint != blueprint_version }}Based on learnings: Treat versions.blueprint != blueprint_version as a communication/parallel-automation scenario by design; keep this check under the "Blueprint version not detected" (communication issue) branch rather than splitting it into a separate mismatch branch.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@nspanel_easy_blueprint.yaml` around lines 10073 - 10100, The "All versions OK" success branch currently only validates format and minimums (checks on versions.blueprint length and tft values) but no longer ensures the echoed versions.blueprint equals the expected blueprint_version; restore the equality check so the success path requires versions.blueprint == blueprint_version (or blueprint_compatible that includes that equality) and move the inverse check versions.blueprint != blueprint_version into the "Blueprint version not detected"/communication issue branch (alias "Blueprint version not detected") so mismatched/parallel-automation echoes are treated as a communication/error case rather than a success; update the condition expressions referencing versions.blueprint, blueprint_version and blueprint_compatible accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@esphome/nspanel_esphome_hw_display.yaml`:
- Around line 210-213: In dump_config and wakeup handlers fix the incorrect use
of the version_tft pointer: in the dump_config logging (function dump_config)
dereference version_tft and log version_tft->state using the uint16_t format
specifier (PRIu16) instead of printing the pointer; in the wakeup handler
(function wakeup) replace the pointer equality check (version_tft == 0) with a
state check (version_tft->state > 0) so the guard activates correctly; update
both locations to reference the uint16_t value (version_tft->state) and use
PRIu16 in the format strings.
In `@esphome/nspanel_esphome_page_boot.yaml`:
- Around line 69-72: The code only publishes version_tft when temp_version_tft >
0, leaving a stale positive sensor value on parse failure; update the handler
around temp_version_tft/tft_ver_valid to always call
version_tft->publish_state(...) — publish temp_version_tft when valid and
publish an explicit invalid value (e.g., 0) when not valid — so that version_tft
is cleared on bad params; adjust the logic around the symbols temp_version_tft,
tft_ver_valid, version_tft and the publish_state calls to implement this.
- Around line 114-115: The call to disp1->set_component_text_printf uses "%s"
but passes a std::string (version_blueprint->state) which is undefined behavior;
change the argument to pass version_blueprint->state.c_str() so the printf-style
API receives a C string (update the call inside the boot page check where
current_page_id == get_page_id("boot") before calling
disp1->set_component_text_printf).
In `@esphome/nspanel_esphome_version.yaml`:
- Line 69: In the ESP_LOGV call using TAG_VERSIONING, replace the direct use of
the template sensor pointer version_tft with its state (version_tft->state),
matching other usages (e.g., the existing line that reads version_tft->state) so
you pass the numeric value to printf instead of a pointer; update the ESP_LOGV
invocation that currently logs "TFT: %" PRIu16, version_tft to use
version_tft->state.
- Line 86: The log call is using PRIu16 (uint16 specifier) for
version_tft->state which is a float; update the ESP_LOGD call to use a float
format specifier (e.g. "%f" or "%.2f") or explicitly cast version_tft->state to
an integer if an integer value is intended; modify the line with
ESP_LOGD("${TAG_VERSIONING}", " TFT: %" PRIu16, version_tft->state) to
use the chosen float specifier and keep TAG_VERSIONING and version_tft->state as
the referenced symbols.
In `@nspanel_easy_blueprint.yaml`:
- Around line 10161-10171: The "Blueprint version mismatch" notification uses
contradictory predicates ('not blueprint_compatible' and 'not esphome_outdated')
so it never fires; replace those two predicates with a direct comparison between
the reported version parts and the minimum required parts (use
'blueprint_version_parts' and 'blueprint_min_parts') so the condition becomes
something like "blueprint_version_parts < blueprint_min_parts" (and keep the
existing checks for length/number), updating the alias block that contains
'Blueprint version mismatch' to reference these symbols instead of
'blueprint_compatible' and 'esphome_outdated'.
---
Outside diff comments:
In `@esphome/nspanel_esphome_addon_upload_tft.yaml`:
- Around line 94-105: The log call using version_tft->state (in the block with
ESP_LOGI("${TAG_UPLOAD_TFT}", "Auto updating TFT ...")) passes a float into a
PRIu8 formatter causing a varargs mismatch; change the format to use PRIu16 and
cast version_tft->state to (uint16_t) before passing it (i.e., update the format
string to use PRIu16 and pass (uint16_t)version_tft->state), keeping the
TAG_UPLOAD_TFT and tft_file_model->active_index() checks unchanged.
In `@nspanel_easy_blueprint.yaml`:
- Around line 10073-10100: The "All versions OK" success branch currently only
validates format and minimums (checks on versions.blueprint length and tft
values) but no longer ensures the echoed versions.blueprint equals the expected
blueprint_version; restore the equality check so the success path requires
versions.blueprint == blueprint_version (or blueprint_compatible that includes
that equality) and move the inverse check versions.blueprint !=
blueprint_version into the "Blueprint version not detected"/communication issue
branch (alias "Blueprint version not detected") so
mismatched/parallel-automation echoes are treated as a communication/error case
rather than a success; update the condition expressions referencing
versions.blueprint, blueprint_version and blueprint_compatible accordingly.
🪄 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: 9146269f-4942-438f-9a49-7f2b9ee7c25e
📒 Files selected for processing (8)
.github/workflows/versioning.ymlcomponents/nspanel_easy/versioning.cppcomponents/nspanel_easy/versioning.hesphome/nspanel_esphome_addon_upload_tft.yamlesphome/nspanel_esphome_hw_display.yamlesphome/nspanel_esphome_page_boot.yamlesphome/nspanel_esphome_version.yamlnspanel_easy_blueprint.yaml
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@esphome/nspanel_esphome_page_boot.yaml`:
- Around line 69-72: Parse the version string with a signed wide type (e.g., use
strtol or std::stol on params[4].c_str()) and validate the resulting long before
converting to uint16_t: check that the value is >= 0 and <= UINT16_MAX, only
then cast to uint16_t and call version_tft->publish_state; otherwise publish
NAN. Apply the same pattern to the other places that currently do direct
atoi-to-uint16_t conversions (the similar checks at the other version variables
and the code paths that set display_settings_received / the wake-up gate) so you
validate range and sign before narrowing.
🪄 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: e62172a4-cf8b-4d9b-9521-347dbdeb2da0
📒 Files selected for processing (4)
esphome/nspanel_esphome_hw_display.yamlesphome/nspanel_esphome_page_boot.yamlesphome/nspanel_esphome_version.yamlnspanel_easy_blueprint.yaml
✅ Files skipped from review due to trivial changes (1)
- nspanel_easy_blueprint.yaml
🚧 Files skipped from review as they are similar to previous changes (2)
- esphome/nspanel_esphome_hw_display.yaml
- esphome/nspanel_esphome_version.yaml
Not used
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
esphome/nspanel_esphome_version.yaml (1)
49-53: Minor redundancy:version_esphomeis published twice.The
version_esphometext_sensor has both:
- A
lambdathat returns"${version}"(runs once on initialization)- An explicit
text_sensor.template.publishinboot_early_routinesBoth set the same compile-time constant. The explicit publish is harmless but technically redundant.
♻️ Optional: Remove redundant lambda
If the
boot_early_routinespublish is the authoritative source, you could simplify:text_sensor: - id: version_esphome name: "Version - ESPHome" platform: template entity_category: diagnostic icon: mdi:tag-text-outline internal: false update_interval: never - lambda: |- - // ESPHome version is a compile-time constant — publish once and never update. - return std::string("${version}"); + # Published by boot_early_routines during boot sequence.Also applies to: 195-204
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@esphome/nspanel_esphome_version.yaml` around lines 49 - 53, The text_sensor id version_esphome is being set twice (once via its lambda returning "${version}" and again via boot_early_routines text_sensor.template.publish), so remove the redundant lambda from the version_esphome text_sensor and rely on the boot_early_routines publish as the authoritative initialization; also make the same change for the duplicate instance referenced around lines 195-204 so only the publish remains. Ensure you edit the text_sensor definition that contains the lambda (identifier: version_esphome) and leave the boot_early_routines publish steps intact.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@components/nspanel_easy/base.h`:
- Around line 38-39: The reserved bitfield width is incorrect: the comment
states "bits 11-15" (5 bits) but the member reserved is defined as 4 bits;
update the bitfield declaration for the reserved member in the struct (the
symbol named reserved in base.h) to be 5 bits so the declaration matches the
documented range 11–15 and avoids future allocation errors.
In `@esphome/nspanel_esphome_version.yaml`:
- Around line 215-220: The reset condition in the on_value lambda for
version_blueprint uses (version_tft->state <= 0) which misses NaN
(uninitialized) states; update the conditional passed to check_versions->execute
in the lambda (where version_blueprint, version_tft, blueprint_status_flags, and
check_versions->execute are referenced) to also test for
std::isnan(version_tft->state), e.g. include "|| std::isnan(version_tft->state)"
alongside the existing checks so uninitialized TFT sensor states trigger the
reset.
---
Nitpick comments:
In `@esphome/nspanel_esphome_version.yaml`:
- Around line 49-53: The text_sensor id version_esphome is being set twice (once
via its lambda returning "${version}" and again via boot_early_routines
text_sensor.template.publish), so remove the redundant lambda from the
version_esphome text_sensor and rely on the boot_early_routines publish as the
authoritative initialization; also make the same change for the duplicate
instance referenced around lines 195-204 so only the publish remains. Ensure you
edit the text_sensor definition that contains the lambda (identifier:
version_esphome) and leave the boot_early_routines publish steps intact.
🪄 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: a6eb989c-cdab-49ed-b974-0935d31c2137
📒 Files selected for processing (10)
components/nspanel_easy/base.hcomponents/nspanel_easy/versioning.cppcomponents/nspanel_easy/versioning.hesphome/nspanel_esphome_addon_upload_tft.yamlesphome/nspanel_esphome_boot.yamlesphome/nspanel_esphome_core.yamlesphome/nspanel_esphome_page_boot.yamlesphome/nspanel_esphome_page_screensaver.yamlesphome/nspanel_esphome_version.yamlnspanel_easy_blueprint.yaml
🚧 Files skipped from review as they are similar to previous changes (3)
- esphome/nspanel_esphome_addon_upload_tft.yaml
- esphome/nspanel_esphome_page_boot.yaml
- nspanel_easy_blueprint.yaml
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@esphome/nspanel_esphome_version.yaml`:
- Line 117: The current mapping converts version_tft->state to unsigned int via
static_cast which is undefined if state is NaN; guard this by testing the
floating value (e.g., std::isnan(version_tft->state)) before casting and produce
a safe fallback string (like "unknown" or "0") when NaN is detected; update the
line that builds the {"tft", ...} entry to use the guarded value and ensure the
translation uses std::to_string only on a valid unsigned int, referencing
version_tft->state and the {"tft", ...} initializer in
nspanel_esphome_version.yaml.
🪄 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: 0a1b6456-b5e7-40ff-8f14-f765fef9d568
📒 Files selected for processing (1)
esphome/nspanel_esphome_version.yaml
Blueprint version now follows the same CalVer scheme (
YYYY.M.seq) as the ESPHome firmware, keeping release versions consistent and easier to track across components. The Blueprint version is included in its name in Home Assistant, so you can always see at a glance which version is installed (closes #86).Three dedicated version sensors are now available in Home Assistant, one for each component (ESPHome, Blueprint, and TFT), replacing the previous combined version string. This makes it easier to see exactly what is running on each panel and to identify which component, if any, needs updating.
Version mismatch notifications are also more precise, telling you specifically whether your Blueprint or ESPHome firmware needs attention and providing clear steps to resolve it.
The boot sequence received some under-the-hood tweaks for a more reliable startup.
Summary by CodeRabbit
New Features
Bug Fixes