From 48fd95c7dbfa0b870dc5f18e315801c6174981cf Mon Sep 17 00:00:00 2001 From: Kevin Eifinger Date: Tue, 12 Nov 2019 15:04:17 +0100 Subject: [PATCH] Fix Here Travel Time unable to find entity on startup (#27237) * add support for entity_id as state of entity * add circular reference detection * voluptuous instead of regex * wait for EVENT_HOMEASSISTANT_START * move delayed_sensor_update to async_added_to_hass * add @callback decorator * remove nested entity resolving --- .../components/here_travel_time/sensor.py | 75 ++++++--- .../here_travel_time/test_sensor.py | 149 +++++++++++++++++- 2 files changed, 198 insertions(+), 26 deletions(-) diff --git a/homeassistant/components/here_travel_time/sensor.py b/homeassistant/components/here_travel_time/sensor.py index afd12bb01c5..0b688a770c5 100755 --- a/homeassistant/components/here_travel_time/sensor.py +++ b/homeassistant/components/here_travel_time/sensor.py @@ -17,8 +17,9 @@ from homeassistant.const import ( CONF_UNIT_SYSTEM, CONF_UNIT_SYSTEM_IMPERIAL, CONF_UNIT_SYSTEM_METRIC, + EVENT_HOMEASSISTANT_START, ) -from homeassistant.core import HomeAssistant, State +from homeassistant.core import HomeAssistant, State, callback from homeassistant.helpers import location import homeassistant.helpers.config_validation as cv from homeassistant.helpers.entity import Entity @@ -89,8 +90,6 @@ UNIT_OF_MEASUREMENT = "min" SCAN_INTERVAL = timedelta(minutes=5) -TRACKABLE_DOMAINS = ["device_tracker", "sensor", "zone", "person"] - NO_ROUTE_ERROR_MESSAGE = "HERE could not find a route based on the input" PLATFORM_SCHEMA = vol.All( @@ -146,15 +145,19 @@ async def async_setup_platform( if config.get(CONF_ORIGIN_LATITUDE) is not None: origin = f"{config[CONF_ORIGIN_LATITUDE]},{config[CONF_ORIGIN_LONGITUDE]}" + origin_entity_id = None else: - origin = config[CONF_ORIGIN_ENTITY_ID] + origin = None + origin_entity_id = config[CONF_ORIGIN_ENTITY_ID] if config.get(CONF_DESTINATION_LATITUDE) is not None: destination = ( f"{config[CONF_DESTINATION_LATITUDE]},{config[CONF_DESTINATION_LONGITUDE]}" ) + destination_entity_id = None else: - destination = config[CONF_DESTINATION_ENTITY_ID] + destination = None + destination_entity_id = config[CONF_DESTINATION_ENTITY_ID] travel_mode = config[CONF_MODE] traffic_mode = config[CONF_TRAFFIC_MODE] @@ -166,9 +169,11 @@ async def async_setup_platform( here_client, travel_mode, traffic_mode, route_mode, units ) - sensor = HERETravelTimeSensor(name, origin, destination, here_data) + sensor = HERETravelTimeSensor( + name, origin, destination, origin_entity_id, destination_entity_id, here_data + ) - async_add_entities([sensor], True) + async_add_entities([sensor]) def _are_valid_client_credentials(here_client: herepy.RoutingApi) -> bool: @@ -194,31 +199,43 @@ class HERETravelTimeSensor(Entity): """Representation of a HERE travel time sensor.""" def __init__( - self, name: str, origin: str, destination: str, here_data: "HERETravelTimeData" + self, + name: str, + origin: str, + destination: str, + origin_entity_id: str, + destination_entity_id: str, + here_data: "HERETravelTimeData", ) -> None: """Initialize the sensor.""" self._name = name + self._origin_entity_id = origin_entity_id + self._destination_entity_id = destination_entity_id self._here_data = here_data self._unit_of_measurement = UNIT_OF_MEASUREMENT - self._origin_entity_id = None - self._destination_entity_id = None self._attrs = { ATTR_UNIT_SYSTEM: self._here_data.units, ATTR_MODE: self._here_data.travel_mode, ATTR_TRAFFIC_MODE: self._here_data.traffic_mode, } - - # Check if location is a trackable entity - if origin.split(".", 1)[0] in TRACKABLE_DOMAINS: - self._origin_entity_id = origin - else: + if self._origin_entity_id is None: self._here_data.origin = origin - if destination.split(".", 1)[0] in TRACKABLE_DOMAINS: - self._destination_entity_id = destination - else: + if self._destination_entity_id is None: self._here_data.destination = destination + async def async_added_to_hass(self) -> None: + """Delay the sensor update to avoid entity not found warnings.""" + + @callback + def delayed_sensor_update(event): + """Update sensor after homeassistant started.""" + self.async_schedule_update_ha_state(True) + + self.hass.bus.async_listen_once( + EVENT_HOMEASSISTANT_START, delayed_sensor_update + ) + @property def state(self) -> Optional[str]: """Return the state of the sensor.""" @@ -309,10 +326,28 @@ class HERETravelTimeSensor(Entity): ) return self._get_location_from_attributes(zone_entity) - # If zone was not found in state then use the state as the location - if entity_id.startswith("sensor."): + # Check if state is valid coordinate set + if self._entity_state_is_valid_coordinate_set(entity.state): return entity.state + _LOGGER.error( + "The state of %s is not a valid set of coordinates: %s", + entity_id, + entity.state, + ) + return None + + @staticmethod + def _entity_state_is_valid_coordinate_set(state: str) -> bool: + """Check that the given string is a valid set of coordinates.""" + schema = vol.Schema(cv.gps) + try: + coordinates = state.split(",") + schema(coordinates) + return True + except (vol.MultipleInvalid): + return False + @staticmethod def _get_location_from_attributes(entity: State) -> str: """Get the lat/long string from an entities attributes.""" diff --git a/tests/components/here_travel_time/test_sensor.py b/tests/components/here_travel_time/test_sensor.py index 783209690a3..1d788c93c66 100644 --- a/tests/components/here_travel_time/test_sensor.py +++ b/tests/components/here_travel_time/test_sensor.py @@ -38,7 +38,7 @@ from homeassistant.components.here_travel_time.sensor import ( TRAVEL_MODE_TRUCK, UNIT_OF_MEASUREMENT, ) -from homeassistant.const import ATTR_ICON +from homeassistant.const import ATTR_ICON, EVENT_HOMEASSISTANT_START from homeassistant.setup import async_setup_component import homeassistant.util.dt as dt_util @@ -176,13 +176,14 @@ async def test_car(hass, requests_mock_car_disabled_response): "app_code": APP_CODE, } } - assert await async_setup_component(hass, DOMAIN, config) + hass.bus.async_fire(EVENT_HOMEASSISTANT_START) + await hass.async_block_till_done() + sensor = hass.states.get("sensor.test") assert sensor.state == "30" assert sensor.attributes.get("unit_of_measurement") == UNIT_OF_MEASUREMENT - assert sensor.attributes.get(ATTR_ATTRIBUTION) is None assert sensor.attributes.get(ATTR_DURATION) == 30.05 assert sensor.attributes.get(ATTR_DISTANCE) == 23.903 @@ -241,8 +242,10 @@ async def test_traffic_mode_enabled(hass, requests_mock_credentials_check): } assert await async_setup_component(hass, DOMAIN, config) - sensor = hass.states.get("sensor.test") + hass.bus.async_fire(EVENT_HOMEASSISTANT_START) + await hass.async_block_till_done() + sensor = hass.states.get("sensor.test") # Test traffic mode enabled assert sensor.attributes.get(ATTR_DURATION) != sensor.attributes.get( ATTR_DURATION_IN_TRAFFIC @@ -266,6 +269,9 @@ async def test_imperial(hass, requests_mock_car_disabled_response): } assert await async_setup_component(hass, DOMAIN, config) + hass.bus.async_fire(EVENT_HOMEASSISTANT_START) + await hass.async_block_till_done() + sensor = hass.states.get("sensor.test") assert sensor.attributes.get(ATTR_DISTANCE) == 14.852635608048994 @@ -295,6 +301,9 @@ async def test_route_mode_shortest(hass, requests_mock_credentials_check): } assert await async_setup_component(hass, DOMAIN, config) + hass.bus.async_fire(EVENT_HOMEASSISTANT_START) + await hass.async_block_till_done() + sensor = hass.states.get("sensor.test") assert sensor.attributes.get(ATTR_DISTANCE) == 18.388 @@ -324,6 +333,9 @@ async def test_route_mode_fastest(hass, requests_mock_credentials_check): } assert await async_setup_component(hass, DOMAIN, config) + hass.bus.async_fire(EVENT_HOMEASSISTANT_START) + await hass.async_block_till_done() + sensor = hass.states.get("sensor.test") assert sensor.attributes.get(ATTR_DISTANCE) == 23.381 @@ -344,6 +356,10 @@ async def test_truck(hass, requests_mock_truck_response): } } assert await async_setup_component(hass, DOMAIN, config) + + hass.bus.async_fire(EVENT_HOMEASSISTANT_START) + await hass.async_block_till_done() + sensor = hass.states.get("sensor.test") _assert_truck_sensor(sensor) @@ -373,6 +389,9 @@ async def test_public_transport(hass, requests_mock_credentials_check): } assert await async_setup_component(hass, DOMAIN, config) + hass.bus.async_fire(EVENT_HOMEASSISTANT_START) + await hass.async_block_till_done() + sensor = hass.states.get("sensor.test") assert sensor.state == "89" assert sensor.attributes.get("unit_of_measurement") == UNIT_OF_MEASUREMENT @@ -421,6 +440,9 @@ async def test_public_transport_time_table(hass, requests_mock_credentials_check } assert await async_setup_component(hass, DOMAIN, config) + hass.bus.async_fire(EVENT_HOMEASSISTANT_START) + await hass.async_block_till_done() + sensor = hass.states.get("sensor.test") assert sensor.state == "80" assert sensor.attributes.get("unit_of_measurement") == UNIT_OF_MEASUREMENT @@ -466,8 +488,12 @@ async def test_pedestrian(hass, requests_mock_credentials_check): "mode": TRAVEL_MODE_PEDESTRIAN, } } + assert await async_setup_component(hass, DOMAIN, config) + hass.bus.async_fire(EVENT_HOMEASSISTANT_START) + await hass.async_block_till_done() + sensor = hass.states.get("sensor.test") assert sensor.state == "211" assert sensor.attributes.get("unit_of_measurement") == UNIT_OF_MEASUREMENT @@ -517,6 +543,9 @@ async def test_bicycle(hass, requests_mock_credentials_check): } assert await async_setup_component(hass, DOMAIN, config) + hass.bus.async_fire(EVENT_HOMEASSISTANT_START) + await hass.async_block_till_done() + sensor = hass.states.get("sensor.test") assert sensor.state == "55" assert sensor.attributes.get("unit_of_measurement") == UNIT_OF_MEASUREMENT @@ -564,8 +593,6 @@ async def test_location_zone(hass, requests_mock_truck_response): }, ] } - assert await async_setup_component(hass, "zone", zone_config) - config = { DOMAIN: { "platform": PLATFORM, @@ -577,8 +604,12 @@ async def test_location_zone(hass, requests_mock_truck_response): "mode": TRAVEL_MODE_TRUCK, } } + assert await async_setup_component(hass, "zone", zone_config) assert await async_setup_component(hass, DOMAIN, config) + hass.bus.async_fire(EVENT_HOMEASSISTANT_START) + await hass.async_block_till_done() + sensor = hass.states.get("sensor.test") _assert_truck_sensor(sensor) @@ -616,6 +647,9 @@ async def test_location_sensor(hass, requests_mock_truck_response): } assert await async_setup_component(hass, DOMAIN, config) + hass.bus.async_fire(EVENT_HOMEASSISTANT_START) + await hass.async_block_till_done() + sensor = hass.states.get("sensor.test") _assert_truck_sensor(sensor) @@ -662,6 +696,9 @@ async def test_location_person(hass, requests_mock_truck_response): } assert await async_setup_component(hass, DOMAIN, config) + hass.bus.async_fire(EVENT_HOMEASSISTANT_START) + await hass.async_block_till_done() + sensor = hass.states.get("sensor.test") _assert_truck_sensor(sensor) @@ -708,6 +745,9 @@ async def test_location_device_tracker(hass, requests_mock_truck_response): } assert await async_setup_component(hass, DOMAIN, config) + hass.bus.async_fire(EVENT_HOMEASSISTANT_START) + await hass.async_block_till_done() + sensor = hass.states.get("sensor.test") _assert_truck_sensor(sensor) @@ -740,6 +780,9 @@ async def test_location_device_tracker_added_after_update( } assert await async_setup_component(hass, DOMAIN, config) + hass.bus.async_fire(EVENT_HOMEASSISTANT_START) + await hass.async_block_till_done() + sensor = hass.states.get("sensor.test") assert len(caplog.records) == 2 assert "Unable to find entity" in caplog.text @@ -806,6 +849,9 @@ async def test_location_device_tracker_in_zone( } assert await async_setup_component(hass, DOMAIN, config) + hass.bus.async_fire(EVENT_HOMEASSISTANT_START) + await hass.async_block_till_done() + sensor = hass.states.get("sensor.test") _assert_truck_sensor(sensor) assert ", getting zone location" in caplog.text @@ -836,6 +882,10 @@ async def test_route_not_found(hass, requests_mock_credentials_check, caplog): } } assert await async_setup_component(hass, DOMAIN, config) + + hass.bus.async_fire(EVENT_HOMEASSISTANT_START) + await hass.async_block_till_done() + assert len(caplog.records) == 1 assert NO_ROUTE_ERROR_MESSAGE in caplog.text @@ -940,8 +990,95 @@ async def test_attribution(hass, requests_mock_credentials_check): } } assert await async_setup_component(hass, DOMAIN, config) + + hass.bus.async_fire(EVENT_HOMEASSISTANT_START) + await hass.async_block_till_done() + sensor = hass.states.get("sensor.test") assert ( sensor.attributes.get(ATTR_ATTRIBUTION) == "With the support of HERE Technologies. All information is provided without warranty of any kind." ) + + +async def test_pattern_entity_state(hass, requests_mock_truck_response, caplog): + """Test that pattern matching the state of an entity works.""" + caplog.set_level(logging.ERROR) + hass.states.async_set("sensor.origin", "invalid") + + config = { + DOMAIN: { + "platform": PLATFORM, + "name": "test", + "origin_entity_id": "sensor.origin", + "destination_latitude": TRUCK_DESTINATION_LATITUDE, + "destination_longitude": TRUCK_DESTINATION_LONGITUDE, + "app_id": APP_ID, + "app_code": APP_CODE, + "mode": TRAVEL_MODE_TRUCK, + } + } + assert await async_setup_component(hass, DOMAIN, config) + + hass.bus.async_fire(EVENT_HOMEASSISTANT_START) + await hass.async_block_till_done() + + assert len(caplog.records) == 1 + assert "is not a valid set of coordinates" in caplog.text + + +async def test_pattern_entity_state_with_space(hass, requests_mock_truck_response): + """Test that pattern matching the state including a space of an entity works.""" + hass.states.async_set( + "sensor.origin", ", ".join([TRUCK_ORIGIN_LATITUDE, TRUCK_ORIGIN_LONGITUDE]) + ) + + config = { + DOMAIN: { + "platform": PLATFORM, + "name": "test", + "origin_entity_id": "sensor.origin", + "destination_latitude": TRUCK_DESTINATION_LATITUDE, + "destination_longitude": TRUCK_DESTINATION_LONGITUDE, + "app_id": APP_ID, + "app_code": APP_CODE, + "mode": TRAVEL_MODE_TRUCK, + } + } + assert await async_setup_component(hass, DOMAIN, config) + + +async def test_delayed_update(hass, requests_mock_truck_response, caplog): + """Test that delayed update does not complain about missing entities.""" + caplog.set_level(logging.WARNING) + + config = { + DOMAIN: { + "platform": PLATFORM, + "name": "test", + "origin_entity_id": "sensor.origin", + "destination_latitude": TRUCK_DESTINATION_LATITUDE, + "destination_longitude": TRUCK_DESTINATION_LONGITUDE, + "app_id": APP_ID, + "app_code": APP_CODE, + "mode": TRAVEL_MODE_TRUCK, + } + } + sensor_config = { + "sensor": { + "platform": "template", + "sensors": [ + {"template_sensor": {"value_template": "{{states('sensor.origin')}}"}} + ], + } + } + assert await async_setup_component(hass, DOMAIN, config) + assert await async_setup_component(hass, "sensor", sensor_config) + hass.states.async_set( + "sensor.origin", ",".join([TRUCK_ORIGIN_LATITUDE, TRUCK_ORIGIN_LONGITUDE]) + ) + + hass.bus.async_fire(EVENT_HOMEASSISTANT_START) + await hass.async_block_till_done() + + assert "Unable to find entity" not in caplog.text