Cleaned PR#1920 - Fix Session Energy bug for chargers that send phase-specific readings#1925
Cleaned PR#1920 - Fix Session Energy bug for chargers that send phase-specific readings#1925ric866 wants to merge 4 commits intolbbrhzn:mainfrom
Conversation
PR lbbrhzn#1920 has got into a mess. This is the cleaned up version.
📝 WalkthroughWalkthroughThe pull request refactors metric normalization in the chargepoint module's phase aggregation logic and introduces session energy derivation when the charger doesn't report session energy directly. Changes include normalizing per-phase metrics through intermediate values, deriving session energy from incoming energy register readings, and updating baseline meter start initialization. Corresponding test cases validate phase processing and session energy calculations. Changes
Sequence DiagramsequenceDiagram
participant Charger as Charger
participant CP as ChargePoint<br/>process_measurands
participant Agg as _sum_l123<br/>(Phase Aggregation)
participant Session as csess<br/>(Session Tracker)
Charger->>CP: Energy.Active.Import.Register<br/>(L1, L2, L3 readings)
activate CP
CP->>Agg: Phase-tagged measurand values
activate Agg
Agg->>Agg: Normalize per-phase<br/>to final_value/final_unit
Agg->>Agg: Sum phase values<br/>L1+L2+L3
alt charger_reports_session_energy == false
Agg->>Session: Check transaction_id active?
activate Session
Session-->>Agg: transaction_id exists
Agg->>Session: Get/Initialize meter_start
alt meter_start missing
Agg->>Session: Set meter_start = current_reading
Agg->>Session: Set session_energy = 0.0
else meter_start exists
Agg->>Session: Derive session_energy<br/>= current_reading - meter_start
end
deactivate Session
end
Agg->>Agg: Store normalized<br/>metric value/unit
deactivate Agg
CP->>CP: Construct MeasurandValue<br/>with normalized fields
deactivate CP
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 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 |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1925 +/- ##
==========================================
+ Coverage 94.94% 94.97% +0.02%
==========================================
Files 12 12
Lines 2946 2963 +17
==========================================
+ Hits 2797 2814 +17
Misses 149 149 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
🧹 Nitpick comments (1)
custom_components/ocpp/chargepoint.py (1)
825-864: Session energy derivation logic looks correct for the intended fix.The implementation properly:
- Only activates for
Energy.Active.Import.Registerwhen charger doesn't report session energy directly- Initializes
meter_startbaseline if missing- Computes session energy as
(final_value - meter_start)with appropriate roundingOne minor observation: when
ms_metric.unit != final_unit(line 860), the session energy is silently not updated. Consider adding a debug log for this mismatch scenario to aid troubleshooting.💡 Optional: Add debug logging for unit mismatch
elif ms_metric.unit == final_unit: se_metric.value = ( round(1000 * (final_value - ms_metric.value)) / 1000 ) se_metric.unit = final_unit + else: + _LOGGER.debug( + "Session energy not updated: unit mismatch (meter_start=%s, current=%s)", + ms_metric.unit, + final_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 825 - 864, Session energy isn't updated when ms_metric.unit != final_unit; add a debug log in the branch handling unit mismatch to aid troubleshooting. Locate the block in chargepoint.py where DEFAULT_MEASURAND and _charger_reports_session_energy are checked and the tx_metric/ms_metric/se_metric logic runs, and insert a debug log (using the component logger used elsewhere in this class) in the else path after the unit comparison (the branch where currently nothing happens when ms_metric.unit != final_unit) that includes identifying info such as target_cid, csess.transaction_id.value, ms_metric.unit and final_unit so operators can see why session energy wasn't computed.
🤖 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 825-864: Session energy isn't updated when ms_metric.unit !=
final_unit; add a debug log in the branch handling unit mismatch to aid
troubleshooting. Locate the block in chargepoint.py where DEFAULT_MEASURAND and
_charger_reports_session_energy are checked and the
tx_metric/ms_metric/se_metric logic runs, and insert a debug log (using the
component logger used elsewhere in this class) in the else path after the unit
comparison (the branch where currently nothing happens when ms_metric.unit !=
final_unit) that includes identifying info such as target_cid,
csess.transaction_id.value, ms_metric.unit and final_unit so operators can see
why session energy wasn't computed.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 3046332b-cd24-4d3e-9c1c-b272422937da
📒 Files selected for processing (2)
custom_components/ocpp/chargepoint.pytests/test_charge_point_core.py
PR #1920 has got into a mess. This is the cleaned up version.
This PR fixes a bug where the Session Energy sensor fails to update during an active charging session for chargers that send phase-specific energy readings (e.g., Energy.Active.Import.Register.L1).
For these chargers, the Session Energy sensor currently remains at 0 for the duration of the charge and only populates with the final value when the StopTransaction payload is received.
Root Cause :
In on_meter_values, the real-time session energy math (Current - Start) is currently trapped inside the if phase is None: block.
If a charger sends a phase-tagged main energy register, it bypasses this block and gets passed to process_phases() via the unprocessed list. While process_phases() correctly aggregates the phases and updates the main lifetime energy sensor, it lacked the logic to simultaneously update the active Session Energy sensor.
Fix :
Added the standard session energy calculation directly to the end of process_phases().
Refactored the final unit assignment to use a final_value variable.
Added a check to verify an active transaction.
If active, it derives the real-time session energy (final_value - meter_start) and updates the metric so the HA dashboard graph updates continuously during the charge.
Testing :
Verified that single-phase chargers sending L1 energy registers now correctly increment the Session Energy sensor in real-time upon receiving periodic MeterValues.
Verified this does not interfere with chargers that natively report session energy (_charger_reports_session_energy).
Summary by CodeRabbit
Release Notes
Bug Fixes
Tests