From 6b5775b4bfb2dd224e564af840095df965a12e5d Mon Sep 17 00:00:00 2001 From: Giovanni Condello Date: Sun, 10 May 2026 11:36:09 +0200 Subject: [PATCH 1/4] fix: forward retain flag through all typed publish methods MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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`. --- src/publisher/core.py | 16 ++++++++++++---- src/publisher/log_publisher.py | 21 +++++++++++++++++---- src/publisher/mqtt_publisher.py | 32 ++++++++++++++++++++++++-------- tests/test_mqtt_publisher.py | 26 ++++++++++++++++++++++++++ 4 files changed, 79 insertions(+), 16 deletions(-) diff --git a/src/publisher/core.py b/src/publisher/core.py index 50fdf6b..5fa8295 100644 --- a/src/publisher/core.py +++ b/src/publisher/core.py @@ -85,19 +85,27 @@ def publish_json( raise NotImplementedError @abstractmethod - def publish_str(self, key: str, value: str, no_prefix: bool = False) -> None: + def publish_str( + self, key: str, value: str, no_prefix: bool = False, *, retain: bool = True + ) -> None: raise NotImplementedError @abstractmethod - def publish_int(self, key: str, value: int, no_prefix: bool = False) -> None: + def publish_int( + self, key: str, value: int, no_prefix: bool = False, *, retain: bool = True + ) -> None: raise NotImplementedError @abstractmethod - def publish_bool(self, key: str, value: bool, no_prefix: bool = False) -> None: + def publish_bool( + self, key: str, value: bool, no_prefix: bool = False, *, retain: bool = True + ) -> None: raise NotImplementedError @abstractmethod - def publish_float(self, key: str, value: float, no_prefix: bool = False) -> None: + def publish_float( + self, key: str, value: float, no_prefix: bool = False, *, retain: bool = True + ) -> None: raise NotImplementedError @abstractmethod diff --git a/src/publisher/log_publisher.py b/src/publisher/log_publisher.py index 00deb67..b9ee33c 100644 --- a/src/publisher/log_publisher.py +++ b/src/publisher/log_publisher.py @@ -31,23 +31,36 @@ def publish_json( *, retain: bool = True, ) -> None: + del retain # console output has no retention semantics anonymized_json = self.dict_to_anonymized_json(data) self.internal_publish(key, anonymized_json) @override - def publish_str(self, key: str, value: str, no_prefix: bool = False) -> None: + def publish_str( + self, key: str, value: str, no_prefix: bool = False, *, retain: bool = True + ) -> None: + del retain self.internal_publish(key, value) @override - def publish_int(self, key: str, value: int, no_prefix: bool = False) -> None: + def publish_int( + self, key: str, value: int, no_prefix: bool = False, *, retain: bool = True + ) -> None: + del retain self.internal_publish(key, value) @override - def publish_bool(self, key: str, value: bool, no_prefix: bool = False) -> None: + def publish_bool( + self, key: str, value: bool, no_prefix: bool = False, *, retain: bool = True + ) -> None: + del retain self.internal_publish(key, value) @override - def publish_float(self, key: str, value: float, no_prefix: bool = False) -> None: + def publish_float( + self, key: str, value: float, no_prefix: bool = False, *, retain: bool = True + ) -> None: + del retain self.internal_publish(key, value) @override diff --git a/src/publisher/mqtt_publisher.py b/src/publisher/mqtt_publisher.py index b441fef..6d3256f 100644 --- a/src/publisher/mqtt_publisher.py +++ b/src/publisher/mqtt_publisher.py @@ -248,20 +248,36 @@ def publish_json( ) @override - def publish_str(self, key: str, value: str, no_prefix: bool = False) -> None: - self.__publish(topic=self.get_topic(key, no_prefix), payload=value) + def publish_str( + self, key: str, value: str, no_prefix: bool = False, *, retain: bool = True + ) -> None: + self.__publish( + topic=self.get_topic(key, no_prefix), payload=value, retain=retain + ) @override - def publish_int(self, key: str, value: int, no_prefix: bool = False) -> None: - self.__publish(topic=self.get_topic(key, no_prefix), payload=value) + def publish_int( + self, key: str, value: int, no_prefix: bool = False, *, retain: bool = True + ) -> None: + self.__publish( + topic=self.get_topic(key, no_prefix), payload=value, retain=retain + ) @override - def publish_bool(self, key: str, value: bool, no_prefix: bool = False) -> None: - self.__publish(topic=self.get_topic(key, no_prefix), payload=value) + def publish_bool( + self, key: str, value: bool, no_prefix: bool = False, *, retain: bool = True + ) -> None: + self.__publish( + topic=self.get_topic(key, no_prefix), payload=value, retain=retain + ) @override - def publish_float(self, key: str, value: float, no_prefix: bool = False) -> None: - self.__publish(topic=self.get_topic(key, no_prefix), payload=value) + def publish_float( + self, key: str, value: float, no_prefix: bool = False, *, retain: bool = True + ) -> None: + self.__publish( + topic=self.get_topic(key, no_prefix), payload=value, retain=retain + ) @override def clear_topic(self, key: str, no_prefix: bool = False) -> None: diff --git a/tests/test_mqtt_publisher.py b/tests/test_mqtt_publisher.py index 2416dc9..4375ef5 100644 --- a/tests/test_mqtt_publisher.py +++ b/tests/test_mqtt_publisher.py @@ -2,6 +2,7 @@ from typing import Any, override import unittest +from unittest.mock import patch from configuration import Configuration, TransportProtocol from publisher.core import MqttCommandListener @@ -98,3 +99,28 @@ async def on_charger_connection_state_changed( self, vin: str, connected: bool ) -> None: pass + + def test_publish_str_default_is_retained(self) -> None: + with patch.object(self.mqtt_client.client, "publish") as m_pub: + self.mqtt_client.publish_str("foo", "bar") + m_pub.assert_called_once_with("saic/foo", "bar", retain=True) + + def test_publish_str_forwards_retain_false(self) -> None: + with patch.object(self.mqtt_client.client, "publish") as m_pub: + self.mqtt_client.publish_str("foo", "bar", retain=False) + m_pub.assert_called_once_with("saic/foo", "bar", retain=False) + + def test_publish_int_forwards_retain_false(self) -> None: + with patch.object(self.mqtt_client.client, "publish") as m_pub: + self.mqtt_client.publish_int("foo", 42, retain=False) + m_pub.assert_called_once_with("saic/foo", 42, retain=False) + + def test_publish_bool_forwards_retain_false(self) -> None: + with patch.object(self.mqtt_client.client, "publish") as m_pub: + self.mqtt_client.publish_bool("foo", True, retain=False) + m_pub.assert_called_once_with("saic/foo", True, retain=False) + + def test_publish_float_forwards_retain_false(self) -> None: + with patch.object(self.mqtt_client.client, "publish") as m_pub: + self.mqtt_client.publish_float("foo", 1.5, retain=False) + m_pub.assert_called_once_with("saic/foo", 1.5, retain=False) From fd490d69cedde3c37fb8501ec1f23f85f48e5302 Mon Sep 17 00:00:00 2001 From: Giovanni Condello Date: Sun, 10 May 2026 11:39:39 +0200 Subject: [PATCH 2/4] fix: log retain flag in ConsolePublisher debug output MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- src/publisher/log_publisher.py | 19 +++++++------------ tests/mocks/__init__.py | 4 ++-- 2 files changed, 9 insertions(+), 14 deletions(-) diff --git a/src/publisher/log_publisher.py b/src/publisher/log_publisher.py index b9ee33c..45e30d2 100644 --- a/src/publisher/log_publisher.py +++ b/src/publisher/log_publisher.py @@ -31,41 +31,36 @@ def publish_json( *, retain: bool = True, ) -> None: - del retain # console output has no retention semantics anonymized_json = self.dict_to_anonymized_json(data) - self.internal_publish(key, anonymized_json) + self.internal_publish(key, anonymized_json, retain=retain) @override def publish_str( self, key: str, value: str, no_prefix: bool = False, *, retain: bool = True ) -> None: - del retain - self.internal_publish(key, value) + self.internal_publish(key, value, retain=retain) @override def publish_int( self, key: str, value: int, no_prefix: bool = False, *, retain: bool = True ) -> None: - del retain - self.internal_publish(key, value) + self.internal_publish(key, value, retain=retain) @override def publish_bool( self, key: str, value: bool, no_prefix: bool = False, *, retain: bool = True ) -> None: - del retain - self.internal_publish(key, value) + self.internal_publish(key, value, retain=retain) @override def publish_float( self, key: str, value: float, no_prefix: bool = False, *, retain: bool = True ) -> None: - del retain - self.internal_publish(key, value) + self.internal_publish(key, value, retain=retain) @override def clear_topic(self, key: str, no_prefix: bool = False) -> None: self.internal_publish(key, None) - def internal_publish(self, key: str, value: Any) -> None: - LOG.debug(f"{key}: {value}") + def internal_publish(self, key: str, value: Any, *, retain: bool = True) -> None: + LOG.debug(f"{key}: {value} (retain={retain})") diff --git a/tests/mocks/__init__.py b/tests/mocks/__init__.py index 9790d3a..de6db2d 100644 --- a/tests/mocks/__init__.py +++ b/tests/mocks/__init__.py @@ -18,7 +18,7 @@ def __init__(self, configuration: Configuration) -> None: self.publish_count: dict[str, int] = {} @override - def internal_publish(self, key: str, value: Any) -> None: + def internal_publish(self, key: str, value: Any, *, retain: bool = True) -> None: self.map[key] = value self.publish_count[key] = self.publish_count.get(key, 0) + 1 - LOG.debug(f"{key}: {value}") + LOG.debug(f"{key}: {value} (retain={retain})") From bcf7c2d16c875157a020d2ecb1623f1014a1d4d0 Mon Sep 17 00:00:00 2001 From: Giovanni Condello Date: Sun, 10 May 2026 11:43:25 +0200 Subject: [PATCH 3/4] chore: have pre-commit invoke mypy without filenames The mypy hook was failing on tests-only commits because pre-commit passes the staged file paths and `mypy ` 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. --- .pre-commit-config.yaml | 1 + 1 file changed, 1 insertion(+) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 6ae070e..d62febd 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -29,6 +29,7 @@ repos: language: system types: - python + pass_filenames: false require_serial: true - id: pytest name: Run pytest From e500cfdce72fcd2fab7e8be539ae9d3b298931e2 Mon Sep 17 00:00:00 2001 From: Giovanni Condello Date: Sun, 10 May 2026 11:43:32 +0200 Subject: [PATCH 4/4] test: round out non-regression coverage for retain flag 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. --- tests/test_mqtt_publisher.py | 36 ++++++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/tests/test_mqtt_publisher.py b/tests/test_mqtt_publisher.py index 4375ef5..1fdeba3 100644 --- a/tests/test_mqtt_publisher.py +++ b/tests/test_mqtt_publisher.py @@ -110,17 +110,53 @@ def test_publish_str_forwards_retain_false(self) -> None: self.mqtt_client.publish_str("foo", "bar", retain=False) m_pub.assert_called_once_with("saic/foo", "bar", retain=False) + def test_publish_int_default_is_retained(self) -> None: + with patch.object(self.mqtt_client.client, "publish") as m_pub: + self.mqtt_client.publish_int("foo", 42) + m_pub.assert_called_once_with("saic/foo", 42, retain=True) + def test_publish_int_forwards_retain_false(self) -> None: with patch.object(self.mqtt_client.client, "publish") as m_pub: self.mqtt_client.publish_int("foo", 42, retain=False) m_pub.assert_called_once_with("saic/foo", 42, retain=False) + def test_publish_bool_default_is_retained(self) -> None: + with patch.object(self.mqtt_client.client, "publish") as m_pub: + self.mqtt_client.publish_bool("foo", True) + m_pub.assert_called_once_with("saic/foo", True, retain=True) + def test_publish_bool_forwards_retain_false(self) -> None: with patch.object(self.mqtt_client.client, "publish") as m_pub: self.mqtt_client.publish_bool("foo", True, retain=False) m_pub.assert_called_once_with("saic/foo", True, retain=False) + def test_publish_float_default_is_retained(self) -> None: + with patch.object(self.mqtt_client.client, "publish") as m_pub: + self.mqtt_client.publish_float("foo", 1.5) + m_pub.assert_called_once_with("saic/foo", 1.5, retain=True) + def test_publish_float_forwards_retain_false(self) -> None: with patch.object(self.mqtt_client.client, "publish") as m_pub: self.mqtt_client.publish_float("foo", 1.5, retain=False) m_pub.assert_called_once_with("saic/foo", 1.5, retain=False) + + def test_publish_json_default_is_retained(self) -> None: + with patch.object(self.mqtt_client.client, "publish") as m_pub: + self.mqtt_client.publish_json("foo", {"a": 1}) + m_pub.assert_called_once() + args, kwargs = m_pub.call_args + assert args[0] == "saic/foo" + assert kwargs == {"retain": True} + + def test_publish_json_forwards_retain_false(self) -> None: + with patch.object(self.mqtt_client.client, "publish") as m_pub: + self.mqtt_client.publish_json("foo", {"a": 1}, retain=False) + m_pub.assert_called_once() + args, kwargs = m_pub.call_args + assert args[0] == "saic/foo" + assert kwargs == {"retain": False} + + def test_clear_topic_publishes_none_retained(self) -> None: + with patch.object(self.mqtt_client.client, "publish") as m_pub: + self.mqtt_client.clear_topic("foo") + m_pub.assert_called_once_with("saic/foo", None, retain=True)