From a600a0e023ad66103c5ed68053b11be1a443b845 Mon Sep 17 00:00:00 2001 From: Michael Hansen Date: Fri, 5 Jan 2024 12:45:41 -0600 Subject: [PATCH] Expose all areas to Assist and ignore empty aliases (#107267) * Expose all areas to Assist * Skip empty entity/area aliases --- .../components/conversation/default_agent.py | 26 +++----- .../conversation/test_default_agent.py | 65 +++++++++++++++++-- 2 files changed, 69 insertions(+), 22 deletions(-) diff --git a/homeassistant/components/conversation/default_agent.py b/homeassistant/components/conversation/default_agent.py index f80c7ae7bbf..6af75b2f0a8 100644 --- a/homeassistant/components/conversation/default_agent.py +++ b/homeassistant/components/conversation/default_agent.py @@ -630,14 +630,12 @@ class DefaultAgent(AbstractConversationAgent): if self._slot_lists is not None: return self._slot_lists - area_ids_with_entities: set[str] = set() entity_registry = er.async_get(self.hass) states = [ state for state in self.hass.states.async_all() if async_should_expose(self.hass, DOMAIN, state.entity_id) ] - devices = dr.async_get(self.hass) # Gather exposed entity names entity_names = [] @@ -660,34 +658,26 @@ class DefaultAgent(AbstractConversationAgent): if entity.aliases: for alias in entity.aliases: + if not alias.strip(): + continue + entity_names.append((alias, alias, context)) # Default name entity_names.append((state.name, state.name, context)) - if entity.area_id: - # Expose area too - area_ids_with_entities.add(entity.area_id) - elif entity.device_id: - # Check device for area as well - device = devices.async_get(entity.device_id) - if (device is not None) and device.area_id: - area_ids_with_entities.add(device.area_id) - - # Gather areas from exposed entities + # Expose all areas areas = ar.async_get(self.hass) area_names = [] - for area_id in area_ids_with_entities: - area = areas.async_get_area(area_id) - if area is None: - continue - + for area in areas.async_list_areas(): area_names.append((area.name, area.id)) if area.aliases: for alias in area.aliases: + if not alias.strip(): + continue + area_names.append((alias, area.id)) - _LOGGER.debug("Exposed areas: %s", area_names) _LOGGER.debug("Exposed entities: %s", entity_names) self._slot_lists = { diff --git a/tests/components/conversation/test_default_agent.py b/tests/components/conversation/test_default_agent.py index 4c1d395a2cc..ac126aa7c6b 100644 --- a/tests/components/conversation/test_default_agent.py +++ b/tests/components/conversation/test_default_agent.py @@ -83,7 +83,7 @@ async def test_exposed_areas( device_registry: dr.DeviceRegistry, entity_registry: er.EntityRegistry, ) -> None: - """Test that only expose areas with an exposed entity/device.""" + """Test that all areas are exposed.""" area_kitchen = area_registry.async_get_or_create("kitchen") area_bedroom = area_registry.async_get_or_create("bedroom") @@ -122,14 +122,20 @@ async def test_exposed_areas( # All is well for the exposed kitchen light assert result.response.response_type == intent.IntentResponseType.ACTION_DONE - # Bedroom is not exposed because it has no exposed entities + # Bedroom has no exposed entities result = await conversation.async_converse( hass, "turn on lights in the bedroom", None, Context(), None ) - # This should be a match failure because the area isn't in the slot list + # This should be an error because the lights in that area are not exposed assert result.response.response_type == intent.IntentResponseType.ERROR - assert result.response.error_code == intent.IntentResponseErrorCode.NO_VALID_TARGETS + assert result.response.error_code == intent.IntentResponseErrorCode.FAILED_TO_HANDLE + + # But we can still ask questions about the bedroom, even with no exposed entities + result = await conversation.async_converse( + hass, "how many lights are on in the bedroom?", None, Context(), None + ) + assert result.response.response_type == intent.IntentResponseType.QUERY_ANSWER async def test_conversation_agent( @@ -463,3 +469,54 @@ async def test_error_match_failure(hass: HomeAssistant, init_components) -> None assert ( result.response.error_code == intent.IntentResponseErrorCode.NO_INTENT_MATCH ) + + +async def test_empty_aliases( + hass: HomeAssistant, + init_components, + area_registry: ar.AreaRegistry, + device_registry: dr.DeviceRegistry, + entity_registry: er.EntityRegistry, +) -> None: + """Test that empty aliases are not added to slot lists.""" + area_kitchen = area_registry.async_get_or_create("kitchen") + assert area_kitchen.id is not None + area_registry.async_update(area_kitchen.id, aliases={" "}) + + entry = MockConfigEntry() + entry.add_to_hass(hass) + kitchen_device = device_registry.async_get_or_create( + config_entry_id=entry.entry_id, + connections=set(), + identifiers={("demo", "id-1234")}, + ) + device_registry.async_update_device(kitchen_device.id, area_id=area_kitchen.id) + + kitchen_light = entity_registry.async_get_or_create("light", "demo", "1234") + entity_registry.async_update_entity( + kitchen_light.entity_id, device_id=kitchen_device.id, aliases={" "} + ) + hass.states.async_set( + kitchen_light.entity_id, "on", attributes={ATTR_FRIENDLY_NAME: "kitchen light"} + ) + + with patch( + "homeassistant.components.conversation.DefaultAgent._recognize", + return_value=None, + ) as mock_recognize_all: + await conversation.async_converse( + hass, "turn on lights in the kitchen", None, Context(), None + ) + + assert mock_recognize_all.call_count > 0 + slot_lists = mock_recognize_all.call_args[0][2] + + # Slot lists should only contain non-empty text + assert slot_lists.keys() == {"area", "name"} + areas = slot_lists["area"] + assert len(areas.values) == 1 + assert areas.values[0].value_out == "kitchen" + + names = slot_lists["name"] + assert len(names.values) == 1 + assert names.values[0].value_out == "kitchen light"