fix(sensor): floor e_consumption_today to 1 dp to prevent TOTAL_INCREASING dips#144
Conversation
|
Warning Review limit reached
More reviews will be available in 32 minutes and 58 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, 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 include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughUpdated the ChangesEnergy consumption sensor fix
🎯 2 (Simple) | ⏱️ ~8 minutes
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
|
I’ve started reviewing this PR. |
There was a problem hiding this comment.
Code Review
This pull request updates the "e_consumption_today" sensor to use "SensorStateClass.TOTAL_INCREASING" and floors its value to one decimal place to prevent transient dips from triggering Home Assistant's strictly-increasing guard. The reviewer pointed out a potential floating-point precision issue where "math.floor(v * 10) / 10" can result in an incorrect lower value (e.g., 5.7 becoming 5.6), and suggested rounding the value to a high precision before flooring. Additionally, the reviewer recommended avoiding defensive "getattr" checks for attributes guaranteed to exist.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
f834e56 to
a05a4e9
Compare
|
I’ve started re-reviewing this PR. |
a05a4e9 to
919a862
Compare
|
I’ve started re-reviewing this PR. |
919a862 to
a0bd6a6
Compare
|
I’ve started re-reviewing this PR. |
…, and reset-threshold The sensor is computed from several registers (PV gen + grid-in - grid-out - AC-charge) polled at slightly different times. When one component updates before the others the result can transiently dip by a few Wh, tripping HA's strictly-increasing guard (#142). Adds a `monotonic: bool` flag to `GivEnergyInverterSensorDescription` and tracks `_monotonic_max` and `_monotonic_date` on the entity. When set, `native_value` applies three layers of logic: 1. Day boundary: when HA's local date changes, reset both fields so the midnight drop to zero passes through as a real decrease and TOTAL_INCREASING starts a new recorder cycle. 2. Large-drop reset (clock-drift guard): if the value drops by more than _MONOTONIC_RESET_THRESHOLD (0.5 kWh) on the same HA date, treat it as a genuine source reset. This handles inverter clocks that lag HA's midnight by one scan interval — the actual counter reset arrives on a subsequent read after the day boundary has already been committed. 3. Intra-day clamp: otherwise, return max(new, session_max) to hide the transient few-Wh dips that triggered the original warning. `GivEnergyInverterSensor` also inherits `RestoreEntity` and seeds `_monotonic_max` from the last persisted state in `async_added_to_hass` (only when the persisted state is from today) so a transient-dip first reading after an HA restart does not undercut the recorder's prior value. Keeping `TOTAL_INCREASING` is important: switching to `TOTAL` (as in the reverted #143) silently corrupts the accumulated `sum` on the midnight reset to zero, because `TOTAL` requires an explicit `last_reset` attribute to distinguish a reset from a decrease. Fixes #142 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
a0bd6a6 to
854249c
Compare
|
I’ve started re-reviewing this PR. |
dewet22-codex
left a comment
There was a problem hiding this comment.
The delayed inverter-clock reset concern is addressed by the large-drop guard, and I have no further concerns with this revision.
What
Floors
e_consumption_todayto one decimal place in thevalue_fnand addssuggested_display_precision=1to match peer daily-energy sensors. Keeps (restores)SensorStateClass.TOTAL_INCREASING.Why
e_consumption_todayis a computed field — givenergy-modbus derives it asPV gen + grid-in - grid-out - AC-chargefrom several registers polled at slightly different times. When one component updates before the others, the result can transiently dip by a few Wh (e.g. 4.7 → 4.6 kWh), tripping HA's strictly-increasing guard (#142).Flooring to 1 dp makes sub-0.1 kWh fluctuations invisible. Keeping
TOTAL_INCREASINGis important: switching toTOTAL(as in the now-reverted #143) would silently corrupt the accumulatedsumon the midnight reset to zero, becauseTOTALrequires an explicitlast_resetattribute to distinguish a reset from a decrease.Display precision of 1 dp matches
e_grid_in_day,e_grid_out_day, ande_pv_day.Fixes #142
Summary by CodeRabbit
Release Notes