From 7e7267f8229e8429e09b8177784b5e52b0abef93 Mon Sep 17 00:00:00 2001 From: Paulus Schoutsen Date: Tue, 20 Apr 2021 09:21:52 -0700 Subject: [PATCH] Send only a single event per incoming Google command (#49449) --- .../components/google_assistant/logbook.py | 21 +-- .../components/google_assistant/smart_home.py | 46 +++---- .../google_assistant/test_logbook.py | 30 +++-- .../google_assistant/test_smart_home.py | 121 +++++------------- 4 files changed, 85 insertions(+), 133 deletions(-) diff --git a/homeassistant/components/google_assistant/logbook.py b/homeassistant/components/google_assistant/logbook.py index ef2bccd2c65..86caa8a9e6c 100644 --- a/homeassistant/components/google_assistant/logbook.py +++ b/homeassistant/components/google_assistant/logbook.py @@ -1,8 +1,7 @@ """Describe logbook events.""" -from homeassistant.const import ATTR_ENTITY_ID from homeassistant.core import callback -from .const import DOMAIN, EVENT_COMMAND_RECEIVED +from .const import DOMAIN, EVENT_COMMAND_RECEIVED, SOURCE_CLOUD COMMON_COMMAND_PREFIX = "action.devices.commands." @@ -14,16 +13,18 @@ def async_describe_events(hass, async_describe_event): @callback def async_describe_logbook_event(event): """Describe a logbook event.""" - entity_id = event.data[ATTR_ENTITY_ID] - state = hass.states.get(entity_id) - name = state.name if state else entity_id + commands = [] - command = event.data["execution"]["command"] - if command.startswith(COMMON_COMMAND_PREFIX): - command = command[len(COMMON_COMMAND_PREFIX) :] + for command_payload in event.data["execution"]: + command = command_payload["command"] + if command.startswith(COMMON_COMMAND_PREFIX): + command = command[len(COMMON_COMMAND_PREFIX) :] + commands.append(command) - message = f"sent command {command} for {name} (via {event.data['source']})" + message = f"sent command {', '.join(commands)}" + if event.data["source"] != SOURCE_CLOUD: + message += f" (via {event.data['source']})" - return {"name": "Google Assistant", "message": message, "entity_id": entity_id} + return {"name": "Google Assistant", "message": message} async_describe_event(DOMAIN, EVENT_COMMAND_RECEIVED, async_describe_logbook_event) diff --git a/homeassistant/components/google_assistant/smart_home.py b/homeassistant/components/google_assistant/smart_home.py index a9a97f047e9..747dc234efe 100644 --- a/homeassistant/components/google_assistant/smart_home.py +++ b/homeassistant/components/google_assistant/smart_home.py @@ -116,21 +116,23 @@ async def async_devices_query(hass, data, payload): https://developers.google.com/assistant/smarthome/develop/process-intents#QUERY """ + payload_devices = payload.get("devices", []) + + hass.bus.async_fire( + EVENT_QUERY_RECEIVED, + { + "request_id": data.request_id, + ATTR_ENTITY_ID: [device["id"] for device in payload_devices], + "source": data.source, + }, + context=data.context, + ) + devices = {} - for device in payload.get("devices", []): + for device in payload_devices: devid = device["id"] state = hass.states.get(devid) - hass.bus.async_fire( - EVENT_QUERY_RECEIVED, - { - "request_id": data.request_id, - ATTR_ENTITY_ID: devid, - "source": data.source, - }, - context=data.context, - ) - if not state: # If we can't find a state, the device is offline devices[devid] = {"online": False} @@ -175,20 +177,20 @@ async def handle_devices_execute(hass, data, payload): results = {} for command in payload["commands"]: + hass.bus.async_fire( + EVENT_COMMAND_RECEIVED, + { + "request_id": data.request_id, + ATTR_ENTITY_ID: [device["id"] for device in command["devices"]], + "execution": command["execution"], + "source": data.source, + }, + context=data.context, + ) + for device, execution in product(command["devices"], command["execution"]): entity_id = device["id"] - hass.bus.async_fire( - EVENT_COMMAND_RECEIVED, - { - "request_id": data.request_id, - ATTR_ENTITY_ID: entity_id, - "execution": execution, - "source": data.source, - }, - context=data.context, - ) - # Happens if error occurred. Skip entity for further processing if entity_id in results: continue diff --git a/tests/components/google_assistant/test_logbook.py b/tests/components/google_assistant/test_logbook.py index 4f996ba038f..09d0b12e417 100644 --- a/tests/components/google_assistant/test_logbook.py +++ b/tests/components/google_assistant/test_logbook.py @@ -32,11 +32,13 @@ async def test_humanify_command_received(hass): EVENT_COMMAND_RECEIVED, { "request_id": "abcd", - ATTR_ENTITY_ID: "light.kitchen", - "execution": { - "command": "action.devices.commands.OnOff", - "params": {"on": True}, - }, + ATTR_ENTITY_ID: ["light.kitchen"], + "execution": [ + { + "command": "action.devices.commands.OnOff", + "params": {"on": True}, + } + ], "source": SOURCE_LOCAL, }, ), @@ -44,11 +46,13 @@ async def test_humanify_command_received(hass): EVENT_COMMAND_RECEIVED, { "request_id": "abcd", - ATTR_ENTITY_ID: "light.non_existing", - "execution": { - "command": "action.devices.commands.OnOff", - "params": {"on": False}, - }, + ATTR_ENTITY_ID: ["light.non_existing"], + "execution": [ + { + "command": "action.devices.commands.OnOff", + "params": {"on": False}, + } + ], "source": SOURCE_CLOUD, }, ), @@ -63,10 +67,8 @@ async def test_humanify_command_received(hass): assert event1["name"] == "Google Assistant" assert event1["domain"] == DOMAIN - assert event1["message"] == "sent command OnOff for The Kitchen Lights (via local)" - assert event1["entity_id"] == "light.kitchen" + assert event1["message"] == "sent command OnOff (via local)" assert event2["name"] == "Google Assistant" assert event2["domain"] == DOMAIN - assert event2["message"] == "sent command OnOff for light.non_existing (via cloud)" - assert event2["entity_id"] == "light.non_existing" + assert event2["message"] == "sent command OnOff" diff --git a/tests/components/google_assistant/test_smart_home.py b/tests/components/google_assistant/test_smart_home.py index 0dfa9e2a5e9..161b4a6f3cb 100644 --- a/tests/components/google_assistant/test_smart_home.py +++ b/tests/components/google_assistant/test_smart_home.py @@ -353,29 +353,16 @@ async def test_query_message(hass): await hass.async_block_till_done() - assert len(events) == 4 + assert len(events) == 1 assert events[0].event_type == EVENT_QUERY_RECEIVED assert events[0].data == { "request_id": REQ_ID, - "entity_id": "light.demo_light", - "source": "cloud", - } - assert events[1].event_type == EVENT_QUERY_RECEIVED - assert events[1].data == { - "request_id": REQ_ID, - "entity_id": "light.another_light", - "source": "cloud", - } - assert events[2].event_type == EVENT_QUERY_RECEIVED - assert events[2].data == { - "request_id": REQ_ID, - "entity_id": "light.color_temp_light", - "source": "cloud", - } - assert events[3].event_type == EVENT_QUERY_RECEIVED - assert events[3].data == { - "request_id": REQ_ID, - "entity_id": "light.non_existing", + "entity_id": [ + "light.demo_light", + "light.another_light", + "light.color_temp_light", + "light.non_existing", + ], "source": "cloud", } @@ -467,65 +454,25 @@ async def test_execute(hass): }, } - assert len(events) == 6 + 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", - "execution": { - "command": "action.devices.commands.OnOff", - "params": {"on": True}, - }, - "source": "cloud", - } - assert events[1].event_type == EVENT_COMMAND_RECEIVED - assert events[1].data == { - "request_id": REQ_ID, - "entity_id": "light.non_existing", - "execution": { - "command": "action.devices.commands.BrightnessAbsolute", - "params": {"brightness": 20}, - }, - "source": "cloud", - } - assert events[2].event_type == EVENT_COMMAND_RECEIVED - assert events[2].data == { - "request_id": REQ_ID, - "entity_id": "light.ceiling_lights", - "execution": { - "command": "action.devices.commands.OnOff", - "params": {"on": True}, - }, - "source": "cloud", - } - assert events[3].event_type == EVENT_COMMAND_RECEIVED - assert events[3].data == { - "request_id": REQ_ID, - "entity_id": "light.ceiling_lights", - "execution": { - "command": "action.devices.commands.BrightnessAbsolute", - "params": {"brightness": 20}, - }, - "source": "cloud", - } - assert events[4].event_type == EVENT_COMMAND_RECEIVED - assert events[4].data == { - "request_id": REQ_ID, - "entity_id": "light.kitchen_lights", - "execution": { - "command": "action.devices.commands.OnOff", - "params": {"on": True}, - }, - "source": "cloud", - } - assert events[5].event_type == EVENT_COMMAND_RECEIVED - assert events[5].data == { - "request_id": REQ_ID, - "entity_id": "light.kitchen_lights", - "execution": { - "command": "action.devices.commands.BrightnessAbsolute", - "params": {"brightness": 20}, - }, + "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", } @@ -543,9 +490,8 @@ async def test_execute(hass): "service": "turn_on", "service_data": {"brightness_pct": 20, "entity_id": "light.ceiling_lights"}, } - assert service_events[0].context == events[2].context - assert service_events[1].context == events[2].context - assert service_events[1].context == events[3].context + 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", @@ -556,9 +502,8 @@ async def test_execute(hass): "service": "turn_on", "service_data": {"brightness_pct": 20, "entity_id": "light.kitchen_lights"}, } - assert service_events[2].context == events[4].context - assert service_events[3].context == events[4].context - assert service_events[3].context == events[5].context + assert service_events[2].context == events[0].context + assert service_events[3].context == events[0].context async def test_raising_error_trait(hass): @@ -618,11 +563,13 @@ async def test_raising_error_trait(hass): assert events[0].event_type == EVENT_COMMAND_RECEIVED assert events[0].data == { "request_id": REQ_ID, - "entity_id": "climate.bla", - "execution": { - "command": "action.devices.commands.ThermostatTemperatureSetpoint", - "params": {"thermostatTemperatureSetpoint": 10}, - }, + "entity_id": ["climate.bla"], + "execution": [ + { + "command": "action.devices.commands.ThermostatTemperatureSetpoint", + "params": {"thermostatTemperatureSetpoint": 10}, + } + ], "source": "cloud", }