Conversation
Update test stubs and publish-behavior tests to match the new abstract method signatures (positional retain=False, qos=0) and rewrite the publisher tests to use AsyncMock against __async_publish instead of patching the synchronous gmqtt client.publish.
- Re-add keyword-only * separator before retain and qos params so callers cannot accidentally pass them positionally - Restore retain=True as the default; flipping to False would silently stop retaining messages for all existing callers - Fix __publish private helper which was also missing the True default - Update tests to match corrected defaults and use keyword args
publish_str/int/bool/float were silently dropping the retain parameter, causing the debug log to always show retain=True regardless of what the caller passed.
_properties.get("retain", 0) called message.properties which is
paho.mqtt.properties.Properties|None, not a dict. On MQTT v3.1.1
(common default) this is None, raising AttributeError on every
incoming message and silently dropping all /set commands.
Change _on_message to accept retained: bool directly, and pass
message.retain (a bool field on aiomqtt.Message) at the call site.
The aiomqtt migration dropped the on_mqtt_reconnected() call that the old gmqtt __on_connect callback made. MqttGateway.on_mqtt_reconnected() resets HomeAssistantGatewayDiscovery and calls reset_ha_discovery() on every vehicle handler, so without this call HA entities would not be re-announced after a broker disconnect/reconnect cycle.
The aiomqtt migration dropped the imported_energy_topic subscription block from __enable_commands. vin_by_imported_energy_topic was initialised in __init__ but never populated, making __handle_imported_energy dead code and silently breaking OpenWB energy-import tracking.
LOG.error("...{e}") was a plain string, always logging the literal
"{e}" instead of the exception value, making subscribe failures
impossible to diagnose from logs.
When self.host is empty __run_loop returned immediately without setting __connected, leaving connect() blocked forever on await self.__connected.wait(). Move the empty-host check into connect() so it returns before creating the task.
test_clear_topic_publishes_none_not_retained asserted retain=True, contradicting its own name. Clearing a retained MQTT topic requires publishing a null payload with retain=True; rename to match.
The suffix was a debug leftover from the initial aiomqtt migration. There is only one MqttPublisher instance so no client ID conflict exists.
Bad credentials, invalid client ID, and auth refusals (MQTT 3.1.1 rc 1, 2, 4, 5) are permanent — retrying every 5 seconds forever gives no useful signal. Raise SystemExit on these, matching the old gmqtt behaviour. rc 3 (server unavailable) is transient and still reconnects.
…1 spec constants Avoids depending on paho-mqtt (a transitive dep) for named CONNACK refusal codes. Values are stable — they are defined by the MQTT 3.1.1 specification (section 3.2.2.3) and will not change.
Fixed 5-second interval hammered a DNS lookup + TCP attempt every 5 s indefinitely against a down broker. Now doubles on each failure (5 s → 10 → 20 → … → 300 s) and resets to the minimum on a successful connection.
run_coroutine_threadsafe is the cross-thread API — calling it from the same event loop thread allocates an unnecessary concurrent.futures.Future and discards it, silently swallowing any unexpected exceptions. Replace with loop.create_task() which is the correct same-loop primitive and whose unhandled exceptions are surfaced by the asyncio runtime.
Two bugs in the permanent-refusal handling: 1. isinstance(e.rc, int) is always False — aiomqtt passes a ReasonCode object, not a plain int. ReasonCode.__eq__ supports int comparison, so the guard is redundant and was silently making the entire branch dead code. Remove the isinstance check. 2. raise SystemExit inside an asyncio Task is caught by the event loop and stored on the task, never propagated. connect() was left hanging on await __connected.wait() forever. Fix: store the error and set __connected so connect() unblocks, then re-raise there.
Migrate to aiomqtt (rebased)
str(retain).lower() produced the Python strings "true"/"false" which json.dumps serialized as JSON strings. Home Assistant's MQTT discovery schema expects a boolean; the string caused schema validation to reject the retain flag, so command topics for number/select entities were not retained on the broker and gateway settings were lost after an HA restart.
…bool fix(ha): emit retain as JSON boolean in discovery payloads
extract_soc_kwh now returns None silently when charge_status is None, which is the expected state for non-EV vehicles on every poll cycle. The WARNING is preserved for EVs where charge_status is present but kWh derivation fails.
Command result topics ("Success" / "Failed: ...") were published with
the default retain=True, so a stale result from a previous run would
persist on the broker and re-appear after a gateway restart. Results
are transient responses — they should not survive a reconnect.
…ned-stale fix(command): publish command results with retain=False
tls_insecure was set for any TLS connection where hostname checking was disabled, not just the self-signed/custom-CA scenario it was intended for. With a public CA cert (the default case), hostname verification should never be bypassed. The warning log already required tls_server_cert_path; align the tls_insecure condition to match.
The warning "Skipping hostname check" was only emitted when a custom CA cert path was also configured. Users connecting with self-signed certs (no CA file) or by IP address would disable hostname verification silently. Move the warning outside the tls_server_cert_path block so it fires for any TLS connection where tls_server_cert_check_hostname=False, and drop the spurious tls_server_cert_path guard on tls_insecure.
…s-custom-ca fix(tls): warn on hostname-check bypass regardless of custom CA cert
…ing-spam fix(extractors): suppress SoC kWh warning spam for non-EV vehicles
Contributor
Author
|
Premature — RC tags go on develop, this PR is for the final stable release only. |
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.
0.13.0
Changed
Fixed
WARNING: Could not extract a valid SoC kWhfor non-EV vehicles (fix(extractors): suppress SoC kWh warning spam for non-EV vehicles #460)retain=False— no stale Success/Failed across restarts (fix(command): publish command results with retain=False #461)retainas JSON boolean in HA discovery payloads (fix(ha): emit retain as JSON boolean in discovery payloads #459)Full Changelog: 0.12.0...0.13.0