-
-
Notifications
You must be signed in to change notification settings - Fork 31.8k
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
Add filters to climate and light service descriptions #86162
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
|
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
|
083221f
to
44d235f
Compare
homeassistant/helpers/selector.py
Outdated
if isinstance(supported_feature, int): | ||
return supported_feature | ||
|
||
known_entity_features = { |
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.
should we generate this in a function decorated with @cache
?
homeassistant/helpers/selector.py
Outdated
"WaterHeaterEntityFeature": WaterHeaterEntityFeature, | ||
} | ||
|
||
_, enum, feature = supported_feature.split(".", 2) |
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.
you should catch if it's only has a single or no .
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.
Voluptuous actually catches this and generates an acceptable error message without catching the ValueError
which is raised if the tuple unpacking fails:
voluptuous.error.MultipleInvalid: not a valid value @ data['filter'][0]['supported_features'][0]
Do you think it adds value to add a more specific validation error, maybe something like:
voluptuous.error.MultipleInvalid: Invalid supported feature 'blah', expected <domain>.<enum>.<member> @ data['filter'][0]['supported_features'][0]
homeassistant/helpers/selector.py
Outdated
@@ -87,6 +152,10 @@ def serialize(self) -> dict[str, dict[str, _T]]: | |||
vol.Optional("domain"): vol.All(cv.ensure_list, [str]), | |||
# Device class of the entity | |||
vol.Optional("device_class"): vol.All(cv.ensure_list, [str]), | |||
# Features supported by the entity | |||
vol.Optional("supported_features"): [ | |||
vol.All(vol.Any(int, str), _validate_supported_feature) |
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.
let's not support int. It's difficult to read
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.
This is required to be able to instantiate a selector from an already validated configuration where the enum strings have been resolved to their integer values.
We do that in tests to test serialization of configuration, I'm not sure if it's actually needed though?
Proposed change
Add filters to
climate
andlight
service descriptionsThis PR makes it possible for service descriptions to add filters both to a service and a service parameters based on the capabilities of the service call target:
entity
target selector has a new configuration parametersupported_features
supported_features
filter lists supported features needed for the service field, the target entity must support at least one of the supported featuresfilter
filter
can be eithersupported_features
orattributes
supported_features
filter lists supported features needed for the service field, the target entity must support at least one of the supported featuresattribute
filter names an attribute and lists possible attribute states needed for the service fieldclimate
andlight
are chosen as targets because they both have services which should be hidden depending on the target, or where service call parameters should be hidden depending on the target.Type of change
Additional information
Checklist
black --fast 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: