added option `--fetch-device-info` to toggle reporting of curtain motor's position (can't enable by default due to `bluepy.btle.BTLEManagementError: Failed to execute management command 'le on'`)

CHANGELOG.md

@@ -6,7 +6,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 ### Added
-- Report position of curtain motor on topic  `homeassistant/cover/switchbot-curtain/MAC_ADDRESS/position`
+- command-line option `--fetch-device-info` enables reporting of curtain motors'
+  position on topic  `homeassistant/cover/switchbot-curtain/MAC_ADDRESS/position`
   after sending stop command.
 
 ## [1.0.0] - 2021-07-25

README.md

@@ -46,6 +46,9 @@ Send `OPEN`, `CLOSE`, or `STOP` to topic `homeassistant/cover/switchbot-curtain/
 $ mosquitto_pub -h MQTT_BROKER -t homeassistant/cover/switchbot-curtain/aa:bb:cc:dd:ee:ff/set -m CLOSE
 ```
 
+The command-line option `--fetch-device-info` enables position reports on topic
+`homeassistant/cover/switchbot-curtain/MAC_ADDRESS/position` after `STOP` commands.
+
 ### Device Passwords
 
 In case some of your Switchbot devices are password-protected,

switchbot_mqtt/__init__.py

@@ -50,10 +50,14 @@ def _mac_address_valid(mac_address: str) -> bool:
 class _MQTTCallbackUserdata:
     # pylint: disable=too-few-public-methods; @dataclasses.dataclass when python_requires>=3.7
     def __init__(
-        self, retry_count: int, device_passwords: typing.Dict[str, str]
+        self,
+        retry_count: int,
+        device_passwords: typing.Dict[str, str],
+        fetch_device_info: bool,
     ) -> None:
         self.retry_count = retry_count
         self.device_passwords = device_passwords
+        self.fetch_device_info = fetch_device_info
 
     def __eq__(self, other: object) -> bool:
         return isinstance(other, type(self)) and vars(self) == vars(other)
@@ -72,7 +76,10 @@ class _MQTTControlledActor(abc.ABC):
 
     @abc.abstractmethod
     def execute_command(
-        self, mqtt_message_payload: bytes, mqtt_client: paho.mqtt.client.Client
+        self,
+        mqtt_message_payload: bytes,
+        mqtt_client: paho.mqtt.client.Client,
+        update_device_info: bool,
     ) -> None:
         raise NotImplementedError()
 
@@ -112,7 +119,10 @@ class _MQTTControlledActor(abc.ABC):
             password=userdata.device_passwords.get(mac_address, None),
         )
         actor.execute_command(
-            mqtt_message_payload=message.payload, mqtt_client=mqtt_client
+            mqtt_message_payload=message.payload,
+            mqtt_client=mqtt_client,
+            # consider calling update+report method directly when adding support for battery levels
+            update_device_info=userdata.fetch_device_info,
         )
 
     @classmethod
@@ -189,7 +199,10 @@ class _ButtonAutomator(_MQTTControlledActor):
         )
 
     def execute_command(
-        self, mqtt_message_payload: bytes, mqtt_client: paho.mqtt.client.Client
+        self,
+        mqtt_message_payload: bytes,
+        mqtt_client: paho.mqtt.client.Client,
+        update_device_info: bool,
     ) -> None:
         # https://www.home-assistant.io/integrations/switch.mqtt/#payload_on
         if mqtt_message_payload.lower() == b"on":
@@ -266,11 +279,17 @@ class _CurtainMotor(_MQTTControlledActor):
         )
 
     def _update_position(self, mqtt_client: paho.mqtt.client.Client) -> None:
+        # Requires running bluepy-helper executable with CAP_NET_ADMIN
+        # https://github.com/IanHarvey/bluepy/issues/313#issuecomment-428324639
+        # https://github.com/fphammerle/switchbot-mqtt/pull/31#issuecomment-840704962
         self._device.update()
         self._report_position(mqtt_client=mqtt_client)
 
     def execute_command(
-        self, mqtt_message_payload: bytes, mqtt_client: paho.mqtt.client.Client
+        self,
+        mqtt_message_payload: bytes,
+        mqtt_client: paho.mqtt.client.Client,
+        update_device_info: bool,
     ) -> None:
         # https://www.home-assistant.io/integrations/cover.mqtt/#payload_open
         if mqtt_message_payload.lower() == b"open":
@@ -297,7 +316,8 @@ class _CurtainMotor(_MQTTControlledActor):
                 # https://www.home-assistant.io/integrations/cover.mqtt/#configuration-variables
                 # https://community.home-assistant.io/t/mqtt-how-to-remove-retained-messages/79029/2
                 self.report_state(mqtt_client=mqtt_client, state=b"")
-                self._update_position(mqtt_client=mqtt_client)
+                if update_device_info:
+                    self._update_position(mqtt_client=mqtt_client)
         else:
             _LOGGER.warning(
                 "unexpected payload %r (expected 'OPEN', 'CLOSE', or 'STOP')",
@@ -327,11 +347,14 @@ def _run(
     mqtt_password: typing.Optional[str],
     retry_count: int,
     device_passwords: typing.Dict[str, str],
+    fetch_device_info: bool,
 ) -> None:
     # https://pypi.org/project/paho-mqtt/
     mqtt_client = paho.mqtt.client.Client(
         userdata=_MQTTCallbackUserdata(
-            retry_count=retry_count, device_passwords=device_passwords
+            retry_count=retry_count,
+            device_passwords=device_passwords,
+            fetch_device_info=fetch_device_info,
         )
     )
     mqtt_client.on_connect = _mqtt_on_connect
@@ -383,6 +406,12 @@ def _main() -> None:
         help="Maximum number of attempts to send a command to a SwitchBot device"
         " (default: %(default)d)",
     )
+    argparser.add_argument(
+        "--fetch-device-info",  # generic name to cover future addition of battery level etc.
+        action="store_true",
+        help="Report curtain motors' position on topic"
+        " homeassistant/cover/switchbot-curtain/MAC_ADDRESS/position after sending stop command.",
+    )
     args = argparser.parse_args()
     if args.mqtt_password_path:
         # .read_text() replaces \r\n with \n
@@ -404,4 +433,5 @@ def _main() -> None:
         mqtt_password=mqtt_password,
         retry_count=args.retry_count,
         device_passwords=device_passwords,
+        fetch_device_info=args.fetch_device_info,
     )

tests/test_actor_base.py

@@ -44,12 +44,19 @@ def test_execute_command_abstract():
             )
 
         def execute_command(
-            self, mqtt_message_payload: bytes, mqtt_client: paho.mqtt.client.Client
+            self,
+            mqtt_message_payload: bytes,
+            mqtt_client: paho.mqtt.client.Client,
+            update_device_info: bool,
         ) -> None:
             super().execute_command(
-                mqtt_message_payload=mqtt_message_payload, mqtt_client=mqtt_client
+                mqtt_message_payload=mqtt_message_payload,
+                mqtt_client=mqtt_client,
+                update_device_info=update_device_info,
             )
 
     actor = _ActorMock(mac_address=None, retry_count=42, password=None)
     with pytest.raises(NotImplementedError):
-        actor.execute_command(mqtt_message_payload=b"dummy", mqtt_client="dummy")
+        actor.execute_command(
+            mqtt_message_payload=b"dummy", mqtt_client="dummy", update_device_info=True
+        )

tests/test_cli.py

@@ -100,6 +100,7 @@ def test__main(
         mqtt_password=expected_password,
         retry_count=expected_retry_count,
         device_passwords={},
+        fetch_device_info=False,
     )
 
 
@@ -144,6 +145,7 @@ def test__main_mqtt_password_file(
         mqtt_password=expected_password,
         retry_count=3,
         device_passwords={},
+        fetch_device_info=False,
     )
 
 
@@ -202,4 +204,34 @@ def test__main_device_password_file(tmpdir, device_passwords):
         mqtt_password=None,
         retry_count=3,
         device_passwords=device_passwords,
+        fetch_device_info=False,
     )
+
+
+def test__main_fetch_device_info():
+    with unittest.mock.patch("switchbot_mqtt._run") as run_mock, unittest.mock.patch(
+        "sys.argv",
+        [
+            "",
+            "--mqtt-host",
+            "localhost",
+        ],
+    ):
+        # pylint: disable=protected-access
+        switchbot_mqtt._main()
+    default_kwargs = dict(
+        mqtt_host="localhost",
+        mqtt_port=1883,
+        mqtt_username=None,
+        mqtt_password=None,
+        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"],
+    ):
+        # pylint: disable=protected-access
+        switchbot_mqtt._main()
+    run_mock.assert_called_once_with(fetch_device_info=True, **default_kwargs)

tests/test_mqtt.py

@@ -36,7 +36,10 @@ import switchbot_mqtt
     "device_passwords",
     [{}, {"11:22:33:44:55:66": "password", "aa:bb:cc:dd:ee:ff": "secret"}],
 )
-def test__run(caplog, mqtt_host, mqtt_port, retry_count, device_passwords):
+@pytest.mark.parametrize("fetch_device_info", [True, False])
+def test__run(
+    caplog, mqtt_host, mqtt_port, retry_count, device_passwords, fetch_device_info
+):
     with unittest.mock.patch(
         "paho.mqtt.client.Client"
     ) as mqtt_client_mock, caplog.at_level(logging.DEBUG):
@@ -47,11 +50,13 @@ def test__run(caplog, mqtt_host, mqtt_port, retry_count, device_passwords):
             mqtt_password=None,
             retry_count=retry_count,
             device_passwords=device_passwords,
+            fetch_device_info=fetch_device_info,
         )
     mqtt_client_mock.assert_called_once_with(
         userdata=switchbot_mqtt._MQTTCallbackUserdata(
             retry_count=retry_count,
             device_passwords=device_passwords,
+            fetch_device_info=fetch_device_info,
         )
     )
     assert not mqtt_client_mock().username_pw_set.called
@@ -111,10 +116,11 @@ def test__run_authentication(mqtt_host, mqtt_port, mqtt_username, mqtt_password)
             mqtt_password=mqtt_password,
             retry_count=7,
             device_passwords={},
+            fetch_device_info=True,
         )
     mqtt_client_mock.assert_called_once_with(
         userdata=switchbot_mqtt._MQTTCallbackUserdata(
-            retry_count=7, device_passwords={}
+            retry_count=7, device_passwords={}, fetch_device_info=True
         )
     )
     mqtt_client_mock().username_pw_set.assert_called_once_with(
@@ -135,6 +141,7 @@ def test__run_authentication_missing_username(mqtt_host, mqtt_port, mqtt_passwor
                 mqtt_password=mqtt_password,
                 retry_count=3,
                 device_passwords={},
+                fetch_device_info=True,
             )
 
 
@@ -149,7 +156,12 @@ def _mock_actor_class(
                 mac_address=mac_address, retry_count=retry_count, password=password
             )
 
-        def execute_command(self, mqtt_message_payload: bytes, mqtt_client: Client):
+        def execute_command(
+            self,
+            mqtt_message_payload: bytes,
+            mqtt_client: Client,
+            update_device_info: bool,
+        ):
             pass
 
     return _ActorMock
@@ -203,6 +215,7 @@ def _mock_actor_class(
     ],
 )
 @pytest.mark.parametrize("retry_count", (3, 42))
+@pytest.mark.parametrize("fetch_device_info", [True, False])
 def test__mqtt_command_callback(
     caplog,
     command_topic_levels: typing.List[switchbot_mqtt._MQTTTopicLevel],
@@ -210,12 +223,15 @@ def test__mqtt_command_callback(
     payload: bytes,
     expected_mac_address: str,
     retry_count: int,
+    fetch_device_info: bool,
 ):
     ActorMock = _mock_actor_class(command_topic_levels)
     message = MQTTMessage(topic=topic)
     message.payload = payload
     callback_userdata = switchbot_mqtt._MQTTCallbackUserdata(
-        retry_count=retry_count, device_passwords={}
+        retry_count=retry_count,
+        device_passwords={},
+        fetch_device_info=fetch_device_info,
     )
     with unittest.mock.patch.object(
         ActorMock, "__init__", return_value=None
@@ -229,7 +245,9 @@ def test__mqtt_command_callback(
         mac_address=expected_mac_address, retry_count=retry_count, password=None
     )
     execute_command_mock.assert_called_once_with(
-        mqtt_client="client_dummy", mqtt_message_payload=payload
+        mqtt_client="client_dummy",
+        mqtt_message_payload=payload,
+        update_device_info=fetch_device_info,
     )
     assert caplog.record_tuples == [
         (
@@ -264,6 +282,7 @@ def test__mqtt_command_callback_password(mac_address, expected_password):
             "aa:bb:cc:dd:ee:ff": "secret",
             "11:22:33:dd:ee:ff": "äöü",
         },
+        fetch_device_info=True,
     )
     with unittest.mock.patch.object(
         ActorMock, "__init__", return_value=None
@@ -275,7 +294,9 @@ def test__mqtt_command_callback_password(mac_address, expected_password):
         mac_address=mac_address, retry_count=3, password=expected_password
     )
     execute_command_mock.assert_called_once_with(
-        mqtt_client="client_dummy", mqtt_message_payload=b"whatever"
+        mqtt_client="client_dummy",
+        mqtt_message_payload=b"whatever",
+        update_device_info=True,
     )
 
 
@@ -302,7 +323,9 @@ def test__mqtt_command_callback_unexpected_topic(caplog, topic: bytes, payload:
     ):
         ActorMock._mqtt_command_callback(
             "client_dummy",
-            switchbot_mqtt._MQTTCallbackUserdata(retry_count=3, device_passwords={}),
+            switchbot_mqtt._MQTTCallbackUserdata(
+                retry_count=3, device_passwords={}, fetch_device_info=True
+            ),
             message,
         )
     init_mock.assert_not_called()
@@ -340,7 +363,9 @@ def test__mqtt_command_callback_invalid_mac_address(
     ):
         ActorMock._mqtt_command_callback(
             "client_dummy",
-            switchbot_mqtt._MQTTCallbackUserdata(retry_count=3, device_passwords={}),
+            switchbot_mqtt._MQTTCallbackUserdata(
+                retry_count=3, device_passwords={}, fetch_device_info=True
+            ),
             message,
         )
     init_mock.assert_not_called()
@@ -379,7 +404,9 @@ def test__mqtt_command_callback_ignore_retained(caplog, topic: bytes, payload: b
     ):
         ActorMock._mqtt_command_callback(
             "client_dummy",
-            switchbot_mqtt._MQTTCallbackUserdata(retry_count=4, device_passwords={}),
+            switchbot_mqtt._MQTTCallbackUserdata(
+                retry_count=4, device_passwords={}, fetch_device_info=True
+            ),
             message,
         )
     init_mock.assert_not_called()
@@ -429,7 +456,12 @@ def test__report_state(
                 mac_address=mac_address, retry_count=retry_count, password=password
             )
 
-        def execute_command(self, mqtt_message_payload: bytes, mqtt_client: Client):
+        def execute_command(
+            self,
+            mqtt_message_payload: bytes,
+            mqtt_client: Client,
+            update_device_info: bool,
+        ):
             pass
 
     mqtt_client_mock = unittest.mock.MagicMock()

tests/test_switchbot_button_automator.py

@@ -64,7 +64,9 @@ def test_execute_command(
             action_name, return_value=command_successful
         ) as action_mock:
             actor.execute_command(
-                mqtt_client="dummy", mqtt_message_payload=message_payload
+                mqtt_client="dummy",
+                mqtt_message_payload=message_payload,
+                update_device_info=True,
             )
     device_init_mock.assert_called_once_with(
         mac=mac_address, password=password, retry_count=retry_count
@@ -107,7 +109,9 @@ def test_execute_command_invalid_payload(caplog, mac_address, message_payload):
         )
         with unittest.mock.patch.object(actor, "report_state") as report_mock:
             actor.execute_command(
-                mqtt_client="dummy", mqtt_message_payload=message_payload
+                mqtt_client="dummy",
+                mqtt_message_payload=message_payload,
+                update_device_info=True,
             )
     device_mock.assert_called_once_with(mac=mac_address, retry_count=21, password=None)
     assert not device_mock().mock_calls  # no methods called
@@ -138,7 +142,11 @@ def test_execute_command_bluetooth_error(caplog, mac_address, message_payload):
     ), caplog.at_level(logging.ERROR):
         switchbot_mqtt._ButtonAutomator(
             mac_address=mac_address, retry_count=3, password=None
-        ).execute_command(mqtt_client="dummy", mqtt_message_payload=message_payload)
+        ).execute_command(
+            mqtt_client="dummy",
+            mqtt_message_payload=message_payload,
+            update_device_info=True,
+        )
     assert caplog.record_tuples == [
         (
             "switchbot",

tests/test_switchbot_curtain_motor.py

@@ -126,6 +126,7 @@ def test__update_position():
         (b"Stop", "switchbot.SwitchbotCurtain.stop"),
     ],
 )
+@pytest.mark.parametrize("report_position_upon_stop", [True, False])
 @pytest.mark.parametrize("command_successful", [True, False])
 def test_execute_command(
     caplog,
@@ -134,6 +135,7 @@ def test_execute_command(
     retry_count,
     message_payload,
     action_name,
+    report_position_upon_stop,
     command_successful,
 ):
     with unittest.mock.patch(
@@ -150,7 +152,9 @@ def test_execute_command(
             actor, "_update_position"
         ) as update_position_mock:
             actor.execute_command(
-                mqtt_client="dummy", mqtt_message_payload=message_payload
+                mqtt_client="dummy",
+                mqtt_message_payload=message_payload,
+                update_device_info=report_position_upon_stop,
             )
     device_init_mock.assert_called_once_with(
         mac=mac_address, password=password, retry_count=retry_count, reverse_mode=True
@@ -187,7 +191,11 @@ def test_execute_command(
             )
         ]
         report_mock.assert_not_called()
-    if action_name == "switchbot.SwitchbotCurtain.stop" and command_successful:
+    if (
+        report_position_upon_stop
+        and action_name == "switchbot.SwitchbotCurtain.stop"
+        and command_successful
+    ):
         update_position_mock.assert_called_once_with(mqtt_client="dummy")
     else:
         update_position_mock.assert_not_called()
@@ -207,7 +215,9 @@ def test_execute_command_invalid_payload(
         )
         with unittest.mock.patch.object(actor, "report_state") as report_mock:
             actor.execute_command(
-                mqtt_client="dummy", mqtt_message_payload=message_payload
+                mqtt_client="dummy",
+                mqtt_message_payload=message_payload,
+                update_device_info=True,
             )
     device_mock.assert_called_once_with(
         mac=mac_address, password=password, retry_count=7, reverse_mode=True
@@ -242,7 +252,11 @@ def test_execute_command_bluetooth_error(caplog, mac_address, message_payload):
     ), caplog.at_level(logging.ERROR):
         switchbot_mqtt._CurtainMotor(
             mac_address=mac_address, retry_count=10, password="secret"
-        ).execute_command(mqtt_client="dummy", mqtt_message_payload=message_payload)
+        ).execute_command(
+            mqtt_client="dummy",
+            mqtt_message_payload=message_payload,
+            update_device_info=True,
+        )
     assert caplog.record_tuples == [
         (
             "switchbot",