Skip to content

Commit c24750e

Browse files
mgazzaclaudeCI
authored
fix(gateway): never enable discharge/export after a failed window write (#5532)
* fix(gateway): never enable discharge/export after a failed window write For a direct Hub battery, adjust_force_export wrote the discharge start and end as two separate set_discharge_slot commands, then enabled the scheduled discharge and switched to export mode regardless of whether the endpoint writes were applied. schedule_write_ok only withheld the later commit record. When the end write was refused after the start was accepted, the Hub was left enabled with a stale wrapping window (e.g. 02:55-00:01) and actively exporting. Send both endpoints in one full-serial set_discharge_slot for direct Hub batteries and verify both endpoint read-backs before continuing. Any refusal, staged or partial ACK, timeout, publication error or swallowed async-bridge exception returns before the enable write, export mode and commit, leaving the commit pending for the next cycle to retry. Both endpoints are tracked in the control ledger and share one backoff key. Read-only, entity ownership and serial checks refuse before publishing. Coordinators keep their staged separate-endpoint sequence; other providers, disable and midnight windows are unchanged. Mirrors the charge-window fix in #5531. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018BaJjLpuMEtQYfvmTRcFJf * fix(gateway): detect coordinator staging before discharge ownership checks A coordinator whose discharge end entity was not (yet) owned by the gateway was refused with an ownership error instead of keeping its staged separate-endpoint sequence. Identify coordinators first, as that decision only needs the entity names and discovery record. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018BaJjLpuMEtQYfvmTRcFJf * fix(gateway): stop an armed export when its window move is refused; no re-poll of a refusal - When the paired window write fails while scheduled export is already enabled, turn the schedule off and return to Eco instead of leaving the previous window exporting on a plan that has moved away from it (skipped in read-only mode). - A refusal of the exact paired command ends the attempt immediately: the poll stops when it arrives, and a retry inside the ack window is not published (_send_control would suppress it anyway), so a refused write no longer holds the engine for the full poll timeout per retry. - Drop the unreachable end-serial comparison (the end entity name is derived from the start, so it carries the same serial) and name the test case after what it exercises. - Document that an exception inside a running event loop reaches the caller as None via run_async's thread, not as an exception. - Test the write/moved counters for the paired path. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018BaJjLpuMEtQYfvmTRcFJf * test(gateway): a successful move of an exporting schedule stays enabled The fail-safe disable applies only to a failed window write. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018BaJjLpuMEtQYfvmTRcFJf * fix(gateway): only an explicit Hub refusal turns off an exporting schedule The fail-safe disable fired on any unverified window write, including a slow but successful one (ACK outside the poll, older firmware without an applied block, telemetry not yet landed). That stopped a correctly re-windowed export for a cycle and toggled enable/mode. Take the disable path only when the Hub refused the exact paired command; an unconfirmed write leaves the running export alone and keeps the commit pending for the next cycle. Document that execute.py still reports "Exporting" for a cycle whose window was refused. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018BaJjLpuMEtQYfvmTRcFJf --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> Co-authored-by: CI <ci@example.com>
1 parent dab5ff6 commit c24750e

4 files changed

Lines changed: 469 additions & 2 deletions

File tree

‎apps/predbat/gateway.py‎

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2737,6 +2737,68 @@ async def _update_charge_slot(self, entity_id, hhmm, serial):
27372737
await self._send_control(entity_id, "set_charge_slot", schedule_json=json.dumps(schedule), serial=serial)
27382738
self.log(f"Info: GatewayMQTT: Charge slot update: {schedule}")
27392739

2740+
def discharge_window_uses_staging(self, start_entity, end_entity):
2741+
"""Identify a coordinator synchronously, before crossing an async bridge."""
2742+
if not isinstance(start_entity, str) or not start_entity.endswith("_discharge_slot1_start"):
2743+
return False
2744+
if end_entity != start_entity.removesuffix("_start") + "_end":
2745+
return False
2746+
serial = self._serial_from_entity_id(start_entity)
2747+
record = getattr(self, "_discovery", {}).get((serial or "").upper(), {})
2748+
return bool(serial) and record.get("type") in GATEWAY_COORDINATOR_TYPES
2749+
2750+
async def update_discharge_window(self, start_entity, end_entity, start, end):
2751+
"""Send an engine discharge window as one full-serial command, not two partial slots.
2752+
2753+
True means publication was attempted, not that the inverter applied it. The
2754+
caller must verify both endpoint entities, which only an applied ACK or
2755+
physical telemetry updates. Coordinators use their existing separate path:
2756+
those devices can stage a disabled slot and apply it only on enable.
2757+
Manual single-endpoint selects keep their path.
2758+
"""
2759+
if getattr(self.base, "set_read_only", False):
2760+
return False
2761+
if self.discharge_window_uses_staging(start_entity, end_entity):
2762+
return False
2763+
kwargs = self._discharge_window_kwargs(start_entity, end_entity, start, end)
2764+
if kwargs is None:
2765+
return False
2766+
if self.discharge_window_refused(start_entity, end_entity, start, end):
2767+
# _send_control would not re-send it inside the ack window, so polling again
2768+
# would only wait out the timeout for an answer that has already arrived
2769+
self.log(f"Info: GatewayMQTT: discharge window {start}-{end} for {kwargs['serial']} was just refused, not retrying yet")
2770+
return False
2771+
await self._send_control(start_entity, "set_discharge_slot", **kwargs)
2772+
return True
2773+
2774+
def _discharge_window_kwargs(self, start_entity, end_entity, start, end):
2775+
"""The paired set_discharge_slot fields for a slot1 start/end entity pair, or None."""
2776+
# The end entity name is derived from the start, so both carry the same serial
2777+
if not isinstance(start_entity, str) or not start_entity.endswith("_discharge_slot1_start"):
2778+
return None
2779+
if end_entity != start_entity.removesuffix("_start") + "_end":
2780+
return None
2781+
serial = self._serial_from_entity_id(start_entity)
2782+
if not serial:
2783+
return None
2784+
try:
2785+
times = [datetime.datetime.strptime(value, "%H:%M:%S") for value in (start, end)]
2786+
except (ValueError, TypeError):
2787+
return None
2788+
schedule = {field: value.hour * 100 + value.minute for field, value in zip(("start", "end"), times)}
2789+
return {"schedule_json": json.dumps(schedule), "serial": serial}
2790+
2791+
def discharge_window_refused(self, start_entity, end_entity, start, end):
2792+
"""Whether the hub refused this exact paired window within the ack window."""
2793+
kwargs = self._discharge_window_kwargs(start_entity, end_entity, start, end)
2794+
if kwargs is None or not (getattr(self, "_ack_subscribed", False) and self._mqtt_connected and self._acks_seen):
2795+
return False
2796+
with self._command_lock:
2797+
entry = self._pending_commands.get(start_entity)
2798+
if entry is None or entry["command"] != "set_discharge_slot" or entry["kwargs"] != kwargs:
2799+
return False
2800+
return entry["state"] == "refused" and _monotonic() - entry["sent_at"] < _COMMAND_ACK_WINDOW
2801+
27402802
async def _update_discharge_slot(self, entity_id, hhmm, serial):
27412803
"""Send set_discharge_slot command with updated start or end time."""
27422804
if "_start" in entity_id:

‎apps/predbat/inverter.py‎

Lines changed: 108 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2957,6 +2957,87 @@ def adjust_idle_time(self, charge_start=None, charge_end=None, discharge_start=N
29572957
self.write_and_poll_option("idle_end_time", idle_end_time_id, idle_end)
29582958
self.idle_end_minutes = idle_end_minutes
29592959

2960+
def write_and_poll_gateway_discharge_window(self, new_start, new_end):
2961+
"""Verify a complete Hub slot together, or return None for another provider.
2962+
2963+
A start equal to the old end cannot be sent alone while discharging is enabled:
2964+
it temporarily disables the slot and the Hub correctly refuses it. Sending
2965+
both bounds also handles swapping the bounds of a midnight-spanning slot.
2966+
"""
2967+
from ha import run_async
2968+
2969+
entities = [self.base.get_arg(name, indirect=False, index=self.id) for name in ("discharge_start_time", "discharge_end_time")]
2970+
owners = [getattr(self.base, "dashboard_index_app", {}).get(entity) for entity in entities]
2971+
if "gateway" not in owners:
2972+
return None
2973+
components = getattr(self.base, "components", None)
2974+
gateway = components.get_component("gateway") if components else None
2975+
names = ("discharge_start_time", "discharge_end_time")
2976+
target = (new_start, new_end)
2977+
if gateway is not None and gateway.discharge_window_uses_staging(*entities):
2978+
return None # Coordinator slot staging must precede its enable write.
2979+
if owners != ["gateway", "gateway"] or gateway is None:
2980+
self.base.record_status(f"Warn: Inverter {self.id} Hub discharge window has inconsistent entity ownership", had_errors=True)
2981+
return False
2982+
if not all(self.check_write_entity("write_and_poll_gateway_discharge_window", name, entity, value) for name, entity, value in zip(names, entities, target)):
2983+
return False
2984+
2985+
def read_pair():
2986+
"""Read both endpoint entities without substituting desired values."""
2987+
return tuple(self.base.get_state_wrapper(entity, refresh=True) for entity in entities)
2988+
2989+
before = read_pair()
2990+
ledger = self.base.control_ledger
2991+
if ledger is not None:
2992+
for name, entity, value, wanted in zip(names, entities, before, target):
2993+
self._ledger_observe(ledger, name, entity, value)
2994+
if value != wanted:
2995+
ledger.note_write_attempt(entity)
2996+
key = "|".join(entities)
2997+
verified = before == target
2998+
for _retry in range(0 if verified else self._write_attempts("discharge_window", key, target)):
2999+
try:
3000+
published = run_async(gateway.update_discharge_window(*entities, *target))
3001+
except Exception as error:
3002+
# Only reached without a running event loop: inside one, run_async's thread hands
3003+
# the exception to the thread exception hook and returns None, handled just below
3004+
self.base.log(f"Warn: Inverter {self.id} Hub discharge window publication failed: {error}")
3005+
break
3006+
if published is not True:
3007+
break
3008+
# A refusal is final for this cycle (the identical command is not re-sent within the
3009+
# ack window), so stop waiting as soon as it arrives rather than polling to the timeout
3010+
self._poll_after_write(entities[0], lambda state: (state == new_start and self.base.get_state_wrapper(entities[1], refresh=True) == new_end) or gateway.discharge_window_refused(*entities, *target))
3011+
verified = read_pair() == target
3012+
if not verified and gateway.discharge_window_refused(*entities, *target):
3013+
break
3014+
if verified:
3015+
self.count_register_writes += 1
3016+
self.registers_moved += sum(old != wanted for old, wanted in zip(before, target))
3017+
break
3018+
self._write_backoff_result("discharge_window", key, target, verified)
3019+
if ledger is not None:
3020+
for name, entity, value in zip(names, entities, target):
3021+
if verified:
3022+
if before == target:
3023+
ledger.record_ownership_from_read(entity, name, value, now=time.time(), generation=self._ledger_generation(entity))
3024+
else:
3025+
ledger.record_write(entity, name, value, now=time.time(), generation=self._ledger_generation(entity))
3026+
else:
3027+
ledger.clear(entity)
3028+
if not verified:
3029+
message = f"Warn: Inverter {self.id} write to Hub discharge window failed: wanted {target}, got {read_pair()}"
3030+
self.base.log(message)
3031+
self.base.record_status(message, had_errors=True)
3032+
return verified
3033+
3034+
def gateway_discharge_window_refused(self, new_start, new_end):
3035+
"""Whether the Hub explicitly refused this exact paired discharge window just now."""
3036+
entities = [self.base.get_arg(name, indirect=False, index=self.id) for name in ("discharge_start_time", "discharge_end_time")]
3037+
components = getattr(self.base, "components", None)
3038+
gateway = components.get_component("gateway") if components else None
3039+
return gateway is not None and gateway.discharge_window_refused(*entities, new_start, new_end)
3040+
29603041
def adjust_force_export(self, force_export, new_start_time=None, new_end_time=None):
29613042
"""
29623043
Adjust force export on/off and set the time window correctly
@@ -3054,8 +3135,33 @@ def adjust_force_export(self, force_export, new_start_time=None, new_end_time=No
30543135
else:
30553136
self.adjust_idle_time(discharge_start="00:00:00", discharge_end="00:00:00")
30563137

3138+
# Direct Hub batteries accept the complete slot as one physical write. Do not
3139+
# enable or enter export mode after a refused, staged or incomplete read-back.
3140+
# Coordinators retain their existing staged-endpoint then enable sequence.
3141+
gateway_window = None
3142+
if (new_start and new_start != old_start) or (new_end and new_end != old_end):
3143+
gateway_window = self.write_and_poll_gateway_discharge_window(new_start or old_start, new_end or old_end)
3144+
if gateway_window is False:
3145+
refused = self.gateway_discharge_window_refused(new_start or old_start, new_end or old_end)
3146+
if force_export and old_discharge_enable and refused and not getattr(self.base, "set_read_only", False):
3147+
# The Hub refused the move, so the previous window is still armed: leaving it
3148+
# would keep exporting on a schedule the plan has moved away from, so stop
3149+
# exporting until a write lands. Only an explicit refusal does this - a write
3150+
# that is merely not confirmed yet (slow ACK, telemetry lag) may well have
3151+
# applied, and the plan still wants to export, so it is left running and the
3152+
# pending commit retries it. (Without force_export the schedule was already
3153+
# turned off above.) The caller is not told: execute.py still reports
3154+
# "Exporting" for this cycle, and the next cycle retries the window.
3155+
self.log("Warn: Inverter {} Hub refused the discharge window, turning off scheduled export".format(self.id))
3156+
self.write_and_poll_switch("scheduled_discharge_enable", self.base.get_arg("scheduled_discharge_enable", indirect=False, index=self.id), False)
3157+
self.adjust_inverter_mode(False)
3158+
self.record_commit("export_window", (new_start, new_end, force_export), False)
3159+
return
3160+
if gateway_window is True:
3161+
changed_start_end = True
3162+
30573163
# Change start time
3058-
if new_start and (new_start != old_start or is_hm_format):
3164+
if gateway_window is None and new_start and (new_start != old_start or is_hm_format):
30593165
self.base.log("Inverter {} set new export start time to {}".format(self.id, new_start))
30603166
if "discharge_start_time" in self.base.args:
30613167
# Always write to this as it is the GE default
@@ -3077,7 +3183,7 @@ def adjust_force_export(self, force_export, new_start_time=None, new_end_time=No
30773183
self.log("Warn: Inverter {} unable write export start time as neither REST or discharge_start_time are set".format(self.id))
30783184

30793185
# Change end time
3080-
if new_end and (new_end != old_end or is_hm_format):
3186+
if gateway_window is None and new_end and (new_end != old_end or is_hm_format):
30813187
self.base.log("Inverter {} Set new export end time to {} was {}".format(self.id, new_end, old_end))
30823188
if "discharge_end_time" in self.base.args:
30833189
# Always write to this as it is the GE default

‎apps/predbat/tests/test_gateway.py‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7444,9 +7444,11 @@ def test_counters_restart_at_local_midnight(self):
74447444
def run_gateway_tests(my_predbat=None):
74457445
"""Run all GatewayMQTT tests. Returns True on failure, False on success."""
74467446
from tests.test_gateway_token_refresh import TestIsAuthFailure, TestApplyRefreshResponse, TestMaybeRefreshOnAuthError
7447+
from tests.test_gateway_discharge_window import TestGatewayDischargeWindow
74477448
from tests.test_gateway_display import TestGatewayDisplayOnly
74487449

74497450
test_classes = [
7451+
TestGatewayDischargeWindow,
74507452
TestGatewayDisplayOnly,
74517453
TestProtobufDecode,
74527454
TestPlanSerialization,

0 commit comments

Comments
 (0)