Fix SolarEdge battery charge/discharge energy always reporting 0#173096
Fix SolarEdge battery charge/discharge energy always reporting 0#173096it-rec wants to merge 1 commit into
Conversation
|
Hey there @frenck, @bdraco, @tronikos, mind taking a look at this pull request as it has been labeled with an integration ( Code owner commandsCode owners of
|
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
This PR changes the SolarEdge battery energy calculation from using unreliable lifeTimeEnergyCharged/lifeTimeEnergyDischarged lifetime counters (which SolarEdge frequently reports as 0) to integrating energy from the reliably-populated power telemetry using the trapezoidal rule.
Changes:
- Added
_battery_energy_from_power()function incoordinator.pythat performs trapezoidal integration of power telemetry to derive charge/discharge energy in Wh. - Replaced the delta-based lifetime counter approach in
SolarEdgeStorageDataService.async_update_datawith the new power integration approach. - Updated all test fixtures, assertions, and snapshots to reflect the new energy calculation method.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| homeassistant/components/solaredge/coordinator.py | Added _battery_energy_from_power() helper and replaced lifetime counter delta logic with power integration. |
| tests/components/solaredge/test_coordinator.py | Added comprehensive parametrized unit tests for _battery_energy_from_power(). |
| tests/components/solaredge/test_sensor.py | Updated inline mock data and assertion values to match the new integration approach. |
| tests/components/solaredge/fixtures/storage_data.json | Expanded fixture with more telemetry points and zeroed lifetime counters to simulate the real-world API behavior. |
| tests/components/solaredge/snapshots/test_sensor.ambr | Updated snapshot expected values. |
d22db39 to
8fc4c88
Compare
joostlek
left a comment
joostlek
left a comment
There was a problem hiding this comment.
IMO we shouldn't integrate the information in the integration level. Instead users can use the integral integration for that instead of doing it ourselves. Integrations should connect with the service/device and not make abstractions on top of that
|
Please take a look at the requested changes, and use the Ready for review button when you are done, thanks 👍 |
SolarEdge frequently reports lifeTimeEnergyCharged/lifeTimeEnergyDischarged as 0 in the storageData response even while the battery is actively cycling, which left the storage_charge_energy/storage_discharge_energy sensors stuck at 0. The energy is instead derived by integrating the reliably-populated power telemetry. Following review feedback, this device-specific parsing now lives in the aiosolaredge library (get_parsed_storage_data, returning typed StorageData) rather than in the integration, so the coordinator just consumes the parsed result. Fixes home-assistant#169964
8fc4c88 to
727df52
Compare
|
Reworked per @joostlek's feedback that device/service-specific code belongs in the library rather than in HA. The power-integration logic has been moved out of the integration into aiosolaredge:
Keeping this in draft until aiosolaredge 1.1.0 is released so CI can install the dependency; happy to adjust the library API if you'd prefer a different shape before it's tagged. |
Proposed change
The SolarEdge battery sensors
storage_charge_energy/storage_discharge_energy(and their per-battery counterparts) were always reporting
0.Root cause: SolarEdge frequently returns
lifeTimeEnergyCharged/lifeTimeEnergyDischargedas0in thestorageDataresponse, even while thebattery is actively charging or discharging. The integration computed the energy
as a delta of those lifetime counters, so it stayed at
0. Thepowertelemetry, in contrast, is reliably populated.
The energy is now derived by trapezoidal integration of the
powertelemetry (positive = charging, negative = discharging).
Following review feedback (@joostlek), this device/service-specific parsing lives
in the aiosolaredge library rather than in the integration:
StorageData/BatteryStorageDataandSolarEdge.get_parsed_storage_data().get_parsed_storage_data()and reads the typedresult; the integration-side
_battery_energy_from_powerhelper and its unittest are removed (the integration logic is unit-tested in the library).
Important
This is a draft until aiosolaredge 1.1.0 (containing
Solarlibs/aiosolaredge#132) is released. The bump to
aiosolaredge==1.1.0isalready included here, so CI will go green once that release is on PyPI.
Verified against a live site: the power integral gave ~4.84 kWh charged vs.
~4.47 kWh from SoC × capacity (~3% = charging losses), while the lifetime
counters stayed at 0 the entire time.
Type of change
Additional information
Checklist
ruff format homeassistant tests)