diff --git a/homeassistant/components/google_assistant/smart_home.py b/homeassistant/components/google_assistant/smart_home.py index c9f6c20c7af..eb7b5e9c9eb 100644 --- a/homeassistant/components/google_assistant/smart_home.py +++ b/homeassistant/components/google_assistant/smart_home.py @@ -17,6 +17,8 @@ from .const import ( from .error import SmartHomeError from .helpers import GoogleEntity, RequestData, async_get_entities +EXECUTE_LIMIT = 2 # Wait 2 seconds for execute to finish + HANDLERS = Registry() _LOGGER = logging.getLogger(__name__) @@ -207,16 +209,23 @@ async def handle_devices_execute(hass, data, payload): entities[entity_id] = GoogleEntity(hass, data.config, state) executions[entity_id] = [execution] - execute_results = await asyncio.gather( - *( - _entity_execute(entities[entity_id], data, execution) - for entity_id, execution in executions.items() + try: + execute_results = await asyncio.wait_for( + asyncio.shield( + asyncio.gather( + *( + _entity_execute(entities[entity_id], data, execution) + for entity_id, execution in executions.items() + ) + ) + ), + EXECUTE_LIMIT, ) - ) - - for entity_id, result in zip(executions, execute_results): - if result is not None: - results[entity_id] = result + for entity_id, result in zip(executions, execute_results): + if result is not None: + results[entity_id] = result + except asyncio.TimeoutError: + pass final_results = list(results.values()) diff --git a/homeassistant/components/google_assistant/trait.py b/homeassistant/components/google_assistant/trait.py index 5801ae6811b..30ea244bac9 100644 --- a/homeassistant/components/google_assistant/trait.py +++ b/homeassistant/components/google_assistant/trait.py @@ -264,7 +264,7 @@ class BrightnessTrait(_Trait): ATTR_ENTITY_ID: self.state.entity_id, light.ATTR_BRIGHTNESS_PCT: params["brightness"], }, - blocking=True, + blocking=not self.config.should_report_state, context=data.context, ) @@ -359,7 +359,7 @@ class OnOffTrait(_Trait): service_domain, service, {ATTR_ENTITY_ID: self.state.entity_id}, - blocking=True, + blocking=not self.config.should_report_state, context=data.context, ) @@ -464,7 +464,7 @@ class ColorSettingTrait(_Trait): light.DOMAIN, SERVICE_TURN_ON, {ATTR_ENTITY_ID: self.state.entity_id, light.ATTR_COLOR_TEMP: temp}, - blocking=True, + blocking=not self.config.should_report_state, context=data.context, ) @@ -479,7 +479,7 @@ class ColorSettingTrait(_Trait): light.DOMAIN, SERVICE_TURN_ON, {ATTR_ENTITY_ID: self.state.entity_id, light.ATTR_HS_COLOR: color}, - blocking=True, + blocking=not self.config.should_report_state, context=data.context, ) @@ -496,7 +496,7 @@ class ColorSettingTrait(_Trait): light.ATTR_HS_COLOR: [color["hue"], saturation], light.ATTR_BRIGHTNESS: brightness, }, - blocking=True, + blocking=not self.config.should_report_state, context=data.context, ) @@ -536,7 +536,8 @@ class SceneTrait(_Trait): self.state.domain, service, {ATTR_ENTITY_ID: self.state.entity_id}, - blocking=self.state.domain not in (button.DOMAIN, script.DOMAIN), + blocking=(not self.config.should_report_state) + and self.state.domain not in (button.DOMAIN, script.DOMAIN), context=data.context, ) @@ -570,7 +571,7 @@ class DockTrait(_Trait): self.state.domain, vacuum.SERVICE_RETURN_TO_BASE, {ATTR_ENTITY_ID: self.state.entity_id}, - blocking=True, + blocking=not self.config.should_report_state, context=data.context, ) @@ -610,7 +611,7 @@ class LocatorTrait(_Trait): self.state.domain, vacuum.SERVICE_LOCATE, {ATTR_ENTITY_ID: self.state.entity_id}, - blocking=True, + blocking=not self.config.should_report_state, context=data.context, ) @@ -731,7 +732,7 @@ class StartStopTrait(_Trait): self.state.domain, vacuum.SERVICE_START, {ATTR_ENTITY_ID: self.state.entity_id}, - blocking=True, + blocking=not self.config.should_report_state, context=data.context, ) else: @@ -739,7 +740,7 @@ class StartStopTrait(_Trait): self.state.domain, vacuum.SERVICE_STOP, {ATTR_ENTITY_ID: self.state.entity_id}, - blocking=True, + blocking=not self.config.should_report_state, context=data.context, ) elif command == COMMAND_PAUSEUNPAUSE: @@ -748,7 +749,7 @@ class StartStopTrait(_Trait): self.state.domain, vacuum.SERVICE_PAUSE, {ATTR_ENTITY_ID: self.state.entity_id}, - blocking=True, + blocking=not self.config.should_report_state, context=data.context, ) else: @@ -756,7 +757,7 @@ class StartStopTrait(_Trait): self.state.domain, vacuum.SERVICE_START, {ATTR_ENTITY_ID: self.state.entity_id}, - blocking=True, + blocking=not self.config.should_report_state, context=data.context, ) @@ -776,7 +777,7 @@ class StartStopTrait(_Trait): self.state.domain, cover.SERVICE_STOP_COVER, {ATTR_ENTITY_ID: self.state.entity_id}, - blocking=True, + blocking=not self.config.should_report_state, context=data.context, ) else: @@ -993,7 +994,7 @@ class TemperatureSettingTrait(_Trait): climate.DOMAIN, climate.SERVICE_SET_TEMPERATURE, {ATTR_ENTITY_ID: self.state.entity_id, ATTR_TEMPERATURE: temp}, - blocking=True, + blocking=not self.config.should_report_state, context=data.context, ) @@ -1041,7 +1042,7 @@ class TemperatureSettingTrait(_Trait): climate.DOMAIN, climate.SERVICE_SET_TEMPERATURE, svc_data, - blocking=True, + blocking=not self.config.should_report_state, context=data.context, ) @@ -1054,7 +1055,7 @@ class TemperatureSettingTrait(_Trait): climate.DOMAIN, SERVICE_TURN_ON, {ATTR_ENTITY_ID: self.state.entity_id}, - blocking=True, + blocking=not self.config.should_report_state, context=data.context, ) return @@ -1064,7 +1065,7 @@ class TemperatureSettingTrait(_Trait): climate.DOMAIN, SERVICE_TURN_OFF, {ATTR_ENTITY_ID: self.state.entity_id}, - blocking=True, + blocking=not self.config.should_report_state, context=data.context, ) return @@ -1077,7 +1078,7 @@ class TemperatureSettingTrait(_Trait): climate.ATTR_PRESET_MODE: self.google_to_preset[target_mode], ATTR_ENTITY_ID: self.state.entity_id, }, - blocking=True, + blocking=not self.config.should_report_state, context=data.context, ) return @@ -1089,7 +1090,7 @@ class TemperatureSettingTrait(_Trait): ATTR_ENTITY_ID: self.state.entity_id, climate.ATTR_HVAC_MODE: self.google_to_hvac[target_mode], }, - blocking=True, + blocking=not self.config.should_report_state, context=data.context, ) @@ -1170,7 +1171,7 @@ class HumiditySettingTrait(_Trait): ATTR_ENTITY_ID: self.state.entity_id, humidifier.ATTR_HUMIDITY: params["humidity"], }, - blocking=True, + blocking=not self.config.should_report_state, context=data.context, ) @@ -1219,7 +1220,7 @@ class LockUnlockTrait(_Trait): lock.DOMAIN, service, {ATTR_ENTITY_ID: self.state.entity_id}, - blocking=True, + blocking=not self.config.should_report_state, context=data.context, ) @@ -1342,7 +1343,7 @@ class ArmDisArmTrait(_Trait): ATTR_ENTITY_ID: self.state.entity_id, ATTR_CODE: data.config.secure_devices_pin, }, - blocking=True, + blocking=not self.config.should_report_state, context=data.context, ) @@ -1430,7 +1431,7 @@ class FanSpeedTrait(_Trait): ATTR_ENTITY_ID: self.state.entity_id, climate.ATTR_FAN_MODE: params["fanSpeed"], }, - blocking=True, + blocking=not self.config.should_report_state, context=data.context, ) @@ -1442,7 +1443,7 @@ class FanSpeedTrait(_Trait): ATTR_ENTITY_ID: self.state.entity_id, fan.ATTR_PERCENTAGE: params["fanSpeedPercent"], }, - blocking=True, + blocking=not self.config.should_report_state, context=data.context, ) @@ -1458,7 +1459,7 @@ class FanSpeedTrait(_Trait): fan.DOMAIN, fan.SERVICE_SET_DIRECTION, {ATTR_ENTITY_ID: self.state.entity_id, fan.ATTR_DIRECTION: direction}, - blocking=True, + blocking=not self.config.should_report_state, context=data.context, ) @@ -1599,7 +1600,7 @@ class ModesTrait(_Trait): ATTR_ENTITY_ID: self.state.entity_id, fan.ATTR_PRESET_MODE: preset_mode, }, - blocking=True, + blocking=not self.config.should_report_state, context=data.context, ) return @@ -1613,7 +1614,7 @@ class ModesTrait(_Trait): ATTR_ENTITY_ID: self.state.entity_id, input_select.ATTR_OPTION: option, }, - blocking=True, + blocking=not self.config.should_report_state, context=data.context, ) return @@ -1627,7 +1628,7 @@ class ModesTrait(_Trait): ATTR_ENTITY_ID: self.state.entity_id, select.ATTR_OPTION: option, }, - blocking=True, + blocking=not self.config.should_report_state, context=data.context, ) return @@ -1641,7 +1642,7 @@ class ModesTrait(_Trait): ATTR_MODE: requested_mode, ATTR_ENTITY_ID: self.state.entity_id, }, - blocking=True, + blocking=not self.config.should_report_state, context=data.context, ) return @@ -1655,7 +1656,7 @@ class ModesTrait(_Trait): ATTR_ENTITY_ID: self.state.entity_id, light.ATTR_EFFECT: requested_effect, }, - blocking=True, + blocking=not self.config.should_report_state, context=data.context, ) return @@ -1670,7 +1671,7 @@ class ModesTrait(_Trait): ATTR_ENTITY_ID: self.state.entity_id, media_player.ATTR_SOUND_MODE: sound_mode, }, - blocking=True, + blocking=not self.config.should_report_state, context=data.context, ) @@ -1744,7 +1745,7 @@ class InputSelectorTrait(_Trait): ATTR_ENTITY_ID: self.state.entity_id, media_player.ATTR_INPUT_SOURCE: requested_source, }, - blocking=True, + blocking=not self.config.should_report_state, context=data.context, ) @@ -1888,7 +1889,11 @@ class OpenCloseTrait(_Trait): _verify_pin_challenge(data, self.state, challenge) await self.hass.services.async_call( - cover.DOMAIN, service, svc_params, blocking=True, context=data.context + cover.DOMAIN, + service, + svc_params, + blocking=not self.config.should_report_state, + context=data.context, ) @@ -1950,7 +1955,7 @@ class VolumeTrait(_Trait): ATTR_ENTITY_ID: self.state.entity_id, media_player.ATTR_MEDIA_VOLUME_LEVEL: level, }, - blocking=True, + blocking=not self.config.should_report_state, context=data.context, ) @@ -1986,7 +1991,7 @@ class VolumeTrait(_Trait): media_player.DOMAIN, svc, {ATTR_ENTITY_ID: self.state.entity_id}, - blocking=True, + blocking=not self.config.should_report_state, context=data.context, ) else: @@ -2008,7 +2013,7 @@ class VolumeTrait(_Trait): ATTR_ENTITY_ID: self.state.entity_id, media_player.ATTR_MEDIA_VOLUME_MUTED: mute, }, - blocking=True, + blocking=not self.config.should_report_state, context=data.context, ) @@ -2170,7 +2175,7 @@ class TransportControlTrait(_Trait): media_player.DOMAIN, service, service_attrs, - blocking=True, + blocking=not self.config.should_report_state, context=data.context, ) @@ -2275,7 +2280,7 @@ class ChannelTrait(_Trait): media_player.ATTR_MEDIA_CONTENT_ID: channel_number, media_player.ATTR_MEDIA_CONTENT_TYPE: MEDIA_TYPE_CHANNEL, }, - blocking=True, + blocking=not self.config.should_report_state, context=data.context, ) diff --git a/tests/components/google_assistant/__init__.py b/tests/components/google_assistant/__init__.py index f7537db18de..562bd4e16cd 100644 --- a/tests/components/google_assistant/__init__.py +++ b/tests/components/google_assistant/__init__.py @@ -20,25 +20,27 @@ class MockConfig(helpers.AbstractConfig): def __init__( self, *, - secure_devices_pin=None, - should_expose=None, - should_2fa=None, + agent_user_ids=None, + enabled=True, entity_config=None, hass=None, - local_sdk_webhook_id=None, local_sdk_user_id=None, - enabled=True, - agent_user_ids=None, + local_sdk_webhook_id=None, + secure_devices_pin=None, + should_2fa=None, + should_expose=None, + should_report_state=False, ): """Initialize config.""" super().__init__(hass) - self._should_expose = should_expose - self._should_2fa = should_2fa - self._secure_devices_pin = secure_devices_pin - self._entity_config = entity_config or {} - self._local_sdk_webhook_id = local_sdk_webhook_id - self._local_sdk_user_id = local_sdk_user_id self._enabled = enabled + self._entity_config = entity_config or {} + self._local_sdk_user_id = local_sdk_user_id + self._local_sdk_webhook_id = local_sdk_webhook_id + self._secure_devices_pin = secure_devices_pin + self._should_2fa = should_2fa + self._should_expose = should_expose + self._should_report_state = should_report_state self._store = mock_google_config_store(agent_user_ids) @property @@ -74,6 +76,11 @@ class MockConfig(helpers.AbstractConfig): """Expose it all.""" return self._should_expose is None or self._should_expose(state) + @property + def should_report_state(self): + """Return if states should be proactively reported.""" + return self._should_report_state + def should_2fa(self, state): """Expose it all.""" return self._should_2fa is None or self._should_2fa(state) diff --git a/tests/components/google_assistant/test_smart_home.py b/tests/components/google_assistant/test_smart_home.py index 9d259290956..911f66bb428 100644 --- a/tests/components/google_assistant/test_smart_home.py +++ b/tests/components/google_assistant/test_smart_home.py @@ -1,5 +1,6 @@ """Test Google Smart Home.""" -from unittest.mock import patch +import asyncio +from unittest.mock import ANY, call, patch import pytest @@ -25,7 +26,7 @@ from homeassistant.components.google_assistant import ( from homeassistant.config import async_process_ha_core_config from homeassistant.const import ATTR_UNIT_OF_MEASUREMENT, TEMP_CELSIUS, __version__ from homeassistant.core import EVENT_CALL_SERVICE, State -from homeassistant.helpers import device_registry +from homeassistant.helpers import device_registry, entity_platform from homeassistant.setup import async_setup_component from . import BASIC_CONFIG, MockConfig @@ -367,7 +368,10 @@ async def test_query_message(hass): } -async def test_execute(hass): +@pytest.mark.parametrize( + "report_state,on,brightness,value", [(False, True, 20, 0.2), (True, ANY, ANY, ANY)] +) +async def test_execute(hass, report_state, on, brightness, value): """Test an execute command.""" await async_setup_component(hass, "light", {"light": {"platform": "demo"}}) await hass.async_block_till_done() @@ -375,45 +379,82 @@ async def test_execute(hass): await hass.services.async_call( "light", "turn_off", {"entity_id": "light.ceiling_lights"}, blocking=True ) + await hass.async_block_till_done() events = async_capture_events(hass, EVENT_COMMAND_RECEIVED) service_events = async_capture_events(hass, EVENT_CALL_SERVICE) - result = await sh.async_handle_message( - hass, - BASIC_CONFIG, - None, - { - "requestId": REQ_ID, - "inputs": [ - { - "intent": "action.devices.EXECUTE", - "payload": { - "commands": [ - { - "devices": [ - {"id": "light.non_existing"}, - {"id": "light.ceiling_lights"}, - {"id": "light.kitchen_lights"}, - ], - "execution": [ - { - "command": "action.devices.commands.OnOff", - "params": {"on": True}, - }, - { - "command": "action.devices.commands.BrightnessAbsolute", - "params": {"brightness": 20}, - }, - ], - } - ] - }, - } - ], - }, - const.SOURCE_CLOUD, - ) + with patch.object( + hass.services, "async_call", wraps=hass.services.async_call + ) as call_service_mock: + result = await sh.async_handle_message( + hass, + MockConfig(should_report_state=report_state), + None, + { + "requestId": REQ_ID, + "inputs": [ + { + "intent": "action.devices.EXECUTE", + "payload": { + "commands": [ + { + "devices": [ + {"id": "light.non_existing"}, + {"id": "light.ceiling_lights"}, + {"id": "light.kitchen_lights"}, + ], + "execution": [ + { + "command": "action.devices.commands.OnOff", + "params": {"on": True}, + }, + { + "command": "action.devices.commands.BrightnessAbsolute", + "params": {"brightness": 20}, + }, + ], + } + ] + }, + } + ], + }, + const.SOURCE_CLOUD, + ) + assert call_service_mock.call_count == 4 + expected_calls = [ + call( + "light", + "turn_on", + {"entity_id": "light.ceiling_lights"}, + blocking=not report_state, + context=ANY, + ), + call( + "light", + "turn_on", + {"entity_id": "light.kitchen_lights"}, + blocking=not report_state, + context=ANY, + ), + call( + "light", + "turn_on", + {"entity_id": "light.ceiling_lights", "brightness_pct": 20}, + blocking=not report_state, + context=ANY, + ), + call( + "light", + "turn_on", + {"entity_id": "light.kitchen_lights", "brightness_pct": 20}, + blocking=not report_state, + context=ANY, + ), + ] + call_service_mock.assert_has_awaits(expected_calls, any_order=True) + await hass.async_block_till_done() assert result == { "requestId": REQ_ID, @@ -428,9 +469,9 @@ async def test_execute(hass): "ids": ["light.ceiling_lights"], "status": "SUCCESS", "states": { - "on": True, + "on": on, "online": True, - "brightness": 20, + "brightness": brightness, "color": {"temperatureK": 2631}, }, }, @@ -440,12 +481,12 @@ async def test_execute(hass): "states": { "on": True, "online": True, - "brightness": 20, + "brightness": brightness, "color": { "spectrumHsv": { "hue": 345, "saturation": 0.75, - "value": 0.2, + "value": value, }, }, }, @@ -506,6 +547,209 @@ async def test_execute(hass): assert service_events[3].context == events[0].context +@pytest.mark.parametrize("report_state,on,brightness,value", [(False, False, ANY, ANY)]) +async def test_execute_times_out(hass, report_state, on, brightness, value): + """Test an execute command which times out.""" + orig_execute_limit = sh.EXECUTE_LIMIT + sh.EXECUTE_LIMIT = 0.02 # Decrease timeout to 20ms + await async_setup_component(hass, "light", {"light": {"platform": "demo"}}) + await hass.async_block_till_done() + + await hass.services.async_call( + "light", "turn_off", {"entity_id": "light.ceiling_lights"}, blocking=True + ) + await hass.async_block_till_done() + + events = async_capture_events(hass, EVENT_COMMAND_RECEIVED) + service_events = async_capture_events(hass, EVENT_CALL_SERVICE) + + platforms = entity_platform.async_get_platforms(hass, "demo") + assert platforms[0].domain == "light" + assert platforms[0].entities["light.ceiling_lights"] + + turn_on_wait = asyncio.Event() + + async def slow_turn_on(*args, **kwargs): + # Make DemoLigt.async_turn_on hang waiting for the turn_on_wait event + await turn_on_wait.wait(), + + with patch.object( + hass.services, "async_call", wraps=hass.services.async_call + ) as call_service_mock, patch.object( + DemoLight, "async_turn_on", wraps=slow_turn_on + ): + result = await sh.async_handle_message( + hass, + MockConfig(should_report_state=report_state), + None, + { + "requestId": REQ_ID, + "inputs": [ + { + "intent": "action.devices.EXECUTE", + "payload": { + "commands": [ + { + "devices": [ + {"id": "light.non_existing"}, + {"id": "light.ceiling_lights"}, + {"id": "light.kitchen_lights"}, + ], + "execution": [ + { + "command": "action.devices.commands.OnOff", + "params": {"on": True}, + }, + { + "command": "action.devices.commands.BrightnessAbsolute", + "params": {"brightness": 20}, + }, + ], + } + ] + }, + } + ], + }, + const.SOURCE_CLOUD, + ) + # Only the two first calls are executed + assert call_service_mock.call_count == 2 + expected_calls = [ + call( + "light", + "turn_on", + {"entity_id": "light.ceiling_lights"}, + blocking=not report_state, + context=ANY, + ), + call( + "light", + "turn_on", + {"entity_id": "light.kitchen_lights"}, + blocking=not report_state, + context=ANY, + ), + ] + call_service_mock.assert_has_awaits(expected_calls, any_order=True) + + turn_on_wait.set() + await hass.async_block_till_done() + # The remaining two calls should now have executed + assert call_service_mock.call_count == 4 + expected_calls.extend( + [ + call( + "light", + "turn_on", + {"entity_id": "light.ceiling_lights", "brightness_pct": 20}, + blocking=not report_state, + context=ANY, + ), + call( + "light", + "turn_on", + {"entity_id": "light.kitchen_lights", "brightness_pct": 20}, + blocking=not report_state, + context=ANY, + ), + ] + ) + call_service_mock.assert_has_awaits(expected_calls, any_order=True) + await hass.async_block_till_done() + + assert result == { + "requestId": REQ_ID, + "payload": { + "commands": [ + { + "ids": ["light.non_existing"], + "status": "ERROR", + "errorCode": "deviceOffline", + }, + { + "ids": ["light.ceiling_lights"], + "status": "SUCCESS", + "states": { + "on": on, + "online": True, + "brightness": brightness, + }, + }, + { + "ids": ["light.kitchen_lights"], + "status": "SUCCESS", + "states": { + "on": True, + "online": True, + "brightness": brightness, + "color": { + "spectrumHsv": { + "hue": 345, + "saturation": 0.75, + "value": value, + }, + }, + }, + }, + ] + }, + } + + assert len(events) == 1 + assert events[0].event_type == EVENT_COMMAND_RECEIVED + assert events[0].data == { + "request_id": REQ_ID, + "entity_id": [ + "light.non_existing", + "light.ceiling_lights", + "light.kitchen_lights", + ], + "execution": [ + { + "command": "action.devices.commands.OnOff", + "params": {"on": True}, + }, + { + "command": "action.devices.commands.BrightnessAbsolute", + "params": {"brightness": 20}, + }, + ], + "source": "cloud", + } + + service_events = sorted( + service_events, key=lambda ev: ev.data["service_data"]["entity_id"] + ) + assert len(service_events) == 4 + assert service_events[0].data == { + "domain": "light", + "service": "turn_on", + "service_data": {"entity_id": "light.ceiling_lights"}, + } + assert service_events[1].data == { + "domain": "light", + "service": "turn_on", + "service_data": {"brightness_pct": 20, "entity_id": "light.ceiling_lights"}, + } + assert service_events[0].context == events[0].context + assert service_events[1].context == events[0].context + assert service_events[2].data == { + "domain": "light", + "service": "turn_on", + "service_data": {"entity_id": "light.kitchen_lights"}, + } + assert service_events[3].data == { + "domain": "light", + "service": "turn_on", + "service_data": {"brightness_pct": 20, "entity_id": "light.kitchen_lights"}, + } + assert service_events[2].context == events[0].context + assert service_events[3].context == events[0].context + + sh.EXECUTE_LIMIT = orig_execute_limit + + async def test_raising_error_trait(hass): """Test raising an error while executing a trait command.""" hass.states.async_set(