Skip to content

Commit 57b7b4a

Browse files
mgazzaclaude
andcommitted
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
1 parent 529e6f8 commit 57b7b4a

3 files changed

Lines changed: 80 additions & 12 deletions

File tree

‎apps/predbat/gateway.py‎

Lines changed: 33 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2675,22 +2675,46 @@ async def update_discharge_window(self, start_entity, end_entity, start, end):
26752675
"""
26762676
if getattr(self.base, "set_read_only", False):
26772677
return False
2678-
if not isinstance(start_entity, str) or not start_entity.endswith("_discharge_slot1_start"):
2679-
return False
2680-
if end_entity != start_entity.removesuffix("_start") + "_end":
2678+
if self.discharge_window_uses_staging(start_entity, end_entity):
26812679
return False
2682-
serial = self._serial_from_entity_id(start_entity)
2683-
if not serial or self._serial_from_entity_id(end_entity) != serial:
2680+
kwargs = self._discharge_window_kwargs(start_entity, end_entity, start, end)
2681+
if kwargs is None:
26842682
return False
2685-
if self.discharge_window_uses_staging(start_entity, end_entity):
2683+
if self.discharge_window_refused(start_entity, end_entity, start, end):
2684+
# _send_control would not re-send it inside the ack window, so polling again
2685+
# would only wait out the timeout for an answer that has already arrived
2686+
self.log(f"Info: GatewayMQTT: discharge window {start}-{end} for {kwargs['serial']} was just refused, not retrying yet")
26862687
return False
2688+
await self._send_control(start_entity, "set_discharge_slot", **kwargs)
2689+
return True
2690+
2691+
def _discharge_window_kwargs(self, start_entity, end_entity, start, end):
2692+
"""The paired set_discharge_slot fields for a slot1 start/end entity pair, or None."""
2693+
# The end entity name is derived from the start, so both carry the same serial
2694+
if not isinstance(start_entity, str) or not start_entity.endswith("_discharge_slot1_start"):
2695+
return None
2696+
if end_entity != start_entity.removesuffix("_start") + "_end":
2697+
return None
2698+
serial = self._serial_from_entity_id(start_entity)
2699+
if not serial:
2700+
return None
26872701
try:
26882702
times = [datetime.datetime.strptime(value, "%H:%M:%S") for value in (start, end)]
26892703
except (ValueError, TypeError):
2690-
return False
2704+
return None
26912705
schedule = {field: value.hour * 100 + value.minute for field, value in zip(("start", "end"), times)}
2692-
await self._send_control(start_entity, "set_discharge_slot", schedule_json=json.dumps(schedule), serial=serial)
2693-
return True
2706+
return {"schedule_json": json.dumps(schedule), "serial": serial}
2707+
2708+
def discharge_window_refused(self, start_entity, end_entity, start, end):
2709+
"""Whether the hub refused this exact paired window within the ack window."""
2710+
kwargs = self._discharge_window_kwargs(start_entity, end_entity, start, end)
2711+
if kwargs is None or not (getattr(self, "_ack_subscribed", False) and self._mqtt_connected and self._acks_seen):
2712+
return False
2713+
with self._command_lock:
2714+
entry = self._pending_commands.get(start_entity)
2715+
if entry is None or entry["command"] != "set_discharge_slot" or entry["kwargs"] != kwargs:
2716+
return False
2717+
return entry["state"] == "refused" and _monotonic() - entry["sent_at"] < _COMMAND_ACK_WINDOW
26942718

26952719
async def _update_discharge_slot(self, entity_id, hhmm, serial):
26962720
"""Send set_discharge_slot command with updated start or end time."""

‎apps/predbat/inverter.py‎

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2969,12 +2969,18 @@ def read_pair():
29692969
try:
29702970
published = run_async(gateway.update_discharge_window(*entities, *target))
29712971
except Exception as error:
2972+
# Only reached without a running event loop: inside one, run_async's thread hands
2973+
# the exception to the thread exception hook and returns None, handled just below
29722974
self.base.log(f"Warn: Inverter {self.id} Hub discharge window publication failed: {error}")
29732975
break
29742976
if published is not True:
29752977
break
2976-
self._poll_after_write(entities[0], lambda state: state == new_start and self.base.get_state_wrapper(entities[1], refresh=True) == new_end)
2978+
# A refusal is final for this cycle (the identical command is not re-sent within the
2979+
# ack window), so stop waiting as soon as it arrives rather than polling to the timeout
2980+
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))
29772981
verified = read_pair() == target
2982+
if not verified and gateway.discharge_window_refused(*entities, *target):
2983+
break
29782984
if verified:
29792985
self.count_register_writes += 1
29802986
self.registers_moved += sum(old != wanted for old, wanted in zip(before, target))
@@ -3099,6 +3105,13 @@ def adjust_force_export(self, force_export, new_start_time=None, new_end_time=No
30993105
if (new_start and new_start != old_start) or (new_end and new_end != old_end):
31003106
gateway_window = self.write_and_poll_gateway_discharge_window(new_start or old_start, new_end or old_end)
31013107
if gateway_window is False:
3108+
if force_export and old_discharge_enable and not getattr(self.base, "set_read_only", False):
3109+
# The previous window is still armed: leaving it would keep exporting on a
3110+
# schedule the plan has moved away from, so stop exporting until a write lands.
3111+
# (Without force_export the schedule was already turned off above.)
3112+
self.log("Warn: Inverter {} Hub discharge window write failed, turning off scheduled export".format(self.id))
3113+
self.write_and_poll_switch("scheduled_discharge_enable", self.base.get_arg("scheduled_discharge_enable", indirect=False, index=self.id), False)
3114+
self.adjust_inverter_mode(False)
31023115
self.record_commit("export_window", (new_start, new_end, force_export), False)
31033116
return
31043117
if gateway_window is True:

‎apps/predbat/tests/test_gateway_discharge_window.py‎

Lines changed: 33 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -140,6 +140,36 @@ def test_pair_applied_before_enable_and_full_serial(self):
140140
self.assertEqual((gw.cache[base.START], gw.cache[base.END], gw.cache[base.ENABLE]), ("02:55:00", "03:01:00", "on"))
141141
self.assertFalse(base.errors)
142142

143+
def test_paired_write_counts_one_write_and_both_moved_registers(self):
144+
"""The pair is one physical write that moves both endpoints."""
145+
inv, gw, base, wire, mode = self.fixture(enabled="on")
146+
self.apply(inv)
147+
self.assertEqual([command["command"] for command in wire], ["set_discharge_slot"])
148+
self.assertEqual((inv.count_register_writes, inv.registers_moved), (1, 2))
149+
150+
def test_refused_window_disables_an_already_exporting_schedule(self):
151+
"""A refused move must not leave the previous enabled window exporting."""
152+
inv, gw, base, wire, mode = self.fixture(outcome="refused", start="02:00:00", end="04:00:00", enabled="on")
153+
self.apply(inv, start="02:30:00", end="03:00:00")
154+
self.assertEqual(json.loads(wire[0]["schedule_json"]), {"start": 230, "end": 300})
155+
self.assertEqual((gw.cache[base.START], gw.cache[base.END]), ("02:00:00", "04:00:00"))
156+
self.assertEqual(gw.cache[base.ENABLE], "off")
157+
self.assertEqual([command.get("enable") for command in wire if command["command"] == "set_discharge_enable"], [False])
158+
inv.adjust_inverter_mode.assert_called_once_with(False)
159+
self.assertTrue(inv.commit_pending["export_window"])
160+
161+
def test_refusal_is_not_polled_to_timeout_or_resent(self):
162+
"""A refusal ends the attempt at once; the identical command is not re-sent or re-polled."""
163+
inv, gw, base, wire, mode = self.fixture(outcome="refused")
164+
inv.inv_write_and_poll_sleep = 3600 # any poll to timeout would hang the test
165+
polls = []
166+
original = inv._poll_after_write
167+
inv._poll_after_write = lambda *args, **kwargs: polls.append(args) or original(*args, **kwargs)
168+
self.apply(inv)
169+
self.assertEqual(len(polls), 1)
170+
self.assertEqual([command["command"] for command in wire], ["set_discharge_slot"])
171+
self.assertTrue(base.errors)
172+
143173
def test_unverified_pair_clears_ledger_and_cannot_activate(self):
144174
"""Refusal, incomplete or staged ACKs, timeout and exceptions fail closed."""
145175
for outcome in ("refused", "partial", "staged", "timeout", "exception"):
@@ -166,15 +196,16 @@ def test_retry_after_refusal_can_apply_and_enable(self):
166196
self.assertFalse(inv.commit_pending["export_window"])
167197
self.assertEqual(json.loads(wire[-2]["schedule_json"]), {"start": 255, "end": 301})
168198

169-
def test_read_only_owner_and_serial_refuse(self):
199+
def test_read_only_owner_and_mismatched_pair_refuse(self):
170200
"""Do not publish across read-only or mismatched endpoint boundaries."""
171-
for reason in ("read_only", "owner", "serial"):
201+
for reason in ("read_only", "owner", "mismatched_pair"):
172202
inv, gw, base, wire, mode = self.fixture()
173203
if reason == "read_only":
174204
base.set_read_only = True
175205
elif reason == "owner":
176206
base.dashboard_index_app[base.END] = "gecloud"
177207
else:
208+
# An end slot of another serial is not this start's pair
178209
entity = base.END.replace("456789", "654321")
179210
base.args["discharge_end_time"] = entity
180211
base.dashboard_index_app[entity] = "gateway"

0 commit comments

Comments
 (0)