Refactor Google Assistant query_device (#12022)

* google_assistant: Refactor query_device

The previous code had issues where domains could break out and end up
with weird brightness values and we weren't enforcing the `on` and
`oneline` keys in the response.

* google_assistant: Add media_player to query test

homeassistant/components/google_assistant/smart_home.py

@@ -37,6 +37,7 @@ from .const import (
 )
 
 HANDLERS = Registry()
+QUERY_HANDLERS = Registry()
 _LOGGER = logging.getLogger(__name__)
 
 # Mapping is [actions schema, primary trait, optional features]
@@ -177,120 +178,145 @@ def entity_to_device(entity: Entity, config: Config, units: UnitSystem):
     return device
 
 
-def query_device(entity: Entity, config: Config, units: UnitSystem) -> dict:
+def celsius(deg: Optional[float], units: UnitSystem) -> Optional[float]:
-    """Take an entity and return a properly formatted device object."""
+    """Convert a float to Celsius and rounds to one decimal place."""
-    def celsius(deg: Optional[float]) -> Optional[float]:
+    if deg is None:
-        """Convert a float to Celsius and rounds to one decimal place."""
+        return None
-        if deg is None:
+    return round(METRIC_SYSTEM.temperature(deg, units.temperature_unit), 1)
-            return None
-        return round(METRIC_SYSTEM.temperature(deg, units.temperature_unit), 1)
 
-    if entity.domain == sensor.DOMAIN:
-        entity_config = config.entity_config.get(entity.entity_id, {})
-        google_domain = entity_config.get(CONF_TYPE)
 
-        if google_domain == climate.DOMAIN:
+@QUERY_HANDLERS.register(sensor.DOMAIN)
-            # check if we have a string value to convert it to number
+def query_response_sensor(
-            value = entity.state
+        entity: Entity, config: Config, units: UnitSystem) -> dict:
-            if isinstance(entity.state, str):
+    """Convert a sensor entity to a QUERY response."""
-                try:
+    entity_config = config.entity_config.get(entity.entity_id, {})
-                    value = float(value)
+    google_domain = entity_config.get(CONF_TYPE)
-                except ValueError:
-                    value = None
-
-            if value is None:
-                raise SmartHomeError(
-                    ERROR_NOT_SUPPORTED,
-                    "Invalid value {} for the climate sensor"
-                    .format(entity.state)
-                )
-
-            # detect if we report temperature or humidity
-            unit_of_measurement = entity.attributes.get(
-                ATTR_UNIT_OF_MEASUREMENT,
-                units.temperature_unit
-            )
-            if unit_of_measurement in [TEMP_FAHRENHEIT, TEMP_CELSIUS]:
-                value = celsius(value)
-                attr = 'thermostatTemperatureAmbient'
-            elif unit_of_measurement == '%':
-                attr = 'thermostatHumidityAmbient'
-            else:
-                raise SmartHomeError(
-                    ERROR_NOT_SUPPORTED,
-                    "Unit {} is not supported by the climate sensor"
-                    .format(unit_of_measurement)
-                )
-
-            return {attr: value}
 
+    if google_domain != climate.DOMAIN:
         raise SmartHomeError(
             ERROR_NOT_SUPPORTED,
             "Sensor type {} is not supported".format(google_domain)
         )
 
-    if entity.domain == climate.DOMAIN:
+    # check if we have a string value to convert it to number
-        mode = entity.attributes.get(climate.ATTR_OPERATION_MODE).lower()
+    value = entity.state
-        if mode not in CLIMATE_SUPPORTED_MODES:
+    if isinstance(entity.state, str):
-            mode = 'heat'
+        try:
-        response = {
+            value = float(value)
-            'thermostatMode': mode,
+        except ValueError:
-            'thermostatTemperatureSetpoint':
+            value = None
-            celsius(entity.attributes.get(climate.ATTR_TEMPERATURE)),
-            'thermostatTemperatureAmbient':
-            celsius(entity.attributes.get(climate.ATTR_CURRENT_TEMPERATURE)),
-            'thermostatTemperatureSetpointHigh':
-            celsius(entity.attributes.get(climate.ATTR_TARGET_TEMP_HIGH)),
-            'thermostatTemperatureSetpointLow':
-            celsius(entity.attributes.get(climate.ATTR_TARGET_TEMP_LOW)),
-            'thermostatHumidityAmbient':
-            entity.attributes.get(climate.ATTR_CURRENT_HUMIDITY),
-        }
-        return {k: v for k, v in response.items() if v is not None}
 
-    final_state = entity.state != STATE_OFF
+    if value is None:
-    final_brightness = entity.attributes.get(light.ATTR_BRIGHTNESS, 255
+        raise SmartHomeError(
-                                             if final_state else 0)
+            ERROR_NOT_SUPPORTED,
+            "Invalid value {} for the climate sensor"
+            .format(entity.state)
+        )
 
-    if entity.domain == media_player.DOMAIN:
+    # detect if we report temperature or humidity
-        level = entity.attributes.get(media_player.ATTR_MEDIA_VOLUME_LEVEL, 1.0
+    unit_of_measurement = entity.attributes.get(
-                                      if final_state else 0.0)
+        ATTR_UNIT_OF_MEASUREMENT,
-        # Convert 0.0-1.0 to 0-255
+        units.temperature_unit
-        final_brightness = round(min(1.0, level) * 255)
+    )
+    if unit_of_measurement in [TEMP_FAHRENHEIT, TEMP_CELSIUS]:
+        value = celsius(value, units)
+        attr = 'thermostatTemperatureAmbient'
+    elif unit_of_measurement == '%':
+        attr = 'thermostatHumidityAmbient'
+    else:
+        raise SmartHomeError(
+            ERROR_NOT_SUPPORTED,
+            "Unit {} is not supported by the climate sensor"
+            .format(unit_of_measurement)
+        )
 
-    if final_brightness is None:
+    return {attr: value}
-        final_brightness = 255 if final_state else 0
 
-    final_brightness = 100 * (final_brightness / 255)
 
-    query_response = {
+@QUERY_HANDLERS.register(climate.DOMAIN)
-        "on": final_state,
+def query_response_climate(
-        "online": True,
+        entity: Entity, config: Config, units: UnitSystem) -> dict:
-        "brightness": int(final_brightness)
+    """Convert a climate entity to a QUERY response."""
+    mode = entity.attributes.get(climate.ATTR_OPERATION_MODE).lower()
+    if mode not in CLIMATE_SUPPORTED_MODES:
+        mode = 'heat'
+    attrs = entity.attributes
+    response = {
+        'thermostatMode': mode,
+        'thermostatTemperatureSetpoint':
+        celsius(attrs.get(climate.ATTR_TEMPERATURE), units),
+        'thermostatTemperatureAmbient':
+        celsius(attrs.get(climate.ATTR_CURRENT_TEMPERATURE), units),
+        'thermostatTemperatureSetpointHigh':
+        celsius(attrs.get(climate.ATTR_TARGET_TEMP_HIGH), units),
+        'thermostatTemperatureSetpointLow':
+        celsius(attrs.get(climate.ATTR_TARGET_TEMP_LOW), units),
+        'thermostatHumidityAmbient':
+        attrs.get(climate.ATTR_CURRENT_HUMIDITY),
     }
+    return {k: v for k, v in response.items() if v is not None}
+
+
+@QUERY_HANDLERS.register(media_player.DOMAIN)
+def query_response_media_player(
+        entity: Entity, config: Config, units: UnitSystem) -> dict:
+    """Convert a media_player entity to a QUERY response."""
+    level = entity.attributes.get(
+        media_player.ATTR_MEDIA_VOLUME_LEVEL,
+        1.0 if entity.state != STATE_OFF else 0.0)
+    # Convert 0.0-1.0 to 0-255
+    brightness = int(level * 100)
+
+    return {'brightness': brightness}
+
+
+@QUERY_HANDLERS.register(light.DOMAIN)
+def query_response_light(
+        entity: Entity, config: Config, units: UnitSystem) -> dict:
+    """Convert a light entity to a QUERY response."""
+    response = {}  # type: Dict[str, Any]
+
+    brightness = entity.attributes.get(light.ATTR_BRIGHTNESS)
+    if brightness is not None:
+        response['brightness'] = int(100 * (brightness / 255))
 
     supported_features = entity.attributes.get(ATTR_SUPPORTED_FEATURES, 0)
     if supported_features & \
        (light.SUPPORT_COLOR_TEMP | light.SUPPORT_RGB_COLOR):
-        query_response["color"] = {}
+        response['color'] = {}
 
         if entity.attributes.get(light.ATTR_COLOR_TEMP) is not None:
-            query_response["color"]["temperature"] = \
+            response['color']['temperature'] = \
                 int(round(color.color_temperature_mired_to_kelvin(
                     entity.attributes.get(light.ATTR_COLOR_TEMP))))
 
         if entity.attributes.get(light.ATTR_COLOR_NAME) is not None:
-            query_response["color"]["name"] = \
+            response['color']['name'] = \
                 entity.attributes.get(light.ATTR_COLOR_NAME)
 
         if entity.attributes.get(light.ATTR_RGB_COLOR) is not None:
             color_rgb = entity.attributes.get(light.ATTR_RGB_COLOR)
             if color_rgb is not None:
-                query_response["color"]["spectrumRGB"] = \
+                response['color']['spectrumRGB'] = \
                     int(color.color_rgb_to_hex(
                         color_rgb[0], color_rgb[1], color_rgb[2]), 16)
 
-    return query_response
+    return response
+
+
+def query_device(entity: Entity, config: Config, units: UnitSystem) -> dict:
+    """Take an entity and return a properly formatted device object."""
+    state = entity.state != STATE_OFF
+    defaults = {
+        'on': state,
+        'online': True
+    }
+
+    handler = QUERY_HANDLERS.get(entity.domain)
+    if callable(handler):
+        defaults.update(handler(entity, config, units))
+
+    return defaults
 
 
 # erroneous bug on old pythons and pylint
@@ -438,11 +464,11 @@ def async_devices_query(hass, config, payload):
         if not state:
             # If we can't find a state, the device is offline
             devices[devid] = {'online': False}
-
+        else:
-        try:
+            try:
-            devices[devid] = query_device(state, config, hass.config.units)
+                devices[devid] = query_device(state, config, hass.config.units)
-        except SmartHomeError as error:
+            except SmartHomeError as error:
-            devices[devid] = {'errorCode': error.code}
+                devices[devid] = {'errorCode': error.code}
 
     return {'devices': devices}
 

tests/components/google_assistant/test_google_assistant.py

@@ -175,6 +175,8 @@ def test_query_request(hass_fixture, assistant_client):
                     'id': "light.bed_light",
                 }, {
                     'id': "light.kitchen_lights",
+                }, {
+                    'id': 'media_player.lounge_room',
                 }]
             }
         }]
@@ -187,12 +189,14 @@ def test_query_request(hass_fixture, assistant_client):
     body = yield from result.json()
     assert body.get('requestId') == reqid
     devices = body['payload']['devices']
-    assert len(devices) == 3
+    assert len(devices) == 4
     assert devices['light.bed_light']['on'] is False
     assert devices['light.ceiling_lights']['on'] is True
     assert devices['light.ceiling_lights']['brightness'] == 70
     assert devices['light.kitchen_lights']['color']['spectrumRGB'] == 16727919
     assert devices['light.kitchen_lights']['color']['temperature'] == 4166
+    assert devices['media_player.lounge_room']['on'] is True
+    assert devices['media_player.lounge_room']['brightness'] == 100
 
 
 @asyncio.coroutine
@@ -225,26 +229,36 @@ def test_query_climate_request(hass_fixture, assistant_client):
     devices = body['payload']['devices']
     assert devices == {
         'climate.heatpump': {
+            'on': True,
+            'online': True,
             'thermostatTemperatureSetpoint': 20.0,
             'thermostatTemperatureAmbient': 25.0,
             'thermostatMode': 'heat',
         },
         'climate.ecobee': {
+            'on': True,
+            'online': True,
             'thermostatTemperatureSetpointHigh': 24,
             'thermostatTemperatureAmbient': 23,
             'thermostatMode': 'heat',
             'thermostatTemperatureSetpointLow': 21
         },
         'climate.hvac': {
+            'on': True,
+            'online': True,
             'thermostatTemperatureSetpoint': 21,
             'thermostatTemperatureAmbient': 22,
             'thermostatMode': 'cool',
             'thermostatHumidityAmbient': 54,
         },
         'sensor.outside_temperature': {
+            'on': True,
+            'online': True,
             'thermostatTemperatureAmbient': 15.6
         },
         'sensor.outside_humidity': {
+            'on': True,
+            'online': True,
             'thermostatHumidityAmbient': 54.0
         }
     }
@@ -280,23 +294,31 @@ def test_query_climate_request_f(hass_fixture, assistant_client):
     devices = body['payload']['devices']
     assert devices == {
         'climate.heatpump': {
+            'on': True,
+            'online': True,
             'thermostatTemperatureSetpoint': -6.7,
             'thermostatTemperatureAmbient': -3.9,
             'thermostatMode': 'heat',
         },
         'climate.ecobee': {
+            'on': True,
+            'online': True,
             'thermostatTemperatureSetpointHigh': -4.4,
             'thermostatTemperatureAmbient': -5,
             'thermostatMode': 'heat',
             'thermostatTemperatureSetpointLow': -6.1,
         },
         'climate.hvac': {
+            'on': True,
+            'online': True,
             'thermostatTemperatureSetpoint': -6.1,
             'thermostatTemperatureAmbient': -5.6,
             'thermostatMode': 'cool',
             'thermostatHumidityAmbient': 54,
         },
         'sensor.outside_temperature': {
+            'on': True,
+            'online': True,
             'thermostatTemperatureAmbient': -9.1
         }
     }
@@ -317,6 +339,8 @@ def test_execute_request(hass_fixture, assistant_client):
                         "id": "light.ceiling_lights",
                     }, {
                         "id": "switch.decorative_lights",
+                    }, {
+                        "id": "media_player.lounge_room",
                     }],
                     "execution": [{
                         "command": "action.devices.commands.OnOff",
@@ -324,6 +348,17 @@ def test_execute_request(hass_fixture, assistant_client):
                             "on": False
                         }
                     }]
+                }, {
+                    "devices": [{
+                        "id": "media_player.walkman",
+                    }],
+                    "execution": [{
+                        "command":
+                        "action.devices.commands.BrightnessAbsolute",
+                        "params": {
+                            "brightness": 70
+                        }
+                    }]
                 }, {
                     "devices": [{
                         "id": "light.kitchen_lights",
@@ -380,7 +415,7 @@ def test_execute_request(hass_fixture, assistant_client):
     body = yield from result.json()
     assert body.get('requestId') == reqid
     commands = body['payload']['commands']
-    assert len(commands) == 6
+    assert len(commands) == 8
 
     ceiling = hass_fixture.states.get('light.ceiling_lights')
     assert ceiling.state == 'off'
@@ -394,3 +429,10 @@ def test_execute_request(hass_fixture, assistant_client):
     assert bed.attributes.get(light.ATTR_RGB_COLOR) == (0, 255, 0)
 
     assert hass_fixture.states.get('switch.decorative_lights').state == 'off'
+
+    walkman = hass_fixture.states.get('media_player.walkman')
+    assert walkman.state == 'playing'
+    assert walkman.attributes.get(media_player.ATTR_MEDIA_VOLUME_LEVEL) == 0.7
+
+    lounge = hass_fixture.states.get('media_player.lounge_room')
+    assert lounge.state == 'off'