From 98049fc5b46ffb4a0ea92174fde495d26cd7ecea Mon Sep 17 00:00:00 2001 From: Donnie Date: Fri, 2 Oct 2020 13:18:56 -0700 Subject: [PATCH 1/9] Add support for the alias field in all action steps. Consolidate logging --- homeassistant/helpers/config_validation.py | 20 +++++++++-- homeassistant/helpers/script.py | 39 ++++++++-------------- tests/helpers/test_config_validation.py | 31 +++++++++++++++++ 3 files changed, 62 insertions(+), 28 deletions(-) diff --git a/homeassistant/helpers/config_validation.py b/homeassistant/helpers/config_validation.py index 6f1c6f465997d..b5cb86bab8985 100644 --- a/homeassistant/helpers/config_validation.py +++ b/homeassistant/helpers/config_validation.py @@ -923,6 +923,7 @@ def script_action(value: Any) -> dict: NUMERIC_STATE_CONDITION_SCHEMA = vol.All( vol.Schema( { + vol.Optional(CONF_ALIAS): string, vol.Required(CONF_CONDITION): "numeric_state", vol.Required(CONF_ENTITY_ID): entity_ids, vol.Optional(CONF_ATTRIBUTE): str, @@ -935,6 +936,7 @@ def script_action(value: Any) -> dict: ) STATE_CONDITION_BASE_SCHEMA = { + vol.Optional(CONF_ALIAS): string, vol.Required(CONF_CONDITION): "state", vol.Required(CONF_ENTITY_ID): entity_ids, vol.Optional(CONF_ATTRIBUTE): str, @@ -975,6 +977,7 @@ def STATE_CONDITION_SCHEMA(value: Any) -> dict: # pylint: disable=invalid-name SUN_CONDITION_SCHEMA = vol.All( vol.Schema( { + vol.Optional(CONF_ALIAS): string, vol.Required(CONF_CONDITION): "sun", vol.Optional("before"): sun_event, vol.Optional("before_offset"): time_period, @@ -989,6 +992,7 @@ def STATE_CONDITION_SCHEMA(value: Any) -> dict: # pylint: disable=invalid-name TEMPLATE_CONDITION_SCHEMA = vol.Schema( { + vol.Optional(CONF_ALIAS): string, vol.Required(CONF_CONDITION): "template", vol.Required(CONF_VALUE_TEMPLATE): template, } @@ -997,6 +1001,7 @@ def STATE_CONDITION_SCHEMA(value: Any) -> dict: # pylint: disable=invalid-name TIME_CONDITION_SCHEMA = vol.All( vol.Schema( { + vol.Optional(CONF_ALIAS): string, vol.Required(CONF_CONDITION): "time", "before": vol.Any(time, vol.All(str, entity_domain("input_datetime"))), "after": vol.Any(time, vol.All(str, entity_domain("input_datetime"))), @@ -1008,6 +1013,7 @@ def STATE_CONDITION_SCHEMA(value: Any) -> dict: # pylint: disable=invalid-name ZONE_CONDITION_SCHEMA = vol.Schema( { + vol.Optional(CONF_ALIAS): string, vol.Required(CONF_CONDITION): "zone", vol.Required(CONF_ENTITY_ID): entity_ids, "zone": entity_ids, @@ -1019,6 +1025,7 @@ def STATE_CONDITION_SCHEMA(value: Any) -> dict: # pylint: disable=invalid-name AND_CONDITION_SCHEMA = vol.Schema( { + vol.Optional(CONF_ALIAS): string, vol.Required(CONF_CONDITION): "and", vol.Required(CONF_CONDITIONS): vol.All( ensure_list, @@ -1030,6 +1037,7 @@ def STATE_CONDITION_SCHEMA(value: Any) -> dict: # pylint: disable=invalid-name OR_CONDITION_SCHEMA = vol.Schema( { + vol.Optional(CONF_ALIAS): string, vol.Required(CONF_CONDITION): "or", vol.Required(CONF_CONDITIONS): vol.All( ensure_list, @@ -1041,6 +1049,7 @@ def STATE_CONDITION_SCHEMA(value: Any) -> dict: # pylint: disable=invalid-name NOT_CONDITION_SCHEMA = vol.Schema( { + vol.Optional(CONF_ALIAS): string, vol.Required(CONF_CONDITION): "not", vol.Required(CONF_CONDITIONS): vol.All( ensure_list, @@ -1052,6 +1061,7 @@ def STATE_CONDITION_SCHEMA(value: Any) -> dict: # pylint: disable=invalid-name DEVICE_CONDITION_BASE_SCHEMA = vol.Schema( { + vol.Optional(CONF_ALIAS): string, vol.Required(CONF_CONDITION): "device", vol.Required(CONF_DEVICE_ID): str, vol.Required(CONF_DOMAIN): str, @@ -1102,12 +1112,18 @@ def STATE_CONDITION_SCHEMA(value: Any) -> dict: # pylint: disable=invalid-name ) DEVICE_ACTION_BASE_SCHEMA = vol.Schema( - {vol.Required(CONF_DEVICE_ID): string, vol.Required(CONF_DOMAIN): str} + { + vol.Optional(CONF_ALIAS): string, + vol.Required(CONF_DEVICE_ID): string, + vol.Required(CONF_DOMAIN): str, + } ) DEVICE_ACTION_SCHEMA = DEVICE_ACTION_BASE_SCHEMA.extend({}, extra=vol.ALLOW_EXTRA) -_SCRIPT_SCENE_SCHEMA = vol.Schema({vol.Required(CONF_SCENE): entity_domain("scene")}) +_SCRIPT_SCENE_SCHEMA = vol.Schema( + {vol.Optional(CONF_ALIAS): string, vol.Required(CONF_SCENE): entity_domain("scene")} +) _SCRIPT_REPEAT_SCHEMA = vol.Schema( { diff --git a/homeassistant/helpers/script.py b/homeassistant/helpers/script.py index 69ba082e57380..594988b82f80d 100644 --- a/homeassistant/helpers/script.py +++ b/homeassistant/helpers/script.py @@ -235,6 +235,11 @@ def _log( msg, *args, level=level, **kwargs ) + def _step_log(self, default_message, delay=None): + self._script.last_action = self._action.get(CONF_ALIAS, default_message) + _timeout = "" if delay is None else f" (timeout: {timedelta(seconds=delay)})" + self._log("Executing step %s%s", self._script.last_action, _timeout) + async def async_run(self) -> None: """Run script.""" try: @@ -327,8 +332,7 @@ async def _async_delay_step(self): """Handle delay.""" delay = self._get_pos_time_period_template(CONF_DELAY) - self._script.last_action = self._action.get(CONF_ALIAS, f"delay {delay}") - self._log("Executing step %s", self._script.last_action) + self._step_log(f"delay {delay}") delay = delay.total_seconds() self._changed() @@ -345,12 +349,7 @@ async def _async_wait_template_step(self): else: delay = None - self._script.last_action = self._action.get(CONF_ALIAS, "wait template") - self._log( - "Executing step %s%s", - self._script.last_action, - "" if delay is None else f" (timeout: {timedelta(seconds=delay)})", - ) + self._step_log("wait template", delay) self._variables["wait"] = {"remaining": delay, "completed": False} @@ -431,8 +430,7 @@ async def async_cancel_long_task() -> None: async def _async_call_service_step(self): """Call the service specified in the action.""" - self._script.last_action = self._action.get(CONF_ALIAS, "call service") - self._log("Executing step %s", self._script.last_action) + self._step_log("call service") params = service.async_prepare_call_from_config( self._hass, self._action, self._variables @@ -467,8 +465,7 @@ async def _async_call_service_step(self): async def _async_device_step(self): """Perform the device automation specified in the action.""" - self._script.last_action = self._action.get(CONF_ALIAS, "device automation") - self._log("Executing step %s", self._script.last_action) + self._step_log("device automation") platform = await device_automation.async_get_device_automation_platform( self._hass, self._action[CONF_DOMAIN], "action" ) @@ -478,8 +475,7 @@ async def _async_device_step(self): async def _async_scene_step(self): """Activate the scene specified in the action.""" - self._script.last_action = self._action.get(CONF_ALIAS, "activate scene") - self._log("Executing step %s", self._script.last_action) + self._step_log("activate scene") await self._hass.services.async_call( scene.DOMAIN, SERVICE_TURN_ON, @@ -490,10 +486,7 @@ async def _async_scene_step(self): async def _async_event_step(self): """Fire an event.""" - self._script.last_action = self._action.get( - CONF_ALIAS, self._action[CONF_EVENT] - ) - self._log("Executing step %s", self._script.last_action) + self._step_log(self._action.get(CONF_ALIAS, self._action[CONF_EVENT])) event_data = {} for conf in [CONF_EVENT_DATA, CONF_EVENT_DATA_TEMPLATE]: if conf not in self._action: @@ -631,12 +624,7 @@ async def _async_wait_for_trigger_step(self): else: delay = None - self._script.last_action = self._action.get(CONF_ALIAS, "wait for trigger") - self._log( - "Executing step %s%s", - self._script.last_action, - "" if delay is None else f" (timeout: {timedelta(seconds=delay)})", - ) + self._step_log("wait for trigger", delay) variables = {**self._variables} self._variables["wait"] = {"remaining": delay, "trigger": None} @@ -685,8 +673,7 @@ def log_cb(level, msg, **kwargs): async def _async_variables_step(self): """Set a variable value.""" - self._script.last_action = self._action.get(CONF_ALIAS, "setting variables") - self._log("Executing step %s", self._script.last_action) + self._step_log("setting variables") self._variables = self._action[CONF_VARIABLES].async_render( self._hass, self._variables, render_as_defaults=False ) diff --git a/tests/helpers/test_config_validation.py b/tests/helpers/test_config_validation.py index 1397e499c7ed5..6eba94de96bf9 100644 --- a/tests/helpers/test_config_validation.py +++ b/tests/helpers/test_config_validation.py @@ -10,6 +10,7 @@ import voluptuous as vol import homeassistant +from homeassistant.const import CONF_ALIAS from homeassistant.helpers import config_validation as cv, template @@ -330,6 +331,31 @@ def test_service(): schema("homeassistant.turn_on") +def test_action_type_schemas_support_alias(): + """Test that all action type schemas allow alias field.""" + + def find_root_schema(node): + """Traverse schema to find the root validation dictionary.""" + if isinstance(node, dict): + return vol.Schema(node) + + if isinstance(node, vol.Schema): + return find_root_schema(node.schema) + + if isinstance(node, vol.All): + return find_root_schema(node.validators[0]) + + return None + + for action_type, action_schema in cv.ACTION_TYPE_SCHEMAS.items(): + # Condition schema is tricky to find a root dictionary for + if action_type == cv.SCRIPT_ACTION_CHECK_CONDITION: + continue + root_schema = find_root_schema(action_schema) + schema_keys = root_schema.schema.keys() + assert CONF_ALIAS in schema_keys + + def test_service_schema(): """Test service_schema validation.""" options = ( @@ -358,6 +384,11 @@ def test_service_schema(): "service": "homeassistant.turn_on", "entity_id": ["light.kitchen", "light.ceiling"], }, + { + "service": "light.turn_on", + "entity_id": "all", + "alias": "turn on kitchen lights", + }, ) for value in options: cv.SERVICE_SCHEMA(value) From e6da390eef7857672f6a20b3f22a357625b93c37 Mon Sep 17 00:00:00 2001 From: Erik Date: Tue, 16 Feb 2021 19:26:41 +0100 Subject: [PATCH 2/9] Allow aliases for choices --- homeassistant/helpers/config_validation.py | 1 + homeassistant/helpers/script.py | 3 ++- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/homeassistant/helpers/config_validation.py b/homeassistant/helpers/config_validation.py index b5cb86bab8985..6e2e3a6928d6b 100644 --- a/homeassistant/helpers/config_validation.py +++ b/homeassistant/helpers/config_validation.py @@ -1151,6 +1151,7 @@ def STATE_CONDITION_SCHEMA(value: Any) -> dict: # pylint: disable=invalid-name ensure_list, [ { + vol.Optional(CONF_ALIAS): string, vol.Required(CONF_CONDITIONS): vol.All( ensure_list, [CONDITION_SCHEMA] ), diff --git a/homeassistant/helpers/script.py b/homeassistant/helpers/script.py index 594988b82f80d..2d913ba550260 100644 --- a/homeassistant/helpers/script.py +++ b/homeassistant/helpers/script.py @@ -1098,10 +1098,11 @@ async def _async_prep_choose_data(self, step): await self._async_get_condition(config) for config in choice.get(CONF_CONDITIONS, []) ] + choice_name = choice.get(CONF_ALIAS, f"choice {idx}") sub_script = Script( self._hass, choice[CONF_SEQUENCE], - f"{self.name}: {step_name}: choice {idx}", + f"{self.name}: {step_name}: {choice_name}", self.domain, running_description=self.running_description, script_mode=SCRIPT_MODE_PARALLEL, From a48d4b1f1df9f9495a39ead7e63185b4dd3ed44a Mon Sep 17 00:00:00 2001 From: Erik Date: Tue, 16 Feb 2021 19:27:53 +0100 Subject: [PATCH 3/9] Update tests --- tests/helpers/test_config_validation.py | 26 ---------- tests/helpers/test_script.py | 68 ++++++++++++++++++++----- 2 files changed, 55 insertions(+), 39 deletions(-) diff --git a/tests/helpers/test_config_validation.py b/tests/helpers/test_config_validation.py index 6eba94de96bf9..d0ae86f8f7edb 100644 --- a/tests/helpers/test_config_validation.py +++ b/tests/helpers/test_config_validation.py @@ -10,7 +10,6 @@ import voluptuous as vol import homeassistant -from homeassistant.const import CONF_ALIAS from homeassistant.helpers import config_validation as cv, template @@ -331,31 +330,6 @@ def test_service(): schema("homeassistant.turn_on") -def test_action_type_schemas_support_alias(): - """Test that all action type schemas allow alias field.""" - - def find_root_schema(node): - """Traverse schema to find the root validation dictionary.""" - if isinstance(node, dict): - return vol.Schema(node) - - if isinstance(node, vol.Schema): - return find_root_schema(node.schema) - - if isinstance(node, vol.All): - return find_root_schema(node.validators[0]) - - return None - - for action_type, action_schema in cv.ACTION_TYPE_SCHEMAS.items(): - # Condition schema is tricky to find a root dictionary for - if action_type == cv.SCRIPT_ACTION_CHECK_CONDITION: - continue - root_schema = find_root_schema(action_schema) - schema_keys = root_schema.schema.keys() - assert CONF_ALIAS in schema_keys - - def test_service_schema(): """Test service_schema validation.""" options = ( diff --git a/tests/helpers/test_script.py b/tests/helpers/test_script.py index a22cf27acdc80..d2946fcd4947e 100644 --- a/tests/helpers/test_script.py +++ b/tests/helpers/test_script.py @@ -50,7 +50,10 @@ async def test_firing_event_basic(hass, caplog): context = Context() events = async_capture_events(hass, event) - sequence = cv.SCRIPT_SCHEMA({"event": event, "event_data": {"hello": "world"}}) + alias = "event step" + sequence = cv.SCRIPT_SCHEMA( + {"alias": alias, "event": event, "event_data": {"hello": "world"}} + ) script_obj = script.Script( hass, sequence, "Test Name", "test_domain", running_description="test script" ) @@ -63,6 +66,7 @@ async def test_firing_event_basic(hass, caplog): assert events[0].data.get("hello") == "world" assert ".test_name:" in caplog.text assert "Test Name: Running test script" in caplog.text + assert f"Executing step {alias}" in caplog.text async def test_firing_event_template(hass): @@ -107,12 +111,15 @@ async def test_firing_event_template(hass): } -async def test_calling_service_basic(hass): +async def test_calling_service_basic(hass, caplog): """Test the calling of a service.""" context = Context() calls = async_mock_service(hass, "test", "script") - sequence = cv.SCRIPT_SCHEMA({"service": "test.script", "data": {"hello": "world"}}) + alias = "service step" + sequence = cv.SCRIPT_SCHEMA( + {"alias": alias, "service": "test.script", "data": {"hello": "world"}} + ) script_obj = script.Script(hass, sequence, "Test Name", "test_domain") await script_obj.async_run(context=context) @@ -121,6 +128,7 @@ async def test_calling_service_basic(hass): assert len(calls) == 1 assert calls[0].context is context assert calls[0].data.get("hello") == "world" + assert f"Executing step {alias}" in caplog.text async def test_calling_service_template(hass): @@ -250,12 +258,13 @@ def heard_event_cb(event): assert len(calls) == 4 -async def test_activating_scene(hass): +async def test_activating_scene(hass, caplog): """Test the activation of a scene.""" context = Context() calls = async_mock_service(hass, scene.DOMAIN, SERVICE_TURN_ON) - sequence = cv.SCRIPT_SCHEMA({"scene": "scene.hello"}) + alias = "scene step" + sequence = cv.SCRIPT_SCHEMA({"alias": alias, "scene": "scene.hello"}) script_obj = script.Script(hass, sequence, "Test Name", "test_domain") await script_obj.async_run(context=context) @@ -264,6 +273,7 @@ async def test_activating_scene(hass): assert len(calls) == 1 assert calls[0].context is context assert calls[0].data.get(ATTR_ENTITY_ID) == "scene.hello" + assert f"Executing step {alias}" in caplog.text @pytest.mark.parametrize("count", [1, 3]) @@ -1063,14 +1073,16 @@ async def test_condition_warning(hass, caplog): assert len(events) == 1 -async def test_condition_basic(hass): +async def test_condition_basic(hass, caplog): """Test if we can use conditions in a script.""" event = "test_event" events = async_capture_events(hass, event) + alias = "condition step" sequence = cv.SCRIPT_SCHEMA( [ {"event": event}, { + "alias": alias, "condition": "template", "value_template": "{{ states.test.entity.state == 'hello' }}", }, @@ -1083,6 +1095,8 @@ async def test_condition_basic(hass): await script_obj.async_run(context=Context()) await hass.async_block_till_done() + assert f"Test condition {alias}: True" in caplog.text + caplog.clear() assert len(events) == 2 hass.states.async_set("test.entity", "goodbye") @@ -1090,6 +1104,7 @@ async def test_condition_basic(hass): await script_obj.async_run(context=Context()) await hass.async_block_till_done() + assert f"Test condition {alias}: False" in caplog.text assert len(events) == 3 @@ -1140,14 +1155,16 @@ async def test_condition_all_cached(hass): assert len(script_obj._config_cache) == 2 -async def test_repeat_count(hass): +async def test_repeat_count(hass, caplog): """Test repeat action w/ count option.""" event = "test_event" events = async_capture_events(hass, event) count = 3 + alias = "condition step" sequence = cv.SCRIPT_SCHEMA( { + "alias": alias, "repeat": { "count": count, "sequence": { @@ -1158,7 +1175,7 @@ async def test_repeat_count(hass): "last": "{{ repeat.last }}", }, }, - } + }, } ) script_obj = script.Script(hass, sequence, "Test Name", "test_domain") @@ -1171,6 +1188,7 @@ async def test_repeat_count(hass): assert event.data.get("first") == (index == 0) assert event.data.get("index") == index + 1 assert event.data.get("last") == (index == count - 1) + assert caplog.text.count(f"Repeating {alias}") == count @pytest.mark.parametrize("condition", ["while", "until"]) @@ -1470,26 +1488,44 @@ async def test_choose_warning(hass, caplog): @pytest.mark.parametrize("var,result", [(1, "first"), (2, "second"), (3, "default")]) -async def test_choose(hass, var, result): +async def test_choose(hass, caplog, var, result): """Test choose action.""" event = "test_event" events = async_capture_events(hass, event) + alias = "choose step" + choice = {1: "choice one", 2: "choice two", 3: None} + aliases = {1: "sequence one", 2: "sequence two", 3: "default sequence"} sequence = cv.SCRIPT_SCHEMA( { + "alias": alias, "choose": [ { + "alias": choice[1], "conditions": { "condition": "template", "value_template": "{{ var == 1 }}", }, - "sequence": {"event": event, "event_data": {"choice": "first"}}, + "sequence": { + "alias": aliases[1], + "event": event, + "event_data": {"choice": "first"}, + }, }, { + "alias": choice[2], "conditions": "{{ var == 2 }}", - "sequence": {"event": event, "event_data": {"choice": "second"}}, + "sequence": { + "alias": aliases[2], + "event": event, + "event_data": {"choice": "second"}, + }, }, ], - "default": {"event": event, "event_data": {"choice": "default"}}, + "default": { + "alias": aliases[3], + "event": event, + "event_data": {"choice": "default"}, + }, } ) script_obj = script.Script(hass, sequence, "Test Name", "test_domain") @@ -1499,6 +1535,10 @@ async def test_choose(hass, var, result): assert len(events) == 1 assert events[0].data["choice"] == result + expected_choice = choice[var] + if var == 3: + expected_choice = "default" + assert f"{alias}: {expected_choice}: Executing step {aliases[var]}" in caplog.text @pytest.mark.parametrize( @@ -2115,9 +2155,10 @@ def started_action(): async def test_set_variable(hass, caplog): """Test setting variables in scripts.""" + alias = "variables step" sequence = cv.SCRIPT_SCHEMA( [ - {"variables": {"variable": "value"}}, + {"alias": alias, "variables": {"variable": "value"}}, {"service": "test.script", "data": {"value": "{{ variable }}"}}, ] ) @@ -2129,6 +2170,7 @@ async def test_set_variable(hass, caplog): await hass.async_block_till_done() assert mock_calls[0].data["value"] == "value" + assert f"Executing step {alias}" in caplog.text async def test_set_redefines_variable(hass, caplog): From 219671d572220a572ed399c5be16a7d1bf64e60a Mon Sep 17 00:00:00 2001 From: Erik Date: Tue, 16 Feb 2021 21:00:22 +0100 Subject: [PATCH 4/9] Add base schemas for script actions and conditions --- homeassistant/helpers/config_validation.py | 44 ++++++++++++---------- 1 file changed, 24 insertions(+), 20 deletions(-) diff --git a/homeassistant/helpers/config_validation.py b/homeassistant/helpers/config_validation.py index 6e2e3a6928d6b..6b0737ae34698 100644 --- a/homeassistant/helpers/config_validation.py +++ b/homeassistant/helpers/config_validation.py @@ -888,9 +888,11 @@ def script_action(value: Any) -> dict: SCRIPT_SCHEMA = vol.All(ensure_list, [script_action]) +SCRIPT_ACTION_BASE_SCHEMA = {vol.Optional(CONF_ALIAS): string} + EVENT_SCHEMA = vol.Schema( { - vol.Optional(CONF_ALIAS): string, + **SCRIPT_ACTION_BASE_SCHEMA, vol.Required(CONF_EVENT): string, vol.Optional(CONF_EVENT_DATA): vol.All(dict, template_complex), vol.Optional(CONF_EVENT_DATA_TEMPLATE): vol.All(dict, template_complex), @@ -900,7 +902,7 @@ def script_action(value: Any) -> dict: SERVICE_SCHEMA = vol.All( vol.Schema( { - vol.Optional(CONF_ALIAS): string, + **SCRIPT_ACTION_BASE_SCHEMA, vol.Exclusive(CONF_SERVICE, "service name"): vol.Any( service, dynamic_template ), @@ -920,10 +922,12 @@ def script_action(value: Any) -> dict: vol.Coerce(float), vol.All(str, entity_domain("input_number")) ) +CONDITION_BASE_SCHEMA = {vol.Optional(CONF_ALIAS): string} + NUMERIC_STATE_CONDITION_SCHEMA = vol.All( vol.Schema( { - vol.Optional(CONF_ALIAS): string, + **CONDITION_BASE_SCHEMA, vol.Required(CONF_CONDITION): "numeric_state", vol.Required(CONF_ENTITY_ID): entity_ids, vol.Optional(CONF_ATTRIBUTE): str, @@ -936,7 +940,7 @@ def script_action(value: Any) -> dict: ) STATE_CONDITION_BASE_SCHEMA = { - vol.Optional(CONF_ALIAS): string, + **CONDITION_BASE_SCHEMA, vol.Required(CONF_CONDITION): "state", vol.Required(CONF_ENTITY_ID): entity_ids, vol.Optional(CONF_ATTRIBUTE): str, @@ -977,7 +981,7 @@ def STATE_CONDITION_SCHEMA(value: Any) -> dict: # pylint: disable=invalid-name SUN_CONDITION_SCHEMA = vol.All( vol.Schema( { - vol.Optional(CONF_ALIAS): string, + **CONDITION_BASE_SCHEMA, vol.Required(CONF_CONDITION): "sun", vol.Optional("before"): sun_event, vol.Optional("before_offset"): time_period, @@ -992,7 +996,7 @@ def STATE_CONDITION_SCHEMA(value: Any) -> dict: # pylint: disable=invalid-name TEMPLATE_CONDITION_SCHEMA = vol.Schema( { - vol.Optional(CONF_ALIAS): string, + **CONDITION_BASE_SCHEMA, vol.Required(CONF_CONDITION): "template", vol.Required(CONF_VALUE_TEMPLATE): template, } @@ -1001,7 +1005,7 @@ def STATE_CONDITION_SCHEMA(value: Any) -> dict: # pylint: disable=invalid-name TIME_CONDITION_SCHEMA = vol.All( vol.Schema( { - vol.Optional(CONF_ALIAS): string, + **CONDITION_BASE_SCHEMA, vol.Required(CONF_CONDITION): "time", "before": vol.Any(time, vol.All(str, entity_domain("input_datetime"))), "after": vol.Any(time, vol.All(str, entity_domain("input_datetime"))), @@ -1013,7 +1017,7 @@ def STATE_CONDITION_SCHEMA(value: Any) -> dict: # pylint: disable=invalid-name ZONE_CONDITION_SCHEMA = vol.Schema( { - vol.Optional(CONF_ALIAS): string, + **CONDITION_BASE_SCHEMA, vol.Required(CONF_CONDITION): "zone", vol.Required(CONF_ENTITY_ID): entity_ids, "zone": entity_ids, @@ -1025,7 +1029,7 @@ def STATE_CONDITION_SCHEMA(value: Any) -> dict: # pylint: disable=invalid-name AND_CONDITION_SCHEMA = vol.Schema( { - vol.Optional(CONF_ALIAS): string, + **CONDITION_BASE_SCHEMA, vol.Required(CONF_CONDITION): "and", vol.Required(CONF_CONDITIONS): vol.All( ensure_list, @@ -1037,7 +1041,7 @@ def STATE_CONDITION_SCHEMA(value: Any) -> dict: # pylint: disable=invalid-name OR_CONDITION_SCHEMA = vol.Schema( { - vol.Optional(CONF_ALIAS): string, + **CONDITION_BASE_SCHEMA, vol.Required(CONF_CONDITION): "or", vol.Required(CONF_CONDITIONS): vol.All( ensure_list, @@ -1049,7 +1053,7 @@ def STATE_CONDITION_SCHEMA(value: Any) -> dict: # pylint: disable=invalid-name NOT_CONDITION_SCHEMA = vol.Schema( { - vol.Optional(CONF_ALIAS): string, + **CONDITION_BASE_SCHEMA, vol.Required(CONF_CONDITION): "not", vol.Required(CONF_CONDITIONS): vol.All( ensure_list, @@ -1061,7 +1065,7 @@ def STATE_CONDITION_SCHEMA(value: Any) -> dict: # pylint: disable=invalid-name DEVICE_CONDITION_BASE_SCHEMA = vol.Schema( { - vol.Optional(CONF_ALIAS): string, + **CONDITION_BASE_SCHEMA, vol.Required(CONF_CONDITION): "device", vol.Required(CONF_DEVICE_ID): str, vol.Required(CONF_DOMAIN): str, @@ -1097,14 +1101,14 @@ def STATE_CONDITION_SCHEMA(value: Any) -> dict: # pylint: disable=invalid-name _SCRIPT_DELAY_SCHEMA = vol.Schema( { - vol.Optional(CONF_ALIAS): string, + **SCRIPT_ACTION_BASE_SCHEMA, vol.Required(CONF_DELAY): positive_time_period_template, } ) _SCRIPT_WAIT_TEMPLATE_SCHEMA = vol.Schema( { - vol.Optional(CONF_ALIAS): string, + **SCRIPT_ACTION_BASE_SCHEMA, vol.Required(CONF_WAIT_TEMPLATE): template, vol.Optional(CONF_TIMEOUT): positive_time_period_template, vol.Optional(CONF_CONTINUE_ON_TIMEOUT): boolean, @@ -1113,7 +1117,7 @@ def STATE_CONDITION_SCHEMA(value: Any) -> dict: # pylint: disable=invalid-name DEVICE_ACTION_BASE_SCHEMA = vol.Schema( { - vol.Optional(CONF_ALIAS): string, + **SCRIPT_ACTION_BASE_SCHEMA, vol.Required(CONF_DEVICE_ID): string, vol.Required(CONF_DOMAIN): str, } @@ -1122,12 +1126,12 @@ def STATE_CONDITION_SCHEMA(value: Any) -> dict: # pylint: disable=invalid-name DEVICE_ACTION_SCHEMA = DEVICE_ACTION_BASE_SCHEMA.extend({}, extra=vol.ALLOW_EXTRA) _SCRIPT_SCENE_SCHEMA = vol.Schema( - {vol.Optional(CONF_ALIAS): string, vol.Required(CONF_SCENE): entity_domain("scene")} + {**SCRIPT_ACTION_BASE_SCHEMA, vol.Required(CONF_SCENE): entity_domain("scene")} ) _SCRIPT_REPEAT_SCHEMA = vol.Schema( { - vol.Optional(CONF_ALIAS): string, + **SCRIPT_ACTION_BASE_SCHEMA, vol.Required(CONF_REPEAT): vol.All( { vol.Exclusive(CONF_COUNT, "repeat"): vol.Any(vol.Coerce(int), template), @@ -1146,7 +1150,7 @@ def STATE_CONDITION_SCHEMA(value: Any) -> dict: # pylint: disable=invalid-name _SCRIPT_CHOOSE_SCHEMA = vol.Schema( { - vol.Optional(CONF_ALIAS): string, + **SCRIPT_ACTION_BASE_SCHEMA, vol.Required(CONF_CHOOSE): vol.All( ensure_list, [ @@ -1165,7 +1169,7 @@ def STATE_CONDITION_SCHEMA(value: Any) -> dict: # pylint: disable=invalid-name _SCRIPT_WAIT_FOR_TRIGGER_SCHEMA = vol.Schema( { - vol.Optional(CONF_ALIAS): string, + **SCRIPT_ACTION_BASE_SCHEMA, vol.Required(CONF_WAIT_FOR_TRIGGER): TRIGGER_SCHEMA, vol.Optional(CONF_TIMEOUT): positive_time_period_template, vol.Optional(CONF_CONTINUE_ON_TIMEOUT): boolean, @@ -1174,7 +1178,7 @@ def STATE_CONDITION_SCHEMA(value: Any) -> dict: # pylint: disable=invalid-name _SCRIPT_SET_SCHEMA = vol.Schema( { - vol.Optional(CONF_ALIAS): string, + **SCRIPT_ACTION_BASE_SCHEMA, vol.Required(CONF_VARIABLES): SCRIPT_VARIABLES_SCHEMA, } ) From 1f5959bdb0f964c2e1dbbb4b93847c5932b16d75 Mon Sep 17 00:00:00 2001 From: Erik Date: Thu, 18 Feb 2021 07:58:50 +0100 Subject: [PATCH 5/9] Add alias to condition tests --- tests/helpers/test_condition.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/tests/helpers/test_condition.py b/tests/helpers/test_condition.py index 1fc1e07da5dfa..571b6c1ffd753 100644 --- a/tests/helpers/test_condition.py +++ b/tests/helpers/test_condition.py @@ -34,6 +34,7 @@ async def test_and_condition(hass): test = await condition.async_from_config( hass, { + "alias": "And Condition", "condition": "and", "conditions": [ { @@ -68,6 +69,7 @@ async def test_and_condition_with_template(hass): "condition": "and", "conditions": [ { + "alias": "Template Condition", "condition": "template", "value_template": '{{ states.sensor.temperature.state == "100" }}', }, @@ -95,6 +97,7 @@ async def test_or_condition(hass): test = await condition.async_from_config( hass, { + "alias": "Or Condition", "condition": "or", "conditions": [ { @@ -153,6 +156,7 @@ async def test_not_condition(hass): test = await condition.async_from_config( hass, { + "alias": "Not Condition", "condition": "not", "conditions": [ { @@ -430,6 +434,7 @@ async def test_multiple_states(hass): "condition": "and", "conditions": [ { + "alias": "State Condition", "condition": "state", "entity_id": "sensor.temperature", "state": ["100", "200"], @@ -699,6 +704,7 @@ async def test_numeric_state_multiple_entities(hass): "condition": "and", "conditions": [ { + "alias": "Numeric State Condition", "condition": "numeric_state", "entity_id": ["sensor.temperature_1", "sensor.temperature_2"], "below": 50, @@ -819,6 +825,7 @@ async def test_zone_multiple_entities(hass): "condition": "and", "conditions": [ { + "alias": "Zone Condition", "condition": "zone", "entity_id": ["device_tracker.person_1", "device_tracker.person_2"], "zone": "zone.home", From a93070b53473ed403963965caf73fc00edfc4476 Mon Sep 17 00:00:00 2001 From: Erik Date: Thu, 18 Feb 2021 08:12:24 +0100 Subject: [PATCH 6/9] Validate schema in time condition tests --- tests/helpers/test_condition.py | 29 +++++++++++++++++++---------- 1 file changed, 19 insertions(+), 10 deletions(-) diff --git a/tests/helpers/test_condition.py b/tests/helpers/test_condition.py index 571b6c1ffd753..39359df9bf2d7 100644 --- a/tests/helpers/test_condition.py +++ b/tests/helpers/test_condition.py @@ -221,36 +221,45 @@ async def test_not_condition_with_template(hass): async def test_time_window(hass): """Test time condition windows.""" - sixam = dt.parse_time("06:00:00") - sixpm = dt.parse_time("18:00:00") + sixam = "06:00:00" + sixpm = "18:00:00" + + test1 = await condition.async_from_config( + hass, + {"alias": "Time Cond", "condition": "time", "after": sixam, "before": sixpm}, + ) + test2 = await condition.async_from_config( + hass, + {"alias": "Time Cond", "condition": "time", "after": sixpm, "before": sixam}, + ) with patch( "homeassistant.helpers.condition.dt_util.now", return_value=dt.now().replace(hour=3), ): - assert not condition.time(hass, after=sixam, before=sixpm) - assert condition.time(hass, after=sixpm, before=sixam) + assert not test1(hass) + assert test2(hass) with patch( "homeassistant.helpers.condition.dt_util.now", return_value=dt.now().replace(hour=9), ): - assert condition.time(hass, after=sixam, before=sixpm) - assert not condition.time(hass, after=sixpm, before=sixam) + assert test1(hass) + assert not test2(hass) with patch( "homeassistant.helpers.condition.dt_util.now", return_value=dt.now().replace(hour=15), ): - assert condition.time(hass, after=sixam, before=sixpm) - assert not condition.time(hass, after=sixpm, before=sixam) + assert test1(hass) + assert not test2(hass) with patch( "homeassistant.helpers.condition.dt_util.now", return_value=dt.now().replace(hour=21), ): - assert not condition.time(hass, after=sixam, before=sixpm) - assert condition.time(hass, after=sixpm, before=sixam) + assert not test1(hass) + assert test2(hass) async def test_time_using_input_datetime(hass): From 454554487323176cfe767c61ebc0c5fc76c7ef51 Mon Sep 17 00:00:00 2001 From: Erik Date: Sun, 21 Feb 2021 00:43:21 +0100 Subject: [PATCH 7/9] Address review comment --- homeassistant/helpers/script.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/homeassistant/helpers/script.py b/homeassistant/helpers/script.py index 2d913ba550260..e231b6bec9413 100644 --- a/homeassistant/helpers/script.py +++ b/homeassistant/helpers/script.py @@ -235,9 +235,11 @@ def _log( msg, *args, level=level, **kwargs ) - def _step_log(self, default_message, delay=None): + def _step_log(self, default_message, timeout=None): self._script.last_action = self._action.get(CONF_ALIAS, default_message) - _timeout = "" if delay is None else f" (timeout: {timedelta(seconds=delay)})" + _timeout = ( + "" if timeout is None else f" (timeout: {timedelta(seconds=timeout)})" + ) self._log("Executing step %s%s", self._script.last_action, _timeout) async def async_run(self) -> None: From b4f2c4845704619d6c3d65bd32ef58d201988ffa Mon Sep 17 00:00:00 2001 From: Erik Date: Sun, 21 Feb 2021 01:39:53 +0100 Subject: [PATCH 8/9] Lint --- homeassistant/helpers/script.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/homeassistant/helpers/script.py b/homeassistant/helpers/script.py index e231b6bec9413..8f2fd9478e4db 100644 --- a/homeassistant/helpers/script.py +++ b/homeassistant/helpers/script.py @@ -18,7 +18,7 @@ cast, ) -from async_timeout import timeout +import async_timeout import voluptuous as vol from homeassistant import exceptions @@ -339,7 +339,7 @@ async def _async_delay_step(self): delay = delay.total_seconds() self._changed() try: - async with timeout(delay): + async with async_timeout.timeout(delay): await self._stop.wait() except asyncio.TimeoutError: pass @@ -383,7 +383,7 @@ def async_script_wait(entity_id, from_s, to_s): self._hass.async_create_task(flag.wait()) for flag in (self._stop, done) ] try: - async with timeout(delay) as to_context: + async with async_timeout.timeout(delay) as to_context: await asyncio.wait(tasks, return_when=asyncio.FIRST_COMPLETED) except asyncio.TimeoutError as ex: if not self._action.get(CONF_CONTINUE_ON_TIMEOUT, True): @@ -661,7 +661,7 @@ def log_cb(level, msg, **kwargs): self._hass.async_create_task(flag.wait()) for flag in (self._stop, done) ] try: - async with timeout(delay) as to_context: + async with async_timeout.timeout(delay) as to_context: await asyncio.wait(tasks, return_when=asyncio.FIRST_COMPLETED) except asyncio.TimeoutError as ex: if not self._action.get(CONF_CONTINUE_ON_TIMEOUT, True): From f593a0ddca6c8fa7ecd4159258094e6ad513dfc6 Mon Sep 17 00:00:00 2001 From: Erik Date: Sun, 21 Feb 2021 01:47:35 +0100 Subject: [PATCH 9/9] Tweak variable naming --- homeassistant/helpers/script.py | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/homeassistant/helpers/script.py b/homeassistant/helpers/script.py index 8f2fd9478e4db..2e8348bcaf847 100644 --- a/homeassistant/helpers/script.py +++ b/homeassistant/helpers/script.py @@ -347,13 +347,13 @@ async def _async_delay_step(self): async def _async_wait_template_step(self): """Handle a wait template.""" if CONF_TIMEOUT in self._action: - delay = self._get_pos_time_period_template(CONF_TIMEOUT).total_seconds() + timeout = self._get_pos_time_period_template(CONF_TIMEOUT).total_seconds() else: - delay = None + timeout = None - self._step_log("wait template", delay) + self._step_log("wait template", timeout) - self._variables["wait"] = {"remaining": delay, "completed": False} + self._variables["wait"] = {"remaining": timeout, "completed": False} wait_template = self._action[CONF_WAIT_TEMPLATE] wait_template.hass = self._hass @@ -367,7 +367,7 @@ async def _async_wait_template_step(self): def async_script_wait(entity_id, from_s, to_s): """Handle script after template condition is true.""" self._variables["wait"] = { - "remaining": to_context.remaining if to_context else delay, + "remaining": to_context.remaining if to_context else timeout, "completed": True, } done.set() @@ -383,7 +383,7 @@ def async_script_wait(entity_id, from_s, to_s): self._hass.async_create_task(flag.wait()) for flag in (self._stop, done) ] try: - async with async_timeout.timeout(delay) as to_context: + async with async_timeout.timeout(timeout) as to_context: await asyncio.wait(tasks, return_when=asyncio.FIRST_COMPLETED) except asyncio.TimeoutError as ex: if not self._action.get(CONF_CONTINUE_ON_TIMEOUT, True): @@ -622,20 +622,20 @@ async def _async_choose_step(self) -> None: async def _async_wait_for_trigger_step(self): """Wait for a trigger event.""" if CONF_TIMEOUT in self._action: - delay = self._get_pos_time_period_template(CONF_TIMEOUT).total_seconds() + timeout = self._get_pos_time_period_template(CONF_TIMEOUT).total_seconds() else: - delay = None + timeout = None - self._step_log("wait for trigger", delay) + self._step_log("wait for trigger", timeout) variables = {**self._variables} - self._variables["wait"] = {"remaining": delay, "trigger": None} + self._variables["wait"] = {"remaining": timeout, "trigger": None} done = asyncio.Event() async def async_done(variables, context=None): self._variables["wait"] = { - "remaining": to_context.remaining if to_context else delay, + "remaining": to_context.remaining if to_context else timeout, "trigger": variables["trigger"], } done.set() @@ -661,7 +661,7 @@ def log_cb(level, msg, **kwargs): self._hass.async_create_task(flag.wait()) for flag in (self._stop, done) ] try: - async with async_timeout.timeout(delay) as to_context: + async with async_timeout.timeout(timeout) as to_context: await asyncio.wait(tasks, return_when=asyncio.FIRST_COMPLETED) except asyncio.TimeoutError as ex: if not self._action.get(CONF_CONTINUE_ON_TIMEOUT, True):