Skip to content

Commit 9638518

Browse files
mgazzaclaude
andcommitted
fix(gateway): dispatch MQTT publishes onto the owning event loop
Every gateway control write (set_reserve, set_charge_rate, slot times, ...) reaches GatewayMQTT via ha.py::run_async(), which runs the call chain on a fresh, throwaway event loop distinct from the persistent loop that owns GatewayMQTT's aiomqtt Client. Calling client.publish() from that other loop binds the QoS-1 publish-confirmation Future to the wrong loop, so every control write stalls for the client's ~10s default timeout before completing — confirmed with a live reproduction against a local broker (10.001s cross-loop vs 0.001s same-loop). _publish_raw() now detects when it's running on a different loop than the one captured from run() and hands the actual publish off via run_coroutine_threadsafe, so it executes on the owning loop instead. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
1 parent f2d87f5 commit 9638518

2 files changed

Lines changed: 75 additions & 1 deletion

File tree

apps/predbat/gateway.py

Lines changed: 29 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -244,6 +244,13 @@ def initialize(self, gateway_device_id=None, mqtt_host=None, mqtt_port=8883, mqt
244244
self._mqtt_client = None
245245
self._mqtt_task = None
246246
self._mqtt_connected = False
247+
# Event loop that owns self._mqtt_client (captured in run()). Control writes
248+
# arrive via ha.py::run_async(), which runs on its own throwaway loop on the
249+
# calling thread — publishing directly from there would bind the aiomqtt
250+
# publish-confirmation Future to the wrong loop and stall for the client's
251+
# ~10s default timeout instead of completing near-instantly. _publish_raw()
252+
# dispatches onto this loop when called from elsewhere.
253+
self._loop = None
247254
self._gateway_online = False
248255
self._last_telemetry_time = 0
249256
self._last_plan_data = None
@@ -536,6 +543,9 @@ async def run(self, seconds, first):
536543
return False
537544

538545
if first:
546+
# Capture the loop that will own the MQTT client/listener task, so
547+
# cross-loop callers (ha.py::run_async()) can dispatch onto it later.
548+
self._loop = asyncio.get_running_loop()
539549
# Start MQTT listener as a background task
540550
self._mqtt_task = asyncio.ensure_future(self._mqtt_loop())
541551
self.log("Info: GatewayMQTT: MQTT listener task started")
@@ -1622,12 +1632,30 @@ async def publish_command(self, command, **kwargs):
16221632
async def _publish_raw(self, topic, payload, retain=False):
16231633
"""Publish raw bytes to an MQTT topic.
16241634
1635+
Must run the actual client.publish() on the event loop that owns
1636+
self._mqtt_client (self._loop). Callers reached via ha.py::run_async()
1637+
(i.e. every control write issued from the synchronous engine thread)
1638+
run on a different, throwaway loop — publishing directly from there
1639+
binds aiomqtt's publish-confirmation Future to the wrong loop and
1640+
stalls for the client's ~10s default timeout instead of completing
1641+
near-instantly. When we're already on the owning loop (e.g. internal
1642+
periodic publishes from run()/housekeeping), publish directly.
1643+
16251644
Args:
16261645
topic: MQTT topic string.
16271646
payload: Bytes to publish.
16281647
retain: Whether to set the retain flag.
16291648
"""
1630-
if self._mqtt_client and self._mqtt_connected:
1649+
if not (self._mqtt_client and self._mqtt_connected):
1650+
return
1651+
1652+
if self._loop is not None and self._loop is not asyncio.get_running_loop():
1653+
future = asyncio.run_coroutine_threadsafe(
1654+
self._mqtt_client.publish(topic, payload, qos=1, retain=retain),
1655+
self._loop,
1656+
)
1657+
await asyncio.wrap_future(future)
1658+
else:
16311659
await self._mqtt_client.publish(topic, payload, qos=1, retain=retain)
16321660

16331661
def is_alive(self):

apps/predbat/tests/test_gateway.py

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4440,6 +4440,51 @@ def test_non_finite_returns_none(self):
44404440
self.assertIsNone(extract_rate_anchors(8.0, float("inf"), 15.0, 12.0))
44414441

44424442

4443+
class TestPublishRawLoopSafety:
4444+
"""_publish_raw() must run the actual client.publish() on the event loop that
4445+
owns the aiomqtt Client (self._loop), even when invoked from a different loop —
4446+
otherwise the publish confirmation Future is bound to the wrong loop and stalls
4447+
for aiomqtt's ~10s default timeout instead of completing near-instantly. This is
4448+
what happens today when a control write reaches GatewayMQTT via
4449+
ha.py::run_async(), which runs the whole call chain on a fresh, throwaway
4450+
event loop distinct from GatewayMQTT's own persistent MQTT loop.
4451+
"""
4452+
4453+
def test_publish_dispatches_to_owning_loop_when_called_cross_loop(self):
4454+
"""A cross-loop _publish_raw() call must execute client.publish() on the
4455+
thread that owns self._loop, not on the calling thread."""
4456+
import asyncio
4457+
import threading
4458+
from gateway import GatewayMQTT
4459+
4460+
gw = GatewayMQTT.__new__(GatewayMQTT)
4461+
gw._mqtt_connected = True
4462+
observed = {}
4463+
4464+
class FakeClient:
4465+
async def publish(self, topic, payload, qos=0, retain=False):
4466+
observed["thread_ident"] = threading.get_ident()
4467+
4468+
gw._mqtt_client = FakeClient()
4469+
4470+
# Real second event loop on its own thread — mimics GatewayMQTT's
4471+
# persistent MQTT loop (owned by a dedicated thread via hass.py::create_task).
4472+
owner_loop = asyncio.new_event_loop()
4473+
owner_thread = threading.Thread(target=owner_loop.run_forever, daemon=True)
4474+
owner_thread.start()
4475+
gw._loop = owner_loop
4476+
4477+
try:
4478+
# Call _publish_raw from a DIFFERENT, freshly-created loop on THIS
4479+
# thread — mirrors ha.py::run_async()'s asyncio.run(coro) pattern.
4480+
asyncio.run(gw._publish_raw("predbat/devices/test/command", b"payload"))
4481+
finally:
4482+
owner_loop.call_soon_threadsafe(owner_loop.stop)
4483+
owner_thread.join(timeout=2)
4484+
4485+
assert observed.get("thread_ident") == owner_thread.ident, "client.publish() ran on the wrong thread/loop"
4486+
4487+
44434488
def run_gateway_tests(my_predbat=None):
44444489
"""Run all GatewayMQTT tests. Returns True on failure, False on success."""
44454490
from tests.test_gateway_token_refresh import TestIsAuthFailure, TestApplyRefreshResponse, TestMaybeRefreshOnAuthError
@@ -4476,6 +4521,7 @@ def run_gateway_tests(my_predbat=None):
44764521
TestApplyRefreshResponse,
44774522
TestMaybeRefreshOnAuthError,
44784523
TestRateAnchors,
4524+
TestPublishRawLoopSafety,
44794525
]
44804526
for cls in test_classes:
44814527
instance = cls()

0 commit comments

Comments
 (0)