Add device automation condition#26313
Conversation
emontnemery
force-pushed
the
device_automation_condition
branch
from
4d54727 to
76f2e60
Compare
There was a problem hiding this comment.
This is now broken because async_from_config is a coroutine function. Is this used anywhere?
There was a problem hiding this comment.
I think that we can remove all of those.
There was a problem hiding this comment.
This is now broken because async_and_from_config is a coroutine function. Is this used anywhere?
There was a problem hiding this comment.
This is now broken because async_or_from_config is a coroutine function. Is this used anywhere?
emontnemery
force-pushed
the
device_automation_condition
branch
from
b1ce321 to
d4efe19
Compare
There was a problem hiding this comment.
If this function is only used in this file, let's prefix it with _.
There was a problem hiding this comment.
Why would we add this to a dictionary? We should just send the list.
| connection.send_result(msg["id"], {"conditions": conditions}) | |
| connection.send_result(msg["id"], conditions) |
We should do the same for triggers (which I thought we already did)
There was a problem hiding this comment.
this feels a bit weird, shouldn't this be is_on and is_off ?
I also wonder if we should move some generic constants to device_automation/const.py
There was a problem hiding this comment.
You say fixed but then we should update these constants to be CONF_IS_ON CONF_IS_OFF ?
There was a problem hiding this comment.
| vol.Required(CONF_TYPE): vol.In([CONF_TURN_OFF, CONF_TURN_ON]), | |
| vol.Required(CONF_TYPE): vol.In([CONF_IS_OFF, CONF_IS_ON]), |
There was a problem hiding this comment.
Oops, now properly fixed!
There was a problem hiding this comment.
This only tests that the keys are the same, not the values.
There was a problem hiding this comment.
| "turn_on": "{name} turned on", | |
| "turn_on": "{name} is on", |
There was a problem hiding this comment.
| "turn_off": "{name} turned off" | |
| "turn_off": "{name} is off" |
There was a problem hiding this comment.
This should not be part of the condition helper, instead I think it should be part of components/device_automation/__init__.py
emontnemery
force-pushed
the
device_automation_condition
branch
from
77b327d to
6cb0a8b
Compare
balloob
left a comment
balloob
left a comment
There was a problem hiding this comment.
This looks great, only a few minor comments left. Ok to merge after this?
emontnemery
changed the title
emontnemery
changed the title
|
Fixed the remaining comments. |
4 participants
Description:
Add websock command to query device for conditions.
Conditions can be provided by the integration that provided the device or the entity integrations that the device has entities with.
As an example of the latter, this PR also adds
turn_onandturn_offconditions tolightentities.This PR is intended as a step towards home-assistant/architecture#178
Pull request with documentation for home-assistant.io (if applicable): home-assistant/home-assistant.io#<home-assistant.io PR number goes here>
Example entry for
configuration.yaml(if applicable):Checklist:
tox. Your PR cannot be merged unless tests passIf user exposed functionality or configuration variables are added/changed:
If the code does not interact with devices: