fix: forward retain flag through all typed publish methods#443
Merged
Conversation
The MQTT broker accepts retain on every PUBLISH frame, but the typed publisher API only honored it for `publish_json`. Calls to `publish_str/int/bool/float` silently retained regardless of caller intent — there was no way to opt out for non-JSON payloads. Add `retain: bool = True` (kwarg-only) to every typed publish method on the `Publisher` ABC and forward it through `MqttPublisher` to the gmqtt client. `ConsolePublisher` accepts and ignores it (no retention semantics for log output). This is API-additive: existing call sites continue to work unchanged since `retain` defaults to `True`.
Surface the retain value in the debug log instead of discarding it via `del retain`. The console publisher exists to mirror what would have been published over MQTT — retention is part of that picture. `internal_publish` (and the test mock override) gain a kwarg-only `retain: bool = True` that gets formatted into the debug line.
The mypy hook was failing on tests-only commits because pre-commit passes the staged file paths and `mypy <file>` runs without the project's `files = ["./src", "./tests"]` config in effect, so imports from `src/` can't be resolved. Set `pass_filenames: false` to match how CI invokes mypy (`poetry run mypy` with no args). mypy's incremental cache keeps this fast on repeat runs.
Pin both the default-retained behavior and explicit retain=False forwarding for every typed publish method (str/int/bool/float/json) plus clear_topic's retained None publish. Existing tests covered str (default + forward) and int/bool/float (forward only); a future change to a default would have slipped through. Add the missing default-retained tests for int/bool/float, both directions for json, and clear_topic.
nanomad
added a commit
that referenced
this pull request
May 10, 2026
Complete the typed publish API by giving `datetime` its own narrow entry point — `publish_datetime` — that stringifies via `datetime_to_str` and forwards to `publish_str` (and now `retain` too). Removes the inline transformation in the dispatch chain. `Publisher.publish()` now forwards `retain` to every arm of the typed API, not just `publish_json`. Previously it was silently dropped for str/int/bool/float; with #443 those typed methods now accept `retain`, so the dispatcher can finally honor the kwarg uniformly. Also folds the two remaining `datetime_to_str(...)` call sites in `vehicle.py` (notify_car_activity, last_failed_refresh setter) through the typed/Publishable APIs, dropping the now-unused import.
nanomad
added a commit
that referenced
this pull request
May 10, 2026
* feat: add Publisher.publish dispatching method
Adds a single non-abstract `publish(key, value, no_prefix=False)` method on
the `Publisher` ABC that dispatches via isinstance to the existing typed
`publish_{bool,int,float,str}` methods, plus a `PublishedValue` type alias
for the union. The `bool`-before-`int` ordering is load-bearing because
`isinstance(True, int)` is `True` in Python.
Conformance tests cover every concrete subclass (`MqttPublisher`,
`ConsolePublisher`, `MessageCapturingConsolePublisher`) plus an
ABC-level minimal subclass, and explicitly lock the bool-vs-int ordering.
* refactor: unify Publishable union and collapse duplicate dispatch sites
Renames `PublishedValue` -> `Publishable` and widens it from
`bool | int | float | str` to also include `dict[str, Any] | datetime`,
matching the full set of value shapes the gateway publishes. Extends
`Publisher.publish` to dispatch the wider union: dicts forward to
`publish_json` (with `retain` plumbed through), datetimes are stringified
via `datetime_to_str` and routed through `publish_str`. An unsupported
runtime type now raises `TypeError` instead of silently no-op-ing.
This subsumes the two duplicate `Publishable` constrained-TypeVar
declarations (in `status_publisher/__init__.py` and `vehicle.py`) and
their two near-identical `_publish_directly` chains, which now collapse
to a single `self.publisher.publish(...)` call. Methods that used to
parametrize over the constrained TypeVar (`_publish`,
`_transform_and_publish`, `__publish`) switch to PEP 695 bounded
generics: `[V: Publishable]`, `[T, V: Publishable]`. This is a small
semantic loosening (subclasses of e.g. `dict` are now valid `V`) but
runtime dispatch is `isinstance`-based and handles subclasses correctly.
Tests grow new conformance cases for the dict (with `retain` forwarding)
and datetime arms plus a regression test for the `TypeError` arm; the
one test patching the deleted `_publish_directly` now patches the
underlying publisher's `publish` instead.
* feat: add publish_datetime and forward retain through dispatcher
Complete the typed publish API by giving `datetime` its own narrow
entry point — `publish_datetime` — that stringifies via
`datetime_to_str` and forwards to `publish_str` (and now `retain` too).
Removes the inline transformation in the dispatch chain.
`Publisher.publish()` now forwards `retain` to every arm of the typed
API, not just `publish_json`. Previously it was silently dropped for
str/int/bool/float; with #443 those typed methods now accept `retain`,
so the dispatcher can finally honor the kwarg uniformly.
Also folds the two remaining `datetime_to_str(...)` call sites in
`vehicle.py` (notify_car_activity, last_failed_refresh setter) through
the typed/Publishable APIs, dropping the now-unused import.
* refactor: type ConsolePublisher.internal_publish with Publishable
`Any` was overly permissive — at runtime `internal_publish` only ever
sees what the typed publish methods route to it (str/int/bool/float)
plus None from `clear_topic`. Reuse the `Publishable | None` alias to
make the contract explicit.
The `MessageCapturingConsolePublisher.map` test inspection store stays
typed `Any` so consumers can `json.loads(...)` serialized payloads
without per-call narrowing.
* refactor: type MqttPublisher.__publish payload with Publishable
Same narrowing as ConsolePublisher.internal_publish: the private
`__publish` only receives Publishable | None at runtime (str/int/bool/
float from the typed methods, str from publish_json after JSON
serialization, None from clear_topic). Replace `Any` with the explicit
alias.
* refactor: introduce WirePayload alias for transport-level publish helpers
Replace `Publishable | None` on `MqttPublisher.__publish` and
`ConsolePublisher.internal_publish` with `WirePayload | None`, where
`WirePayload = bool | int | float | str` — the precise set of values
that crosses the publisher/transport boundary after the typed
publish_* methods do their stringification.
This catches accidental misuse if a future caller tried to hand a raw
`dict` or `datetime` to a wire-level helper, and matches what gmqtt
can actually serialize without surprises.
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
1 participant
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
The MQTT broker accepts
retainon every PUBLISH frame, but the typed publisher API only honored it forpublish_json. Calls topublish_str/int/bool/floatsilently retained regardless of caller intent — there was no way to opt out for non-JSON payloads.This PR adds
retain: bool = True(kwarg-only) to every typed publish method on thePublisherABC and forwards it throughMqttPublisherto the gmqtt client.ConsolePublisheraccepts and ignores it (no retention semantics for log output).API-additive: existing call sites continue to work unchanged since
retaindefaults toTrue.Surfaced while reviewing #442, where the dispatcher's
Publisher.publish(retain=...)only forwardedretainto thedictarm. Splitting this fix out so it can land independently and #442 can rebase on a develop where retain is already first-class.Test plan
publish_str/int/bool/floatretain forwarding (default +retain=False) onMqttPublishervia patchedclient.publish.