Skip to content

Commit a53c4c8

Browse files
fix(deye): apply the SOC floor only at the API boundary
Copilot review on #4363 spotted that publish_schedule_settings_ha() clamped the published reserve while apply_reserve_live() stored the raw value, so the two could disagree. Resolved the other way round from the suggestion. Clamping at entry as well would make the internal state self-consistent but would not fix the problem described: Predbat writes this entity and then reads it back to confirm (write_and_poll_value), so if it writes 4 and we publish 14 the read-back cannot match whichever end is clamped, and it retries until it gives up — the exact "didn't complete got 0.0" failure this component was just fixed for. The entity is Predbat's control surface, so it now echoes what Predbat wrote, everywhere: the entity read, the control event and the published state all store and emit the raw value. The floor is applied in one place, build_dynamic_payload, immediately before the payload leaves for the API — which is what actually protects the battery, and cannot be bypassed by any caller. Narrow in practice: adjust_reserve already clamps to reserve_percent, which comes from the battery_min_soc sensor this component publishes, so the automatic path can never write below the floor. A manually configured install that does not map battery_min_soc defaults it to 4.0 and could. The clamp now logs once per serial when it actually lifts a value. Routine before Predbat has written the reserve; a persistent complaint means Predbat is planning below the inverter's floor, which is worth seeing rather than silently correcting. Adds a test that the reserve entity echoes a below-floor write verbatim while the payload built from it is still lifted to the floor. Verified to fail against the clamped-publish version: "expected the entity to echo 4, got 14". Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent adc2ab3 commit a53c4c8

3 files changed

Lines changed: 58 additions & 11 deletions

File tree

apps/predbat/deye.py

Lines changed: 20 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -129,6 +129,7 @@ def initialize(
129129
self._tier_refreshed = {}
130130
self._cache_restored = False
131131
self._saved_ratings = None # signature of the ratings last written, to skip no-op saves
132+
self._soc_floor_warned = set()
132133
self.battery_nominal_voltage = self._as_float(battery_nominal_voltage, 0.0)
133134
# auth_method defaults to app_credentials, but an injected access token with no
134135
# developer app credentials can only be oauth — and in app_credentials mode
@@ -639,9 +640,17 @@ def build_dynamic_payload(self, sn, schedule, current_soc, now_minutes=None):
639640
# 14%. Applied here, at the last step, so no other caller can bypass it.
640641
floor = self.battery_reserve_min(sn)
641642
if floor > 0:
643+
lifted = False
642644
for slot in slots:
643645
if slot.get(TOU_FIELD["soc"], 0) < floor:
644646
slot[TOU_FIELD["soc"]] = floor
647+
lifted = True
648+
# Say so once per serial. Routine before Predbat has written the reserve, but a
649+
# persistent complaint means Predbat is planning below the inverter's floor —
650+
# usually battery_min_soc not being mapped, which is worth surfacing.
651+
if lifted and sn not in self._soc_floor_warned:
652+
self._soc_floor_warned.add(sn)
653+
self.log(f"Info: DEYE {sn} raising requested slot SOC to the inverter's {floor}% floor (config/battery battLowCapacity)")
645654
return {
646655
"deviceSn": sn,
647656
"workMode": active["work_mode"],
@@ -802,15 +811,16 @@ async def publish_data(self):
802811
async def publish_schedule_settings_ha(self, sn):
803812
"""Publish the charge/export schedule control entities for one inverter."""
804813
local = self.local_schedule.get(sn, {})
805-
# The entity starts life at 0 and only reaches a real value once Predbat writes it,
806-
# so surface the inverter's own floor as both the published value and the entity
807-
# minimum rather than advertising a reserve the hardware would never honour.
808-
floor = self.battery_reserve_min(sn)
809-
reserve = max(int(local.get("reserve", 0)), floor)
814+
# Deliberately NOT clamped to the inverter floor. This entity is Predbat's control
815+
# surface: it writes a value then reads it back to confirm (write_and_poll_value),
816+
# so publishing anything other than what was written guarantees a mismatch and a
817+
# retry storm. The floor is enforced at the API boundary instead, in
818+
# build_dynamic_payload, which is what actually protects the battery.
819+
reserve = int(local.get("reserve", 0))
810820
self.dashboard_item(
811821
self._control_name("number", sn, "battery_schedule_reserve"),
812822
state=reserve,
813-
attributes={"min": floor, "max": 100, "step": 1, "unit_of_measurement": "%", "friendly_name": f"DEYE {sn} Battery Schedule Reserve", "icon": "mdi:gauge"},
823+
attributes={"min": 0, "max": 100, "step": 1, "unit_of_measurement": "%", "friendly_name": f"DEYE {sn} Battery Schedule Reserve", "icon": "mdi:gauge"},
814824
app="deye",
815825
)
816826
for direction in ("charge", "export"):
@@ -842,10 +852,7 @@ async def get_schedule_settings_ha(self, sn):
842852
before Predbat republishes - falls back to 0 rather than raising and crashing the
843853
reconciliation loop (mirrors the Fox defensiveness).
844854
"""
845-
# Clamped to the inverter's own floor, exactly as fox.py does with fdsoc_min: the
846-
# entity reads 0 until Predbat has written it, and 0 is below what the hardware
847-
# will honour.
848-
schedule = {"reserve": max(int(self._as_float(self.get_state_wrapper(self._control_name("number", sn, "battery_schedule_reserve"), default=0), 0)), self.battery_reserve_min(sn))}
855+
schedule = {"reserve": int(self._as_float(self.get_state_wrapper(self._control_name("number", sn, "battery_schedule_reserve"), default=0), 0))}
849856
for direction in ("charge", "export"):
850857
schedule[direction] = {
851858
"enable": self.get_state_wrapper(self._control_name("switch", sn, f"battery_schedule_{direction}_enable"), default="off") == "on",
@@ -912,7 +919,9 @@ def update_local_schedule(self, sn, entity_id, value):
912919
"""
913920
schedule = self.local_schedule.setdefault(sn, {})
914921
if entity_id.endswith("battery_schedule_reserve"):
915-
schedule["reserve"] = max(int(self._as_float(value, 0)), self.battery_reserve_min(sn))
922+
# Stored exactly as written; the floor is applied at the API boundary so the
923+
# read-back Predbat performs always matches what it wrote.
924+
schedule["reserve"] = int(self._as_float(value, 0))
916925
return True
917926
direction = "charge" if "_charge_" in entity_id else ("export" if "_export_" in entity_id else None)
918927
if not direction:

apps/predbat/tests/test_deye_api.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@ def __init__(self, auth_method="app_credentials", data_center="eu", inverter_sn=
5050
self.cached_values = {}
5151
self._tier_refreshed = {}
5252
self._cache_restored = False
53+
self._soc_floor_warned = set()
5354
self.log_messages = []
5455
self.local_tz = pytz.timezone("Europe/London")
5556
self.base = MagicMock()

apps/predbat/tests/test_deye_publish.py

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -498,6 +498,42 @@ async def fake_apply(sn, force=False):
498498
assert not failed, "test_write_button_applies_and_is_not_stored_as_schedule"
499499

500500

501+
def test_reserve_entity_echoes_the_written_value_even_below_the_floor():
502+
"""The reserve entity must echo exactly what Predbat wrote, floor or no floor.
503+
504+
Predbat writes this entity then reads it back to confirm (write_and_poll_value), so
505+
republishing a clamped value can never match what was written and would retry until it
506+
gave up — the same "didn't complete got 0.0" failure this component already had. The
507+
floor is enforced at the API boundary in build_dynamic_payload instead, which is what
508+
actually protects the battery.
509+
"""
510+
failed = False
511+
d = RecordingDeye()
512+
d.device_list = ["INV1"]
513+
d.device_values = {"INV1": {"soc": 50.0}}
514+
d.device_battery_config = {"INV1": {"battLowCapacity": 14}}
515+
entity = "number.predbat_deye_inv1_battery_schedule_reserve"
516+
import tests.test_infra as ti
517+
518+
async def fake_apply(sn, schedule, current_soc, force=False):
519+
"""Stand in for the live control write."""
520+
return True
521+
522+
# A below-floor write is echoed verbatim so the read-back matches
523+
with patch.object(d, "apply_dynamic_control", side_effect=fake_apply):
524+
ti.run_async(d.number_event(entity, 4))
525+
if d.published.get(entity) != 4:
526+
print(f"ERROR: expected the entity to echo 4, got {d.published.get(entity)!r}")
527+
failed = True
528+
529+
# ...but the payload that reaches the inverter is still lifted to the floor
530+
socs = [s["soc"] for s in d.build_dynamic_payload("INV1", d.local_schedule["INV1"], 50)["timeUseSettingItems"]]
531+
if any(s < 14 for s in socs):
532+
print(f"ERROR: the payload must still respect the 14% floor: {socs}")
533+
failed = True
534+
assert not failed, "test_reserve_entity_echoes_the_written_value_even_below_the_floor"
535+
536+
501537
def test_reserve_write_is_republished():
502538
"""A reserve change is pushed to the inverter and republished for the read-back."""
503539
failed = False
@@ -559,6 +595,7 @@ def run_deye_publish_tests(my_predbat):
559595
("control_writes_republished", test_control_entity_writes_are_republished),
560596
("write_button_not_stored", test_write_button_applies_and_is_not_stored_as_schedule),
561597
("reserve_republished", test_reserve_write_is_republished),
598+
("reserve_echoes_written", test_reserve_entity_echoes_the_written_value_even_below_the_floor),
562599
("unknown_entity_ignored", test_unrelated_entity_does_not_corrupt_schedule),
563600
]:
564601
try:

0 commit comments

Comments
 (0)