Skip to content

Commit fbdd541

Browse files
tomer-wCopilot
andcommitted
Fix boot race: retry on any startup error and clean up paho threads
When HA and Victron boot simultaneously (power outage, no UPS), the integration can fail permanently because: 1. Hub.start() only catches CannotConnectError, but connect() can also raise NotConnectedError (if connection drops mid-setup) or other exceptions. Uncaught exceptions cause HA to treat the failure as permanent (no retry). Fix: catch Exception (except AuthenticationError) and convert to ConfigEntryNotReady so HA retries with backoff. 2. connect() calls loop_start() early, creating a paho background thread. If any later step fails, loop_stop() is never called, leaking the thread. Each HA retry leaks another thread. Fix: try/except around post-loop_start() code that calls loop_stop() on failure. 3. disconnect() calls client.disconnect() but not loop_stop(), so the background thread is never explicitly stopped. Fix: add loop_stop(). Fixes #461 Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 6374e2f1-1fb3-453d-a1c2-dddf69a62f61
1 parent ac92079 commit fbdd541

2 files changed

Lines changed: 20 additions & 43 deletions

File tree

  • custom_components/victron_mqtt

custom_components/victron_mqtt/_vendor/victron_mqtt/hub.py

Lines changed: 19 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -211,7 +211,6 @@ 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
215214
self._first_full_publish = True
216215
self._notified_device_ids: set[str] = set()
217216
self._connect_failed_attempts = 0
@@ -368,14 +367,21 @@ async def connect(self) -> None:
368367

369368
_LOGGER.info("Starting paho mqtt")
370369
self._client.loop_start()
371-
_LOGGER.info("Connecting")
372-
self._client.connect_async(self.host, self.port)
373-
_LOGGER.info("Waiting for connection event")
374-
await self._wait_for_connect()
375-
if self._connect_failed_reason is not None:
376-
reraise_same_exception(self._connect_failed_reason)
377-
_LOGGER.info("Successfully connected to MQTT broker at %s:%d", self.host, self.port)
378-
await self._wait_for_installation_id(expected_id=self._expected_installation_id)
370+
try:
371+
_LOGGER.info("Connecting")
372+
self._client.connect_async(self.host, self.port)
373+
_LOGGER.info("Waiting for connection event")
374+
await self._wait_for_connect()
375+
if self._connect_failed_reason is not None:
376+
reraise_same_exception(self._connect_failed_reason)
377+
_LOGGER.info("Successfully connected to MQTT broker at %s:%d", self.host, self.port)
378+
await self._wait_for_installation_id(expected_id=self._expected_installation_id)
379+
except Exception:
380+
# If anything fails after loop_start(), stop the paho thread to avoid leaking it.
381+
# On HA retry a new Hub and paho client will be created.
382+
_LOGGER.info("Connection setup failed, stopping paho mqtt loop")
383+
self._client.loop_stop()
384+
raise
379385
assert self._installation_id is not None
380386
# First we need to replace the installation ID in the subscription topics
381387
new_list: list[str] = []
@@ -415,24 +421,12 @@ def _on_connect(
415421
reason_code: ReasonCode,
416422
properties: Properties | None = None,
417423
) -> 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
422424
try:
423425
self._on_connect_internal(client, userdata, flags, reason_code, properties)
424426
except Exception as exc:
425427
_LOGGER.exception("_on_connect exception %s: %s", type(exc), exc)
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-
)
428+
self._connect_failed_reason = exc
429+
client.disconnect()
436430

437431
try:
438432
self._schedule_threadsafe(self._connected_event.set)
@@ -464,14 +458,7 @@ def _on_connect_internal(
464458
f"Failed to connect to MQTT broker: {self.host}:{self.port}. Error: {connack_string(reason_code)}"
465459
)
466460

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")
461+
_LOGGER.info("Connected to MQTT broker successfully")
475462
self._setup_subscriptions()
476463

477464
def _on_disconnect(
@@ -483,7 +470,6 @@ def _on_disconnect(
483470
_properties: Properties | None = None,
484471
) -> None:
485472
"""Handle disconnection callback."""
486-
self._subscribed = False
487473
if reason_code != 0:
488474
_LOGGER.warning(
489475
"Unexpected disconnection from MQTT broker. Error: %s. flags: %s, Reconnecting...",
@@ -839,6 +825,7 @@ async def disconnect(self) -> None:
839825
self._stop_keepalive_loop()
840826
await asyncio.sleep(0.1)
841827
self._client.disconnect() # need to call disconnect so the paho thread will terminate
828+
self._client.loop_stop() # stop the background thread started by loop_start()
842829
_LOGGER.info("Disconnected from MQTT broker")
843830
# Give a small delay to allow any pending MQTT messages to be processed
844831
await asyncio.sleep(0.1)
@@ -873,15 +860,6 @@ async def _keepalive_loop(self) -> None:
873860
count = 0
874861
while True:
875862
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-
885863
self._keepalive()
886864
await asyncio.sleep(30)
887865
# We should keep alive all metrics every 60 seconds
@@ -1070,7 +1048,6 @@ def _setup_subscriptions(self) -> None:
10701048
assert self.installation_id is not None
10711049
self._subscribe(f"N/{self.installation_id}/full_publish_completed")
10721050
_LOGGER.info("Subscribed to full_publish_completed notification")
1073-
self._subscribed = True
10741051
self._keepalive(True)
10751052

10761053
async def _wait_for_connect(self) -> None:

custom_components/victron_mqtt/hub.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -118,7 +118,7 @@ async def start(self) -> None:
118118
raise ConfigEntryAuthFailed(
119119
f"Authentication failed for {self.host}: {auth_error}"
120120
) from auth_error
121-
except CannotConnectError as connect_error:
121+
except Exception as connect_error:
122122
raise ConfigEntryNotReady(
123123
f"Cannot connect to the hub: {connect_error}"
124124
) from connect_error

0 commit comments

Comments
 (0)