New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Specific Assist errors for domain/device class #107302
Specific Assist errors for domain/device class #107302
Conversation
Hey there @home-assistant/core, mind taking a look at this pull request as it has been labeled with an integration ( Code owner commandsCode owners of
|
if no_states_error.area: | ||
# Check device classes first, since it's more specific than domain | ||
if no_states_error.device_classes: | ||
# No exposed entities of a particular class in an area. | ||
# Example: "close the bedroom windows" | ||
error_response_type = ResponseType.NO_DEVICE_CLASS | ||
error_response_args["area"] = no_states_error.area | ||
|
||
# Only use the first device class for the error message | ||
error_response_args["device_class"] = next( | ||
iter(no_states_error.device_classes) | ||
) | ||
elif no_states_error.domains: | ||
# No exposed entities of a domain in an area. | ||
# Example: "turn on lights in kitchen" | ||
error_response_type = ResponseType.NO_DOMAIN | ||
error_response_args["area"] = no_states_error.area | ||
|
||
# Only use the first domain for the error message | ||
error_response_args["domain"] = next(iter(no_states_error.domains)) | ||
|
||
return error_response_type, error_response_args |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if no_states_error.area: | |
# Check device classes first, since it's more specific than domain | |
if no_states_error.device_classes: | |
# No exposed entities of a particular class in an area. | |
# Example: "close the bedroom windows" | |
error_response_type = ResponseType.NO_DEVICE_CLASS | |
error_response_args["area"] = no_states_error.area | |
# Only use the first device class for the error message | |
error_response_args["device_class"] = next( | |
iter(no_states_error.device_classes) | |
) | |
elif no_states_error.domains: | |
# No exposed entities of a domain in an area. | |
# Example: "turn on lights in kitchen" | |
error_response_type = ResponseType.NO_DOMAIN | |
error_response_args["area"] = no_states_error.area | |
# Only use the first domain for the error message | |
error_response_args["domain"] = next(iter(no_states_error.domains)) | |
return error_response_type, error_response_args | |
if not no_states_error.area: | |
return error_response_type, error_response_args | |
# Check device classes first, since it's more specific than domain | |
if no_states_error.device_classes: | |
# No exposed entities of a particular class in an area. | |
# Example: "close the bedroom windows" | |
error_response_type = ResponseType.NO_DEVICE_CLASS | |
error_response_args["area"] = no_states_error.area | |
# Only use the first device class for the error message | |
error_response_args["device_class"] = next( | |
iter(no_states_error.device_classes) | |
) | |
return error_response_type, error_response_args | |
# No exposed entities of a domain in an area. | |
# Example: "turn on lights in kitchen" | |
error_response_type = ResponseType.NO_DOMAIN | |
error_response_args["area"] = no_states_error.area | |
# Only use the first domain for the error message | |
error_response_args["domain"] = next(iter(no_states_error.domains)) | |
return error_response_type, error_response_args |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I refactored the top guard clause to exit early if there isn't an area as well as at least one domain/device class.
Otherwise, this suggestion could fail on next(iter(no_states_error.domains))
I've marked this PR, as changes are requested that need to be processed. Thanks! 👍 ../Frenck |
Breaking change
Proposed change
Use specific Assist errors for domain/device class. For example, "turn on kitchen lights" will now respond with "kitchen does not contain a light" instead of "Sorry, I couldn't understand that". Likewise, "close bedroom windows" will respond "bedroom does not contain a window".
To make this work, the
ServiceIntentHandler
was modified to raise a newNoStatesMatchedError
, which is bubbled up byintent.async_handle
to the default agent. This error must be raised inside the intent handler because it is only after filtering entities based on the constraints that we know nothing matches [1]. The error must be bubbled up because only the default agent knows how to produce the correct translated responses.[1] Additionally, other intent handlers may not care if nothing matches.
GetStateIntentHandler
is one of these, which would just respond "0" to "how many lights are in the kitchen" if there are no lights instead of raising an error.Type of change
Additional information
Checklist
ruff format homeassistant tests
)If user exposed functionality or configuration variables are added/changed:
If the code communicates with devices, web services, or third-party tools:
Updated and included derived files by running:
python3 -m script.hassfest
.requirements_all.txt
.Updated by running
python3 -m script.gen_requirements_all
..coveragerc
.To help with the load of incoming pull requests: