Skip to content

Commit 58cec78

Browse files
mgazzaclaude
andcommitted
fix(gateway): bound attempt ids, detect acks during a failing publish, time replays
Review follow-ups: - prune the per-attempt id list with the kept ids; - roll back a failed publish only if no ack of any kind arrived meanwhile (an ack counter, not an outcome comparison); - a replay of the newest id after another send applied the value still restarts the refusal window. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018BaJjLpuMEtQYfvmTRcFJf
1 parent 504e512 commit 58cec78

2 files changed

Lines changed: 55 additions & 6 deletions

File tree

‎apps/predbat/gateway.py‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1973,7 +1973,7 @@ async def _send_control(self, entity_id, command, **kwargs):
19731973
self._acks_seen = False
19741974
self.log(f"Warn: GatewayMQTT: {command} for {kwargs.get('serial')} was never acknowledged, re-sending until telemetry confirms it")
19751975
if not same:
1976-
entry = {"command_ids": [], "outcomes": {}, "command": command, "kwargs": dict(kwargs), "state": "sent", "sent_at": now, "resent": False, "cached": None, "cached_ids": [], "attempt_ids": []}
1976+
entry = {"command_ids": [], "outcomes": {}, "command": command, "kwargs": dict(kwargs), "state": "sent", "sent_at": now, "resent": False, "cached": None, "cached_ids": [], "attempt_ids": [], "acks": 0}
19771977
self._pending_commands[entity_id] = entry
19781978
previous = {"command_ids": list(entry["command_ids"]), "attempt_ids": list(entry["attempt_ids"]), "outcomes": dict(entry["outcomes"]), "state": entry["state"], "sent_at": entry["sent_at"], "resent": entry["resent"], "cached": entry["cached"]}
19791979
if entry["state"] != "sent":
@@ -1988,9 +1988,9 @@ async def _send_control(self, entity_id, command, **kwargs):
19881988
# Earlier ids of the same value stay matchable: an ack for any of them confirms it
19891989
entry["command_ids"] = entry["command_ids"][-(_COMMAND_ACK_IDS_KEPT - 1) :] + [command_ref]
19901990
entry["outcomes"] = {key: value for key, value in entry["outcomes"].items() if key in entry["command_ids"]}
1991-
entry["attempt_ids"] = entry["attempt_ids"] + [command_ref]
1991+
entry["attempt_ids"] = [ref for ref in entry["attempt_ids"] if ref in entry["command_ids"]] + [command_ref]
19921992
entry["sent_at"] = now
1993-
outcomes_sent = dict(entry["outcomes"])
1993+
acks_before = entry["acks"]
19941994

19951995
try:
19961996
await self.publish_command(command, command_id=command_id, **kwargs)
@@ -2003,7 +2003,7 @@ async def _send_control(self, entity_id, command, **kwargs):
20032003
if self._pending_commands.get(entity_id) is entry and command_ref in entry["command_ids"] and command_ref not in entry["outcomes"]:
20042004
if not previous["command_ids"]:
20052005
del self._pending_commands[entity_id]
2006-
elif entry["outcomes"] == outcomes_sent:
2006+
elif entry["acks"] == acks_before:
20072007
# No ack arrived meanwhile: back to how the earlier attempt left it,
20082008
# including any id pruned to make room for this one
20092009
entry.update(previous)
@@ -2041,6 +2041,7 @@ def _process_ack(self, data):
20412041
if entry is None or ack.get("command", entry["command"]) != entry["command"]:
20422042
return
20432043
self._acks_seen = True
2044+
entry["acks"] += 1 # lets a failing publish tell whether any ack arrived meanwhile
20442045
# A replay for the newest id, before any unit answered it, means the hub has seen that
20452046
# id before and never considered this send. Drop the id; if nothing else has answered
20462047
# the value, let the next attempt go out under a fresh id straight away. For an older
@@ -2051,6 +2052,7 @@ def _process_ack(self, data):
20512052
# Another send in this attempt already applied the value: only forget this id
20522053
entry["command_ids"].remove(command_id)
20532054
entry["attempt_ids"].remove(command_id)
2055+
entry["sent_at"] = _monotonic() # still a refusal: the window runs from it
20542056
elif drop_replay:
20552057
del self._pending_commands[entity_id]
20562058
else:

‎apps/predbat/tests/test_gateway.py‎

Lines changed: 49 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5177,15 +5177,21 @@ def test_replay_for_the_newest_id_after_an_earlier_send_applied(self):
51775177
self._run(gw.number_event(self.CHARGE_RATE, 2000))
51785178
clock["now"] = 1030.0
51795179
self._run(gw.number_event(self.CHARGE_RATE, 2000))
5180+
clock["now"] = 1031.0
51805181
self._ack(gw, "PBAT1", "set_charge_rate", applied={"power_w": 2000, "slot": 0})
5182+
clock["now"] = 1059.0
51815183
self._ack(gw, "PBAT2", "set_charge_rate", ok=False, error="replay")
51825184
entry = gw._pending_commands[self.CHARGE_RATE]
51835185
assert entry["command_ids"] == ["PBAT1"]
51845186
assert entry["state"] == "applied"
51855187
assert self._logged(gw, "already applied by another send")
5186-
clock["now"] = 1040.0
5188+
# The replay is a refusal too: the window runs from it
5189+
clock["now"] = 1060.0
51875190
self._run(gw.number_event(self.CHARGE_RATE, 2000))
5188-
assert len(gw._published) == 2
5191+
assert len(gw._published) == 2
5192+
clock["now"] = 1089.0
5193+
self._run(gw.number_event(self.CHARGE_RATE, 2000))
5194+
assert len(gw._published) == 3
51895195

51905196
def test_replay_for_the_newest_id_with_only_an_older_attempts_ok_drops_the_entry(self):
51915197
"""An ok from an earlier attempt does not count: a replay of the fresh attempt's id re-sends straight away."""
@@ -5232,6 +5238,34 @@ async def failing_publish_command(command, command_id=None, **kwargs):
52325238
self._ack(gw, "PBAT1", "set_charge_rate", applied={"power_w": 2000, "slot": 0})
52335239
assert gw._pending_commands[self.CHARGE_RATE]["state"] == "applied"
52345240

5241+
def test_repeated_refusal_during_a_failing_publish_is_not_rolled_back(self):
5242+
"""A refusal repeated while a later publish awaits the broker keeps its restarted window when that publish fails."""
5243+
gw = self._make_gateway()
5244+
clock, patcher = self._clock()
5245+
with patcher:
5246+
self._run(gw.number_event(self.CHARGE_RATE, 2000))
5247+
self._ack(gw, "PBAT1", "set_charge_rate", ok=False, error="not_managed")
5248+
5249+
async def refusal_then_fail(command, command_id=None, **kwargs):
5250+
clock["now"] = 1035.0
5251+
self._ack(gw, "PBAT1", "set_charge_rate", ok=False, error="not_managed")
5252+
raise RuntimeError("broker gone")
5253+
5254+
real_publish = gw.publish_command
5255+
gw.publish_command = refusal_then_fail
5256+
clock["now"] = 1031.0
5257+
try:
5258+
self._run(gw.number_event(self.CHARGE_RATE, 2000))
5259+
except RuntimeError:
5260+
pass
5261+
gw.publish_command = real_publish
5262+
entry = gw._pending_commands[self.CHARGE_RATE]
5263+
assert entry["command_ids"] == ["PBAT1"]
5264+
assert entry["sent_at"] == 1035.0
5265+
clock["now"] = 1036.0
5266+
self._run(gw.number_event(self.CHARGE_RATE, 2000))
5267+
assert len(gw._published) == 1
5268+
52355269
def test_outcomes_do_not_grow_past_the_kept_ids(self):
52365270
"""Per-id outcomes are pruned with the ids, so a value re-sent for a long time does not grow the entry."""
52375271
gw = self._make_gateway()
@@ -5245,6 +5279,19 @@ def test_outcomes_do_not_grow_past_the_kept_ids(self):
52455279
assert len(entry["command_ids"]) == 16
52465280
assert set(entry["outcomes"]) <= set(entry["command_ids"])
52475281

5282+
def test_attempt_ids_do_not_grow_without_acks(self):
5283+
"""Subscribed but no acks ever: every call re-sends, and the id lists stay bounded."""
5284+
gw = self._make_gateway(acks_seen=False)
5285+
clock, patcher = self._clock()
5286+
with patcher:
5287+
for step in range(50):
5288+
clock["now"] = 1000.0 + step * 2
5289+
self._run(gw.number_event(self.CHARGE_RATE, 2000))
5290+
entry = gw._pending_commands[self.CHARGE_RATE]
5291+
assert len(gw._published) == 50
5292+
assert len(entry["command_ids"]) == 16
5293+
assert len(entry["attempt_ids"]) == 16
5294+
52485295
def test_second_ok_ack_does_not_overwrite_cache(self):
52495296
"""Only the first ok ack for a command updates the cached slot times."""
52505297
gw = self._make_gateway()

0 commit comments

Comments
 (0)