Skip to content

Commit ac92079

Browse files
tomer-wCopilot
andcommitted
fix: prevent auto-reconnect from being killed when subscription setup fails
When the MQTT broker restarts (e.g. Victron power cycle, power outage), paho-mqtt auto-reconnects and fires the on_connect callback. Previously, if _setup_subscriptions() raised ANY exception during reconnection, the _on_connect handler called client.disconnect(), which permanently killed paho's auto-reconnect mechanism. The integration would never recover without a manual reload. Changes: - Only call client.disconnect() on first-connect errors (auth failures, connection refused). On reconnection, log the error and let paho continue auto-reconnecting. - Track subscription state with _subscribed flag, cleared on disconnect. - Retry failed subscriptions from the keepalive loop (runs every 30s), so even if subscription setup fails during reconnection, it will be retried automatically. - Add reconnection-specific logging (session_present flag, retry status) for better diagnostics. Closes #461 Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 6374e2f1-1fb3-453d-a1c2-dddf69a62f61
1 parent 578e0d6 commit ac92079

1 file changed

Lines changed: 34 additions & 3 deletions

File tree

  • custom_components/victron_mqtt/_vendor/victron_mqtt

custom_components/victron_mqtt/_vendor/victron_mqtt/hub.py

Lines changed: 34 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -211,6 +211,7 @@ def __init__(
211211
self._fallback_placeholders: dict[str, FallbackPlaceholder] = {}
212212
self._all_metrics: dict[str, Metric] = {}
213213
self._first_connect = True
214+
self._subscribed = False
214215
self._first_full_publish = True
215216
self._notified_device_ids: set[str] = set()
216217
self._connect_failed_attempts = 0
@@ -414,12 +415,24 @@ def _on_connect(
414415
reason_code: ReasonCode,
415416
properties: Properties | None = None,
416417
) -> None:
418+
# Capture before _on_connect_internal changes it so we can distinguish
419+
# first connect (where we must surface errors) from auto-reconnect
420+
# (where we must NOT call disconnect, or paho stops retrying).
421+
is_initial_connect = self._first_connect
417422
try:
418423
self._on_connect_internal(client, userdata, flags, reason_code, properties)
419424
except Exception as exc:
420425
_LOGGER.exception("_on_connect exception %s: %s", type(exc), exc)
421-
self._connect_failed_reason = exc
422-
client.disconnect()
426+
if is_initial_connect:
427+
# First connect: propagate the error so the caller can handle it
428+
self._connect_failed_reason = exc
429+
client.disconnect()
430+
else:
431+
# Reconnect: do NOT disconnect — let paho keep auto-reconnecting.
432+
# Subscriptions will be retried from the keepalive loop.
433+
_LOGGER.warning(
434+
"Subscription setup failed during reconnect, will retry from keepalive loop"
435+
)
423436

424437
try:
425438
self._schedule_threadsafe(self._connected_event.set)
@@ -451,7 +464,14 @@ def _on_connect_internal(
451464
f"Failed to connect to MQTT broker: {self.host}:{self.port}. Error: {connack_string(reason_code)}"
452465
)
453466

454-
_LOGGER.info("Connected to MQTT broker successfully")
467+
is_reconnect = not self._first_connect
468+
if is_reconnect:
469+
_LOGGER.info(
470+
"Reconnected to MQTT broker (session_present=%s). Re-establishing subscriptions...",
471+
flags.session_present,
472+
)
473+
else:
474+
_LOGGER.info("Connected to MQTT broker successfully")
455475
self._setup_subscriptions()
456476

457477
def _on_disconnect(
@@ -463,6 +483,7 @@ def _on_disconnect(
463483
_properties: Properties | None = None,
464484
) -> None:
465485
"""Handle disconnection callback."""
486+
self._subscribed = False
466487
if reason_code != 0:
467488
_LOGGER.warning(
468489
"Unexpected disconnection from MQTT broker. Error: %s. flags: %s, Reconnecting...",
@@ -852,6 +873,15 @@ async def _keepalive_loop(self) -> None:
852873
count = 0
853874
while True:
854875
try:
876+
# If subscriptions were lost (e.g. failed during reconnect), retry them
877+
if not self._subscribed and self._client is not None and self._client.is_connected():
878+
_LOGGER.info("Subscriptions not established, retrying subscription setup...")
879+
try:
880+
self._setup_subscriptions()
881+
_LOGGER.info("Subscription retry succeeded")
882+
except Exception as sub_exc:
883+
_LOGGER.warning("Subscription retry failed, will try again: %s", sub_exc)
884+
855885
self._keepalive()
856886
await asyncio.sleep(30)
857887
# We should keep alive all metrics every 60 seconds
@@ -1040,6 +1070,7 @@ def _setup_subscriptions(self) -> None:
10401070
assert self.installation_id is not None
10411071
self._subscribe(f"N/{self.installation_id}/full_publish_completed")
10421072
_LOGGER.info("Subscribed to full_publish_completed notification")
1073+
self._subscribed = True
10431074
self._keepalive(True)
10441075

10451076
async def _wait_for_connect(self) -> None:

0 commit comments

Comments
 (0)