Don't wait for Google Assistant service calls when reporting state (#59832)

* Don't wait for Google Assistant service calls when reporting state

* Update tests

* Add test
This commit is contained in:
Erik Montnemery 2021-11-29 18:34:38 +01:00 committed by GitHub
parent d980ca7e04
commit 814a742518
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 366 additions and 101 deletions

View File

@ -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())

View File

@ -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,
)

View File

@ -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)

View File

@ -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(