Open
Conversation
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…tions run_coroutine_threadsafe is meant for cross-thread scheduling but was being used within the same event loop. The returned Future was discarded, silently swallowing any publish or subscribe failures. Use create_task instead with a done callback to surface errors in the logs. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The migration to aiomqtt dropped the on_mqtt_reconnected() call, which resets HA discovery state so it gets re-published on the next update cycle. Without this, HA entities are never re-discovered after a broker reconnect. The explicit re-subscribe call is not needed as paho-mqtt handles resubscription automatically. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The reconnection loop previously caught all MqttError exceptions and retried every 5 seconds, even for fatal errors like bad credentials or protocol mismatch. Now MqttConnectError is caught separately and only "Server unavailable" (the sole transient CONNACK refusal) triggers a retry. All other connection refusals raise SystemExit, matching the old gmqtt behavior. Also logs the actual exception in all error paths and fixes asyncio.CancelledError import path. Supports both MQTT v3.1.1 and v5 reason codes. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
connect() awaited __connected.wait() which would block forever if the run loop exited before establishing a connection (e.g. missing host, unexpected exception). Now races the connection event against the task itself using asyncio.wait. Also upgrades missing MQTT host from a silent INFO log to a fatal SystemExit. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The aiomqtt migration changed the default retain from True to False across all publish methods. This meant vehicle state data was no longer retained on the broker, so MQTT clients connecting after a publish would not see any data until the next refresh cycle. Restore the old behavior where all publishes are retained by default. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The error log used a plain string "{e}" instead of a format specifier,
printing the literal text {e} instead of the exception. Also changed
raise e to bare raise to preserve the original traceback.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
__connected.set() was called before __on_connect() finished, so callers of connect() could proceed before subscriptions and keepalive were established. Move the event set to after __on_connect() completes, and remove the now-incorrect __connected.is_set() guard from __enable_commands since it runs before the event is set. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Replace bare int with Literal[0, 1, 2] across the Publisher hierarchy, matching the Transport type alias pattern already in the PR. This provides static type checking to prevent invalid QoS values. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
nanomad
force-pushed
the
chore/aiomqtt-fixes
branch
from
6c618f1 to
49d8afd
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Migrates from gmqtt to aiomqtt (async wrapper around paho-mqtt), based on @bj00rn's work in SAIC-iSmart-API#371 with significant bug fixes and improvements on top.
Original work (by @bj00rn)
retain=True,qos=1for birth/LWT messagesBug fixes on top
run_coroutine_threadsafewithcreate_task— was silently swallowing all publish/subscribe failureson_mqtt_reconnectedcallback — HA discovery was never re-pushed after broker reconnectconnect()from hanging forever — if the run loop exits before connecting, the error now propagatesretain=Truedefault — the migration accidentally made all vehicle state publishes non-retained"a"suffix from MQTT client identifier__enable_commandserror log ("{e}"→%s)__on_connectcompletes, not beforeImprovements
QoS = Literal[0, 1, 2]type alias for MQTT quality of service levelTest plan
🤖 Generated with Claude Code