Matching duplicate named entities is now an error in Assist (#110050)
* Matching duplicate named entities is now an error * Update snapshot * Only use area idpull/110078/head
parent
e4382a494c
commit
f5884c6279
|
@ -316,6 +316,20 @@ class DefaultAgent(AbstractConversationAgent):
|
|||
),
|
||||
conversation_id,
|
||||
)
|
||||
except intent.DuplicateNamesMatchedError as duplicate_names_error:
|
||||
# Intent was valid, but two or more entities with the same name matched.
|
||||
(
|
||||
error_response_type,
|
||||
error_response_args,
|
||||
) = _get_duplicate_names_matched_response(duplicate_names_error)
|
||||
return _make_error_result(
|
||||
language,
|
||||
intent.IntentResponseErrorCode.NO_VALID_TARGETS,
|
||||
self._get_error_text(
|
||||
error_response_type, lang_intents, **error_response_args
|
||||
),
|
||||
conversation_id,
|
||||
)
|
||||
except intent.IntentHandleError:
|
||||
# Intent was valid and entities matched constraints, but an error
|
||||
# occurred during handling.
|
||||
|
@ -753,7 +767,7 @@ class DefaultAgent(AbstractConversationAgent):
|
|||
if not alias.strip():
|
||||
continue
|
||||
|
||||
entity_names.append((alias, state.name, context))
|
||||
entity_names.append((alias, alias, context))
|
||||
|
||||
# Default name
|
||||
entity_names.append((state.name, state.name, context))
|
||||
|
@ -992,6 +1006,20 @@ def _get_no_states_matched_response(
|
|||
return ErrorKey.NO_INTENT, {}
|
||||
|
||||
|
||||
def _get_duplicate_names_matched_response(
|
||||
duplicate_names_error: intent.DuplicateNamesMatchedError,
|
||||
) -> tuple[ErrorKey, dict[str, Any]]:
|
||||
"""Return key and template arguments for error when intent returns duplicate matches."""
|
||||
|
||||
if duplicate_names_error.area:
|
||||
return ErrorKey.DUPLICATE_ENTITIES_IN_AREA, {
|
||||
"entity": duplicate_names_error.name,
|
||||
"area": duplicate_names_error.area,
|
||||
}
|
||||
|
||||
return ErrorKey.DUPLICATE_ENTITIES, {"entity": duplicate_names_error.name}
|
||||
|
||||
|
||||
def _collect_list_references(expression: Expression, list_names: set[str]) -> None:
|
||||
"""Collect list reference names recursively."""
|
||||
if isinstance(expression, Sequence):
|
||||
|
|
|
@ -156,16 +156,18 @@ class GetStateIntentHandler(intent.IntentHandler):
|
|||
slots = self.async_validate_slots(intent_obj.slots)
|
||||
|
||||
# Entity name to match
|
||||
entity_name: str | None = slots.get("name", {}).get("value")
|
||||
name_slot = slots.get("name", {})
|
||||
entity_name: str | None = name_slot.get("value")
|
||||
entity_text: str | None = name_slot.get("text")
|
||||
|
||||
# Look up area first to fail early
|
||||
area_name = slots.get("area", {}).get("value")
|
||||
area_slot = slots.get("area", {})
|
||||
area_id = area_slot.get("value")
|
||||
area_name = area_slot.get("text")
|
||||
area: ar.AreaEntry | None = None
|
||||
if area_name is not None:
|
||||
if area_id is not None:
|
||||
areas = ar.async_get(hass)
|
||||
area = areas.async_get_area(area_name) or areas.async_get_area_by_name(
|
||||
area_name
|
||||
)
|
||||
area = areas.async_get_area(area_id)
|
||||
if area is None:
|
||||
raise intent.IntentHandleError(f"No area named {area_name}")
|
||||
|
||||
|
@ -205,6 +207,13 @@ class GetStateIntentHandler(intent.IntentHandler):
|
|||
intent_obj.assistant,
|
||||
)
|
||||
|
||||
if entity_name and (len(states) > 1):
|
||||
# Multiple entities matched for the same name
|
||||
raise intent.DuplicateNamesMatchedError(
|
||||
name=entity_text or entity_name,
|
||||
area=area_name or area_id,
|
||||
)
|
||||
|
||||
# Create response
|
||||
response = intent_obj.create_response()
|
||||
response.response_type = intent.IntentResponseType.QUERY_ANSWER
|
||||
|
|
|
@ -155,6 +155,17 @@ class NoStatesMatchedError(IntentError):
|
|||
self.device_classes = device_classes
|
||||
|
||||
|
||||
class DuplicateNamesMatchedError(IntentError):
|
||||
"""Error when two or more entities with the same name matched."""
|
||||
|
||||
def __init__(self, name: str, area: str | None) -> None:
|
||||
"""Initialize error."""
|
||||
super().__init__()
|
||||
|
||||
self.name = name
|
||||
self.area = area
|
||||
|
||||
|
||||
def _is_device_class(
|
||||
state: State,
|
||||
entity: entity_registry.RegistryEntry | None,
|
||||
|
@ -318,8 +329,6 @@ def async_match_states(
|
|||
for state, entity in states_and_entities:
|
||||
if _has_name(state, entity, name):
|
||||
yield state
|
||||
break
|
||||
|
||||
else:
|
||||
# Not filtered by name
|
||||
for state, _entity in states_and_entities:
|
||||
|
@ -416,9 +425,7 @@ class ServiceIntentHandler(IntentHandler):
|
|||
area: area_registry.AreaEntry | None = None
|
||||
if area_id is not None:
|
||||
areas = area_registry.async_get(hass)
|
||||
area = areas.async_get_area(area_id) or areas.async_get_area_by_name(
|
||||
area_name
|
||||
)
|
||||
area = areas.async_get_area(area_id)
|
||||
if area is None:
|
||||
raise IntentHandleError(f"No area named {area_name}")
|
||||
|
||||
|
@ -453,6 +460,13 @@ class ServiceIntentHandler(IntentHandler):
|
|||
device_classes=device_classes,
|
||||
)
|
||||
|
||||
if entity_name and (len(states) > 1):
|
||||
# Multiple entities matched for the same name
|
||||
raise DuplicateNamesMatchedError(
|
||||
name=entity_text or entity_name,
|
||||
area=area_name or area_id,
|
||||
)
|
||||
|
||||
response = await self.async_handle_states(intent_obj, states, area)
|
||||
|
||||
# Make the matched states available in the response
|
||||
|
|
|
@ -1397,7 +1397,7 @@
|
|||
'name': dict({
|
||||
'name': 'name',
|
||||
'text': 'my cool light',
|
||||
'value': 'kitchen',
|
||||
'value': 'my cool light',
|
||||
}),
|
||||
}),
|
||||
'intent': dict({
|
||||
|
@ -1422,7 +1422,7 @@
|
|||
'name': dict({
|
||||
'name': 'name',
|
||||
'text': 'my cool light',
|
||||
'value': 'kitchen',
|
||||
'value': 'my cool light',
|
||||
}),
|
||||
}),
|
||||
'intent': dict({
|
||||
|
|
|
@ -614,6 +614,115 @@ async def test_error_no_intent(hass: HomeAssistant, init_components) -> None:
|
|||
)
|
||||
|
||||
|
||||
async def test_error_duplicate_names(
|
||||
hass: HomeAssistant, init_components, entity_registry: er.EntityRegistry
|
||||
) -> None:
|
||||
"""Test error message when multiple devices have the same name (or alias)."""
|
||||
kitchen_light_1 = entity_registry.async_get_or_create("light", "demo", "1234")
|
||||
kitchen_light_2 = entity_registry.async_get_or_create("light", "demo", "5678")
|
||||
|
||||
# Same name and alias
|
||||
for light in (kitchen_light_1, kitchen_light_2):
|
||||
light = entity_registry.async_update_entity(
|
||||
light.entity_id,
|
||||
name="kitchen light",
|
||||
aliases={"overhead light"},
|
||||
)
|
||||
hass.states.async_set(
|
||||
light.entity_id,
|
||||
"off",
|
||||
attributes={ATTR_FRIENDLY_NAME: light.name},
|
||||
)
|
||||
|
||||
# Check name and alias
|
||||
for name in ("kitchen light", "overhead light"):
|
||||
# command
|
||||
result = await conversation.async_converse(
|
||||
hass, f"turn on {name}", None, Context(), None
|
||||
)
|
||||
assert result.response.response_type == intent.IntentResponseType.ERROR
|
||||
assert (
|
||||
result.response.error_code
|
||||
== intent.IntentResponseErrorCode.NO_VALID_TARGETS
|
||||
)
|
||||
assert (
|
||||
result.response.speech["plain"]["speech"]
|
||||
== f"Sorry, there are multiple devices called {name}"
|
||||
)
|
||||
|
||||
# question
|
||||
result = await conversation.async_converse(
|
||||
hass, f"is {name} on?", None, Context(), None
|
||||
)
|
||||
assert result.response.response_type == intent.IntentResponseType.ERROR
|
||||
assert (
|
||||
result.response.error_code
|
||||
== intent.IntentResponseErrorCode.NO_VALID_TARGETS
|
||||
)
|
||||
assert (
|
||||
result.response.speech["plain"]["speech"]
|
||||
== f"Sorry, there are multiple devices called {name}"
|
||||
)
|
||||
|
||||
|
||||
async def test_error_duplicate_names_in_area(
|
||||
hass: HomeAssistant,
|
||||
init_components,
|
||||
area_registry: ar.AreaRegistry,
|
||||
entity_registry: er.EntityRegistry,
|
||||
) -> None:
|
||||
"""Test error message when multiple devices have the same name (or alias)."""
|
||||
area_kitchen = area_registry.async_get_or_create("kitchen_id")
|
||||
area_kitchen = area_registry.async_update(area_kitchen.id, name="kitchen")
|
||||
|
||||
kitchen_light_1 = entity_registry.async_get_or_create("light", "demo", "1234")
|
||||
kitchen_light_2 = entity_registry.async_get_or_create("light", "demo", "5678")
|
||||
|
||||
# Same name and alias
|
||||
for light in (kitchen_light_1, kitchen_light_2):
|
||||
light = entity_registry.async_update_entity(
|
||||
light.entity_id,
|
||||
name="kitchen light",
|
||||
area_id=area_kitchen.id,
|
||||
aliases={"overhead light"},
|
||||
)
|
||||
hass.states.async_set(
|
||||
light.entity_id,
|
||||
"off",
|
||||
attributes={ATTR_FRIENDLY_NAME: light.name},
|
||||
)
|
||||
|
||||
# Check name and alias
|
||||
for name in ("kitchen light", "overhead light"):
|
||||
# command
|
||||
result = await conversation.async_converse(
|
||||
hass, f"turn on {name} in {area_kitchen.name}", None, Context(), None
|
||||
)
|
||||
assert result.response.response_type == intent.IntentResponseType.ERROR
|
||||
assert (
|
||||
result.response.error_code
|
||||
== intent.IntentResponseErrorCode.NO_VALID_TARGETS
|
||||
)
|
||||
assert (
|
||||
result.response.speech["plain"]["speech"]
|
||||
== f"Sorry, there are multiple devices called {name} in the {area_kitchen.name} area"
|
||||
)
|
||||
|
||||
# question
|
||||
result = await conversation.async_converse(
|
||||
hass, f"is {name} on in the {area_kitchen.name}?", None, Context(), None
|
||||
)
|
||||
assert result.response.response_type == intent.IntentResponseType.ERROR
|
||||
assert (
|
||||
result.response.error_code
|
||||
== intent.IntentResponseErrorCode.NO_VALID_TARGETS
|
||||
)
|
||||
assert (
|
||||
result.response.speech["plain"]["speech"]
|
||||
== f"Sorry, there are multiple devices called {name} in the {area_kitchen.name} area"
|
||||
)
|
||||
|
||||
|
||||
async def test_no_states_matched_default_error(
|
||||
hass: HomeAssistant, init_components, area_registry: ar.AreaRegistry
|
||||
) -> None:
|
||||
|
@ -794,3 +903,112 @@ async def test_same_named_entities_in_different_areas(
|
|||
assert len(result.response.matched_states) == 1
|
||||
assert result.response.matched_states[0].entity_id == bedroom_light.entity_id
|
||||
assert calls[0].data.get("entity_id") == [bedroom_light.entity_id]
|
||||
|
||||
# Targeting a duplicate name should fail
|
||||
result = await conversation.async_converse(
|
||||
hass, "turn on overhead light", None, Context(), None
|
||||
)
|
||||
assert result.response.response_type == intent.IntentResponseType.ERROR
|
||||
|
||||
# Querying a duplicate name should also fail
|
||||
result = await conversation.async_converse(
|
||||
hass, "is the overhead light on?", None, Context(), None
|
||||
)
|
||||
assert result.response.response_type == intent.IntentResponseType.ERROR
|
||||
|
||||
# But we can still ask questions that don't rely on the name
|
||||
result = await conversation.async_converse(
|
||||
hass, "how many lights are on?", None, Context(), None
|
||||
)
|
||||
assert result.response.response_type == intent.IntentResponseType.QUERY_ANSWER
|
||||
|
||||
|
||||
async def test_same_aliased_entities_in_different_areas(
|
||||
hass: HomeAssistant,
|
||||
init_components,
|
||||
area_registry: ar.AreaRegistry,
|
||||
entity_registry: er.EntityRegistry,
|
||||
) -> None:
|
||||
"""Test that entities with the same alias (but different names) in different areas can be targeted."""
|
||||
area_kitchen = area_registry.async_get_or_create("kitchen_id")
|
||||
area_kitchen = area_registry.async_update(area_kitchen.id, name="kitchen")
|
||||
|
||||
area_bedroom = area_registry.async_get_or_create("bedroom_id")
|
||||
area_bedroom = area_registry.async_update(area_bedroom.id, name="bedroom")
|
||||
|
||||
# Both lights have the same alias, but are in different areas
|
||||
kitchen_light = entity_registry.async_get_or_create("light", "demo", "1234")
|
||||
kitchen_light = entity_registry.async_update_entity(
|
||||
kitchen_light.entity_id,
|
||||
area_id=area_kitchen.id,
|
||||
name="kitchen overhead light",
|
||||
aliases={"overhead light"},
|
||||
)
|
||||
hass.states.async_set(
|
||||
kitchen_light.entity_id,
|
||||
"off",
|
||||
attributes={ATTR_FRIENDLY_NAME: kitchen_light.name},
|
||||
)
|
||||
|
||||
bedroom_light = entity_registry.async_get_or_create("light", "demo", "5678")
|
||||
bedroom_light = entity_registry.async_update_entity(
|
||||
bedroom_light.entity_id,
|
||||
area_id=area_bedroom.id,
|
||||
name="bedroom overhead light",
|
||||
aliases={"overhead light"},
|
||||
)
|
||||
hass.states.async_set(
|
||||
bedroom_light.entity_id,
|
||||
"off",
|
||||
attributes={ATTR_FRIENDLY_NAME: bedroom_light.name},
|
||||
)
|
||||
|
||||
# Target kitchen light
|
||||
calls = async_mock_service(hass, "light", "turn_on")
|
||||
result = await conversation.async_converse(
|
||||
hass, "turn on overhead light in the kitchen", None, Context(), None
|
||||
)
|
||||
await hass.async_block_till_done()
|
||||
|
||||
assert len(calls) == 1
|
||||
assert result.response.response_type == intent.IntentResponseType.ACTION_DONE
|
||||
assert result.response.intent is not None
|
||||
assert result.response.intent.slots.get("name", {}).get("value") == "overhead light"
|
||||
assert result.response.intent.slots.get("name", {}).get("text") == "overhead light"
|
||||
assert len(result.response.matched_states) == 1
|
||||
assert result.response.matched_states[0].entity_id == kitchen_light.entity_id
|
||||
assert calls[0].data.get("entity_id") == [kitchen_light.entity_id]
|
||||
|
||||
# Target bedroom light
|
||||
calls.clear()
|
||||
result = await conversation.async_converse(
|
||||
hass, "turn on overhead light in the bedroom", None, Context(), None
|
||||
)
|
||||
await hass.async_block_till_done()
|
||||
|
||||
assert len(calls) == 1
|
||||
assert result.response.response_type == intent.IntentResponseType.ACTION_DONE
|
||||
assert result.response.intent is not None
|
||||
assert result.response.intent.slots.get("name", {}).get("value") == "overhead light"
|
||||
assert result.response.intent.slots.get("name", {}).get("text") == "overhead light"
|
||||
assert len(result.response.matched_states) == 1
|
||||
assert result.response.matched_states[0].entity_id == bedroom_light.entity_id
|
||||
assert calls[0].data.get("entity_id") == [bedroom_light.entity_id]
|
||||
|
||||
# Targeting a duplicate alias should fail
|
||||
result = await conversation.async_converse(
|
||||
hass, "turn on overhead light", None, Context(), None
|
||||
)
|
||||
assert result.response.response_type == intent.IntentResponseType.ERROR
|
||||
|
||||
# Querying a duplicate alias should also fail
|
||||
result = await conversation.async_converse(
|
||||
hass, "is the overhead light on?", None, Context(), None
|
||||
)
|
||||
assert result.response.response_type == intent.IntentResponseType.ERROR
|
||||
|
||||
# But we can still ask questions that don't rely on the alias
|
||||
result = await conversation.async_converse(
|
||||
hass, "how many lights are on?", None, Context(), None
|
||||
)
|
||||
assert result.response.response_type == intent.IntentResponseType.QUERY_ANSWER
|
||||
|
|
Loading…
Reference in New Issue