From 7258bc6457d628e97eb2f66d65dc3314800b3eb0 Mon Sep 17 00:00:00 2001 From: Jan Bouwhuis Date: Mon, 25 Sep 2023 22:17:29 +0200 Subject: [PATCH] Avoid redundant calls to async_write_ha_state in mqtt vacuum (#100799) * Avoid redundant calls to async_write_ha_state * Add comment * Rephrase --- .../components/mqtt/vacuum/schema_legacy.py | 20 +++++++-- .../components/mqtt/vacuum/schema_state.py | 8 ++-- tests/components/mqtt/test_legacy_vacuum.py | 41 +++++++++++++++++++ tests/components/mqtt/test_state_vacuum.py | 38 +++++++++++++++++ 4 files changed, 100 insertions(+), 7 deletions(-) diff --git a/homeassistant/components/mqtt/vacuum/schema_legacy.py b/homeassistant/components/mqtt/vacuum/schema_legacy.py index 516a7772c11..478a91baaba 100644 --- a/homeassistant/components/mqtt/vacuum/schema_legacy.py +++ b/homeassistant/components/mqtt/vacuum/schema_legacy.py @@ -30,14 +30,14 @@ from .. import subscription from ..config import MQTT_BASE_SCHEMA from ..const import CONF_COMMAND_TOPIC, CONF_ENCODING, CONF_QOS, CONF_RETAIN from ..debug_info import log_messages -from ..mixins import MQTT_ENTITY_COMMON_SCHEMA, MqttEntity +from ..mixins import MQTT_ENTITY_COMMON_SCHEMA, MqttEntity, write_state_on_attr_change from ..models import ( MqttValueTemplate, PayloadSentinel, ReceiveMessage, ReceivePayloadType, ) -from ..util import get_mqtt_data, valid_publish_topic +from ..util import valid_publish_topic from .const import MQTT_VACUUM_ATTRIBUTES_BLOCKED from .schema import MQTT_VACUUM_SCHEMA, services_to_strings, strings_to_services @@ -313,6 +313,20 @@ class MqttVacuum(MqttEntity, VacuumEntity): @callback @log_messages(self.hass, self.entity_id) + @write_state_on_attr_change( + self, + { + "_attr_battery_level", + "_attr_fan_speed", + "_attr_is_on", + # We track _attr_status and _charging as they are used to + # To determine the batery_icon. + # We do not need to track _docked as it is + # not leading to entity changes directly. + "_attr_status", + "_charging", + }, + ) def message_received(msg: ReceiveMessage) -> None: """Handle new MQTT message.""" if ( @@ -387,8 +401,6 @@ class MqttVacuum(MqttEntity, VacuumEntity): if fan_speed and fan_speed is not PayloadSentinel.DEFAULT: self._attr_fan_speed = str(fan_speed) - get_mqtt_data(self.hass).state_write_requests.write_state_request(self) - topics_list = {topic for topic in self._state_topics.values() if topic} self._sub_state = subscription.async_prepare_subscribe_topics( self.hass, diff --git a/homeassistant/components/mqtt/vacuum/schema_state.py b/homeassistant/components/mqtt/vacuum/schema_state.py index 5113e19f097..425202adea2 100644 --- a/homeassistant/components/mqtt/vacuum/schema_state.py +++ b/homeassistant/components/mqtt/vacuum/schema_state.py @@ -38,9 +38,9 @@ from ..const import ( CONF_STATE_TOPIC, ) from ..debug_info import log_messages -from ..mixins import MQTT_ENTITY_COMMON_SCHEMA, MqttEntity +from ..mixins import MQTT_ENTITY_COMMON_SCHEMA, MqttEntity, write_state_on_attr_change from ..models import ReceiveMessage -from ..util import get_mqtt_data, valid_publish_topic +from ..util import valid_publish_topic from .const import MQTT_VACUUM_ATTRIBUTES_BLOCKED from .schema import MQTT_VACUUM_SCHEMA, services_to_strings, strings_to_services @@ -231,6 +231,9 @@ class MqttStateVacuum(MqttEntity, StateVacuumEntity): @callback @log_messages(self.hass, self.entity_id) + @write_state_on_attr_change( + self, {"_attr_battery_level", "_attr_fan_speed", "_attr_state"} + ) def state_message_received(msg: ReceiveMessage) -> None: """Handle state MQTT message.""" payload = json_loads_object(msg.payload) @@ -242,7 +245,6 @@ class MqttStateVacuum(MqttEntity, StateVacuumEntity): ) del payload[STATE] self._update_state_attributes(payload) - get_mqtt_data(self.hass).state_write_requests.write_state_request(self) if state_topic := self._config.get(CONF_STATE_TOPIC): topics["state_position_topic"] = { diff --git a/tests/components/mqtt/test_legacy_vacuum.py b/tests/components/mqtt/test_legacy_vacuum.py index 85e3bdd12b9..c7d17ed47a0 100644 --- a/tests/components/mqtt/test_legacy_vacuum.py +++ b/tests/components/mqtt/test_legacy_vacuum.py @@ -63,6 +63,7 @@ from .test_common import ( help_test_setting_attribute_via_mqtt_json_message, help_test_setting_attribute_with_template, help_test_setting_blocked_attribute_via_mqtt_json_message, + help_test_skipped_async_ha_write_state, help_test_unique_id, help_test_update_with_json_attrs_bad_json, help_test_update_with_json_attrs_not_dict, @@ -1099,3 +1100,43 @@ async def test_setup_manual_entity_from_yaml( await mqtt_mock_entry() platform = vacuum.DOMAIN assert hass.states.get(f"{platform}.mqtttest") + + +@pytest.mark.parametrize( + "hass_config", + [ + help_custom_config( + vacuum.DOMAIN, + DEFAULT_CONFIG, + ( + { + "availability_topic": "availability-topic", + "json_attributes_topic": "json-attributes-topic", + }, + ), + ) + ], +) +@pytest.mark.parametrize( + ("topic", "payload1", "payload2"), + [ + ("availability-topic", "online", "offline"), + ("json-attributes-topic", '{"attr1": "val1"}', '{"attr1": "val2"}'), + ("vacuum/state", '{"battery_level": 71}', '{"battery_level": 60}'), + ("vacuum/state", '{"docked": true}', '{"docked": false}'), + ("vacuum/state", '{"cleaning": true}', '{"cleaning": false}'), + ("vacuum/state", '{"fan_speed": "max"}', '{"fan_speed": "min"}'), + ("vacuum/state", '{"error": "some error"}', '{"error": "other error"}'), + ("vacuum/state", '{"charging": true}', '{"charging": false}'), + ], +) +async def test_skipped_async_ha_write_state( + hass: HomeAssistant, + mqtt_mock_entry: MqttMockHAClientGenerator, + topic: str, + payload1: str, + payload2: str, +) -> None: + """Test a write state command is only called when there is change.""" + await mqtt_mock_entry() + await help_test_skipped_async_ha_write_state(hass, topic, payload1, payload2) diff --git a/tests/components/mqtt/test_state_vacuum.py b/tests/components/mqtt/test_state_vacuum.py index a24884941fc..40bd5158280 100644 --- a/tests/components/mqtt/test_state_vacuum.py +++ b/tests/components/mqtt/test_state_vacuum.py @@ -58,6 +58,7 @@ from .test_common import ( help_test_setting_attribute_via_mqtt_json_message, help_test_setting_attribute_with_template, help_test_setting_blocked_attribute_via_mqtt_json_message, + help_test_skipped_async_ha_write_state, help_test_unique_id, help_test_update_with_json_attrs_bad_json, help_test_update_with_json_attrs_not_dict, @@ -821,3 +822,40 @@ async def test_setup_manual_entity_from_yaml( await mqtt_mock_entry() platform = vacuum.DOMAIN assert hass.states.get(f"{platform}.mqtttest") + + +@pytest.mark.parametrize( + "hass_config", + [ + help_custom_config( + vacuum.DOMAIN, + DEFAULT_CONFIG, + ( + { + "availability_topic": "availability-topic", + "json_attributes_topic": "json-attributes-topic", + }, + ), + ) + ], +) +@pytest.mark.parametrize( + ("topic", "payload1", "payload2"), + [ + ("availability-topic", "online", "offline"), + ("json-attributes-topic", '{"attr1": "val1"}', '{"attr1": "val2"}'), + ("vacuum/state", '{"state": "cleaning"}', '{"state": "docked"}'), + ("vacuum/state", '{"battery_level": 71}', '{"battery_level": 60}'), + ("vacuum/state", '{"fan_speed": "max"}', '{"fan_speed": "min"}'), + ], +) +async def test_skipped_async_ha_write_state( + hass: HomeAssistant, + mqtt_mock_entry: MqttMockHAClientGenerator, + topic: str, + payload1: str, + payload2: str, +) -> None: + """Test a write state command is only called when there is change.""" + await mqtt_mock_entry() + await help_test_skipped_async_ha_write_state(hass, topic, payload1, payload2)