From 8e441ba03b9006f7a678c8364538473c5e55fea7 Mon Sep 17 00:00:00 2001 From: Phil Kates Date: Tue, 30 Jan 2018 01:19:24 -0800 Subject: [PATCH] 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 --- .../components/google_assistant/smart_home.py | 200 ++++++++++-------- .../google_assistant/test_google_assistant.py | 46 +++- 2 files changed, 157 insertions(+), 89 deletions(-) diff --git a/homeassistant/components/google_assistant/smart_home.py b/homeassistant/components/google_assistant/smart_home.py index 0cc6f9c3f83..b718c009160 100644 --- a/homeassistant/components/google_assistant/smart_home.py +++ b/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: - """Take an entity and return a properly formatted device object.""" - def celsius(deg: Optional[float]) -> Optional[float]: - """Convert a float to Celsius and rounds to one decimal place.""" - if deg is None: - return None - return round(METRIC_SYSTEM.temperature(deg, units.temperature_unit), 1) +def celsius(deg: Optional[float], units: UnitSystem) -> Optional[float]: + """Convert a float to Celsius and rounds to one decimal place.""" + if deg is None: + 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: - # check if we have a string value to convert it to number - value = entity.state - if isinstance(entity.state, str): - try: - value = float(value) - 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} +@QUERY_HANDLERS.register(sensor.DOMAIN) +def query_response_sensor( + entity: Entity, config: Config, units: UnitSystem) -> dict: + """Convert a sensor entity to a QUERY response.""" + entity_config = config.entity_config.get(entity.entity_id, {}) + google_domain = entity_config.get(CONF_TYPE) + if google_domain != climate.DOMAIN: raise SmartHomeError( ERROR_NOT_SUPPORTED, "Sensor type {} is not supported".format(google_domain) ) - if entity.domain == climate.DOMAIN: - mode = entity.attributes.get(climate.ATTR_OPERATION_MODE).lower() - if mode not in CLIMATE_SUPPORTED_MODES: - mode = 'heat' - response = { - 'thermostatMode': mode, - 'thermostatTemperatureSetpoint': - 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} + # check if we have a string value to convert it to number + value = entity.state + if isinstance(entity.state, str): + try: + value = float(value) + except ValueError: + value = None - final_state = entity.state != STATE_OFF - final_brightness = entity.attributes.get(light.ATTR_BRIGHTNESS, 255 - if final_state else 0) + if value is None: + raise SmartHomeError( + ERROR_NOT_SUPPORTED, + "Invalid value {} for the climate sensor" + .format(entity.state) + ) - if entity.domain == media_player.DOMAIN: - level = entity.attributes.get(media_player.ATTR_MEDIA_VOLUME_LEVEL, 1.0 - if final_state else 0.0) - # Convert 0.0-1.0 to 0-255 - final_brightness = round(min(1.0, level) * 255) + # 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, 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: - final_brightness = 255 if final_state else 0 + return {attr: value} - final_brightness = 100 * (final_brightness / 255) - query_response = { - "on": final_state, - "online": True, - "brightness": int(final_brightness) +@QUERY_HANDLERS.register(climate.DOMAIN) +def query_response_climate( + entity: Entity, config: Config, units: UnitSystem) -> dict: + """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} - - try: - devices[devid] = query_device(state, config, hass.config.units) - except SmartHomeError as error: - devices[devid] = {'errorCode': error.code} + else: + try: + devices[devid] = query_device(state, config, hass.config.units) + except SmartHomeError as error: + devices[devid] = {'errorCode': error.code} return {'devices': devices} diff --git a/tests/components/google_assistant/test_google_assistant.py b/tests/components/google_assistant/test_google_assistant.py index 9fc35bc17b1..43c36d1ca2a 100644 --- a/tests/components/google_assistant/test_google_assistant.py +++ b/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'