mqtt: extract shared control topic helpers (#345)

Author: filiplajszczak

src/batcontrol/mqtt_api.py

@@ -51,10 +51,15 @@
 logger = logging.getLogger(__name__)
 logger.info('Loading module')
 
+TOPIC_MODE = 'mode'
+TOPIC_CHARGE_RATE = 'charge_rate'
+TOPIC_LIMIT_BATTERY_CHARGE_RATE = 'limit_battery_charge_rate'
+TOPIC_API_OVERRIDE_ACTIVE = 'api_override_active'
+TOPIC_SET_SUFFIX = '/set'
+
 
 class MqttApi:
     """ MQTT API to publish data from batcontrol to MQTT for further processing+visualization"""
-    SET_SUFFIX = '/set'
 
     def __init__(self, config: dict, interval_minutes: int = 60):
         self.config = config
@@ -151,6 +156,14 @@ def wait_ready(self) -> bool:
 
         return True
 
+    def _topic(self, topic: str) -> str:
+        """Build a state topic below the configured base topic."""
+        return f"{self.base_topic}/{topic}"
+
+    def _set_topic(self, topic: str) -> str:
+        """Build a command topic below the configured base topic."""
+        return f"{self.base_topic}/{topic}{TOPIC_SET_SUFFIX}"
+
     def _handle_message(self, client, userdata, message):  # pylint: disable=unused-argument
         """ Handle and dispatch incoming messages"""
         logger.debug('Received message on %s', message.topic)
@@ -179,7 +192,7 @@ def register_set_callback(
         """ Generic- register a callback for changing values inside batcontrol via
             MQTT set topics
         """
-        topic_string = self.base_topic + "/" + topic + MqttApi.SET_SUFFIX
+        topic_string = self._set_topic(topic)
         logger.debug('Registering callback for %s', topic_string)
         # set api endpoints, generic subscription
         self.callbacks[topic_string] = {
@@ -192,22 +205,22 @@ def publish_mode(self, mode: int) -> None:
             /mode
         """
         if self.client.is_connected():
-            self.client.publish(self.base_topic + '/mode', mode)
+            self.client.publish(self._topic(TOPIC_MODE), mode)
 
     def publish_charge_rate(self, rate: float) -> None:
         """ Publish the forced charge rate in W to MQTT
             /charge_rate
         """
         if self.client.is_connected():
-            self.client.publish(self.base_topic + '/charge_rate', rate)
+            self.client.publish(self._topic(TOPIC_CHARGE_RATE), rate)
 
     def publish_limit_battery_charge_rate(self, limit: int) -> None:
         """ Publish dynamic battery charge rate limit to MQTT
             /limit_battery_charge_rate
         """
         if self.client.is_connected():
             self.client.publish(
-                self.base_topic + '/limit_battery_charge_rate',
+                self._topic(TOPIC_LIMIT_BATTERY_CHARGE_RATE),
                 limit
             )
 
@@ -458,7 +471,7 @@ def publish_api_override_active(self, active: bool) -> None:
         """
         if self.client.is_connected():
             self.client.publish(
-                self.base_topic + '/api_override_active',
+                self._topic(TOPIC_API_OVERRIDE_ACTIVE),
                 str(active).lower(),
                 retain=True
             )
@@ -518,10 +531,8 @@ def send_mqtt_discovery_messages(self) -> None:
             "number",
             "power",
             "W",
-            self.base_topic +
-            "/charge_rate",
-            self.base_topic +
-            "/charge_rate/set",
+            self._topic(TOPIC_CHARGE_RATE),
+            self._set_topic(TOPIC_CHARGE_RATE),
             entity_category="config",
             min_value=0,
             max_value=10000,
@@ -533,10 +544,8 @@ def send_mqtt_discovery_messages(self) -> None:
             "number",
             "power",
             "W",
-            self.base_topic +
-            "/limit_battery_charge_rate",
-            self.base_topic +
-            "/limit_battery_charge_rate/set",
+            self._topic(TOPIC_LIMIT_BATTERY_CHARGE_RATE),
+            self._set_topic(TOPIC_LIMIT_BATTERY_CHARGE_RATE),
             entity_category="config",
             min_value=-1,
             max_value=10000,
@@ -648,7 +657,7 @@ def send_mqtt_discovery_messages(self) -> None:
             "binary_sensor",
             None,
             None,
-            self.base_topic + "/api_override_active",
+            self._topic(TOPIC_API_OVERRIDE_ACTIVE),
             entity_category="diagnostic",
             value_template="{% if value == 'true' %}ON{% else %}OFF{% endif %}")
 
@@ -788,8 +797,8 @@ def send_mqtt_discovery_for_mode(self) -> None:
             "select",
             None,
             None,
-            self.base_topic + "/mode",
-            self.base_topic + "/mode/set",
+            self._topic(TOPIC_MODE),
+            self._set_topic(TOPIC_MODE),
             entity_category=None,
             options=[
                 "Charge from Grid",

tests/batcontrol/test_mqtt_api.py

@@ -25,6 +25,22 @@ def _make_message(topic: str, payload):
     return msg
 
 
+def _make_discovery_stub():
+    """Return a minimal stub for discovery helper tests."""
+    api = MagicMock(spec=MqttApi)
+    api.base_topic = 'batcontrol'
+    api.publish_mqtt_discovery_message = MagicMock()
+    api._topic = MqttApi._topic.__get__(api, MqttApi)
+    api._set_topic = MqttApi._set_topic.__get__(api, MqttApi)
+    api.send_mqtt_discovery_for_mode = (
+        MqttApi.send_mqtt_discovery_for_mode.__get__(api, MqttApi)
+    )
+    api.send_mqtt_discovery_messages = (
+        MqttApi.send_mqtt_discovery_messages.__get__(api, MqttApi)
+    )
+    return api
+
+
 class TestHandleMessageBytesDecoding:
     """_handle_message must decode bytes payloads before calling convert."""
 
@@ -93,12 +109,7 @@ class TestModeDiscovery:
     """Mode discovery should expose the full externally supported mode model."""
 
     def test_mode_discovery_includes_limit_battery_charge_mode(self):
-        api = MagicMock(spec=MqttApi)
-        api.base_topic = 'batcontrol'
-        api.publish_mqtt_discovery_message = MagicMock()
-        api.send_mqtt_discovery_for_mode = (
-            MqttApi.send_mqtt_discovery_for_mode.__get__(api, MqttApi)
-        )
+        api = _make_discovery_stub()
 
         api.send_mqtt_discovery_for_mode()
 
@@ -120,13 +131,7 @@ class TestDiscoveryMessages:
     """Discovery should expose key externally visible runtime state."""
 
     def test_discovery_includes_limit_battery_charge_rate_number(self):
-        api = MagicMock(spec=MqttApi)
-        api.base_topic = 'batcontrol'
-        api.publish_mqtt_discovery_message = MagicMock()
-        api.send_mqtt_discovery_for_mode = MagicMock()
-        api.send_mqtt_discovery_messages = (
-            MqttApi.send_mqtt_discovery_messages.__get__(api, MqttApi)
-        )
+        api = _make_discovery_stub()
 
         api.send_mqtt_discovery_messages()
 
@@ -148,13 +153,7 @@ def test_discovery_includes_limit_battery_charge_rate_number(self):
         )
 
     def test_discovery_includes_api_override_active_binary_sensor(self):
-        api = MagicMock(spec=MqttApi)
-        api.base_topic = 'batcontrol'
-        api.publish_mqtt_discovery_message = MagicMock()
-        api.send_mqtt_discovery_for_mode = MagicMock()
-        api.send_mqtt_discovery_messages = (
-            MqttApi.send_mqtt_discovery_messages.__get__(api, MqttApi)
-        )
+        api = _make_discovery_stub()
 
         api.send_mqtt_discovery_messages()