From 9947875774e85ade715fe963d2fa1112624a1394 Mon Sep 17 00:00:00 2001 From: Fabian Peter Hammerle Date: Tue, 30 Aug 2022 21:39:50 +0200 Subject: [PATCH 1/2] fix topic of birth & last will message `homeassistant/{switchbot_mqtt->switchbot-mqtt}/status` (old kept for backward compatibility) --- CHANGELOG.md | 5 +++++ switchbot_mqtt/__init__.py | 26 +++++++++++++++----------- tests/test_mqtt.py | 16 ++++++++++------ 3 files changed, 30 insertions(+), 17 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4568ee5..65a89e9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,11 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). ## [Unreleased] +### Fixed +- Publish birth and last will message on expected/documented topic + `homeassistant/switchbot-mqtt/status` + (old/undocumented topic `homeassistant/switchbot_mqtt/status` kept for + backward compatibility until next major release) ## [3.3.0] - 2022-08-30 ### Added diff --git a/switchbot_mqtt/__init__.py b/switchbot_mqtt/__init__.py index 34cd390..f03fe5f 100644 --- a/switchbot_mqtt/__init__.py +++ b/switchbot_mqtt/__init__.py @@ -27,7 +27,9 @@ _LOGGER = logging.getLogger(__name__) -_MQTT_AVAILABILITY_TOPIC = "switchbot_mqtt/status" +_MQTT_AVAILABILITY_TOPIC = "switchbot-mqtt/status" +# for backward compatability, will be removed in next major release: +_MQTT_AVAILABILITY_TOPIC_LEGACY = "switchbot_mqtt/status" # "online" and "offline" to match home assistant's default settings # https://www.home-assistant.io/integrations/switch.mqtt/#payload_available _MQTT_BIRTH_PAYLOAD = "online" @@ -52,11 +54,12 @@ def _mqtt_on_connect( else mqtt_broker_host, mqtt_broker_port, ) - mqtt_client.publish( - topic=userdata.mqtt_topic_prefix + _MQTT_AVAILABILITY_TOPIC, - payload=_MQTT_BIRTH_PAYLOAD, - retain=True, - ) + for topic in (_MQTT_AVAILABILITY_TOPIC, _MQTT_AVAILABILITY_TOPIC_LEGACY): + mqtt_client.publish( + topic=userdata.mqtt_topic_prefix + topic, + payload=_MQTT_BIRTH_PAYLOAD, + retain=True, + ) _ButtonAutomator.mqtt_subscribe(mqtt_client=mqtt_client, settings=userdata) _CurtainMotor.mqtt_subscribe(mqtt_client=mqtt_client, settings=userdata) @@ -95,11 +98,12 @@ def _run( mqtt_client.username_pw_set(username=mqtt_username, password=mqtt_password) elif mqtt_password: raise ValueError("Missing MQTT username") - mqtt_client.will_set( - topic=mqtt_topic_prefix + _MQTT_AVAILABILITY_TOPIC, - payload=_MQTT_LAST_WILL_PAYLOAD, - retain=True, - ) + for topic in (_MQTT_AVAILABILITY_TOPIC, _MQTT_AVAILABILITY_TOPIC_LEGACY): + mqtt_client.will_set( + topic=mqtt_topic_prefix + topic, + payload=_MQTT_LAST_WILL_PAYLOAD, + retain=True, + ) mqtt_client.connect(host=mqtt_host, port=mqtt_port) # https://github.com/eclipse/paho.mqtt.python/blob/master/src/paho/mqtt/client.py#L1740 mqtt_client.loop_forever() diff --git a/tests/test_mqtt.py b/tests/test_mqtt.py index 375ad9d..731e4c3 100644 --- a/tests/test_mqtt.py +++ b/tests/test_mqtt.py @@ -65,9 +65,10 @@ def test__mqtt_on_connect( {}, 0, ) - mqtt_client.publish.assert_called_once_with( - topic="whatever/switchbot_mqtt/status", payload="online", retain=True - ) + assert mqtt_client.publish.call_args_list == [ + unittest.mock.call(topic=f"whatever/{t}/status", payload="online", retain=True) + for t in ("switchbot-mqtt", "switchbot_mqtt") + ] assert mqtt_client.subscribe.call_args_list == [ unittest.mock.call("whatever/switch/switchbot/+/set"), unittest.mock.call("whatever/cover/switchbot-curtain/+/set"), @@ -141,9 +142,12 @@ def test__run( ) assert not mqtt_client_mock().username_pw_set.called mqtt_client_mock().tls_set.assert_called_once_with(ca_certs=None) - mqtt_client_mock().will_set.assert_called_once_with( - topic="homeassistant/switchbot_mqtt/status", payload="offline", retain=True - ) + assert mqtt_client_mock().will_set.call_args_list == [ + unittest.mock.call( + topic=f"homeassistant/{t}/status", payload="offline", retain=True + ) + for t in ("switchbot-mqtt", "switchbot_mqtt") + ] mqtt_client_mock().connect.assert_called_once_with(host=mqtt_host, port=mqtt_port) mqtt_client_mock().socket().getpeername.return_value = (mqtt_host, mqtt_port) with caplog.at_level(logging.DEBUG): From 9d8a18c183849fb6f54f965f2777327b9ab0d666 Mon Sep 17 00:00:00 2001 From: Fabian Peter Hammerle Date: Wed, 31 Aug 2022 16:31:07 +0200 Subject: [PATCH 2/2] fix ineffective last will message setting (by disabling birth & last will on `homeassistant/switchbot_mqtt/status`) partially reverts commit 9947875774e85ade715fe963d2fa1112624a1394 https://github.com/fphammerle/switchbot-mqtt/issues/106#issue-1356357552 https://github.com/fphammerle/switchbot-mqtt/pull/105 --- CHANGELOG.md | 5 ++--- switchbot_mqtt/__init__.py | 24 ++++++++++-------------- tests/test_mqtt.py | 16 ++++++---------- 3 files changed, 18 insertions(+), 27 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 65a89e9..77847ad 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,9 +7,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] ### Fixed - Publish birth and last will message on expected/documented topic - `homeassistant/switchbot-mqtt/status` - (old/undocumented topic `homeassistant/switchbot_mqtt/status` kept for - backward compatibility until next major release) + `homeassistant/switchbot-mqtt/status` instead of topic + `homeassistant/switchbot_mqtt/status`. ## [3.3.0] - 2022-08-30 ### Added diff --git a/switchbot_mqtt/__init__.py b/switchbot_mqtt/__init__.py index f03fe5f..3f978b2 100644 --- a/switchbot_mqtt/__init__.py +++ b/switchbot_mqtt/__init__.py @@ -28,8 +28,6 @@ _LOGGER = logging.getLogger(__name__) _MQTT_AVAILABILITY_TOPIC = "switchbot-mqtt/status" -# for backward compatability, will be removed in next major release: -_MQTT_AVAILABILITY_TOPIC_LEGACY = "switchbot_mqtt/status" # "online" and "offline" to match home assistant's default settings # https://www.home-assistant.io/integrations/switch.mqtt/#payload_available _MQTT_BIRTH_PAYLOAD = "online" @@ -54,12 +52,11 @@ def _mqtt_on_connect( else mqtt_broker_host, mqtt_broker_port, ) - for topic in (_MQTT_AVAILABILITY_TOPIC, _MQTT_AVAILABILITY_TOPIC_LEGACY): - mqtt_client.publish( - topic=userdata.mqtt_topic_prefix + topic, - payload=_MQTT_BIRTH_PAYLOAD, - retain=True, - ) + mqtt_client.publish( + topic=userdata.mqtt_topic_prefix + _MQTT_AVAILABILITY_TOPIC, + payload=_MQTT_BIRTH_PAYLOAD, + retain=True, + ) _ButtonAutomator.mqtt_subscribe(mqtt_client=mqtt_client, settings=userdata) _CurtainMotor.mqtt_subscribe(mqtt_client=mqtt_client, settings=userdata) @@ -98,12 +95,11 @@ def _run( mqtt_client.username_pw_set(username=mqtt_username, password=mqtt_password) elif mqtt_password: raise ValueError("Missing MQTT username") - for topic in (_MQTT_AVAILABILITY_TOPIC, _MQTT_AVAILABILITY_TOPIC_LEGACY): - mqtt_client.will_set( - topic=mqtt_topic_prefix + topic, - payload=_MQTT_LAST_WILL_PAYLOAD, - retain=True, - ) + mqtt_client.will_set( + topic=mqtt_topic_prefix + _MQTT_AVAILABILITY_TOPIC, + payload=_MQTT_LAST_WILL_PAYLOAD, + retain=True, + ) mqtt_client.connect(host=mqtt_host, port=mqtt_port) # https://github.com/eclipse/paho.mqtt.python/blob/master/src/paho/mqtt/client.py#L1740 mqtt_client.loop_forever() diff --git a/tests/test_mqtt.py b/tests/test_mqtt.py index 731e4c3..1b105be 100644 --- a/tests/test_mqtt.py +++ b/tests/test_mqtt.py @@ -65,10 +65,9 @@ def test__mqtt_on_connect( {}, 0, ) - assert mqtt_client.publish.call_args_list == [ - unittest.mock.call(topic=f"whatever/{t}/status", payload="online", retain=True) - for t in ("switchbot-mqtt", "switchbot_mqtt") - ] + mqtt_client.publish.assert_called_once_with( + topic="whatever/switchbot-mqtt/status", payload="online", retain=True + ) assert mqtt_client.subscribe.call_args_list == [ unittest.mock.call("whatever/switch/switchbot/+/set"), unittest.mock.call("whatever/cover/switchbot-curtain/+/set"), @@ -142,12 +141,9 @@ def test__run( ) assert not mqtt_client_mock().username_pw_set.called mqtt_client_mock().tls_set.assert_called_once_with(ca_certs=None) - assert mqtt_client_mock().will_set.call_args_list == [ - unittest.mock.call( - topic=f"homeassistant/{t}/status", payload="offline", retain=True - ) - for t in ("switchbot-mqtt", "switchbot_mqtt") - ] + mqtt_client_mock().will_set.assert_called_once_with( + topic="homeassistant/switchbot-mqtt/status", payload="offline", retain=True + ) mqtt_client_mock().connect.assert_called_once_with(host=mqtt_host, port=mqtt_port) mqtt_client_mock().socket().getpeername.return_value = (mqtt_host, mqtt_port) with caplog.at_level(logging.DEBUG):