-
-
Notifications
You must be signed in to change notification settings - Fork 38.4k
Add optional speed parameter to cover movement actions #176231
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
base: dev
Are you sure you want to change the base?
Changes from all commits
349ae63
0c07a0c
40fc350
a517db0
6dc6227
3e0656c
9aa818b
979f73e
976a8ad
3beb006
3125b78
dc80f60
f143781
2020094
a92d470
6aff465
57386d9
f6437a7
78b683b
f610c9a
362460b
21fa59c
bc4faf2
6cd651d
a169847
61b601c
b29b1e4
0c8cf3f
4388df6
20b0565
af13c60
f70a4dc
1cd0e1f
59919a5
22e02c8
e155949
2e86f05
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -6,13 +6,31 @@ open_cover: | |
| domain: cover | ||
| supported_features: | ||
| - cover.CoverEntityFeature.OPEN | ||
| fields: | ||
| speed: | ||
| example: "fast" | ||
| filter: | ||
| supported_features: | ||
| - cover.CoverEntityFeature.SPEED | ||
| selector: | ||
| state: | ||
| attribute: speed | ||
|
wollew marked this conversation as resolved.
Comment on lines
+10
to
+17
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should this be filtered for entities that don't support speeds?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is that possible? The docs say I need to specify at least one value but these are all unknown in this case. And I also could not find an example. Or did I misunderstand this?
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I guess it would be possible if there was a supported_feature flag for speeds? I just worry it looks bad when entities without speeds show a speed dropdown that is just blank/empty. It might confuse people into thinking somehow they could change the speed and they wouldn't know what to do with it.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I think so, but I understood home-assistant/architecture#789 (reply in thread) as "we don't need that". It seems we do, do you want me to change it?
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I will leave that decision up to the core reviewers. It may be that when it was first thought that it wasn't needed, this wasn't considered.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
But I do see two "problems" with this implementation;
@MartinHjelmare what do you think?
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could you elaborate what you mean by "supported_features maps to services"? It seems like it's used in lots more ways than just that. Is there a rule that supported feature is only allowed if it gates an entire service? Seems weirdly restrictive, given it is also used for other things.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Right, misunderstanding from my side. I think we need to have an entity feature flag for speed 👍🏼
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. added the feature flag and filtering
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, speed should be a supported feature. It is used to express what service actions and what action parameters are supported.
wollew marked this conversation as resolved.
wollew marked this conversation as resolved.
wollew marked this conversation as resolved.
wollew marked this conversation as resolved.
karwosts marked this conversation as resolved.
|
||
|
|
||
| close_cover: | ||
| target: | ||
| entity: | ||
| domain: cover | ||
| supported_features: | ||
| - cover.CoverEntityFeature.CLOSE | ||
| fields: | ||
| speed: | ||
| example: "fast" | ||
| filter: | ||
| supported_features: | ||
| - cover.CoverEntityFeature.SPEED | ||
| selector: | ||
| state: | ||
| attribute: speed | ||
|
karwosts marked this conversation as resolved.
|
||
|
|
||
| toggle: | ||
| target: | ||
|
|
@@ -36,6 +54,14 @@ set_cover_position: | |
| min: 0 | ||
| max: 100 | ||
| unit_of_measurement: "%" | ||
| speed: | ||
| filter: | ||
| supported_features: | ||
| - cover.CoverEntityFeature.SPEED | ||
| example: "fast" | ||
| selector: | ||
| state: | ||
| attribute: speed | ||
|
karwosts marked this conversation as resolved.
|
||
|
|
||
| stop_cover: | ||
| target: | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.