refactor: add param `mqtt_topic_prefix` to internal function `_run`

https://github.com/fphammerle/switchbot-mqtt/issues/70

switchbot_mqtt/__init__.py

@@ -46,9 +46,10 @@ def _run(
     *,
     mqtt_host: str,
     mqtt_port: int,
+    mqtt_disable_tls: bool,
     mqtt_username: typing.Optional[str],
     mqtt_password: typing.Optional[str],
-    mqtt_disable_tls: bool,
+    mqtt_topic_prefix: str,
     retry_count: int,
     device_passwords: typing.Dict[str, str],
     fetch_device_info: bool,
@@ -59,6 +60,7 @@ def _run(
             retry_count=retry_count,
             device_passwords=device_passwords,
             fetch_device_info=fetch_device_info,
+            mqtt_topic_prefix=mqtt_topic_prefix,
         )
     )
     mqtt_client.on_connect = _mqtt_on_connect

switchbot_mqtt/_actors/base.py

@@ -52,9 +52,7 @@ class _MQTTCallbackUserdata:
     retry_count: int
     device_passwords: typing.Dict[str, str]
     fetch_device_info: bool
-    # "homeassistant/" for historic reasons.
+    mqtt_topic_prefix: str
-    # will be parametrized via command-line argument in the future.
-    mqtt_topic_prefix: str = "homeassistant/"
 
 
 class _MQTTControlledActor(abc.ABC):

switchbot_mqtt/_cli.py

@@ -27,10 +27,10 @@ import switchbot
 
 import switchbot_mqtt
 from switchbot_mqtt._actors import _ButtonAutomator, _CurtainMotor
-from switchbot_mqtt._actors.base import _MQTTCallbackUserdata
 
 _MQTT_DEFAULT_PORT = 1883
 _MQTT_DEFAULT_TLS_PORT = 8883
+_MQTT_TOPIC_PREFIX = "homeassistant/"  # for historic reasons
 
 _LOGGER = logging.getLogger(__name__)
 
@@ -86,25 +86,25 @@ def _main() -> None:
         action="store_true",
         help="Report devices' battery level on topic "
         + _ButtonAutomator.get_mqtt_battery_percentage_topic(
-            prefix=_MQTTCallbackUserdata.mqtt_topic_prefix, mac_address="MAC_ADDRESS"
+            prefix=_MQTT_TOPIC_PREFIX, mac_address="MAC_ADDRESS"
         )
         + " or, respectively,"
         + _CurtainMotor.get_mqtt_battery_percentage_topic(
-            prefix=_MQTTCallbackUserdata.mqtt_topic_prefix, mac_address="MAC_ADDRESS"
+            prefix=_MQTT_TOPIC_PREFIX, mac_address="MAC_ADDRESS"
         )
         + " after every command. Additionally report curtain motors' position on topic "
         + _CurtainMotor.get_mqtt_position_topic(
-            prefix=_MQTTCallbackUserdata.mqtt_topic_prefix, mac_address="MAC_ADDRESS"
+            prefix=_MQTT_TOPIC_PREFIX, mac_address="MAC_ADDRESS"
         )
         + " after executing stop commands."
         " When this option is enabled, the mentioned reports may also be requested"
         " by sending a MQTT message to the topic "
         + _ButtonAutomator.get_mqtt_update_device_info_topic(
-            prefix=_MQTTCallbackUserdata.mqtt_topic_prefix, mac_address="MAC_ADDRESS"
+            prefix=_MQTT_TOPIC_PREFIX, mac_address="MAC_ADDRESS"
         )
         + " or "
         + _CurtainMotor.get_mqtt_update_device_info_topic(
-            prefix=_MQTTCallbackUserdata.mqtt_topic_prefix, mac_address="MAC_ADDRESS"
+            prefix=_MQTT_TOPIC_PREFIX, mac_address="MAC_ADDRESS"
         )
         + ". This option can also be enabled by assigning a non-empty value to the"
         " environment variable FETCH_DEVICE_INFO.",
@@ -149,9 +149,10 @@ def _main() -> None:
     switchbot_mqtt._run(  # pylint: disable=protected-access; internal
         mqtt_host=args.mqtt_host,
         mqtt_port=mqtt_port,
+        mqtt_disable_tls=not args.mqtt_enable_tls,
         mqtt_username=args.mqtt_username,
         mqtt_password=mqtt_password,
-        mqtt_disable_tls=not args.mqtt_enable_tls,
+        mqtt_topic_prefix=_MQTT_TOPIC_PREFIX,
         retry_count=args.retry_count,
         device_passwords=device_passwords,
         fetch_device_info=args.fetch_device_info

tests/test_cli.py

@@ -108,9 +108,10 @@ def test__main(
     run_mock.assert_called_once_with(
         mqtt_host=expected_mqtt_host,
         mqtt_port=expected_mqtt_port,
+        mqtt_disable_tls=True,
         mqtt_username=expected_username,
         mqtt_password=expected_password,
-        mqtt_disable_tls=True,
+        mqtt_topic_prefix="homeassistant/",
         retry_count=expected_retry_count,
         device_passwords={},
         fetch_device_info=False,
@@ -152,9 +153,10 @@ def test__main_mqtt_password_file(
     run_mock.assert_called_once_with(
         mqtt_host="localhost",
         mqtt_port=1883,
+        mqtt_disable_tls=True,
         mqtt_username="me",
         mqtt_password=expected_password,
-        mqtt_disable_tls=True,
+        mqtt_topic_prefix="homeassistant/",
         retry_count=3,
         device_passwords={},
         fetch_device_info=False,
@@ -214,9 +216,10 @@ def test__main_device_password_file(
     run_mock.assert_called_once_with(
         mqtt_host="localhost",
         mqtt_port=1883,
+        mqtt_disable_tls=True,
         mqtt_username=None,
         mqtt_password=None,
-        mqtt_disable_tls=True,
+        mqtt_topic_prefix="homeassistant/",
         retry_count=3,
         device_passwords=device_passwords,
         fetch_device_info=False,
@@ -224,10 +227,12 @@ def test__main_device_password_file(
 
 
 _RUN_DEFAULT_KWARGS: typing.Dict[str, typing.Any] = {
+    "mqtt_host": "localhost",
     "mqtt_port": 1883,
+    "mqtt_disable_tls": True,
     "mqtt_username": None,
     "mqtt_password": None,
-    "mqtt_disable_tls": False,
+    "mqtt_topic_prefix": "homeassistant/",
     "retry_count": 3,
     "device_passwords": {},
     "fetch_device_info": False,
@@ -255,7 +260,12 @@ def test__main_mqtt_enable_tls() -> None:
     ):
         switchbot_mqtt._cli._main()
     run_mock.assert_called_once_with(
-        **{**_RUN_DEFAULT_KWARGS, "mqtt_host": "mqtt.local", "mqtt_port": 8883}
+        **{
+            **_RUN_DEFAULT_KWARGS,
+            "mqtt_host": "mqtt.local",
+            "mqtt_disable_tls": False,
+            "mqtt_port": 8883,
+        }
     )
 
 
@@ -266,7 +276,12 @@ def test__main_mqtt_enable_tls_overwrite_port() -> None:
     ):
         switchbot_mqtt._cli._main()
     run_mock.assert_called_once_with(
-        **{**_RUN_DEFAULT_KWARGS, "mqtt_host": "mqtt.local", "mqtt_port": 1883}
+        **{
+            **_RUN_DEFAULT_KWARGS,
+            "mqtt_host": "mqtt.local",
+            "mqtt_disable_tls": False,
+            "mqtt_port": 1883,
+        }
     )
 
 
@@ -293,40 +308,41 @@ def test__main_fetch_device_info() -> None:
         ],
     ):
         switchbot_mqtt._cli._main()
-    default_kwargs = dict(
+    run_mock.assert_called_once_with(
-        mqtt_host="localhost",
+        **{**_RUN_DEFAULT_KWARGS, "fetch_device_info": False}
-        mqtt_port=1883,
-        mqtt_username=None,
-        mqtt_password=None,
-        mqtt_disable_tls=True,
-        retry_count=3,
-        device_passwords={},
     )
-    run_mock.assert_called_once_with(fetch_device_info=False, **default_kwargs)
     with unittest.mock.patch("switchbot_mqtt._run") as run_mock, unittest.mock.patch(
         "sys.argv",
         ["", "--mqtt-host", "localhost", "--fetch-device-info"],
     ):
         switchbot_mqtt._cli._main()
-    run_mock.assert_called_once_with(fetch_device_info=True, **default_kwargs)
+    run_mock.assert_called_once_with(
+        **{**_RUN_DEFAULT_KWARGS, "fetch_device_info": True}
+    )
     with unittest.mock.patch("switchbot_mqtt._run") as run_mock, unittest.mock.patch(
         "sys.argv",
         ["", "--mqtt-host", "localhost"],
     ), unittest.mock.patch.dict("os.environ", {"FETCH_DEVICE_INFO": "21"}):
         switchbot_mqtt._cli._main()
-    run_mock.assert_called_once_with(fetch_device_info=True, **default_kwargs)
+    run_mock.assert_called_once_with(
+        **{**_RUN_DEFAULT_KWARGS, "fetch_device_info": True}
+    )
     with unittest.mock.patch("switchbot_mqtt._run") as run_mock, unittest.mock.patch(
         "sys.argv",
         ["", "--mqtt-host", "localhost"],
     ), unittest.mock.patch.dict("os.environ", {"FETCH_DEVICE_INFO": ""}):
         switchbot_mqtt._cli._main()
-    run_mock.assert_called_once_with(fetch_device_info=False, **default_kwargs)
+    run_mock.assert_called_once_with(
+        **{**_RUN_DEFAULT_KWARGS, "fetch_device_info": False}
+    )
     with unittest.mock.patch("switchbot_mqtt._run") as run_mock, unittest.mock.patch(
         "sys.argv",
         ["", "--mqtt-host", "localhost"],
     ), unittest.mock.patch.dict("os.environ", {"FETCH_DEVICE_INFO": " "}):
         switchbot_mqtt._cli._main()
-    run_mock.assert_called_once_with(fetch_device_info=True, **default_kwargs)
+    run_mock.assert_called_once_with(
+        **{**_RUN_DEFAULT_KWARGS, "fetch_device_info": True}
+    )
 
 
 @pytest.mark.parametrize(

tests/test_mqtt.py

@@ -56,9 +56,10 @@ def test__run(
         switchbot_mqtt._run(
             mqtt_host=mqtt_host,
             mqtt_port=mqtt_port,
+            mqtt_disable_tls=False,
             mqtt_username=None,
             mqtt_password=None,
-            mqtt_disable_tls=False,
+            mqtt_topic_prefix="homeassistant/",
             retry_count=retry_count,
             device_passwords=device_passwords,
             fetch_device_info=fetch_device_info,
@@ -71,6 +72,7 @@ def test__run(
         retry_count=retry_count,
         device_passwords=device_passwords,
         fetch_device_info=fetch_device_info,
+        mqtt_topic_prefix="homeassistant/",
     )
     assert not mqtt_client_mock().username_pw_set.called
     mqtt_client_mock().tls_set.assert_called_once_with(ca_certs=None)
@@ -137,9 +139,10 @@ def test__run_tls(
         switchbot_mqtt._run(
             mqtt_host="mqtt.local",
             mqtt_port=1234,
+            mqtt_disable_tls=mqtt_disable_tls,
             mqtt_username=None,
             mqtt_password=None,
-            mqtt_disable_tls=mqtt_disable_tls,
+            mqtt_topic_prefix="prfx",
             retry_count=21,
             device_passwords={},
             fetch_device_info=True,
@@ -168,16 +171,20 @@ def test__run_authentication(
         switchbot_mqtt._run(
             mqtt_host=mqtt_host,
             mqtt_port=mqtt_port,
+            mqtt_disable_tls=True,
             mqtt_username=mqtt_username,
             mqtt_password=mqtt_password,
-            mqtt_disable_tls=True,
+            mqtt_topic_prefix="prfx",
             retry_count=7,
             device_passwords={},
             fetch_device_info=True,
         )
     mqtt_client_mock.assert_called_once_with(
         userdata=_MQTTCallbackUserdata(
-            retry_count=7, device_passwords={}, fetch_device_info=True
+            retry_count=7,
+            device_passwords={},
+            fetch_device_info=True,
+            mqtt_topic_prefix="prfx",
         )
     )
     mqtt_client_mock().username_pw_set.assert_called_once_with(
@@ -196,9 +203,10 @@ def test__run_authentication_missing_username(
             switchbot_mqtt._run(
                 mqtt_host=mqtt_host,
                 mqtt_port=mqtt_port,
+                mqtt_disable_tls=True,
                 mqtt_username=None,
                 mqtt_password=mqtt_password,
-                mqtt_disable_tls=True,
+                mqtt_topic_prefix="whatever",
                 retry_count=3,
                 device_passwords={},
                 fetch_device_info=True,
@@ -242,7 +250,7 @@ def _mock_actor_class(
     [
         (
             switchbot_mqtt._actors._ButtonAutomator._MQTT_UPDATE_DEVICE_INFO_TOPIC_LEVELS,
-            b"homeassistant/switch/switchbot/aa:bb:cc:dd:ee:ff/request-device-info",
+            b"prfx/switch/switchbot/aa:bb:cc:dd:ee:ff/request-device-info",
             "aa:bb:cc:dd:ee:ff",
         ),
     ],
@@ -262,6 +270,7 @@ def test__mqtt_update_device_info_callback(
         retry_count=21,  # tested in test__mqtt_command_callback
         device_passwords={},
         fetch_device_info=True,
+        mqtt_topic_prefix="prfx/",
     )
     with unittest.mock.patch.object(
         ActorMock, "__init__", return_value=None
@@ -277,7 +286,7 @@ def test__mqtt_update_device_info_callback(
         mac_address=expected_mac_address, retry_count=21, password=None
     )
     update_mock.assert_called_once_with(
-        mqtt_client="client_dummy", mqtt_topic_prefix="homeassistant/"
+        mqtt_client="client_dummy", mqtt_topic_prefix="prfx/"
     )
     assert caplog.record_tuples == [
         (
@@ -307,7 +316,10 @@ def test__mqtt_update_device_info_callback_ignore_retained(
         ActorMock._mqtt_update_device_info_callback(
             "client_dummy",
             _MQTTCallbackUserdata(
-                retry_count=21, device_passwords={}, fetch_device_info=True
+                retry_count=21,
+                device_passwords={},
+                fetch_device_info=True,
+                mqtt_topic_prefix="ignored",
             ),
             message,
         )
@@ -499,7 +511,10 @@ def test__mqtt_command_callback_unexpected_topic(
         ActorMock._mqtt_command_callback(
             "client_dummy",
             _MQTTCallbackUserdata(
-                retry_count=3, device_passwords={}, fetch_device_info=True
+                retry_count=3,
+                device_passwords={},
+                fetch_device_info=True,
+                mqtt_topic_prefix="homeassistant/",
             ),
             message,
         )
@@ -526,7 +541,7 @@ def test__mqtt_command_callback_invalid_mac_address(
     ActorMock = _mock_actor_class(
         command_topic_levels=_ButtonAutomator.MQTT_COMMAND_TOPIC_LEVELS
     )
-    topic = f"homeassistant/switch/switchbot/{mac_address}/set".encode()
+    topic = f"mqttprefix-switch/switchbot/{mac_address}/set".encode()
     message = MQTTMessage(topic=topic)
     message.payload = payload
     with unittest.mock.patch.object(
@@ -539,7 +554,10 @@ def test__mqtt_command_callback_invalid_mac_address(
         ActorMock._mqtt_command_callback(
             "client_dummy",
             _MQTTCallbackUserdata(
-                retry_count=3, device_passwords={}, fetch_device_info=True
+                retry_count=3,
+                device_passwords={},
+                fetch_device_info=True,
+                mqtt_topic_prefix="mqttprefix-",
             ),
             message,
         )
@@ -582,7 +600,10 @@ def test__mqtt_command_callback_ignore_retained(
         ActorMock._mqtt_command_callback(
             "client_dummy",
             _MQTTCallbackUserdata(
-                retry_count=4, device_passwords={}, fetch_device_info=True
+                retry_count=4,
+                device_passwords={},
+                fetch_device_info=True,
+                mqtt_topic_prefix="homeassistant/",
             ),
             message,
         )

tests/test_switchbot_curtain_motor_position.py

@@ -33,19 +33,19 @@ from switchbot_mqtt._actors.base import _MQTTCallbackUserdata
     ("topic", "payload", "expected_mac_address", "expected_position_percent"),
     [
         (
-            b"homeassistant/cover/switchbot-curtain/aa:bb:cc:dd:ee:ff/position/set-percent",
+            b"home/cover/switchbot-curtain/aa:bb:cc:dd:ee:ff/position/set-percent",
             b"42",
             "aa:bb:cc:dd:ee:ff",
             42,
         ),
         (
-            b"homeassistant/cover/switchbot-curtain/11:22:33:44:55:66/position/set-percent",
+            b"home/cover/switchbot-curtain/11:22:33:44:55:66/position/set-percent",
             b"0",
             "11:22:33:44:55:66",
             0,
         ),
         (
-            b"homeassistant/cover/switchbot-curtain/11:22:33:44:55:66/position/set-percent",
+            b"home/cover/switchbot-curtain/11:22:33:44:55:66/position/set-percent",
             b"100",
             "11:22:33:44:55:66",
             100,
@@ -65,6 +65,7 @@ def test__mqtt_set_position_callback(
         retry_count=retry_count,
         device_passwords={},
         fetch_device_info=False,
+        mqtt_topic_prefix="home/",
     )
     message = MQTTMessage(topic=topic)
     message.payload = payload
@@ -85,7 +86,7 @@ def test__mqtt_set_position_callback(
         (
             "switchbot_mqtt._actors",
             logging.DEBUG,
-            f"received topic=homeassistant/cover/switchbot-curtain/{expected_mac_address}"
+            f"received topic=home/cover/switchbot-curtain/{expected_mac_address}"
             f"/position/set-percent payload=b'{expected_position_percent}'",
         ),
         (
@@ -104,6 +105,7 @@ def test__mqtt_set_position_callback_ignore_retained(
         retry_count=3,
         device_passwords={},
         fetch_device_info=False,
+        mqtt_topic_prefix="whatever",
     )
     message = MQTTMessage(
         topic=b"homeassistant/cover/switchbot-curtain/aa:bb:cc:dd:ee:ff/position/set-percent"
@@ -161,9 +163,10 @@ def test__mqtt_set_position_callback_invalid_mac_address(
         retry_count=3,
         device_passwords={},
         fetch_device_info=False,
+        mqtt_topic_prefix="tnatsissaemoh/",
     )
     message = MQTTMessage(
-        topic=b"homeassistant/cover/switchbot-curtain/aa:bb:cc:dd:ee/position/set-percent"
+        topic=b"tnatsissaemoh/cover/switchbot-curtain/aa:bb:cc:dd:ee/position/set-percent"
     )
     message.payload = b"42"
     with unittest.mock.patch(
@@ -191,6 +194,7 @@ def test__mqtt_set_position_callback_invalid_position(
         retry_count=3,
         device_passwords={},
         fetch_device_info=False,
+        mqtt_topic_prefix="homeassistant/",
     )
     message = MQTTMessage(
         topic=b"homeassistant/cover/switchbot-curtain/aa:bb:cc:dd:ee:ff/position/set-percent"
@@ -220,9 +224,10 @@ def test__mqtt_set_position_callback_command_failed(
         retry_count=3,
         device_passwords={},
         fetch_device_info=False,
+        mqtt_topic_prefix="",
     )
     message = MQTTMessage(
-        topic=b"homeassistant/cover/switchbot-curtain/aa:bb:cc:dd:ee:ff/position/set-percent"
+        topic=b"cover/switchbot-curtain/aa:bb:cc:dd:ee:ff/position/set-percent"
     )
     message.payload = b"21"
     with unittest.mock.patch(