fix: round energy/power values to avoid total_increasing violations#1918
Open
rishabhvaish wants to merge 3 commits intolbbrhzn:mainfrom
Open
fix: round energy/power values to avoid total_increasing violations#1918rishabhvaish wants to merge 3 commits intolbbrhzn:mainfrom
rishabhvaish wants to merge 3 commits intolbbrhzn:mainfrom
Conversation
Some chargers report energy readings with floating-point noise (e.g. 65.9999999998035 Wh instead of 66 Wh). After the Wh-to-kWh conversion this produces values like 0.0659999999998035 instead of 0.066, which can appear to decrease and triggers HA warnings for sensors with state_class total_increasing. Round all Wh/W → kWh/kW conversions to 3 decimal places (1 Wh / 1 W precision), consistent with the rounding already used for the session-energy derivation on the meter_start path. Fixes lbbrhzn#1916
Contributor
📝 WalkthroughWalkthroughIntroduce a KILO_UNIT_PRECISION constant and apply rounding to 3 decimal places for kW/kWh conversions in the chargepoint module: per-phase aggregation, measurand normalization, and the Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 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)
📝 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
custom_components/ocpp/chargepoint.py (1)
824-833: Consider centralizing precision to avoid drift across conversion paths.
3is now repeated in multiple conversion branches. A shared constant (or helper) would keep behavior consistent if precision policy changes.♻️ Suggested refactor
+KILO_UNIT_ROUND_DIGITS = 3 ... - self._metrics[(target_cid, metric)].value = round(metric_value / 1000, 3) + self._metrics[(target_cid, metric)].value = round( + metric_value / 1000, KILO_UNIT_ROUND_DIGITS + ) ... - return round(measurand_value.value / 1000, 3) + return round(measurand_value.value / 1000, KILO_UNIT_ROUND_DIGITS) ... - return round(measurand_value.value, 3) + return round(measurand_value.value, KILO_UNIT_ROUND_DIGITS) ... - value = round(value / 1000, 3) + value = round(value / 1000, KILO_UNIT_ROUND_DIGITS)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@custom_components/ocpp/chargepoint.py` around lines 824 - 833, Centralize the rounding precision used in the energy conversion code by replacing the duplicated literal 3 with a shared constant or helper used by the conversion function that handles measurand_value (the function that "Convert energy value from charger to kWh"); introduce a single PRECISION (e.g., PRECISION_KWH) or a helper like round_kwh(value) and use it in both branches where measurand_value.unit is "Wh" or None and the fallback branch so future precision changes affect all paths consistently.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@custom_components/ocpp/chargepoint.py`:
- Around line 824-833: Centralize the rounding precision used in the energy
conversion code by replacing the duplicated literal 3 with a shared constant or
helper used by the conversion function that handles measurand_value (the
function that "Convert energy value from charger to kWh"); introduce a single
PRECISION (e.g., PRECISION_KWH) or a helper like round_kwh(value) and use it in
both branches where measurand_value.unit is "Wh" or None and the fallback branch
so future precision changes affect all paths consistently.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: d8a8f4a9-45c9-4736-b3ac-37790a25d07b
📒 Files selected for processing (1)
custom_components/ocpp/chargepoint.py
rishabhvaish
temporarily deployed
to
continuous-integration
GitHub Actions
Inactive
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1918 +/- ##
=======================================
Coverage 94.92% 94.92%
=======================================
Files 12 12
Lines 2957 2957
=======================================
Hits 2807 2807
Misses 150 150 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
custom_components/ocpp/chargepoint.py (1)
828-836: Centralize the precision constant to avoid future drift.The logic is good, but the repeated
3precision literal across conversion paths is easy to desynchronize later. Consider a shared constant (e.g.,KILO_UNIT_PRECISION = 3) reused here and in other conversion branches.♻️ Suggested refactor
+KILO_UNIT_PRECISION = 3 + `@staticmethod` def get_energy_kwh(measurand_value: MeasurandValue) -> float: @@ - if (measurand_value.unit == "Wh") or (measurand_value.unit is None): - return round(measurand_value.value / 1000, 3) - return round(measurand_value.value, 3) + if (measurand_value.unit == "Wh") or (measurand_value.unit is None): + return round(measurand_value.value / 1000, KILO_UNIT_PRECISION) + return round(measurand_value.value, KILO_UNIT_PRECISION)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@custom_components/ocpp/chargepoint.py` around lines 828 - 836, Add a module-level constant (e.g., KILO_UNIT_PRECISION = 3) and replace the hardcoded precision literal in the energy conversion function that contains the "Convert energy value from charger to kWh" docstring with that constant; update both branches that call round(measurand_value.value / 1000, 3) and round(measurand_value.value, 3) to use KILO_UNIT_PRECISION, and also reuse that constant in other related conversion branches in chargepoint.py to keep precision centralized and consistent (refer to the measurand_value usage in the conversion function to locate the code).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@custom_components/ocpp/chargepoint.py`:
- Around line 828-836: Add a module-level constant (e.g., KILO_UNIT_PRECISION =
3) and replace the hardcoded precision literal in the energy conversion function
that contains the "Convert energy value from charger to kWh" docstring with that
constant; update both branches that call round(measurand_value.value / 1000, 3)
and round(measurand_value.value, 3) to use KILO_UNIT_PRECISION, and also reuse
that constant in other related conversion branches in chargepoint.py to keep
precision centralized and consistent (refer to the measurand_value usage in the
conversion function to locate the code).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 6587efe5-c2bd-497a-bd39-b49cf11fe9b1
📒 Files selected for processing (1)
custom_components/ocpp/chargepoint.py
Centralizes the repeated precision literal (3) used when converting W→kW and Wh→kWh into a single KILO_UNIT_PRECISION constant to prevent future drift. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
custom_components/ocpp/chargepoint.py (1)
1005-1011: Consider usingKILO_UNIT_PRECISIONfor consistency.Line 1008 computes session energy using
round(1000 * (value - ms_metric.value)) / 1000, which achieves the same 3-decimal precision but with a different expression. For maintainability and to leverage the new constant:♻️ Suggested refactor for consistency
elif ms_metric.unit == unit: self._metrics[ (target_cid, csess.session_energy.value) - ].value = round(1000 * (value - ms_metric.value)) / 1000 + ].value = round(value - ms_metric.value, KILO_UNIT_PRECISION) self._metrics[ (target_cid, csess.session_energy.value) ].unit = unit🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@custom_components/ocpp/chargepoint.py` around lines 1005 - 1011, Replace the hard-coded 3-decimal rounding expression in the block that assigns session energy (inside the ms_metric.unit == unit branch where self._metrics[(target_cid, csess.session_energy.value)].value is set) with the existing KILO_UNIT_PRECISION constant to standardize precision; update the assignment to round to KILO_UNIT_PRECISION (e.g., using round((value - ms_metric.value), KILO_UNIT_PRECISION) or equivalent) and leave the unit assignment unchanged so the metric uses the shared precision constant.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@custom_components/ocpp/chargepoint.py`:
- Around line 1005-1011: Replace the hard-coded 3-decimal rounding expression in
the block that assigns session energy (inside the ms_metric.unit == unit branch
where self._metrics[(target_cid, csess.session_energy.value)].value is set) with
the existing KILO_UNIT_PRECISION constant to standardize precision; update the
assignment to round to KILO_UNIT_PRECISION (e.g., using round((value -
ms_metric.value), KILO_UNIT_PRECISION) or equivalent) and leave the unit
assignment unchanged so the metric uses the shared precision constant.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 4bb68bb0-455f-4eb1-b63f-b6620bbb3748
📒 Files selected for processing (1)
custom_components/ocpp/chargepoint.py
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
1 participant
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.
Some chargers report energy readings with floating-point noise (e.g. 65.9999999998035 Wh instead of 66 Wh). After the Wh-to-kWh division this produces values like 0.0659999999998035 instead of 0.066, which can appear to decrease slightly and triggers the HA recorder warning:
I've added
round(..., 3)(1 Wh / 1 W precision) to all the Wh-to-kWh and W-to-kW conversion paths inchargepoint.py. This is consistent with the rounding already used for the session-energy derivation on the meter_start path (line 998).Fixes #1916
Summary by CodeRabbit