Skip to content

Commit 97ca706

Browse files
CIclaude
andcommitted
fix(solax): use each inverter's last known yield instead of holding the plant
get_plant_pv_yield() held the whole plant whenever any inverter was missing, which treated three different cases alike. An inverter that failed a read after being read before now contributes its last known totalYield, a lifetime counter does not go backwards so the sum cannot dip. An inverter that reads fine but has no totalYield counts as 0 rather than holding the plant for ever. The value is held only when an inverter that onlineStatus says should be answering failed its read and has never given a yield. The last known yield is seeded from the inverter's own total yield sensor, so an inverter that no longer answers stays in the sum across a restart. The plant pv_power and grid_power follow the same narrowing: an offline inverter counts as 0W rather than holding them. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
1 parent 9f46987 commit 97ca706

3 files changed

Lines changed: 151 additions & 35 deletions

File tree

‎apps/predbat/solax.py‎

Lines changed: 41 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -390,6 +390,9 @@ def initialize(self, client_id, client_secret, region="eu", plant_id=None, autom
390390
# Plants whose PV energy has been built from their inverters, these never drop back to the plant total
391391
self.plant_pv_from_devices = set()
392392

393+
# Last known lifetime yield of each inverter, so that a failed read does not drop it from the plant sum
394+
self.inverter_last_yield = {}
395+
393396
# When the plant and device info was last read successfully, seeded from the storage cache on
394397
# startup so that a restart does not re-read data that is still current, plus when it was last
395398
# attempted so that a failing read is retried at a sensible rate rather than every cycle
@@ -613,19 +616,49 @@ async def write_setting_from_event(self, entity_id, value):
613616
await self.publish_controls()
614617
return True
615618

619+
def inverter_expected(self, device_sn):
620+
"""
621+
True unless the device info says the inverter is offline, an offline inverter is not expected to answer
622+
"""
623+
return self.device_info.get(device_sn, {}).get("onlineStatus", 1) != 0
624+
625+
def get_inverter_last_yield(self, plant_id, device_sn):
626+
"""
627+
Last known lifetime yield of an inverter in kWh, or None if it has never given one
628+
629+
Updated only when a read returns a number. With nothing known yet it is seeded from the inverter's
630+
own total yield sensor, which survives a restart, so that an inverter which no longer answers
631+
stays in the plant sum rather than stepping it down
632+
"""
633+
device_yield = None
634+
if device_sn not in self.realtime_device_failed:
635+
device_yield = self.realtime_device_data.get(device_sn, {}).get("totalYield")
636+
elif device_sn not in self.inverter_last_yield:
637+
device_yield = self.get_state_wrapper(f"sensor.{self.prefix}_solax_{plant_id}_{device_sn}_total_yield", default=None)
638+
try:
639+
self.inverter_last_yield[device_sn] = float(device_yield)
640+
except (ValueError, TypeError):
641+
pass
642+
return self.inverter_last_yield.get(device_sn)
643+
616644
def get_plant_pv_yield(self, plant_id, plant_yield):
617645
"""
618646
Lifetime PV energy for a plant in kWh, summed from its inverters rather than taken from the plant total
619647
620648
The plant level totalYield includes the AC output of an AC-coupled battery inverter, so battery
621-
discharge reads as generation (GH#5356). Returns None when the value should be held: a sum with an
622-
inverter missing dips and then recovers, which reads as a burst of generation on a cumulative counter
649+
discharge reads as generation (GH#5356). Each inverter contributes its last known yield, a lifetime
650+
counter does not go backwards so a failed read cannot make the sum dip. An inverter with no yield of
651+
its own counts as 0. Returns None when the value should be held, which is only when an inverter that
652+
should be answering failed its read and has never given a yield
623653
"""
624654
pv_yield = 0.0
625655
for device_sn in self.plant_inverters.get(plant_id, []):
626-
device_yield = self.realtime_device_data.get(device_sn, {}).get("totalYield")
627-
if device_sn in self.realtime_device_failed or device_yield is None:
628-
return None
656+
device_yield = self.get_inverter_last_yield(plant_id, device_sn)
657+
if device_yield is None:
658+
read_ok = device_sn in self.realtime_device_data and device_sn not in self.realtime_device_failed
659+
if not read_ok and self.inverter_expected(device_sn):
660+
return None
661+
device_yield = 0.0
629662
pv_yield += device_yield
630663
if pv_yield > 0:
631664
self.plant_pv_from_devices.add(plant_id)
@@ -2520,9 +2553,9 @@ async def publish_device_realtime_data(self):
25202553
battery = saved["battery"] if saved["battery"] is not None else 0
25212554
load_power = pv - battery - grid
25222555

2523-
# Plant PV and grid power, held when an inverter in the plant was not read on this cycle as a
2524-
# sum with one inverter missing would under-read
2525-
if not any(inverter_sn in self.realtime_device_failed or inverter_sn not in self.realtime_device_data for inverter_sn in self.plant_inverters.get(plant_id, [])):
2556+
# Plant PV and grid power, held when an inverter that should be answering was not read on this
2557+
# cycle as a sum with it missing would under-read. An offline inverter counts as 0W
2558+
if not any((inverter_sn in self.realtime_device_failed or inverter_sn not in self.realtime_device_data) and self.inverter_expected(inverter_sn) for inverter_sn in self.plant_inverters.get(plant_id, [])):
25262559
plant_name = next((plant.get("plantName", plant_id) for plant in self.plant_info if plant.get("plantId", "unknown").lower().replace(" ", "_") == plant_id), plant_id)
25272560
self.dashboard_item(
25282561
f"sensor.{self.prefix}_solax_{plant_id}_pv_power",

‎apps/predbat/tests/test_solax.py‎

Lines changed: 109 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@ def __init__(self, prefix="predbat"):
4141
self.realtime_plant_failed = set() # Plants whose last realtime read failed
4242
self.realtime_device_failed = set() # Devices whose last realtime read failed
4343
self.plant_pv_from_devices = set() # Plants whose PV energy has been built from their inverters
44+
self.inverter_last_yield = {} # Last known lifetime yield of each inverter
4445
self._storage = None # No storage cache by default in tests
4546
self.log_messages = []
4647
self.dashboard_items = {}
@@ -1462,46 +1463,52 @@ def state_of(api, entity_id):
14621463
else:
14631464
print("✓ PV yield and load built from the inverters, plant total yield unchanged")
14641465

1465-
# Test 2: A failed inverter read holds the PV yield and load, a sum with one inverter missing would dip
1466-
print("Test 2: Failed inverter read holds PV yield and load")
1467-
api.realtime_device_data[pv_sn]["totalYield"] = 10270.0
1468-
api.realtime_device_failed.add(hybrid_sn)
1466+
# Test 2: A failed inverter read uses that inverter's last known yield, a lifetime counter does not go
1467+
# backwards so the sum cannot dip, and the load keeps following the plant counters
1468+
print("Test 2: Failed inverter read uses its last known yield")
1469+
api.realtime_data[test_plant_id]["totalImported"] = 8001.0
1470+
api.realtime_device_failed.add(pv_sn)
14691471
await api.publish_plant_info()
14701472

14711473
if state_of(api, pv_entity) != 10269.0:
1472-
print(f"**** ERROR: PV yield should be held at 10269.0 while an inverter read failed, got {state_of(api, pv_entity)} ****")
1474+
print(f"**** ERROR: PV yield should stay at the last known 10269.0 while an inverter read failed, got {state_of(api, pv_entity)} ****")
14731475
failed = True
1474-
elif abs(state_of(api, load_entity) - expected_load) > 0.01:
1475-
print(f"**** ERROR: Total load should be held at {expected_load} while an inverter read failed, got {state_of(api, load_entity)} ****")
1476+
elif abs(state_of(api, load_entity) - (expected_load + 1.0)) > 0.01:
1477+
print(f"**** ERROR: Total load should still be published while an inverter read failed, expected {expected_load + 1.0}, got {state_of(api, load_entity)} ****")
14761478
failed = True
14771479
else:
1478-
print("✓ PV yield and load held on a failed inverter read")
1480+
print("✓ Last known yield used on a failed inverter read, load still published")
14791481

1480-
# Test 3: The next good read publishes again
1481-
print("Test 3: A good read publishes again")
1482-
api.realtime_device_failed.discard(hybrid_sn)
1482+
# Test 3: The next good read moves on from the last known value
1483+
print("Test 3: A good read publishes the new yield")
1484+
api.realtime_device_data[pv_sn]["totalYield"] = 10270.0
1485+
api.realtime_device_failed.discard(pv_sn)
14831486
await api.publish_plant_info()
14841487

14851488
if state_of(api, pv_entity) != 10270.0:
1486-
print(f"**** ERROR: PV yield should be 10270.0 once every inverter is read, got {state_of(api, pv_entity)} ****")
1489+
print(f"**** ERROR: PV yield should be 10270.0 once the inverter is read again, got {state_of(api, pv_entity)} ****")
14871490
failed = True
14881491
else:
1489-
print("✓ PV yield published once every inverter is read")
1492+
print("✓ PV yield follows the inverter once it is read again")
14901493

1491-
# Test 4: An inverter that has not been read yet, or has no yield in its data, also holds
1492-
print("Test 4: An unread inverter holds")
1493-
api = build_api()
1494-
del api.realtime_device_data[pv_sn]
1495-
await api.publish_plant_info()
1494+
# Test 4: An inverter that should be answering but has never given a yield holds, this is the startup case
1495+
print("Test 4: An unread inverter with no known yield holds")
1496+
for online_status in [None, 1]:
1497+
api = build_api()
1498+
del api.realtime_device_data[pv_sn]
1499+
api.realtime_device_failed.add(pv_sn)
1500+
if online_status is not None:
1501+
api.device_info[pv_sn]["onlineStatus"] = online_status
1502+
await api.publish_plant_info()
14961503

1497-
if pv_entity in api.dashboard_items or load_entity in api.dashboard_items:
1498-
print("**** ERROR: PV yield and load should not be published before every inverter is read ****")
1499-
failed = True
1500-
elif yield_entity not in api.dashboard_items:
1501-
print("**** ERROR: The plant totals do not depend on the inverters and should still be published ****")
1502-
failed = True
1503-
else:
1504-
print("✓ PV yield and load not published before every inverter is read")
1504+
if pv_entity in api.dashboard_items or load_entity in api.dashboard_items:
1505+
print(f"**** ERROR: PV yield and load should not be published before an online inverter is read (onlineStatus {online_status}) ****")
1506+
failed = True
1507+
elif yield_entity not in api.dashboard_items:
1508+
print("**** ERROR: The plant totals do not depend on the inverters and should still be published ****")
1509+
failed = True
1510+
else:
1511+
print(f"✓ PV yield and load not published before an online inverter is read (onlineStatus {online_status})")
15051512

15061513
# Test 5: No inverter reports a yield of its own, so the plant figure is used as before
15071514
print("Test 5: Falls back to the plant yield when no inverter reports one")
@@ -1528,6 +1535,72 @@ def state_of(api, entity_id):
15281535
else:
15291536
print("✓ PV yield held rather than switching to the plant figure")
15301537

1538+
# Test 7: An inverter that reads fine but has no totalYield counts as 0, alone in a plant that leaves the
1539+
# plant figure as the only PV figure there is
1540+
print("Test 7: An inverter with no totalYield of its own counts as 0")
1541+
api = build_api()
1542+
api.plant_inverters[test_plant_id] = [hybrid_sn]
1543+
del api.realtime_device_data[hybrid_sn]["totalYield"]
1544+
await api.publish_plant_info()
1545+
alone_yield = state_of(api, pv_entity)
1546+
1547+
api = build_api()
1548+
del api.realtime_device_data[hybrid_sn]["totalYield"]
1549+
await api.publish_plant_info()
1550+
1551+
if alone_yield != 19569.5:
1552+
print(f"**** ERROR: PV yield should fall back to the plant figure 19569.5 when the only inverter has no totalYield, got {alone_yield} ****")
1553+
failed = True
1554+
elif state_of(api, pv_entity) != 10269.0:
1555+
print(f"**** ERROR: PV yield should be 10269.0 from the other inverter when one has no totalYield, got {state_of(api, pv_entity)} ****")
1556+
failed = True
1557+
else:
1558+
print("✓ An inverter with no totalYield counts as 0 rather than holding the plant")
1559+
1560+
# Test 8: An offline inverter that has never reported contributes 0 rather than holding the plant
1561+
print("Test 8: An offline inverter that never reported contributes 0")
1562+
api = build_api()
1563+
del api.realtime_device_data[hybrid_sn]
1564+
api.realtime_device_failed.add(hybrid_sn)
1565+
api.device_info[hybrid_sn]["onlineStatus"] = 0
1566+
await api.publish_plant_info()
1567+
1568+
if state_of(api, pv_entity) != 10269.0:
1569+
print(f"**** ERROR: PV yield should be 10269.0 with an offline inverter counted as 0, got {state_of(api, pv_entity)} ****")
1570+
failed = True
1571+
else:
1572+
print("✓ Offline inverter with no known yield counted as 0")
1573+
1574+
# Test 9: After a restart the last known yield is seeded from the inverter's own sensor, so a dead
1575+
# inverter stays in the sum rather than stepping the total down
1576+
print("Test 9: Last known yield is seeded from the inverter sensor after a restart")
1577+
api = build_api()
1578+
api.realtime_device_data[hybrid_sn] = {}
1579+
api.realtime_device_failed.add(hybrid_sn)
1580+
api.device_info[hybrid_sn]["onlineStatus"] = 0
1581+
api.dashboard_items[f"sensor.predbat_solax_{test_plant_id}_{hybrid_sn}_total_yield"] = {"state": 459.1, "attributes": {}}
1582+
await api.publish_plant_info()
1583+
1584+
if state_of(api, pv_entity) is None or abs(state_of(api, pv_entity) - 10728.1) > 0.01:
1585+
print(f"**** ERROR: PV yield should be 10728.1 with the dead inverter seeded from its sensor, got {state_of(api, pv_entity)} ****")
1586+
failed = True
1587+
else:
1588+
print("✓ Dead inverter's yield seeded from its sensor")
1589+
1590+
# Test 10: A sensor state that is not a number is not used as a seed
1591+
print("Test 10: A non-numeric sensor state is not a seed")
1592+
api = build_api()
1593+
del api.realtime_device_data[pv_sn]
1594+
api.realtime_device_failed.add(pv_sn)
1595+
api.dashboard_items[f"sensor.predbat_solax_{test_plant_id}_{pv_sn}_total_yield"] = {"state": "unknown", "attributes": {}}
1596+
await api.publish_plant_info()
1597+
1598+
if pv_entity in api.dashboard_items:
1599+
print(f"**** ERROR: PV yield should be held when the only seed is not a number, got {state_of(api, pv_entity)} ****")
1600+
failed = True
1601+
else:
1602+
print("✓ Non-numeric sensor state ignored")
1603+
15311604
return failed
15321605

15331606

@@ -6230,6 +6303,16 @@ async def test_publish_device_realtime_data_main():
62306303
print("✓ Plant pv_power and grid_power held when an inverter failed its read")
62316304

62326305
# Republished on the next good read
6306+
# An offline inverter is not expected to answer, so it counts as 0W rather than holding the plant
6307+
api10.device_info["INV_B"]["onlineStatus"] = 0
6308+
await api10.publish_device_realtime_data()
6309+
if api10.dashboard_items[plant_pv_sensor10]["state"] != 900 or api10.dashboard_items[plant_grid_sensor10]["state"] != -200:
6310+
print(f"**** ERROR: Plant pv_power/grid_power should be 900/-200 with the offline inverter counted as 0, got {api10.dashboard_items[plant_pv_sensor10]['state']}/{api10.dashboard_items[plant_grid_sensor10]['state']} ****")
6311+
failed = True
6312+
else:
6313+
print("✓ Offline inverter counted as 0W in the plant pv_power and grid_power")
6314+
api10.device_info["INV_B"]["onlineStatus"] = 1
6315+
62336316
api10.realtime_device_failed.discard("INV_B")
62346317
await api10.publish_device_realtime_data()
62356318
if api10.dashboard_items[plant_pv_sensor10]["state"] != 1400:

0 commit comments

Comments
 (0)