From 3ae9ea3f19fe087982ed004eeef65acbd8bd3ddb Mon Sep 17 00:00:00 2001 From: Erik Montnemery Date: Mon, 14 Jul 2025 20:18:21 +0200 Subject: [PATCH] Do not add generic_thermostat config entry to source device (#148728) --- .../components/generic_thermostat/__init__.py | 50 +++- .../components/generic_thermostat/climate.py | 39 +-- .../generic_thermostat/config_flow.py | 2 + .../generic_thermostat/test_init.py | 265 +++++++++++++++--- 4 files changed, 292 insertions(+), 64 deletions(-) diff --git a/homeassistant/components/generic_thermostat/__init__.py b/homeassistant/components/generic_thermostat/__init__.py index 3e2af8598de..98cd9a02baa 100644 --- a/homeassistant/components/generic_thermostat/__init__.py +++ b/homeassistant/components/generic_thermostat/__init__.py @@ -1,5 +1,7 @@ """The generic_thermostat component.""" +import logging + from homeassistant.config_entries import ConfigEntry from homeassistant.core import Event, HomeAssistant from homeassistant.helpers import entity_registry as er @@ -8,14 +10,20 @@ from homeassistant.helpers.device import ( async_remove_stale_devices_links_keep_entity_device, ) from homeassistant.helpers.event import async_track_entity_registry_updated_event -from homeassistant.helpers.helper_integration import async_handle_source_entity_changes +from homeassistant.helpers.helper_integration import ( + async_handle_source_entity_changes, + async_remove_helper_config_entry_from_source_device, +) from .const import CONF_HEATER, CONF_SENSOR, PLATFORMS +_LOGGER = logging.getLogger(__name__) + async def async_setup_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool: """Set up from a config entry.""" + # This can be removed in HA Core 2026.2 async_remove_stale_devices_links_keep_entity_device( hass, entry.entry_id, @@ -28,23 +36,19 @@ async def async_setup_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool: options={**entry.options, CONF_HEATER: source_entity_id}, ) - async def source_entity_removed() -> None: - # The source entity has been removed, we need to clean the device links. - async_remove_stale_devices_links_keep_entity_device(hass, entry.entry_id, None) - entry.async_on_unload( # We use async_handle_source_entity_changes to track changes to the heater, but # not the temperature sensor because the generic_hygrostat adds itself to the # heater's device. async_handle_source_entity_changes( hass, + add_helper_config_entry_to_device=False, helper_config_entry_id=entry.entry_id, set_source_entity_id_or_uuid=set_humidifier_entity_id_or_uuid, source_device_id=async_entity_id_to_device_id( hass, entry.options[CONF_HEATER] ), source_entity_id_or_uuid=entry.options[CONF_HEATER], - source_entity_removed=source_entity_removed, ) ) @@ -75,6 +79,40 @@ async def async_setup_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool: return True +async def async_migrate_entry(hass: HomeAssistant, config_entry: ConfigEntry) -> bool: + """Migrate old entry.""" + _LOGGER.debug( + "Migrating from version %s.%s", config_entry.version, config_entry.minor_version + ) + + if config_entry.version > 1: + # This means the user has downgraded from a future version + return False + if config_entry.version == 1: + options = {**config_entry.options} + if config_entry.minor_version < 2: + # Remove the generic_thermostat config entry from the source device + if source_device_id := async_entity_id_to_device_id( + hass, options[CONF_HEATER] + ): + async_remove_helper_config_entry_from_source_device( + hass, + helper_config_entry_id=config_entry.entry_id, + source_device_id=source_device_id, + ) + hass.config_entries.async_update_entry( + config_entry, options=options, minor_version=2 + ) + + _LOGGER.debug( + "Migration to version %s.%s successful", + config_entry.version, + config_entry.minor_version, + ) + + return True + + async def config_entry_update_listener(hass: HomeAssistant, entry: ConfigEntry) -> None: """Update listener, called when the config entry options are changed.""" await hass.config_entries.async_reload(entry.entry_id) diff --git a/homeassistant/components/generic_thermostat/climate.py b/homeassistant/components/generic_thermostat/climate.py index 185040f02c9..76fcc4acdde 100644 --- a/homeassistant/components/generic_thermostat/climate.py +++ b/homeassistant/components/generic_thermostat/climate.py @@ -48,7 +48,7 @@ from homeassistant.core import ( ) from homeassistant.exceptions import ConditionError from homeassistant.helpers import condition, config_validation as cv -from homeassistant.helpers.device import async_device_info_to_link_from_entity +from homeassistant.helpers.device import async_entity_id_to_device from homeassistant.helpers.entity_platform import ( AddConfigEntryEntitiesCallback, AddEntitiesCallback, @@ -182,23 +182,23 @@ async def _async_setup_config( [ GenericThermostat( hass, - name, - heater_entity_id, - sensor_entity_id, - min_temp, - max_temp, - target_temp, - ac_mode, - min_cycle_duration, - cold_tolerance, - hot_tolerance, - keep_alive, - initial_hvac_mode, - presets, - precision, - target_temperature_step, - unit, - unique_id, + name=name, + heater_entity_id=heater_entity_id, + sensor_entity_id=sensor_entity_id, + min_temp=min_temp, + max_temp=max_temp, + target_temp=target_temp, + ac_mode=ac_mode, + min_cycle_duration=min_cycle_duration, + cold_tolerance=cold_tolerance, + hot_tolerance=hot_tolerance, + keep_alive=keep_alive, + initial_hvac_mode=initial_hvac_mode, + presets=presets, + precision=precision, + target_temperature_step=target_temperature_step, + unit=unit, + unique_id=unique_id, ) ] ) @@ -212,6 +212,7 @@ class GenericThermostat(ClimateEntity, RestoreEntity): def __init__( self, hass: HomeAssistant, + *, name: str, heater_entity_id: str, sensor_entity_id: str, @@ -234,7 +235,7 @@ class GenericThermostat(ClimateEntity, RestoreEntity): self._attr_name = name self.heater_entity_id = heater_entity_id self.sensor_entity_id = sensor_entity_id - self._attr_device_info = async_device_info_to_link_from_entity( + self.device_entry = async_entity_id_to_device( hass, heater_entity_id, ) diff --git a/homeassistant/components/generic_thermostat/config_flow.py b/homeassistant/components/generic_thermostat/config_flow.py index 1fbeaefde6b..b69106597d1 100644 --- a/homeassistant/components/generic_thermostat/config_flow.py +++ b/homeassistant/components/generic_thermostat/config_flow.py @@ -100,6 +100,8 @@ OPTIONS_FLOW = { class ConfigFlowHandler(SchemaConfigFlowHandler, domain=DOMAIN): """Handle a config or options flow.""" + MINOR_VERSION = 2 + config_flow = CONFIG_FLOW options_flow = OPTIONS_FLOW diff --git a/tests/components/generic_thermostat/test_init.py b/tests/components/generic_thermostat/test_init.py index 9131e3ffdd4..ceca7ecc444 100644 --- a/tests/components/generic_thermostat/test_init.py +++ b/tests/components/generic_thermostat/test_init.py @@ -9,8 +9,8 @@ import pytest from homeassistant.components import generic_thermostat from homeassistant.components.generic_thermostat.config_flow import ConfigFlowHandler from homeassistant.components.generic_thermostat.const import DOMAIN -from homeassistant.config_entries import ConfigEntry -from homeassistant.core import Event, HomeAssistant +from homeassistant.config_entries import ConfigEntry, ConfigEntryState +from homeassistant.core import Event, HomeAssistant, callback from homeassistant.helpers import device_registry as dr, entity_registry as er from homeassistant.helpers.event import async_track_entity_registry_updated_event @@ -117,10 +117,20 @@ def generic_thermostat_config_entry( return config_entry +@pytest.fixture +def expected_helper_device_id( + request: pytest.FixtureRequest, + switch_device: dr.DeviceEntry, +) -> str | None: + """Fixture to provide the expected helper device ID.""" + return switch_device.id if request.param == "switch_device_id" else None + + def track_entity_registry_actions(hass: HomeAssistant, entity_id: str) -> list[str]: """Track entity registry actions for an entity.""" events = [] + @callback def add_event(event: Event[er.EventEntityRegistryUpdatedData]) -> None: """Add entity registry updated event to the list.""" events.append(event.data["action"]) @@ -199,7 +209,7 @@ async def test_device_cleaning( devices_before_reload = device_registry.devices.get_devices_for_config_entry_id( helper_config_entry.entry_id ) - assert len(devices_before_reload) == 3 + assert len(devices_before_reload) == 2 # Config entry reload await hass.config_entries.async_reload(helper_config_entry.entry_id) @@ -214,9 +224,7 @@ async def test_device_cleaning( devices_after_reload = device_registry.devices.get_devices_for_config_entry_id( helper_config_entry.entry_id ) - assert len(devices_after_reload) == 1 - - assert devices_after_reload[0].id == source_device1_entry.id + assert len(devices_after_reload) == 0 @pytest.mark.usefixtures( @@ -227,8 +235,12 @@ async def test_device_cleaning( "switch_device", ) @pytest.mark.parametrize( - ("source_entity_id", "helper_in_device", "expected_events"), - [("switch.test_unique", True, ["update"]), ("sensor.test_unique", False, [])], + ("source_entity_id", "expected_helper_device_id", "expected_events"), + [ + ("switch.test_unique", None, ["update"]), + ("sensor.test_unique", "switch_device_id", []), + ], + indirect=["expected_helper_device_id"], ) async def test_async_handle_source_entity_changes_source_entity_removed( hass: HomeAssistant, @@ -237,7 +249,84 @@ async def test_async_handle_source_entity_changes_source_entity_removed( generic_thermostat_config_entry: MockConfigEntry, switch_entity_entry: er.RegistryEntry, source_entity_id: str, - helper_in_device: bool, + expected_helper_device_id: str | None, + expected_events: list[str], +) -> None: + """Test the generic_thermostat config entry is removed when the source entity is removed.""" + source_entity_entry = entity_registry.async_get(source_entity_id) + + assert await hass.config_entries.async_setup( + generic_thermostat_config_entry.entry_id + ) + await hass.async_block_till_done() + + generic_thermostat_entity_entry = entity_registry.async_get( + "climate.my_generic_thermostat" + ) + assert generic_thermostat_entity_entry.device_id == switch_entity_entry.device_id + + source_device = device_registry.async_get(source_entity_entry.device_id) + assert generic_thermostat_config_entry.entry_id not in source_device.config_entries + + events = track_entity_registry_actions( + hass, generic_thermostat_entity_entry.entity_id + ) + + # Remove the source entity's config entry from the device, this removes the + # source entity + with patch( + "homeassistant.components.generic_thermostat.async_unload_entry", + wraps=generic_thermostat.async_unload_entry, + ) as mock_unload_entry: + device_registry.async_update_device( + source_device.id, remove_config_entry_id=source_entity_entry.config_entry_id + ) + await hass.async_block_till_done() + await hass.async_block_till_done() + mock_unload_entry.assert_not_called() + + # Check that the helper entity is linked to the expected source device + generic_thermostat_entity_entry = entity_registry.async_get( + "climate.my_generic_thermostat" + ) + assert generic_thermostat_entity_entry.device_id == expected_helper_device_id + + # Check that the device is removed + assert not device_registry.async_get(source_device.id) + + # Check that the generic_thermostat config entry is not removed + assert ( + generic_thermostat_config_entry.entry_id + in hass.config_entries.async_entry_ids() + ) + + # Check we got the expected events + assert events == expected_events + + +@pytest.mark.usefixtures( + "sensor_config_entry", + "sensor_device", + "sensor_entity_entry", + "switch_config_entry", + "switch_device", +) +@pytest.mark.parametrize( + ("source_entity_id", "expected_helper_device_id", "expected_events"), + [ + ("switch.test_unique", None, ["update"]), + ("sensor.test_unique", "switch_device_id", []), + ], + indirect=["expected_helper_device_id"], +) +async def test_async_handle_source_entity_changes_source_entity_removed_shared_device( + hass: HomeAssistant, + device_registry: dr.DeviceRegistry, + entity_registry: er.EntityRegistry, + generic_thermostat_config_entry: MockConfigEntry, + switch_entity_entry: er.RegistryEntry, + source_entity_id: str, + expected_helper_device_id: str | None, expected_events: list[str], ) -> None: """Test the generic_thermostat config entry is removed when the source entity is removed.""" @@ -261,9 +350,7 @@ async def test_async_handle_source_entity_changes_source_entity_removed( assert generic_thermostat_entity_entry.device_id == switch_entity_entry.device_id source_device = device_registry.async_get(source_entity_entry.device_id) - assert ( - generic_thermostat_config_entry.entry_id in source_device.config_entries - ) == helper_in_device + assert generic_thermostat_config_entry.entry_id not in source_device.config_entries events = track_entity_registry_actions( hass, generic_thermostat_entity_entry.entity_id @@ -282,6 +369,13 @@ async def test_async_handle_source_entity_changes_source_entity_removed( await hass.async_block_till_done() mock_unload_entry.assert_not_called() + # Check that the helper entity is linked to the expected source device + switch_entity_entry = entity_registry.async_get("switch.test_unique") + generic_thermostat_entity_entry = entity_registry.async_get( + "climate.my_generic_thermostat" + ) + assert generic_thermostat_entity_entry.device_id == expected_helper_device_id + # Check if the generic_thermostat config entry is not in the device source_device = device_registry.async_get(source_device.id) assert generic_thermostat_config_entry.entry_id not in source_device.config_entries @@ -304,8 +398,17 @@ async def test_async_handle_source_entity_changes_source_entity_removed( "switch_device", ) @pytest.mark.parametrize( - ("source_entity_id", "helper_in_device", "unload_entry_calls", "expected_events"), - [("switch.test_unique", True, 1, ["update"]), ("sensor.test_unique", False, 0, [])], + ( + "source_entity_id", + "unload_entry_calls", + "expected_helper_device_id", + "expected_events", + ), + [ + ("switch.test_unique", 1, None, ["update"]), + ("sensor.test_unique", 0, "switch_device_id", []), + ], + indirect=["expected_helper_device_id"], ) async def test_async_handle_source_entity_changes_source_entity_removed_from_device( hass: HomeAssistant, @@ -314,8 +417,8 @@ async def test_async_handle_source_entity_changes_source_entity_removed_from_dev generic_thermostat_config_entry: MockConfigEntry, switch_entity_entry: er.RegistryEntry, source_entity_id: str, - helper_in_device: bool, unload_entry_calls: int, + expected_helper_device_id: str | None, expected_events: list[str], ) -> None: """Test the source entity removed from the source device.""" @@ -332,9 +435,7 @@ async def test_async_handle_source_entity_changes_source_entity_removed_from_dev assert generic_thermostat_entity_entry.device_id == switch_entity_entry.device_id source_device = device_registry.async_get(source_entity_entry.device_id) - assert ( - generic_thermostat_config_entry.entry_id in source_device.config_entries - ) == helper_in_device + assert generic_thermostat_config_entry.entry_id not in source_device.config_entries events = track_entity_registry_actions( hass, generic_thermostat_entity_entry.entity_id @@ -351,7 +452,13 @@ async def test_async_handle_source_entity_changes_source_entity_removed_from_dev await hass.async_block_till_done() assert len(mock_unload_entry.mock_calls) == unload_entry_calls - # Check that the generic_thermostat config entry is removed from the device + # Check that the helper entity is linked to the expected source device + generic_thermostat_entity_entry = entity_registry.async_get( + "climate.my_generic_thermostat" + ) + assert generic_thermostat_entity_entry.device_id == expected_helper_device_id + + # Check that the generic_thermostat config entry is not in the device source_device = device_registry.async_get(source_device.id) assert generic_thermostat_config_entry.entry_id not in source_device.config_entries @@ -373,8 +480,8 @@ async def test_async_handle_source_entity_changes_source_entity_removed_from_dev "switch_device", ) @pytest.mark.parametrize( - ("source_entity_id", "helper_in_device", "unload_entry_calls", "expected_events"), - [("switch.test_unique", True, 1, ["update"]), ("sensor.test_unique", False, 0, [])], + ("source_entity_id", "unload_entry_calls", "expected_events"), + [("switch.test_unique", 1, ["update"]), ("sensor.test_unique", 0, [])], ) async def test_async_handle_source_entity_changes_source_entity_moved_other_device( hass: HomeAssistant, @@ -383,7 +490,6 @@ async def test_async_handle_source_entity_changes_source_entity_moved_other_devi generic_thermostat_config_entry: MockConfigEntry, switch_entity_entry: er.RegistryEntry, source_entity_id: str, - helper_in_device: bool, unload_entry_calls: int, expected_events: list[str], ) -> None: @@ -406,9 +512,7 @@ async def test_async_handle_source_entity_changes_source_entity_moved_other_devi assert generic_thermostat_entity_entry.device_id == switch_entity_entry.device_id source_device = device_registry.async_get(source_entity_entry.device_id) - assert ( - generic_thermostat_config_entry.entry_id in source_device.config_entries - ) == helper_in_device + assert generic_thermostat_config_entry.entry_id not in source_device.config_entries source_device_2 = device_registry.async_get(source_device_2.id) assert ( generic_thermostat_config_entry.entry_id not in source_device_2.config_entries @@ -429,13 +533,20 @@ async def test_async_handle_source_entity_changes_source_entity_moved_other_devi await hass.async_block_till_done() assert len(mock_unload_entry.mock_calls) == unload_entry_calls - # Check that the generic_thermostat config entry is moved to the other device + # Check that the helper entity is linked to the expected source device + switch_entity_entry = entity_registry.async_get(switch_entity_entry.entity_id) + generic_thermostat_entity_entry = entity_registry.async_get( + "climate.my_generic_thermostat" + ) + assert generic_thermostat_entity_entry.device_id == switch_entity_entry.device_id + + # Check that the generic_thermostat config entry is not in any of the devices source_device = device_registry.async_get(source_device.id) assert generic_thermostat_config_entry.entry_id not in source_device.config_entries source_device_2 = device_registry.async_get(source_device_2.id) assert ( - generic_thermostat_config_entry.entry_id in source_device_2.config_entries - ) == helper_in_device + generic_thermostat_config_entry.entry_id not in source_device_2.config_entries + ) # Check that the generic_thermostat config entry is not removed assert ( @@ -455,10 +566,10 @@ async def test_async_handle_source_entity_changes_source_entity_moved_other_devi "switch_device", ) @pytest.mark.parametrize( - ("source_entity_id", "new_entity_id", "helper_in_device", "config_key"), + ("source_entity_id", "new_entity_id", "config_key"), [ - ("switch.test_unique", "switch.new_entity_id", True, "heater"), - ("sensor.test_unique", "sensor.new_entity_id", False, "target_sensor"), + ("switch.test_unique", "switch.new_entity_id", "heater"), + ("sensor.test_unique", "sensor.new_entity_id", "target_sensor"), ], ) async def test_async_handle_source_entity_new_entity_id( @@ -469,7 +580,6 @@ async def test_async_handle_source_entity_new_entity_id( switch_entity_entry: er.RegistryEntry, source_entity_id: str, new_entity_id: str, - helper_in_device: bool, config_key: str, ) -> None: """Test the source entity's entity ID is changed.""" @@ -486,9 +596,7 @@ async def test_async_handle_source_entity_new_entity_id( assert generic_thermostat_entity_entry.device_id == switch_entity_entry.device_id source_device = device_registry.async_get(source_entity_entry.device_id) - assert ( - generic_thermostat_config_entry.entry_id in source_device.config_entries - ) == helper_in_device + assert generic_thermostat_config_entry.entry_id not in source_device.config_entries events = track_entity_registry_actions( hass, generic_thermostat_entity_entry.entity_id @@ -508,11 +616,9 @@ async def test_async_handle_source_entity_new_entity_id( # Check that the generic_thermostat config entry is updated with the new entity ID assert generic_thermostat_config_entry.options[config_key] == new_entity_id - # Check that the helper config is still in the device + # Check that the helper config is not in the device source_device = device_registry.async_get(source_device.id) - assert ( - generic_thermostat_config_entry.entry_id in source_device.config_entries - ) == helper_in_device + assert generic_thermostat_config_entry.entry_id not in source_device.config_entries # Check that the generic_thermostat config entry is not removed assert ( @@ -522,3 +628,84 @@ async def test_async_handle_source_entity_new_entity_id( # Check we got the expected events assert events == [] + + +@pytest.mark.usefixtures("sensor_device") +async def test_migration_1_1( + hass: HomeAssistant, + device_registry: dr.DeviceRegistry, + entity_registry: er.EntityRegistry, + sensor_entity_entry: er.RegistryEntry, + switch_device: dr.DeviceEntry, + switch_entity_entry: er.RegistryEntry, +) -> None: + """Test migration from v1.1 removes generic_thermostat config entry from device.""" + + generic_thermostat_config_entry = MockConfigEntry( + data={}, + domain=DOMAIN, + options={ + "name": "My generic thermostat", + "heater": switch_entity_entry.entity_id, + "target_sensor": sensor_entity_entry.entity_id, + "ac_mode": False, + "cold_tolerance": 0.3, + "hot_tolerance": 0.3, + }, + title="My generic thermostat", + version=1, + minor_version=1, + ) + generic_thermostat_config_entry.add_to_hass(hass) + + # Add the helper config entry to the device + device_registry.async_update_device( + switch_device.id, add_config_entry_id=generic_thermostat_config_entry.entry_id + ) + + # Check preconditions + switch_device = device_registry.async_get(switch_device.id) + assert generic_thermostat_config_entry.entry_id in switch_device.config_entries + + await hass.config_entries.async_setup(generic_thermostat_config_entry.entry_id) + await hass.async_block_till_done() + + assert generic_thermostat_config_entry.state is ConfigEntryState.LOADED + + # Check that the helper config entry is removed from the device and the helper + # entity is linked to the source device + switch_device = device_registry.async_get(switch_device.id) + assert generic_thermostat_config_entry.entry_id not in switch_device.config_entries + generic_thermostat_entity_entry = entity_registry.async_get( + "climate.my_generic_thermostat" + ) + assert generic_thermostat_entity_entry.device_id == switch_entity_entry.device_id + + assert generic_thermostat_config_entry.version == 1 + assert generic_thermostat_config_entry.minor_version == 2 + + +async def test_migration_from_future_version( + hass: HomeAssistant, +) -> None: + """Test migration from future version.""" + config_entry = MockConfigEntry( + data={}, + domain=DOMAIN, + options={ + "name": "My generic thermostat", + "heater": "switch.test", + "target_sensor": "sensor.test", + "ac_mode": False, + "cold_tolerance": 0.3, + "hot_tolerance": 0.3, + }, + title="My generic thermostat", + version=2, + minor_version=1, + ) + config_entry.add_to_hass(hass) + await hass.config_entries.async_setup(config_entry.entry_id) + await hass.async_block_till_done() + + assert config_entry.state is ConfigEntryState.MIGRATION_ERROR