Add device automation condition #26313

emontnemery · 2019-08-31T08:19:25Z

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_on and turn_off conditions to light entities.

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:

The code change is tested and works locally.
Local tests pass with tox. Your PR cannot be merged unless tests pass
There is no commented out code in this PR.
I have followed the development checklist

If user exposed functionality or configuration variables are added/changed:

Documentation added/updated in home-assistant.io

If the code does not interact with devices:

Tests have been added to verify that the new code works.

emontnemery · 2019-09-01T07:54:57Z

homeassistant/helpers/condition.py

+        check_factory = check_factory.func
+
+    if asyncio.iscoroutinefunction(check_factory):
+        return await cast(Callable[..., bool], factory(hass, config, config_validation))
    return cast(Callable[..., bool], factory(config, config_validation))


 from_config = _threaded_factory(async_from_config)


This is now broken because async_from_config is a coroutine function. Is this used anywhere?

I think that we can remove all of those.

OK, removed.

emontnemery · 2019-09-01T07:55:09Z

homeassistant/helpers/condition.py

@@ -134,23 +141,20 @@ def if_and_condition(
 and_from_config = _threaded_factory(async_and_from_config)


This is now broken because async_and_from_config is a coroutine function. Is this used anywhere?

emontnemery · 2019-09-01T07:55:18Z

homeassistant/helpers/condition.py

@@ -166,6 +170,17 @@ def if_or_condition(
 or_from_config = _threaded_factory(async_or_from_config)


This is now broken because async_or_from_config is a coroutine function. Is this used anywhere?

homeassistant/helpers/condition.py

balloob · 2019-09-03T21:25:13Z

homeassistant/components/device_automation/__init__.py



-async def async_get_device_automation_triggers(hass, device_id):
-    """List device triggers."""
+async def async_get_device_automations(hass, fname, device_id):


If this function is only used in this file, let's prefix it with _.

balloob · 2019-09-03T21:25:58Z

homeassistant/components/device_automation/__init__.py

+    conditions = await async_get_device_automations(
+        hass, "async_get_conditions", device_id
+    )
+    connection.send_result(msg["id"], {"conditions": conditions})


Why would we add this to a dictionary? We should just send the list.

Suggested change

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)

balloob · 2019-09-03T21:27:04Z

homeassistant/components/light/device_automation.py

+            vol.Optional(CONF_DEVICE_ID): str,
+            vol.Required(CONF_DOMAIN): DOMAIN,
+            vol.Required(CONF_ENTITY_ID): cv.entity_id,
+            vol.Required(CONF_TYPE): vol.In([CONF_TURN_OFF, CONF_TURN_ON]),


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

You say fixed but then we should update these constants to be CONF_IS_ON CONF_IS_OFF ?

Suggested change

vol.Required(CONF_TYPE): vol.In([CONF_TURN_OFF, CONF_TURN_ON]),

vol.Required(CONF_TYPE): vol.In([CONF_IS_OFF, CONF_IS_ON]),

Oops, now properly fixed!

balloob · 2019-09-03T23:48:05Z

tests/components/device_automation/test_init.py

@@ -21,7 +21,7 @@ def entity_reg(hass):
    return mock_registry(hass)


-def _same_triggers(a, b):
+def _same_dicts(a, b):


This only tests that the keys are the same, not the values.

balloob · 2019-09-03T23:56:13Z

homeassistant/components/light/strings.json

@@ -1,5 +1,9 @@
 {
  "device_automation": {
+    "condition_type": {
+      "turn_on": "{name} turned on",


Suggested change

"turn_on": "{name} turned on",

"turn_on": "{name} is on",

balloob · 2019-09-03T23:56:20Z

homeassistant/components/light/strings.json

@@ -1,5 +1,9 @@
 {
  "device_automation": {
+    "condition_type": {
+      "turn_on": "{name} turned on",
+      "turn_off": "{name} turned off"


Suggested change

"turn_off": "{name} turned off"

"turn_off": "{name} is off"

homeassistant/helpers/condition.py

balloob · 2019-09-04T00:00:39Z

homeassistant/helpers/condition.py

@@ -163,7 +161,18 @@ def if_or_condition(
    return if_or_condition


-or_from_config = _threaded_factory(async_or_from_config)
+async def async_device_from_config(


This should not be part of the condition helper, instead I think it should be part of components/device_automation/__init__.py

balloob

This looks great, only a few minor comments left. Ok to merge after this?

emontnemery · 2019-09-05T14:46:43Z

Fixed the remaining comments.

emontnemery requested a review from a team as a code owner August 31, 2019 08:19

homeassistant added the cla-signed label Aug 31, 2019

probot-home-assistant bot added core has-tests integration: automation integration: device_automation integration: light labels Aug 31, 2019

project-bot bot added this to Needs review in Dev Aug 31, 2019

emontnemery closed this Aug 31, 2019

emontnemery force-pushed the device_automation_condition branch from 4d54727 to 76f2e60 Compare August 31, 2019 08:26

Dev automation moved this from Needs review to Cancelled Aug 31, 2019

emontnemery reopened this Aug 31, 2019

Dev automation moved this from Cancelled to Needs review Aug 31, 2019

MartinHjelmare moved this from Needs review to Incoming in Dev Aug 31, 2019

emontnemery commented Sep 1, 2019

View reviewed changes

emontnemery force-pushed the device_automation_condition branch from b1ce321 to d4efe19 Compare September 2, 2019 20:31

emontnemery mentioned this pull request Sep 3, 2019

Add device conditions to automation editor. home-assistant/frontend#3595

Merged

balloob reviewed Sep 3, 2019

View reviewed changes

homeassistant/helpers/condition.py Show resolved Hide resolved

balloob reviewed Sep 4, 2019

View reviewed changes

emontnemery added 5 commits September 4, 2019 19:11

Add support for device conditions

c58b58f

Lint

234476a

Update test case

26faef4

Make and+or conditions async, adjust tests

590407d

Cleanup tests

4b7a5ff

emontnemery added 5 commits September 4, 2019 19:11

Remove non callback versions of conditions, correct typing

8e8afcf

Correct typing

a485792

Update light/strings.json

61dab4a

Address review comments

9d65b41

Make device automation lists simple lists, not dicts

6cb0a8b

emontnemery force-pushed the device_automation_condition branch from 77b327d to 6cb0a8b Compare September 4, 2019 17:11

Add device_automation/const.py

d8608fd

balloob approved these changes Sep 5, 2019

View reviewed changes

Dev automation moved this from Incoming to Reviewer approved Sep 5, 2019

Use IS_ON/IS_OFF everywhere for conditions

98e73ca

emontnemery changed the title ~~WIP: Device automation condition~~ Device automation condition Sep 5, 2019

emontnemery changed the title ~~Device automation condition~~ Add device automation condition Sep 5, 2019

emontnemery merged commit f7dc537 into home-assistant:dev Sep 5, 2019

Dev automation moved this from Reviewer approved to Done Sep 5, 2019

emontnemery deleted the device_automation_condition branch September 6, 2019 10:02

lock bot locked and limited conversation to collaborators Sep 7, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add device automation condition #26313

Add device automation condition #26313

emontnemery commented Aug 31, 2019 •

edited

emontnemery Sep 1, 2019

balloob Sep 3, 2019

emontnemery Sep 4, 2019

emontnemery Sep 1, 2019

emontnemery Sep 4, 2019

emontnemery Sep 1, 2019

emontnemery Sep 4, 2019

balloob Sep 3, 2019

emontnemery Sep 4, 2019

balloob Sep 3, 2019

emontnemery Sep 4, 2019 •

edited

balloob Sep 3, 2019

emontnemery Sep 4, 2019

balloob Sep 5, 2019

balloob Sep 5, 2019

emontnemery Sep 5, 2019

balloob Sep 3, 2019

emontnemery Sep 4, 2019

balloob Sep 3, 2019

emontnemery Sep 4, 2019

balloob Sep 3, 2019

emontnemery Sep 4, 2019

balloob Sep 4, 2019

emontnemery Sep 4, 2019

balloob left a comment

emontnemery commented Sep 5, 2019

		@@ -134,23 +141,20 @@ def if_and_condition(
		and_from_config = _threaded_factory(async_and_from_config)

		@@ -166,6 +170,17 @@ def if_or_condition(
		or_from_config = _threaded_factory(async_or_from_config)

	connection.send_result(msg["id"], {"conditions": conditions})
	connection.send_result(msg["id"], conditions)

	vol.Required(CONF_TYPE): vol.In([CONF_TURN_OFF, CONF_TURN_ON]),
	vol.Required(CONF_TYPE): vol.In([CONF_IS_OFF, CONF_IS_ON]),

Add device automation condition #26313

Add device automation condition #26313

Conversation

emontnemery commented Aug 31, 2019 • edited

Description:

Example entry for configuration.yaml (if applicable):

Checklist:

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

emontnemery Sep 4, 2019 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

balloob left a comment

Choose a reason for hiding this comment

emontnemery commented Sep 5, 2019

emontnemery commented Aug 31, 2019 •

edited

Example entry for `configuration.yaml` (if applicable):

emontnemery Sep 4, 2019 •

edited