fix: TFT-driven values display to follow units separator settings#156
Merged
fix: TFT-driven values display to follow units separator settings#156
Conversation
Contributor
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 9 minutes and 38 seconds. ⌛ 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: Path: .coderabbit.yml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (9)
📝 WalkthroughWalkthroughThis PR introduces a configurable units separator system for localization, centralizes sleep/wake control flow using dedicated scripts, adds TFT baud-rate auto-detection diagnostics, and updates version thresholds across ESPHome and HMI components. Changes span configuration, HMI files, and documentation. Changes
Sequence DiagramsequenceDiagram
participant User as User (Button Press)
participant Device as ESPHome Device
participant Screen as Nextion Screen
participant UART as UART Bus
User->>Device: Press "Scan Baud Rate" button
Device->>Device: Log scan start
Device->>Screen: Power cycle (toggle switch with delays)
loop For each candidate baud rate
Device->>Device: Set tf_uart baud rate
Device->>Device: Reload UART settings
Device->>UART: Flush RX noise
Device->>Screen: Send "get sleep" command
Screen->>UART: Respond with data (if compatible)
Device->>UART: Wait for response (timeout)
Device->>UART: Flush response buffer
alt Response received
Device->>Device: Mark baud rate as FOUND
Note over Device: Exit loop on first match
else No response
Device->>Device: Revert to 115200 baud
Note over Device: Continue to next candidate
end
end
Device->>Device: Update baud_rate_active state
alt Fallback inactive OR rate differs
Device->>Screen: Trigger TFT upload
else Fallback active AND rate matches
Device->>Device: Skip TFT upload
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
🚥 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 |
edwardtfn
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
hmi/dev/nextion2text/nspanel_portrait/fan.txt (1)
232-260:⚠️ Potential issue | 🟠 MajorApply
home.units_sep.txtto every fan display update path.The slider paths now use the separator, but the up/down buttons and
timer0still writeva0.txt+"%";timer0also overwrites the correctly formatted slider value after release.🐛 Proposed fix
@@ - fan_value.txt=va0.txt+"%" + fan_value.txt=va0.txt + fan_value.txt+=home.units_sep.txt + fan_value.txt+="%" @@ - fan_value.txt=va0.txt+"%" + fan_value.txt=va0.txt + fan_value.txt+=home.units_sep.txt + fan_value.txt+="%" @@ - fan_value.txt=va0.txt+"%" + fan_value.txt=va0.txt + fan_value.txt+=home.units_sep.txt + fan_value.txt+="%" fansetting.txt="fan,percentage,"+va0.txtAlso applies to: 304-312, 359-367, 437-443
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@hmi/dev/nextion2text/nspanel_portrait/fan.txt` around lines 232 - 260, The fan display formatting is inconsistent: ensure every code path that writes fan_value.txt (Touch Release Event, Touch Move Event, the up/down button handlers and the timer0 handler) appends home.units_sep.txt before the "%" just like the slider paths do, and prevent timer0 from overwriting the correctly formatted slider value after release by either moving or gating timer0.en=1 or making the timer0 handler itself write the fully formatted string (va0.txt + home.units_sep.txt + "%") instead of just va0.txt+"%"; update references in the Touch Release Event (fanslider, fan_value.txt, button_up.pco, button_down.pco, timer0.en) and the timer0 handler accordingly.hmi/dev/nextion2text/nspanel_landscape/fan.txt (1)
232-260:⚠️ Potential issue | 🟡 MinorApply
home.units_sep.txtto all fan value writers.The slider paths now use the separator, but the up/down buttons and timer still write
va0.txt+"%", so button-driven changes will keep showing the old formatting.🐛 Proposed fix for the remaining fan display paths
if(fanslider.val<fanslider.maxval) { fanslider.val++ sys0=100*fanslider.val/fanslider.maxval covx sys0,va0.txt,0,0 - fan_value.txt=va0.txt+"%" + fan_value.txt=va0.txt + fan_value.txt+=home.units_sep.txt + fan_value.txt+="%" } @@ if(fanslider.val>fanslider.minval) { fanslider.val-- sys0=100*fanslider.val/fanslider.maxval covx sys0,va0.txt,0,0 - fan_value.txt=va0.txt+"%" + fan_value.txt=va0.txt + fan_value.txt+=home.units_sep.txt + fan_value.txt+="%" } @@ timer0.en=0 sys0=100*fanslider.val/fanslider.maxval covx sys0,va0.txt,0,0 - fan_value.txt=va0.txt+"%" + fan_value.txt=va0.txt + fan_value.txt+=home.units_sep.txt + fan_value.txt+="%" fansetting.txt="fan,percentage,"+va0.txtAlso applies to: 304-367, 438-443
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@hmi/dev/nextion2text/nspanel_landscape/fan.txt` around lines 232 - 260, The fan display formatting is inconsistent: ensure every place that writes the fan value uses the same pattern as the slider (convert percentage to va0.txt, then append home.units_sep.txt and "%" before assigning to fan_value.txt). Update all occurrences that currently do fan_value.txt=va0.txt+"%" (including Touch Release handler branch outcomes, the Touch Move handler duplicates, button-driven handlers referenced by button_up/button_down actions, and any timer-driven writers such as timer0) to use the sequence covx sys0,va0.txt,0,0 then fan_value.txt=va0.txt; fan_value.txt+=home.units_sep.txt; fan_value.txt+="%". This ensures fanslider, button_up, button_down and timer-based updates all use the separator consistently.
🧹 Nitpick comments (1)
esphome/nspanel_esphome_addon_upload_tft.yaml (1)
138-149: Replace hardcoded baud rates with the existing substitutions.Lines 138 (
115200) and 148 (921600) duplicate the${BAUD_RATE_FALLBACK}and${BAUD_RATE}values used throughout the project (seeesphome/nspanel_esphome_hw_uart.yaml). If those defaults ever change, this diagnostic will silently drift.♻️ Proposed change
} else { ESP_LOGD("nspanel.hw.uart", "Baud scan: no response at %" PRIu32 " bps", rate); // Revert to a safe rate before next attempt - tf_uart->set_baud_rate(115200); + tf_uart->set_baud_rate(${BAUD_RATE_FALLBACK}); tf_uart->load_settings(); delay(50); App.feed_wdt(); } } if (!found) { ESP_LOGE("nspanel.hw.uart", "Baud scan: no response at any rate"); // Restore compiled primary rate so the system is not left stranded - tf_uart->set_baud_rate(921600); + tf_uart->set_baud_rate(${BAUD_RATE}); tf_uart->load_settings(); }🤖 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 138 - 149, The code uses hardcoded baud rates in the tf_uart->set_baud_rate calls (currently 115200 and 921600) which duplicate project defaults; replace those literals with the existing substitution variables used elsewhere (${BAUD_RATE_FALLBACK} for the fallback rate and ${BAUD_RATE} for the compiled primary rate) so tf_uart->set_baud_rate(...) and subsequent tf_uart->load_settings() use the substituted constants instead of magic numbers; locate the two occurrences around the baud scan logic (the if (!found) block and the branch that sets 115200) and swap the numeric literals for the corresponding substitution tokens.
🤖 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_addon_upload_tft.yaml`:
- Around line 81-168: The scan lambda currently sets local found but never
updates system state, so the post-scan gate and nextion_upload compute
upload_baud from stale system_flags.baud_fallback_active; modify the scan lambda
(the block that iterates BAUD_RATES, uses tf_uart, disp1->send_command and sets
found) to set a persistent flag (e.g., system_flags.baud_scan_success = found)
and also update system_flags.baud_fallback_active to reflect whether the final
UART rate differs from the compiled ${BAUD_RATE}; then change the gate condition
and the nextion_upload upload_baud logic to read the new
system_flags.baud_scan_success (or the updated baud_fallback_active) so both the
trigger and the computed upload_baud match the actual UART state detected by the
scan.
In `@nspanel_easy_blueprint.yaml`:
- Around line 9378-9383: The template currently inserts units_separator before
the precipitation probability unit causing "40%" to become "40 %"; update the
precipitation probability branch so it concatenates the unit directly without
units_separator—i.e., use (precipitation_probability ~
weather_units.precipitation_probability) instead of (precipitation_probability ~
units_separator ~ weather_units.precipitation_probability) while leaving the
precipitation branch unchanged; adjust the expression around the variable
precipitation_probability in the block that renders value for
precipitation/precipitation_probability.
---
Outside diff comments:
In `@hmi/dev/nextion2text/nspanel_landscape/fan.txt`:
- Around line 232-260: The fan display formatting is inconsistent: ensure every
place that writes the fan value uses the same pattern as the slider (convert
percentage to va0.txt, then append home.units_sep.txt and "%" before assigning
to fan_value.txt). Update all occurrences that currently do
fan_value.txt=va0.txt+"%" (including Touch Release handler branch outcomes, the
Touch Move handler duplicates, button-driven handlers referenced by
button_up/button_down actions, and any timer-driven writers such as timer0) to
use the sequence covx sys0,va0.txt,0,0 then fan_value.txt=va0.txt;
fan_value.txt+=home.units_sep.txt; fan_value.txt+="%". This ensures fanslider,
button_up, button_down and timer-based updates all use the separator
consistently.
In `@hmi/dev/nextion2text/nspanel_portrait/fan.txt`:
- Around line 232-260: The fan display formatting is inconsistent: ensure every
code path that writes fan_value.txt (Touch Release Event, Touch Move Event, the
up/down button handlers and the timer0 handler) appends home.units_sep.txt
before the "%" just like the slider paths do, and prevent timer0 from
overwriting the correctly formatted slider value after release by either moving
or gating timer0.en=1 or making the timer0 handler itself write the fully
formatted string (va0.txt + home.units_sep.txt + "%") instead of just
va0.txt+"%"; update references in the Touch Release Event (fanslider,
fan_value.txt, button_up.pco, button_down.pco, timer0.en) and the timer0 handler
accordingly.
---
Nitpick comments:
In `@esphome/nspanel_esphome_addon_upload_tft.yaml`:
- Around line 138-149: The code uses hardcoded baud rates in the
tf_uart->set_baud_rate calls (currently 115200 and 921600) which duplicate
project defaults; replace those literals with the existing substitution
variables used elsewhere (${BAUD_RATE_FALLBACK} for the fallback rate and
${BAUD_RATE} for the compiled primary rate) so tf_uart->set_baud_rate(...) and
subsequent tf_uart->load_settings() use the substituted constants instead of
magic numbers; locate the two occurrences around the baud scan logic (the if
(!found) block and the branch that sets 115200) and swap the numeric literals
for the corresponding substitution tokens.
🪄 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: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: 3b558234-5d1b-476e-9878-9a91fc9e6f81
📒 Files selected for processing (26)
.github/workflows/esphome_build.yml.test/esphome_common_customizations.yamlcomponents/nspanel_easy/page_home.hdocs/customization.mdesphome/nspanel_esphome_addon_upload_tft.yamlesphome/nspanel_esphome_localization.yamlesphome/nspanel_esphome_page_screensaver.yamlesphome/nspanel_esphome_version.yamlhmi/dev/nextion2text/nspanel_easy_landscape/boot.txthmi/dev/nextion2text/nspanel_landscape/boot.txthmi/dev/nextion2text/nspanel_landscape/cover.txthmi/dev/nextion2text/nspanel_landscape/fan.txthmi/dev/nextion2text/nspanel_landscape/home.txthmi/dev/nextion2text/nspanel_landscape/light.txthmi/dev/nextion2text/nspanel_portrait/boot.txthmi/dev/nextion2text/nspanel_portrait/cover.txthmi/dev/nextion2text/nspanel_portrait/fan.txthmi/dev/nextion2text/nspanel_portrait/home.txthmi/dev/nextion2text/nspanel_portrait/light.txthmi/nspanel_easy_landscape.hmihmi/nspanel_easy_landscape.tfthmi/nspanel_landscape.hmihmi/nspanel_landscape.tfthmi/nspanel_portrait.hmihmi/nspanel_portrait.tftnspanel_easy_blueprint.yaml
|
Works Shown. Thx! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Improvements