Fix: Theme change during sleep causes mixed theme on wake-up#49
Fix: Theme change during sleep causes mixed theme on wake-up#49
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. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (23)
📝 WalkthroughWalkthroughAdds NSPanel Easy weather support (C++ state, condition lookup, picture selection), integrates weather handling into ESPHome pages, introduces "Where to Buy" docs, renames TFT model options toward "Legacy"/"Easy", and adds complete landscape HMI (boot/home) assets and logic. Changes
Sequence Diagram(s)sequenceDiagram
participant User as User/API
participant ESPHome
participant Component as nspanel_easy (C++)
participant HMI as HMI Display
User->>ESPHome: Send weather_state text
ESPHome->>Component: normalise_weather_condition(cond)
Component->>Component: get_weather_index(cond)
Component->>Component: get_weather_pics(index)
ESPHome->>Component: select_weather_variant(is_new_device, is_dark_theme)
Component-->>ESPHome: picture_id
ESPHome->>HMI: update home_weather_pic(picture_id)
HMI->>HMI: render weather icon
Estimated Code Review Effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
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)
📝 Coding Plan
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 |
It includes pic mapping
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
esphome/nspanel_esphome_addon_upload_tft.yaml (1)
351-351: Option label should match the resolved TFT artifact.Line 351 says “NSPanel Easy EU (under construction)”, but this selection resolves to
nspanel_easy_landscape.tft(Line 213). Please align the label to avoid selecting an unintended layout.Proposed text alignment
- - "NSPanel Easy EU (under construction)" + - "NSPanel Easy Landscape (under construction)"🤖 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` at line 351, The option label "NSPanel Easy EU (under construction)" is inconsistent with the artifact it resolves to (nspanel_easy_landscape.tft); update the option label in the selection mapping so it clearly matches the artifact (for example "NSPanel Easy EU (Landscape)" or "NSPanel Easy EU - Landscape") where the choice that references nspanel_easy_landscape.tft is defined (look for the option string currently set to "NSPanel Easy EU (under construction)" and change it to the new label).docs/where_to_buy.md (1)
67-67: Missing newline at end of file.Most style guides and many tools expect a trailing newline at the end of text files.
Proposed fix
consider [buying me an ice cream](https://www.buymeacoffee.com/edwardfirmo) 🍦 +🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/where_to_buy.md` at line 67, The file ends without a trailing newline; add a single newline character at the end of the file so the final line "consider [buying me an ice cream](https://www.buymeacoffee.com/edwardfirmo) 🍦" is followed by a newline (ensure your editor or commit adds the EOF newline).components/nspanel_easy/weather.h (1)
185-186: strncpy may not null-terminate if input fills the buffer.If
conditionis exactly 31 characters,strncpywon't null-terminatebuf, causingstrcmpat line 192 to read past the buffer. The buffer is zero-initialized, so this is safe in practice, but explicitly setting the last byte guards against future refactors.Defensive fix
char buf[32] = {}; strncpy(buf, condition, sizeof(buf) - 1); + buf[sizeof(buf) - 1] = '\0'; // Ensure null-termination normalise_weather_condition(buf, sizeof(buf));🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/nspanel_easy/weather.h` around lines 185 - 186, The current use of strncpy to copy condition into buf (char buf[32]) can leave buf non-null-terminated when condition is exactly 31 chars; update the copy so the final byte is always NUL-terminated (e.g., after strncpy assign buf[sizeof(buf)-1] = '\0' or replace strncpy with a bounded-copy that guarantees termination such as strlcpy), ensuring subsequent strcmp or other string ops on buf are safe; locate the buffer and copy site around the buf/condition usage to apply the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/where_to_buy.md`:
- Line 45: The markdownlint directive on the line currently reads "<!--
markdownlint-disable MD013 MD033 -->" but should re-enable the rules; replace
that duplicate disable directive with the enable directive "<!--
markdownlint-enable MD013 MD033 -->" so the rules disabled earlier (MD013,
MD033) are properly re-enabled; locate the duplicate directive string in the
document and update it to "enable".
In `@hmi/dev/nspanel_easy_landscape/boot.txt`:
- Line 542: The substr call is trimming the first log line with an incorrect +2
/ -2 offset, causing one extra character from the next line to be dropped;
update the substr invocation that references log_body.txt (the call with
log_start_pos.val and log_len.val) to use +1 and -1 (or otherwise use the actual
newline length) instead of +2/-2 so the slice starts at log_start_pos.val+1 and
length is log_len.val-log_start_pos.val-1, ensuring you only remove the single
newline character.
---
Nitpick comments:
In `@components/nspanel_easy/weather.h`:
- Around line 185-186: The current use of strncpy to copy condition into buf
(char buf[32]) can leave buf non-null-terminated when condition is exactly 31
chars; update the copy so the final byte is always NUL-terminated (e.g., after
strncpy assign buf[sizeof(buf)-1] = '\0' or replace strncpy with a bounded-copy
that guarantees termination such as strlcpy), ensuring subsequent strcmp or
other string ops on buf are safe; locate the buffer and copy site around the
buf/condition usage to apply the change.
In `@docs/where_to_buy.md`:
- Line 67: The file ends without a trailing newline; add a single newline
character at the end of the file so the final line "consider [buying me an ice
cream](https://www.buymeacoffee.com/edwardfirmo) 🍦" is followed by a newline
(ensure your editor or commit adds the EOF newline).
In `@esphome/nspanel_esphome_addon_upload_tft.yaml`:
- Line 351: The option label "NSPanel Easy EU (under construction)" is
inconsistent with the artifact it resolves to (nspanel_easy_landscape.tft);
update the option label in the selection mapping so it clearly matches the
artifact (for example "NSPanel Easy EU (Landscape)" or "NSPanel Easy EU -
Landscape") where the choice that references nspanel_easy_landscape.tft is
defined (look for the option string currently set to "NSPanel Easy EU (under
construction)" and change it to the new label).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: db8222dd-f96c-47f0-8dd3-9624ad761cd8
⛔ Files ignored due to path filters (59)
hmi/dev/ui/Easy/weather-icons_dark-theme/clear-night.pngis excluded by!**/*.pnghmi/dev/ui/Easy/weather-icons_dark-theme/clear-night.svgis excluded by!**/*.svghmi/dev/ui/Easy/weather-icons_dark-theme/cloudy.pngis excluded by!**/*.pnghmi/dev/ui/Easy/weather-icons_dark-theme/cloudy.svgis excluded by!**/*.svghmi/dev/ui/Easy/weather-icons_dark-theme/fog.pngis excluded by!**/*.pnghmi/dev/ui/Easy/weather-icons_dark-theme/fog.svgis excluded by!**/*.svghmi/dev/ui/Easy/weather-icons_dark-theme/hail.pngis excluded by!**/*.pnghmi/dev/ui/Easy/weather-icons_dark-theme/hail.svgis excluded by!**/*.svghmi/dev/ui/Easy/weather-icons_dark-theme/lightning-rainy.pngis excluded by!**/*.pnghmi/dev/ui/Easy/weather-icons_dark-theme/lightning-rainy.svgis excluded by!**/*.svghmi/dev/ui/Easy/weather-icons_dark-theme/lightning.pngis excluded by!**/*.pnghmi/dev/ui/Easy/weather-icons_dark-theme/lightning.svgis excluded by!**/*.svghmi/dev/ui/Easy/weather-icons_dark-theme/partlycloudy-night.pngis excluded by!**/*.pnghmi/dev/ui/Easy/weather-icons_dark-theme/partlycloudy-night.svgis excluded by!**/*.svghmi/dev/ui/Easy/weather-icons_dark-theme/partlycloudy.pngis excluded by!**/*.pnghmi/dev/ui/Easy/weather-icons_dark-theme/partlycloudy.svgis excluded by!**/*.svghmi/dev/ui/Easy/weather-icons_dark-theme/pouring.pngis excluded by!**/*.pnghmi/dev/ui/Easy/weather-icons_dark-theme/pouring.svgis excluded by!**/*.svghmi/dev/ui/Easy/weather-icons_dark-theme/rainy.pngis excluded by!**/*.pnghmi/dev/ui/Easy/weather-icons_dark-theme/rainy.svgis excluded by!**/*.svghmi/dev/ui/Easy/weather-icons_dark-theme/snowy-rainy.pngis excluded by!**/*.pnghmi/dev/ui/Easy/weather-icons_dark-theme/snowy-rainy.svgis excluded by!**/*.svghmi/dev/ui/Easy/weather-icons_dark-theme/snowy.pngis excluded by!**/*.pnghmi/dev/ui/Easy/weather-icons_dark-theme/snowy.svgis excluded by!**/*.svghmi/dev/ui/Easy/weather-icons_dark-theme/sunny.pngis excluded by!**/*.pnghmi/dev/ui/Easy/weather-icons_dark-theme/sunny.svgis excluded by!**/*.svghmi/dev/ui/Easy/weather-icons_dark-theme/windy.pngis excluded by!**/*.pnghmi/dev/ui/Easy/weather-icons_dark-theme/windy.svgis excluded by!**/*.svghmi/dev/ui/Easy/weather-icons_light-theme/clear-night.pngis excluded by!**/*.pnghmi/dev/ui/Easy/weather-icons_light-theme/clear-night.svgis excluded by!**/*.svghmi/dev/ui/Easy/weather-icons_light-theme/cloudy.pngis excluded by!**/*.pnghmi/dev/ui/Easy/weather-icons_light-theme/cloudy.svgis excluded by!**/*.svghmi/dev/ui/Easy/weather-icons_light-theme/fog.pngis excluded by!**/*.pnghmi/dev/ui/Easy/weather-icons_light-theme/fog.svgis excluded by!**/*.svghmi/dev/ui/Easy/weather-icons_light-theme/hail.pngis excluded by!**/*.pnghmi/dev/ui/Easy/weather-icons_light-theme/hail.svgis excluded by!**/*.svghmi/dev/ui/Easy/weather-icons_light-theme/lightning-rainy.pngis excluded by!**/*.pnghmi/dev/ui/Easy/weather-icons_light-theme/lightning-rainy.svgis excluded by!**/*.svghmi/dev/ui/Easy/weather-icons_light-theme/lightning.pngis excluded by!**/*.pnghmi/dev/ui/Easy/weather-icons_light-theme/lightning.svgis excluded by!**/*.svghmi/dev/ui/Easy/weather-icons_light-theme/partlycloudy-night.pngis excluded by!**/*.pnghmi/dev/ui/Easy/weather-icons_light-theme/partlycloudy-night.svgis excluded by!**/*.svghmi/dev/ui/Easy/weather-icons_light-theme/partlycloudy.pngis excluded by!**/*.pnghmi/dev/ui/Easy/weather-icons_light-theme/partlycloudy.svgis excluded by!**/*.svghmi/dev/ui/Easy/weather-icons_light-theme/pouring.pngis excluded by!**/*.pnghmi/dev/ui/Easy/weather-icons_light-theme/pouring.svgis excluded by!**/*.svghmi/dev/ui/Easy/weather-icons_light-theme/rainy.pngis excluded by!**/*.pnghmi/dev/ui/Easy/weather-icons_light-theme/rainy.svgis excluded by!**/*.svghmi/dev/ui/Easy/weather-icons_light-theme/snowy-rainy.pngis excluded by!**/*.pnghmi/dev/ui/Easy/weather-icons_light-theme/snowy-rainy.svgis excluded by!**/*.svghmi/dev/ui/Easy/weather-icons_light-theme/snowy.pngis excluded by!**/*.pnghmi/dev/ui/Easy/weather-icons_light-theme/snowy.svgis excluded by!**/*.svghmi/dev/ui/Easy/weather-icons_light-theme/sunny.pngis excluded by!**/*.pnghmi/dev/ui/Easy/weather-icons_light-theme/sunny.svgis excluded by!**/*.svghmi/dev/ui/Easy/weather-icons_light-theme/windy.pngis excluded by!**/*.pnghmi/dev/ui/Easy/weather-icons_light-theme/windy.svgis excluded by!**/*.svghmi/dev/ui/fonts/Roboto-Bold.ttfis excluded by!**/*.ttfhmi/dev/ui/fonts/Roboto-Regular.ttfis excluded by!**/*.ttfhmi/dev/ui/fonts/materialdesignicons-webfont.ttfis excluded by!**/*.ttf
📒 Files selected for processing (33)
README.mdcomponents/nspanel_easy/weather.cppcomponents/nspanel_easy/weather.hdocs/where_to_buy.mdesphome/nspanel_esphome_addon_upload_tft.yamlesphome/nspanel_esphome_hw_display.yamlesphome/nspanel_esphome_page_home.yamlhmi/boot_temp.pagehmi/dev/nspanel_easy_landscape/Program.s.txthmi/dev/nspanel_easy_landscape/boot.txthmi/dev/nspanel_easy_landscape/home.txthmi/dev/ui/fonts/10.zihmi/dev/ui/fonts/7.zihmi/dev/ui/fonts/8.zihmi/dev/ui/fonts/MDI 15.zihmi/dev/ui/fonts/MDI 25.zihmi/dev/ui/fonts/Roboto Bold 12.zihmi/dev/ui/fonts/Roboto Bold 30.zihmi/dev/ui/fonts/Roboto Bold 40.zihmi/dev/ui/fonts/Roboto Bold 42.zihmi/dev/ui/fonts/Roboto Regular 12.zihmi/dev/ui/fonts/Roboto-Bold 80 (UTF_8).zihmi/dev/ui/fonts/Roboto-Regular 10 (UTF_8).zihmi/dev/ui/fonts/Roboto-Regular 15 (UTF_8).zihmi/dev/ui/fonts/Roboto-Regular 24 (UTF_8).zihmi/dev/ui/fonts/Roboto-Regular 25 (UTF_8).zihmi/dev/ui/fonts/Roboto-Regular 32 (UTF_8).zihmi/dev/ui/fonts/Roboto-Regular 48 (UTF_8).zihmi/dev/ui/fonts/Roboto-Regular 6 (UTF_8).zihmi/dev/ui/fonts/Roboto-Regular 64 (UTF_8).zihmi/dev/ui/fonts/Roboto-Regular 80 (UTF_8).zihmi/nspanel_easy_landscape.hmihmi/nspanel_easy_landscape.tft
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
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 `@docs/where_to_buy.md`:
- Line 38: The table row currently uses the label "🇬🇧 EU (United Kingdom)"
which is incorrect; update that label to "🇬🇧 UK (United Kingdom)" (or "🇬🇧
Europe (United Kingdom)" if you intended shipping geography) by replacing the
"EU (United Kingdom)" text in the row string that starts with "| 🇬🇧 EU (United
Kingdom) |" so the row becomes "| 🇬🇧 UK (United Kingdom) | ..." to reflect the
correct region name.
Theme changes applied while the panel was sleeping resulted in a partially updated display — some components reflected the new theme while others retained the previous one. This is now correctly handled by ensuring the full theme is applied on wake-up.
Summary by CodeRabbit
New Features
Documentation
Changes