Add heartbeat floor to Reduce DB Reporting ambient sensors#105
Add heartbeat floor to Reduce DB Reporting ambient sensors#105bharvey88 wants to merge 1 commit into
Conversation
The reduce-gated ambient sensors (ESP temp, LTR390 light/UV) used a hardcoded delta with no time floor, so with Reduce DB Reporting on they could freeze and never show a value on boot. Give them a delta-OR-hourly-heartbeat floor and publish the first post-boot reading. Radar filters, BME280 pressure, and the toggle-off path are unchanged. Related to ApolloAutomation/MSR-2#70 🤖 Generated with [Claude Code](https://claude.com/claude-code)
WalkthroughThe ESPHome firmware version is updated to ChangesSensor reporting updates
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested labels: Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Integrations/ESPHome/Core.yaml`:
- Around line 411-413: Update the reporting conditions for sys_esp_temperature
at Integrations/ESPHome/Core.yaml lines 411-413, LTR390 Light at lines 629-631,
and LTR390 UV at lines 654-656 to report only on the first publish or when the
current NAN state differs from the previous NAN state, while preserving the
hourly heartbeat and existing thresholds (5.0 for temperature, 20.0 for LTR390
sensors); apply the equivalent state-change logic at all three sites.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d2d3aa73-a5f0-4b9b-9c3d-abd6a31082e1
📒 Files selected for processing (1)
Integrations/ESPHome/Core.yaml
| if (isnan(last_reported_value) || | ||
| abs(current_value - last_reported_value) >= 5.0 || | ||
| (now - last_report_time) >= 3600000UL) { |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Handle NAN values gracefully to avoid error-hiding and database spam.
The current logic has a significant flaw when the sensor fails and returns NAN:
- If the sensor drops offline and starts returning
NANwhile operating normally,abs(NAN - last) >= thresholdevaluates tofalse. The error is suppressed until the hourly heartbeat fires, hiding the sensor failure from the system for up to an hour. - Once the heartbeat fires,
last_reported_valuebecomesNAN. On subsequent polls,isnan(last_reported_value)staystrue, causing EVERYNANreading to bypass the checks and report immediately. This spams the database with errors, defeating the "Reduce DB Reporting" intent.
To fix this, check explicitly if this is the first publish or if the NAN state has changed (i.e., newly failed or newly recovered) instead of checking only if the last value was NAN.
Integrations/ESPHome/Core.yaml#L411-L413: Apply the robust state-change check for ESP Temperature.Integrations/ESPHome/Core.yaml#L629-L631: Apply the robust state-change check for LTR390 Light.Integrations/ESPHome/Core.yaml#L654-L656: Apply the robust state-change check for LTR390 UV.
🛠️ Proposed fix for `sys_esp_temperature` (lines 411-413)
- if (isnan(last_reported_value) ||
- abs(current_value - last_reported_value) >= 5.0 ||
- (now - last_report_time) >= 3600000UL) {
+ bool is_first_publish = last_report_time == 0;
+ bool state_changed = isnan(current_value) != isnan(last_reported_value);
+ if (is_first_publish || state_changed ||
+ abs(current_value - last_reported_value) >= 5.0 ||
+ (now - last_report_time) >= 3600000UL) {(Apply the equivalent change with >= 20.0 for the LTR390 sensors at lines 629-631 and 654-656)
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (isnan(last_reported_value) || | |
| abs(current_value - last_reported_value) >= 5.0 || | |
| (now - last_report_time) >= 3600000UL) { | |
| bool is_first_publish = last_report_time == 0; | |
| bool state_changed = isnan(current_value) != isnan(last_reported_value); | |
| if (is_first_publish || state_changed || | |
| abs(current_value - last_reported_value) >= 5.0 || | |
| (now - last_report_time) >= 3600000UL) { |
📍 Affects 1 file
Integrations/ESPHome/Core.yaml#L411-L413(this comment)Integrations/ESPHome/Core.yaml#L629-L631Integrations/ESPHome/Core.yaml#L654-L656
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@Integrations/ESPHome/Core.yaml` around lines 411 - 413, Update the reporting
conditions for sys_esp_temperature at Integrations/ESPHome/Core.yaml lines
411-413, LTR390 Light at lines 629-631, and LTR390 UV at lines 654-656 to report
only on the first publish or when the current NAN state differs from the
previous NAN state, while preserving the hourly heartbeat and existing
thresholds (5.0 for temperature, 20.0 for LTR390 sensors); apply the equivalent
state-change logic at all three sites.
1 participant
Version: 26.7.14.1
What does this implement/fix?
Companion to the MSR-2 fix for ApolloAutomation/MSR-2#70. MSR-1 uses a BME280 (not a DPS310), and its pressure channel is not reduce-gated — but its other reduce-gated ambient sensors share the same "hardcoded delta, no time floor" pattern that can make a sensor look frozen when Reduce DB Reporting is on.
Changes (Reduce DB Reporting behaviour only — the toggle-off path is unchanged):
esphome configvalidates.Types of changes
Checklist / Checklijst:
If user-visible functionality or configuration variables are added/modified:
🤖 Generated with Claude Code
Summary by CodeRabbit
Bug Fixes
Chores