Bound MQTT broker operations with a per-node timeout
Without one, aiomqtt's disconnect acknowledgement has no deadline, so a subscriber cancelled while its socket is dead never finishes unwinding and teardown abandons the task. The knob is per node because brokers differ.
This commit is contained in:
@@ -83,6 +83,8 @@ class MqttNode(Node):
|
|||||||
- ``qos`` (int): Quality of Service level 0, 1, or 2 (default: 0)
|
- ``qos`` (int): Quality of Service level 0, 1, or 2 (default: 0)
|
||||||
- ``retain`` (bool): Retain flag for published messages (default: False)
|
- ``retain`` (bool): Retain flag for published messages (default: False)
|
||||||
- ``keepalive`` (int): Keepalive interval in seconds (default: 60)
|
- ``keepalive`` (int): Keepalive interval in seconds (default: 60)
|
||||||
|
- ``timeout`` (float): Deadline for a broker operation in seconds
|
||||||
|
(default: 10.0)
|
||||||
:type params: dict
|
:type params: dict
|
||||||
:param name: Optional name for the node.
|
:param name: Optional name for the node.
|
||||||
:type name: str | None
|
:type name: str | None
|
||||||
@@ -151,6 +153,7 @@ class MqttNode(Node):
|
|||||||
"qos",
|
"qos",
|
||||||
"retain",
|
"retain",
|
||||||
"keepalive",
|
"keepalive",
|
||||||
|
"timeout",
|
||||||
"json_keys",
|
"json_keys",
|
||||||
"_topic_to_ports",
|
"_topic_to_ports",
|
||||||
"_wildcards",
|
"_wildcards",
|
||||||
@@ -175,6 +178,15 @@ class MqttNode(Node):
|
|||||||
qos: int = 0
|
qos: int = 0
|
||||||
retain: bool = False
|
retain: bool = False
|
||||||
keepalive: int = 60
|
keepalive: int = 60
|
||||||
|
# Bounds every broker operation: subscribe, publish, and the
|
||||||
|
# disconnect acknowledgement on the way out. Without one a client
|
||||||
|
# whose socket died waits for that ack forever, and the task never
|
||||||
|
# finishes unwinding. Brokers differ, so it is per node.
|
||||||
|
timeout: float = Field(
|
||||||
|
default=10.0,
|
||||||
|
gt=0,
|
||||||
|
description="Give up on a broker operation after this many seconds.",
|
||||||
|
)
|
||||||
# Which key to lift out of a JSON object payload. A device that wraps
|
# Which key to lift out of a JSON object payload. A device that wraps
|
||||||
# its reading — Victron's ``{"value": 5}`` — is otherwise a Python node
|
# its reading — Victron's ``{"value": 5}`` — is otherwise a Python node
|
||||||
# per port. One key for every port, or a per-port mapping.
|
# per port. One key for every port, or a per-port mapping.
|
||||||
@@ -247,6 +259,7 @@ class MqttNode(Node):
|
|||||||
self.qos = cfg.qos
|
self.qos = cfg.qos
|
||||||
self.retain = cfg.retain
|
self.retain = cfg.retain
|
||||||
self.keepalive = cfg.keepalive
|
self.keepalive = cfg.keepalive
|
||||||
|
self.timeout = cfg.timeout
|
||||||
|
|
||||||
# Runtime state
|
# Runtime state
|
||||||
self._subscription_task: asyncio.Task[None] | None = None
|
self._subscription_task: asyncio.Task[None] | None = None
|
||||||
@@ -372,6 +385,7 @@ class MqttNode(Node):
|
|||||||
password=self.password,
|
password=self.password,
|
||||||
identifier=self.client_id,
|
identifier=self.client_id,
|
||||||
keepalive=self.keepalive,
|
keepalive=self.keepalive,
|
||||||
|
timeout=self.timeout,
|
||||||
) as client:
|
) as client:
|
||||||
self.report_health("ok")
|
self.report_health("ok")
|
||||||
while True:
|
while True:
|
||||||
@@ -393,6 +407,7 @@ class MqttNode(Node):
|
|||||||
password=self.password,
|
password=self.password,
|
||||||
identifier=self.client_id,
|
identifier=self.client_id,
|
||||||
keepalive=self.keepalive,
|
keepalive=self.keepalive,
|
||||||
|
timeout=self.timeout,
|
||||||
) as client:
|
) as client:
|
||||||
await self._publish_with(client, data)
|
await self._publish_with(client, data)
|
||||||
|
|
||||||
@@ -556,6 +571,7 @@ class MqttNode(Node):
|
|||||||
password=self.password,
|
password=self.password,
|
||||||
identifier=self.client_id,
|
identifier=self.client_id,
|
||||||
keepalive=self.keepalive,
|
keepalive=self.keepalive,
|
||||||
|
timeout=self.timeout,
|
||||||
) as client:
|
) as client:
|
||||||
# Subscribe to every unique topic
|
# Subscribe to every unique topic
|
||||||
for topic in self._topic_to_ports:
|
for topic in self._topic_to_ports:
|
||||||
|
|||||||
@@ -89,3 +89,33 @@ def test_a_string_goes_on_the_wire_bare():
|
|||||||
asyncio.run(node._publish_with(client, {"plug": "ON", "level": 60}))
|
asyncio.run(node._publish_with(client, {"plug": "ON", "level": 60}))
|
||||||
|
|
||||||
assert client.published == [("actor/plug", "ON"), ("light/level", "60")]
|
assert client.published == [("actor/plug", "ON"), ("light/level", "60")]
|
||||||
|
|
||||||
|
|
||||||
|
def test_the_configured_timeout_reaches_the_broker_client(monkeypatch):
|
||||||
|
"""Without one, a dead socket makes the disconnect ack wait forever."""
|
||||||
|
import aiomqtt
|
||||||
|
|
||||||
|
seen: dict = {}
|
||||||
|
|
||||||
|
class FakeClient:
|
||||||
|
def __init__(self, **kwargs):
|
||||||
|
seen.update(kwargs)
|
||||||
|
|
||||||
|
async def __aenter__(self):
|
||||||
|
return self
|
||||||
|
|
||||||
|
async def __aexit__(self, *_):
|
||||||
|
return False
|
||||||
|
|
||||||
|
async def publish(self, *_, **__):
|
||||||
|
return None
|
||||||
|
|
||||||
|
monkeypatch.setattr(aiomqtt, "Client", FakeClient)
|
||||||
|
|
||||||
|
node = MqttNode(
|
||||||
|
requires=[MessageSpec(name="setpoint", port="setpoint", dtype=DType.FLOAT)],
|
||||||
|
params={"topic": {"setpoint": "heating/setpoint"}, "timeout": 2.5},
|
||||||
|
)
|
||||||
|
asyncio.run(node._publish_once({"setpoint": 21.0}))
|
||||||
|
|
||||||
|
assert seen["timeout"] == 2.5
|
||||||
|
|||||||
Reference in New Issue
Block a user