From 488dae43d42cfb3145979ca6815b9fda6e751b54 Mon Sep 17 00:00:00 2001 From: Jan Bouwhuis Date: Wed, 13 Mar 2024 11:04:59 +0100 Subject: [PATCH] Improve lists for MQTT integration (#113184) * Improve lists for MQTT integration * Extra diagnostics tests * Revert changes where the original version was probably faster * Revert change to gather and await in series --- homeassistant/components/mqtt/client.py | 8 ++- homeassistant/components/mqtt/climate.py | 24 ++++---- homeassistant/components/mqtt/debug_info.py | 31 +++++----- .../components/mqtt/device_trigger.py | 16 ++---- homeassistant/components/mqtt/diagnostics.py | 56 ++++++++++--------- homeassistant/components/mqtt/discovery.py | 19 ++++--- homeassistant/components/mqtt/fan.py | 19 +++---- homeassistant/components/mqtt/humidifier.py | 16 +++--- homeassistant/components/mqtt/siren.py | 16 ++++-- homeassistant/components/mqtt/water_heater.py | 27 ++++----- tests/components/mqtt/test_diagnostics.py | 28 +++++++++- 11 files changed, 144 insertions(+), 116 deletions(-) diff --git a/homeassistant/components/mqtt/client.py b/homeassistant/components/mqtt/client.py index 1cded579c53..b2fab355c41 100644 --- a/homeassistant/components/mqtt/client.py +++ b/homeassistant/components/mqtt/client.py @@ -812,9 +812,11 @@ class MQTT: subscriptions: list[Subscription] = [] if topic in self._simple_subscriptions: subscriptions.extend(self._simple_subscriptions[topic]) - for subscription in self._wildcard_subscriptions: - if subscription.matcher(topic): - subscriptions.append(subscription) + subscriptions.extend( + subscription + for subscription in self._wildcard_subscriptions + if subscription.matcher(topic) + ) return subscriptions @callback diff --git a/homeassistant/components/mqtt/climate.py b/homeassistant/components/mqtt/climate.py index 2deffaea042..567fe2540fd 100644 --- a/homeassistant/components/mqtt/climate.py +++ b/homeassistant/components/mqtt/climate.py @@ -661,15 +661,12 @@ class MqttClimate(MqttTemperatureControlEntity, ClimateEntity): self._optimistic or CONF_PRESET_MODE_STATE_TOPIC not in config ) - value_templates: dict[str, Template | None] = {} - for key in VALUE_TEMPLATE_KEYS: - value_templates[key] = None - if CONF_VALUE_TEMPLATE in config: - value_templates = { - key: config.get(CONF_VALUE_TEMPLATE) for key in VALUE_TEMPLATE_KEYS - } - for key in VALUE_TEMPLATE_KEYS & config.keys(): - value_templates[key] = config[key] + value_templates: dict[str, Template | None] = { + key: config.get(CONF_VALUE_TEMPLATE) for key in VALUE_TEMPLATE_KEYS + } + value_templates.update( + {key: config[key] for key in VALUE_TEMPLATE_KEYS & config.keys()} + ) self._value_templates = { key: MqttValueTemplate( template, @@ -678,11 +675,10 @@ class MqttClimate(MqttTemperatureControlEntity, ClimateEntity): for key, template in value_templates.items() } - self._command_templates = {} - for key in COMMAND_TEMPLATE_KEYS: - self._command_templates[key] = MqttCommandTemplate( - config.get(key), entity=self - ).async_render + self._command_templates = { + key: MqttCommandTemplate(config.get(key), entity=self).async_render + for key in COMMAND_TEMPLATE_KEYS + } support = ClimateEntityFeature.TURN_ON | ClimateEntityFeature.TURN_OFF if (self._topic[CONF_TEMP_STATE_TOPIC] is not None) or ( diff --git a/homeassistant/components/mqtt/debug_info.py b/homeassistant/components/mqtt/debug_info.py index a3841c27fa3..7ff93a6bd06 100644 --- a/homeassistant/components/mqtt/debug_info.py +++ b/homeassistant/components/mqtt/debug_info.py @@ -239,11 +239,14 @@ def info_for_config_entry(hass: HomeAssistant) -> dict[str, list[Any]]: mqtt_data = get_mqtt_data(hass) mqtt_info: dict[str, list[Any]] = {"entities": [], "triggers": []} - for entity_id in mqtt_data.debug_info_entities: - mqtt_info["entities"].append(_info_for_entity(hass, entity_id)) + mqtt_info["entities"].extend( + _info_for_entity(hass, entity_id) for entity_id in mqtt_data.debug_info_entities + ) - for trigger_key in mqtt_data.debug_info_triggers: - mqtt_info["triggers"].append(_info_for_trigger(hass, trigger_key)) + mqtt_info["triggers"].extend( + _info_for_trigger(hass, trigger_key) + for trigger_key in mqtt_data.debug_info_triggers + ) return mqtt_info @@ -259,16 +262,16 @@ def info_for_device(hass: HomeAssistant, device_id: str) -> dict[str, list[Any]] entries = er.async_entries_for_device( entity_registry, device_id, include_disabled_entities=True ) - for entry in entries: - if entry.entity_id not in mqtt_data.debug_info_entities: - continue + mqtt_info["entities"].extend( + _info_for_entity(hass, entry.entity_id) + for entry in entries + if entry.entity_id in mqtt_data.debug_info_entities + ) - mqtt_info["entities"].append(_info_for_entity(hass, entry.entity_id)) - - for trigger_key, trigger in mqtt_data.debug_info_triggers.items(): - if trigger["device_id"] != device_id: - continue - - mqtt_info["triggers"].append(_info_for_trigger(hass, trigger_key)) + mqtt_info["triggers"].extend( + _info_for_trigger(hass, trigger_key) + for trigger_key, trigger in mqtt_data.debug_info_triggers.items() + if trigger["device_id"] == device_id + ) return mqtt_info diff --git a/homeassistant/components/mqtt/device_trigger.py b/homeassistant/components/mqtt/device_trigger.py index 8ca5e460419..db94305f9d7 100644 --- a/homeassistant/components/mqtt/device_trigger.py +++ b/homeassistant/components/mqtt/device_trigger.py @@ -353,24 +353,20 @@ async def async_get_triggers( ) -> list[dict[str, str]]: """List device triggers for MQTT devices.""" mqtt_data = get_mqtt_data(hass) - triggers: list[dict[str, str]] = [] if not mqtt_data.device_triggers: - return triggers + return [] - for trig in mqtt_data.device_triggers.values(): - if trig.device_id != device_id or trig.topic is None: - continue - - trigger = { + return [ + { **MQTT_TRIGGER_BASE, "device_id": device_id, "type": trig.type, "subtype": trig.subtype, } - triggers.append(trigger) - - return triggers + for trig in mqtt_data.device_triggers.values() + if trig.device_id == device_id and trig.topic is not None + ] async def async_attach_trigger( diff --git a/homeassistant/components/mqtt/diagnostics.py b/homeassistant/components/mqtt/diagnostics.py index 8b278c1e8d6..9c0f59fe8c3 100644 --- a/homeassistant/components/mqtt/diagnostics.py +++ b/homeassistant/components/mqtt/diagnostics.py @@ -95,36 +95,40 @@ def _async_device_as_dict(hass: HomeAssistant, device: DeviceEntry) -> dict[str, include_disabled_entities=True, ) - for entity_entry in entities: + def _state_dict(entity_entry: er.RegistryEntry) -> dict[str, Any] | None: state = hass.states.get(entity_entry.entity_id) - state_dict = None - if state: - state_dict = dict(state.as_dict()) + if not state: + return None - # The context doesn't provide useful information in this case. - state_dict.pop("context", None) + state_dict = dict(state.as_dict()) - entity_domain = split_entity_id(state.entity_id)[0] + # The context doesn't provide useful information in this case. + state_dict.pop("context", None) - # Retract some sensitive state attributes - if entity_domain == device_tracker.DOMAIN: - state_dict["attributes"] = async_redact_data( - state_dict["attributes"], REDACT_STATE_DEVICE_TRACKER - ) + entity_domain = split_entity_id(state.entity_id)[0] - data["entities"].append( - { - "device_class": entity_entry.device_class, - "disabled_by": entity_entry.disabled_by, - "disabled": entity_entry.disabled, - "entity_category": entity_entry.entity_category, - "entity_id": entity_entry.entity_id, - "icon": entity_entry.icon, - "original_device_class": entity_entry.original_device_class, - "original_icon": entity_entry.original_icon, - "state": state_dict, - "unit_of_measurement": entity_entry.unit_of_measurement, - } - ) + # Retract some sensitive state attributes + if entity_domain == device_tracker.DOMAIN: + state_dict["attributes"] = async_redact_data( + state_dict["attributes"], REDACT_STATE_DEVICE_TRACKER + ) + return state_dict + + data["entities"].extend( + { + "device_class": entity_entry.device_class, + "disabled_by": entity_entry.disabled_by, + "disabled": entity_entry.disabled, + "entity_category": entity_entry.entity_category, + "entity_id": entity_entry.entity_id, + "icon": entity_entry.icon, + "original_device_class": entity_entry.original_device_class, + "original_icon": entity_entry.original_icon, + "state": state_dict, + "unit_of_measurement": entity_entry.unit_of_measurement, + } + for entity_entry in entities + if (state_dict := _state_dict(entity_entry)) is not None + ) return data diff --git a/homeassistant/components/mqtt/discovery.py b/homeassistant/components/mqtt/discovery.py index 218413d79f5..0e1b0a799d6 100644 --- a/homeassistant/components/mqtt/discovery.py +++ b/homeassistant/components/mqtt/discovery.py @@ -403,14 +403,17 @@ async def async_start( # noqa: C901 ): mqtt_data.integration_unsubscribe.pop(key)() - for topic in topics: - key = f"{integration}_{topic}" - mqtt_data.integration_unsubscribe[key] = await mqtt.async_subscribe( - hass, - topic, - functools.partial(async_integration_message_received, integration), - 0, - ) + mqtt_data.integration_unsubscribe.update( + { + f"{integration}_{topic}": await mqtt.async_subscribe( + hass, + topic, + functools.partial(async_integration_message_received, integration), + 0, + ) + for topic in topics + } + ) async def async_stop(hass: HomeAssistant) -> None: diff --git a/homeassistant/components/mqtt/fan.py b/homeassistant/components/mqtt/fan.py index 848e50e3e2e..0fed4ab666e 100644 --- a/homeassistant/components/mqtt/fan.py +++ b/homeassistant/components/mqtt/fan.py @@ -319,13 +319,11 @@ class MqttFan(MqttEntity, FanEntity): ATTR_PRESET_MODE: config.get(CONF_PRESET_MODE_COMMAND_TEMPLATE), ATTR_OSCILLATING: config.get(CONF_OSCILLATION_COMMAND_TEMPLATE), } - self._command_templates = {} - for key, tpl in command_templates.items(): - self._command_templates[key] = MqttCommandTemplate( - tpl, entity=self - ).async_render + self._command_templates = { + key: MqttCommandTemplate(tpl, entity=self).async_render + for key, tpl in command_templates.items() + } - self._value_templates = {} value_templates: dict[str, Template | None] = { CONF_STATE: config.get(CONF_STATE_VALUE_TEMPLATE), ATTR_DIRECTION: config.get(CONF_DIRECTION_VALUE_TEMPLATE), @@ -333,11 +331,12 @@ class MqttFan(MqttEntity, FanEntity): ATTR_PRESET_MODE: config.get(CONF_PRESET_MODE_VALUE_TEMPLATE), ATTR_OSCILLATING: config.get(CONF_OSCILLATION_VALUE_TEMPLATE), } - for key, tpl in value_templates.items(): - self._value_templates[key] = MqttValueTemplate( - tpl, - entity=self, + self._value_templates = { + key: MqttValueTemplate( + tpl, entity=self ).async_render_with_possible_json_value + for key, tpl in value_templates.items() + } def _prepare_subscribe_topics(self) -> None: """(Re)Subscribe to topics.""" diff --git a/homeassistant/components/mqtt/humidifier.py b/homeassistant/components/mqtt/humidifier.py index 400b8154c8d..f3b9cf4c4ff 100644 --- a/homeassistant/components/mqtt/humidifier.py +++ b/homeassistant/components/mqtt/humidifier.py @@ -254,18 +254,16 @@ class MqttHumidifier(MqttEntity, HumidifierEntity): ) self._optimistic_mode = optimistic or self._topic[CONF_MODE_STATE_TOPIC] is None - self._command_templates = {} command_templates: dict[str, Template | None] = { CONF_STATE: config.get(CONF_COMMAND_TEMPLATE), ATTR_HUMIDITY: config.get(CONF_TARGET_HUMIDITY_COMMAND_TEMPLATE), ATTR_MODE: config.get(CONF_MODE_COMMAND_TEMPLATE), } - for key, tpl in command_templates.items(): - self._command_templates[key] = MqttCommandTemplate( - tpl, entity=self - ).async_render + self._command_templates = { + key: MqttCommandTemplate(tpl, entity=self).async_render + for key, tpl in command_templates.items() + } - self._value_templates = {} value_templates: dict[str, Template | None] = { ATTR_ACTION: config.get(CONF_ACTION_TEMPLATE), ATTR_CURRENT_HUMIDITY: config.get(CONF_CURRENT_HUMIDITY_TEMPLATE), @@ -273,11 +271,13 @@ class MqttHumidifier(MqttEntity, HumidifierEntity): ATTR_HUMIDITY: config.get(CONF_TARGET_HUMIDITY_STATE_TEMPLATE), ATTR_MODE: config.get(CONF_MODE_STATE_TEMPLATE), } - for key, tpl in value_templates.items(): - self._value_templates[key] = MqttValueTemplate( + self._value_templates = { + key: MqttValueTemplate( tpl, entity=self, ).async_render_with_possible_json_value + for key, tpl in value_templates.items() + } def add_subscription( self, diff --git a/homeassistant/components/mqtt/siren.py b/homeassistant/components/mqtt/siren.py index 5eda05d531f..e360416db7c 100644 --- a/homeassistant/components/mqtt/siren.py +++ b/homeassistant/components/mqtt/siren.py @@ -364,9 +364,13 @@ class MqttSiren(MqttEntity, SirenEntity): def _update(self, data: SirenTurnOnServiceParameters) -> None: """Update the extra siren state attributes.""" - for attribute, support in SUPPORTED_ATTRIBUTES.items(): - if self._attr_supported_features & support and attribute in data: - data_attr = data[attribute] # type: ignore[literal-required] - if self._extra_attributes.get(attribute) == data_attr: - continue - self._extra_attributes[attribute] = data_attr + self._extra_attributes.update( + { + attribute: data_attr + for attribute, support in SUPPORTED_ATTRIBUTES.items() + if self._attr_supported_features & support + and attribute in data + and (data_attr := data[attribute]) # type: ignore[literal-required] + != self._extra_attributes.get(attribute) + } + ) diff --git a/homeassistant/components/mqtt/water_heater.py b/homeassistant/components/mqtt/water_heater.py index f8061873c69..09db5fc33e7 100644 --- a/homeassistant/components/mqtt/water_heater.py +++ b/homeassistant/components/mqtt/water_heater.py @@ -226,28 +226,23 @@ class MqttWaterHeater(MqttTemperatureControlEntity, WaterHeaterEntity): if self._topic[CONF_MODE_STATE_TOPIC] is None or self._optimistic: self._attr_current_operation = STATE_OFF - value_templates: dict[str, Template | None] = {} - for key in VALUE_TEMPLATE_KEYS: - value_templates[key] = None - if CONF_VALUE_TEMPLATE in config: - value_templates = { - key: config.get(CONF_VALUE_TEMPLATE) for key in VALUE_TEMPLATE_KEYS - } - for key in VALUE_TEMPLATE_KEYS & config.keys(): - value_templates[key] = config[key] + value_templates: dict[str, Template | None] = { + key: config.get(CONF_VALUE_TEMPLATE) for key in VALUE_TEMPLATE_KEYS + } + value_templates.update( + {key: config[key] for key in VALUE_TEMPLATE_KEYS & config.keys()} + ) self._value_templates = { key: MqttValueTemplate( - template, - entity=self, + template, entity=self ).async_render_with_possible_json_value for key, template in value_templates.items() } - self._command_templates = {} - for key in COMMAND_TEMPLATE_KEYS: - self._command_templates[key] = MqttCommandTemplate( - config.get(key), entity=self - ).async_render + self._command_templates = { + key: MqttCommandTemplate(config.get(key), entity=self).async_render + for key in COMMAND_TEMPLATE_KEYS + } support = WaterHeaterEntityFeature(0) if (self._topic[CONF_TEMP_STATE_TOPIC] is not None) or ( diff --git a/tests/components/mqtt/test_diagnostics.py b/tests/components/mqtt/test_diagnostics.py index 9ca903b88b7..8959ad0f719 100644 --- a/tests/components/mqtt/test_diagnostics.py +++ b/tests/components/mqtt/test_diagnostics.py @@ -7,7 +7,7 @@ import pytest from homeassistant.components import mqtt from homeassistant.core import HomeAssistant -from homeassistant.helpers import device_registry as dr +from homeassistant.helpers import device_registry as dr, entity_registry as er from tests.common import async_fire_mqtt_message from tests.components.diagnostics import ( @@ -25,6 +25,7 @@ default_config = { async def test_entry_diagnostics( hass: HomeAssistant, device_registry: dr.DeviceRegistry, + entity_registry: er.EntityRegistry, hass_client: ClientSessionGenerator, mqtt_mock_entry: MqttMockHAClientGenerator, ) -> None: @@ -260,3 +261,28 @@ async def test_redact_diagnostics( "mqtt_config": expected_config, "mqtt_debug_info": expected_debug_info, } + + # Disable the entity and remove the state + ent_registry = er.async_get(hass) + device_tracker_entry = er.async_entries_for_device(ent_registry, device_entry.id)[0] + ent_registry.async_update_entity( + device_tracker_entry.entity_id, disabled_by=er.RegistryEntryDisabler.USER + ) + hass.states.async_remove(device_tracker_entry.entity_id) + + # Assert disabled entries are filtered + assert await get_diagnostics_for_device( + hass, hass_client, config_entry, device_entry + ) == { + "connected": True, + "device": { + "id": device_entry.id, + "name": None, + "name_by_user": None, + "disabled": False, + "disabled_by": None, + "entities": [], + }, + "mqtt_config": expected_config, + "mqtt_debug_info": {"entities": [], "triggers": []}, + }