From 727df52ebe143342ce2a42c6aa912fc7688355e3 Mon Sep 17 00:00:00 2001 From: it-rec <19797875+it-rec@users.noreply.github.com> Date: Fri, 5 Jun 2026 12:24:41 +0200 Subject: [PATCH] Use aiosolaredge to parse SolarEdge battery storage data 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 #169964 --- .../components/solaredge/coordinator.py | 79 +++++----------- .../components/solaredge/manifest.json | 2 +- requirements_all.txt | 2 +- tests/components/solaredge/conftest.py | 7 +- .../solaredge/fixtures/storage_data.json | 30 ++++-- .../solaredge/snapshots/test_sensor.ambr | 12 +-- tests/components/solaredge/test_sensor.py | 94 ++++++++++--------- 7 files changed, 106 insertions(+), 120 deletions(-) diff --git a/homeassistant/components/solaredge/coordinator.py b/homeassistant/components/solaredge/coordinator.py index a897b9c1b14e3f..cd0f5df27563d9 100644 --- a/homeassistant/components/solaredge/coordinator.py +++ b/homeassistant/components/solaredge/coordinator.py @@ -347,70 +347,35 @@ async def async_update_data(self) -> None: """Update the data from the SolarEdge Monitoring API.""" now = dt_util.now() start_of_day = now.replace(hour=0, minute=0, second=0, microsecond=0) - data = await self.api.get_storage_data( - self.site_id, - start_of_day, - now, - ) - storage_data = data.get("storageData") - if storage_data is None: - raise UpdateFailed("Storage data not available from API") - - batteries = storage_data.get("batteries") - if batteries is None: - raise UpdateFailed("Battery data not available from API") + try: + storage = await self.api.get_parsed_storage_data( + self.site_id, + start_of_day, + now, + ) + except KeyError as ex: + raise UpdateFailed("Storage data not available from API") from ex self.data = {} self.attributes = {} - if not batteries: + if not storage.batteries: LOGGER.debug("No batteries found in storage data") return - # Aggregate totals across all batteries - total_charge_energy = 0.0 - total_discharge_energy = 0.0 - - for battery in batteries: - serial = battery.get("serialNumber") - if not serial: - LOGGER.debug("Skipping battery without serialNumber") - continue - - telemetries = battery.get("telemetries", []) - - if not telemetries: - continue - - latest = telemetries[-1] - - # Per-battery current values - self.data[f"{serial}_state_of_charge"] = latest.get( - "batteryPercentageState" - ) - self.data[f"{serial}_power"] = latest.get("power") - - # Compute daily charge/discharge delta from lifetime counters - if len(telemetries) >= 2: - first = telemetries[0] - charge_energy = latest.get("lifeTimeEnergyCharged", 0.0) - first.get( - "lifeTimeEnergyCharged", 0.0 - ) - discharge_energy = latest.get( - "lifeTimeEnergyDischarged", 0.0 - ) - first.get("lifeTimeEnergyDischarged", 0.0) - else: - charge_energy = 0.0 - discharge_energy = 0.0 - - total_charge_energy += charge_energy - total_discharge_energy += discharge_energy - - self.data[f"{serial}_charge_energy"] = charge_energy - self.data[f"{serial}_discharge_energy"] = discharge_energy - - self.data["charge_energy"] = total_charge_energy - self.data["discharge_energy"] = total_discharge_energy + # aiosolaredge derives the charged/discharged energy by integrating the + # power telemetry, because SolarEdge frequently leaves + # lifeTimeEnergyCharged/Discharged at 0 even while the battery is + # cycling. See issue #169964. + for battery in storage.batteries: + serial = battery.serial_number + self.data[f"{serial}_state_of_charge"] = battery.state_of_charge + self.data[f"{serial}_power"] = battery.power + self.data[f"{serial}_charge_energy"] = battery.charge_energy + self.data[f"{serial}_discharge_energy"] = battery.discharge_energy + + self.data["charge_energy"] = storage.total_charge_energy + self.data["discharge_energy"] = storage.total_discharge_energy LOGGER.debug("Updated SolarEdge storage data: %s", self.data) diff --git a/homeassistant/components/solaredge/manifest.json b/homeassistant/components/solaredge/manifest.json index 276ddaf0c821e7..b2787694b1a7cf 100644 --- a/homeassistant/components/solaredge/manifest.json +++ b/homeassistant/components/solaredge/manifest.json @@ -14,5 +14,5 @@ "integration_type": "device", "iot_class": "cloud_polling", "loggers": ["aiosolaredge", "solaredge_web"], - "requirements": ["aiosolaredge==1.0.2", "solaredge-web==0.0.1"] + "requirements": ["aiosolaredge==1.1.0", "solaredge-web==0.0.1"] } diff --git a/requirements_all.txt b/requirements_all.txt index 18bebd84990a1e..ca5b1ff5ca5b87 100644 --- a/requirements_all.txt +++ b/requirements_all.txt @@ -414,7 +414,7 @@ aioskybell==22.7.0 aioslimproto==3.0.0 # homeassistant.components.solaredge -aiosolaredge==1.0.2 +aiosolaredge==1.1.0 # homeassistant.components.steamist aiosteamist==1.0.1 diff --git a/tests/components/solaredge/conftest.py b/tests/components/solaredge/conftest.py index 21cd8153d51c62..83302144da5225 100644 --- a/tests/components/solaredge/conftest.py +++ b/tests/components/solaredge/conftest.py @@ -3,6 +3,7 @@ from collections.abc import Generator from unittest.mock import AsyncMock, Mock, patch +from aiosolaredge import StorageData import pytest from homeassistant.components.solaredge.const import CONF_SITE_ID, DOMAIN @@ -71,8 +72,10 @@ def mock_solaredge_api_fixture() -> Generator[Mock]: api.get_energy_details = AsyncMock( return_value=load_json_object_fixture("energy_details.json", DOMAIN) ) - api.get_storage_data = AsyncMock( - return_value=load_json_object_fixture("storage_data.json", DOMAIN) + api.get_parsed_storage_data = AsyncMock( + return_value=StorageData.from_response( + load_json_object_fixture("storage_data.json", DOMAIN) + ) ) with ( patch( diff --git a/tests/components/solaredge/fixtures/storage_data.json b/tests/components/solaredge/fixtures/storage_data.json index 21f4065467139b..02db7e5c74c98a 100644 --- a/tests/components/solaredge/fixtures/storage_data.json +++ b/tests/components/solaredge/fixtures/storage_data.json @@ -6,17 +6,31 @@ "telemetries": [ { "timeStamp": "2025-01-01 00:00:00", - "lifeTimeEnergyCharged": 1000.0, - "lifeTimeEnergyDischarged": 500.0, + "lifeTimeEnergyCharged": 0, + "lifeTimeEnergyDischarged": 0, "batteryPercentageState": 50.0, - "power": 100.0 + "power": 0.0 }, { - "timeStamp": "2025-01-01 12:00:00", - "lifeTimeEnergyCharged": 1500.0, - "lifeTimeEnergyDischarged": 800.0, - "batteryPercentageState": 75.0, - "power": 200.0 + "timeStamp": "2025-01-01 01:00:00", + "lifeTimeEnergyCharged": 0, + "lifeTimeEnergyDischarged": 0, + "batteryPercentageState": 80.0, + "power": 2000.0 + }, + { + "timeStamp": "2025-01-01 02:00:00", + "lifeTimeEnergyCharged": 0, + "lifeTimeEnergyDischarged": 0, + "batteryPercentageState": 90.0, + "power": 0.0 + }, + { + "timeStamp": "2025-01-01 03:00:00", + "lifeTimeEnergyCharged": 0, + "lifeTimeEnergyDischarged": 0, + "batteryPercentageState": 70.0, + "power": -1000.0 } ] } diff --git a/tests/components/solaredge/snapshots/test_sensor.ambr b/tests/components/solaredge/snapshots/test_sensor.ambr index 8c8e20267bf885..d01d1aa8434927 100644 --- a/tests/components/solaredge/snapshots/test_sensor.ambr +++ b/tests/components/solaredge/snapshots/test_sensor.ambr @@ -54,7 +54,7 @@ 'last_changed': , 'last_reported': , 'last_updated': , - 'state': '500.0', + 'state': '2000.0', }) # --- # name: test_all_entities[sensor.battery_bat001_discharge_energy_today-entry] @@ -112,7 +112,7 @@ 'last_changed': , 'last_reported': , 'last_updated': , - 'state': '300.0', + 'state': '500.0', }) # --- # name: test_all_entities[sensor.battery_bat001_power-entry] @@ -170,7 +170,7 @@ 'last_changed': , 'last_reported': , 'last_updated': , - 'state': '200.0', + 'state': '-1000.0', }) # --- # name: test_all_entities[sensor.battery_bat001_state_of_charge-entry] @@ -225,7 +225,7 @@ 'last_changed': , 'last_reported': , 'last_updated': , - 'state': '75.0', + 'state': '70.0', }) # --- # name: test_all_entities[sensor.solaredge_batteries-entry] @@ -1462,7 +1462,7 @@ 'last_changed': , 'last_reported': , 'last_updated': , - 'state': '500.0', + 'state': '2000.0', }) # --- # name: test_all_entities[sensor.solaredge_storage_discharge_energy_today-entry] @@ -1520,7 +1520,7 @@ 'last_changed': , 'last_reported': , 'last_updated': , - 'state': '300.0', + 'state': '500.0', }) # --- # name: test_all_entities[sensor.solaredge_storage_flow_direction-entry] diff --git a/tests/components/solaredge/test_sensor.py b/tests/components/solaredge/test_sensor.py index 72e9647eebd5cc..a2b755cbeb7e23 100644 --- a/tests/components/solaredge/test_sensor.py +++ b/tests/components/solaredge/test_sensor.py @@ -3,6 +3,7 @@ from unittest.mock import AsyncMock, MagicMock, Mock, patch from aiohttp import ClientError +from aiosolaredge import StorageData from freezegun.api import FrozenDateTimeFactory import pytest from syrupy.assertion import SnapshotAssertion @@ -26,17 +27,17 @@ "telemetries": [ { "timeStamp": "2025-01-01 00:00:00", - "lifeTimeEnergyCharged": 1000.0, - "lifeTimeEnergyDischarged": 500.0, + "lifeTimeEnergyCharged": 0, + "lifeTimeEnergyDischarged": 0, "batteryPercentageState": 50.0, - "power": 100.0, + "power": 0.0, }, { - "timeStamp": "2025-01-01 12:00:00", - "lifeTimeEnergyCharged": 1500.0, - "lifeTimeEnergyDischarged": 800.0, + "timeStamp": "2025-01-01 02:00:00", + "lifeTimeEnergyCharged": 0, + "lifeTimeEnergyDischarged": 0, "batteryPercentageState": 75.0, - "power": 200.0, + "power": 2000.0, }, ], }, @@ -45,17 +46,17 @@ "telemetries": [ { "timeStamp": "2025-01-01 00:00:00", - "lifeTimeEnergyCharged": 2000.0, - "lifeTimeEnergyDischarged": 1000.0, + "lifeTimeEnergyCharged": 0, + "lifeTimeEnergyDischarged": 0, "batteryPercentageState": 40.0, - "power": 150.0, + "power": 0.0, }, { - "timeStamp": "2025-01-01 12:00:00", - "lifeTimeEnergyCharged": 2700.0, - "lifeTimeEnergyDischarged": 1400.0, + "timeStamp": "2025-01-01 02:00:00", + "lifeTimeEnergyCharged": 0, + "lifeTimeEnergyDischarged": 0, "batteryPercentageState": 80.0, - "power": 250.0, + "power": 1500.0, }, ], }, @@ -365,11 +366,14 @@ async def test_storage_data_service( state = hass.states.get(charge_entry) assert state is not None - assert float(state.state) == 500.0 # 1500 - 1000 + # Integrated from the power telemetry: 1000 Wh (0 -> 2000 W over 1 h) + + # 1000 Wh (2000 -> 0 W over 1 h) = 2000 Wh. + assert float(state.state) == 2000.0 state = hass.states.get(discharge_entry) assert state is not None - assert float(state.state) == 300.0 # 800 - 500 + # 500 Wh discharged (0 -> -1000 W over 1 h). + assert float(state.state) == 500.0 # Per-battery entities for BAT001 bat_charge = entity_registry.async_get_entity_id( @@ -391,19 +395,19 @@ async def test_storage_data_service( state = hass.states.get(bat_charge) assert state is not None - assert float(state.state) == 500.0 + assert float(state.state) == 2000.0 state = hass.states.get(bat_discharge) assert state is not None - assert float(state.state) == 300.0 + assert float(state.state) == 500.0 state = hass.states.get(bat_soc) assert state is not None - assert float(state.state) == 75.0 + assert float(state.state) == 70.0 state = hass.states.get(bat_power) assert state is not None - assert float(state.state) == 200.0 + assert float(state.state) == -1000.0 @pytest.mark.usefixtures("entity_registry_enabled_by_default") @@ -417,7 +421,9 @@ async def test_storage_data_service_multi_battery( """Test storage data service aggregates data across multiple batteries.""" inventory = solaredge_api.get_inventory.return_value inventory["Inventory"]["batteries"] = [{"SN": "BAT001"}, {"SN": "BAT002"}] - solaredge_api.get_storage_data.return_value = STORAGE_DATA_MULTI_BATTERY + solaredge_api.get_parsed_storage_data.return_value = StorageData.from_response( + STORAGE_DATA_MULTI_BATTERY + ) await setup_integration(hass, mock_config_entry) @@ -430,10 +436,10 @@ async def test_storage_data_service_multi_battery( assert charge_entry is not None assert discharge_entry is not None - # BAT001: charge=500 (1500-1000), discharge=300 (800-500) - # BAT002: charge=700 (2700-2000), discharge=400 (1400-1000) - assert float(hass.states.get(charge_entry).state) == 1200.0 - assert float(hass.states.get(discharge_entry).state) == 700.0 + # Integrated from power telemetry over the 2 h window: + # BAT001: charge=2000 (avg 1000 W), BAT002: charge=1500 (avg 750 W). + assert float(hass.states.get(charge_entry).state) == 3500.0 + assert float(hass.states.get(discharge_entry).state) == 0.0 bat1_soc = entity_registry.async_get_entity_id( "sensor", DOMAIN, f"{SITE_ID}_BAT001_battery_state_of_charge" @@ -449,7 +455,7 @@ async def test_storage_data_service_multi_battery( ) assert bat2_charge is not None assert bat2_soc is not None - assert float(hass.states.get(bat2_charge).state) == 700.0 + assert float(hass.states.get(bat2_charge).state) == 1500.0 assert float(hass.states.get(bat2_soc).state) == 80.0 @@ -485,7 +491,7 @@ async def test_storage_data_service_api_error( entity_registry: er.EntityRegistry, ) -> None: """Test storage sensors are unavailable when the storage API errors out.""" - solaredge_api.get_storage_data.side_effect = Exception("API error") + solaredge_api.get_parsed_storage_data.side_effect = Exception("API error") await setup_integration(hass, mock_config_entry) @@ -502,11 +508,6 @@ async def test_storage_data_service_api_error( assert hass.states.get(discharge_entry).state == STATE_UNAVAILABLE -@pytest.mark.parametrize( - "bad_response", - [{"unexpected": {}}, {"storageData": {"otherField": "value"}}], - ids=["missing_storageData", "missing_batteries"], -) @pytest.mark.usefixtures("entity_registry_enabled_by_default") async def test_storage_data_missing_keys_in_response( recorder_mock: Recorder, @@ -514,10 +515,13 @@ async def test_storage_data_missing_keys_in_response( mock_config_entry: MockConfigEntry, solaredge_api: Mock, entity_registry: er.EntityRegistry, - bad_response: dict, ) -> None: - """Test storage sensors unavailable with missing required keys.""" - solaredge_api.get_storage_data.return_value = bad_response + """Test storage sensors unavailable with missing required keys. + + aiosolaredge raises KeyError when the storageData response is missing the + storageData/batteries keys; the coordinator turns that into an UpdateFailed. + """ + solaredge_api.get_parsed_storage_data.side_effect = KeyError("storageData") await setup_integration(hass, mock_config_entry) @@ -583,20 +587,20 @@ async def test_storage_service_deferred_after_inventory_failure( [ # Empty batteries list → data service returns early, aggregate stays unset. ({"storageData": {"batteries": []}}, STATE_UNKNOWN), - # Battery missing the serialNumber key → skipped in the loop, aggregate - # falls through with the initial 0.0 totals. - ({"storageData": {"batteries": [{"telemetries": []}]}}, "0.0"), - # Battery with no telemetries → skipped after the serial check. + # Battery missing the serialNumber key → dropped by the parser, leaving no + # usable battery, so the data service returns early. + ({"storageData": {"batteries": [{"telemetries": []}]}}, STATE_UNKNOWN), + # Battery with no telemetries → dropped by the parser too. ( { "storageData": { "batteries": [{"serialNumber": "BAT001", "telemetries": []}] } }, - "0.0", + STATE_UNKNOWN, ), - # Battery with a single telemetry → can't compute a delta, contributes - # 0.0 to the aggregate via the len < 2 branch. + # Battery with a single telemetry → no intervals to integrate, so it + # contributes 0.0 to the aggregate. ( { "storageData": { @@ -606,8 +610,6 @@ async def test_storage_service_deferred_after_inventory_failure( "telemetries": [ { "timeStamp": "2025-01-01 00:00:00", - "lifeTimeEnergyCharged": 1000.0, - "lifeTimeEnergyDischarged": 500.0, "batteryPercentageState": 50.0, "power": 100.0, } @@ -637,7 +639,9 @@ async def test_storage_data_service_handles_malformed_responses( expected_charge_state: str, ) -> None: """Test storage tolerates batteries without serial/telemetries.""" - solaredge_api.get_storage_data.return_value = storage_response + solaredge_api.get_parsed_storage_data.return_value = StorageData.from_response( + storage_response + ) await setup_integration(hass, mock_config_entry)