Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Person: Ignore unavailable states #21058

Merged
merged 2 commits into from Feb 14, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
44 changes: 23 additions & 21 deletions homeassistant/components/person/__init__.py
Expand Up @@ -15,7 +15,7 @@
DOMAIN as DEVICE_TRACKER_DOMAIN)
from homeassistant.const import (
ATTR_ID, ATTR_LATITUDE, ATTR_LONGITUDE, CONF_ID, CONF_NAME,
EVENT_HOMEASSISTANT_START)
EVENT_HOMEASSISTANT_START, STATE_UNKNOWN, STATE_UNAVAILABLE)
from homeassistant.core import callback, Event
from homeassistant.auth import EVENT_USER_REMOVED
import homeassistant.helpers.config_validation as cv
Expand All @@ -25,7 +25,6 @@
from homeassistant.helpers.restore_state import RestoreEntity
from homeassistant.components import websocket_api
from homeassistant.helpers.typing import HomeAssistantType, ConfigType
from homeassistant.util import dt as dt_util
from homeassistant.loader import bind_hass

_LOGGER = logging.getLogger(__name__)
Expand All @@ -38,6 +37,8 @@
STORAGE_KEY = DOMAIN
STORAGE_VERSION = 1
SAVE_DELAY = 10
# Device tracker states to ignore
IGNORE_STATES = (STATE_UNKNOWN, STATE_UNAVAILABLE)

PERSON_SCHEMA = vol.Schema({
vol.Required(CONF_ID): cv.string,
Expand Down Expand Up @@ -350,30 +351,31 @@ def person_updated(self):
trackers = self._config.get(CONF_DEVICE_TRACKERS)

if trackers:
def sort_key(state):
if state:
return state.last_updated
return dt_util.utc_from_timestamp(0)

latest = max(
[self.hass.states.get(entity_id) for entity_id in trackers],
key=sort_key
)

@callback
def async_handle_tracker_update(entity, old_state, new_state):
"""Handle the device tracker state changes."""
self._parse_source_state(new_state)
self.async_schedule_update_ha_state()

_LOGGER.debug(
"Subscribe to device trackers for %s", self.entity_id)

self._unsub_track_device = async_track_state_change(
self.hass, trackers, async_handle_tracker_update)
self.hass, trackers, self._async_handle_tracker_update)

else:
latest = None
self._update_state()

@callback
def _async_handle_tracker_update(self, entity, old_state, new_state):
"""Handle the device tracker state changes."""
self._update_state()

@callback
def _update_state(self):
"""Update the state."""
latest = None
for entity_id in self._config.get(CONF_DEVICE_TRACKERS, []):
state = self.hass.states.get(entity_id)

if not state or state.state in IGNORE_STATES:
continue

if latest is None or state.last_updated > latest.last_updated:
latest = state

if latest:
self._parse_source_state(latest)
Expand Down
39 changes: 38 additions & 1 deletion tests/components/person/test_init.py
Expand Up @@ -29,7 +29,7 @@ def storage_setup(hass, hass_storage, hass_admin_user):
'id': '1234',
'name': 'tracked person',
'user_id': hass_admin_user.id,
'device_trackers': DEVICE_TRACKER
'device_trackers': [DEVICE_TRACKER]
}
]
}
Expand Down Expand Up @@ -189,6 +189,43 @@ async def test_setup_two_trackers(hass, hass_admin_user):
assert state.attributes.get(ATTR_USER_ID) == user_id


async def test_ignore_unavailable_states(hass, hass_admin_user):
"""Test set up person with two device trackers, one unavailable."""
user_id = hass_admin_user.id
config = {DOMAIN: {
'id': '1234', 'name': 'tracked person', 'user_id': user_id,
'device_trackers': [DEVICE_TRACKER, DEVICE_TRACKER_2]}}
assert await async_setup_component(hass, DOMAIN, config)

state = hass.states.get('person.tracked_person')
assert state.state == STATE_UNKNOWN

hass.bus.async_fire(EVENT_HOMEASSISTANT_START)
await hass.async_block_till_done()
hass.states.async_set(DEVICE_TRACKER, 'home')
await hass.async_block_till_done()
hass.states.async_set(DEVICE_TRACKER, 'unavailable')
await hass.async_block_till_done()

# Unknown, as only 1 device tracker has a state, but we ignore that one
state = hass.states.get('person.tracked_person')
assert state.state == STATE_UNKNOWN
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this be state 'home' since the device tracker had that state first, before becoming unknown state?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The moment device tracker updates to unavailable, it's no longer home. So person should not rely on a no-longer available state in current implementation. I guess we can tweak in future.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But we don't set the person state explicitly to unknown. We just ignore unknown or unavailable device tracker states when reacting to the device tracker change. I don't understand why this test works.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because I changed how the person entity determines state. It will fetch all new states for each entity and decides which one to take. So by overriding the only state that was there to one that is ignored, no state will be found.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I forgot that we do set state explicitly to None after check at line 380.


hass.states.async_set(DEVICE_TRACKER_2, 'not_home')
await hass.async_block_till_done()

# Take state of tracker 2
state = hass.states.get('person.tracked_person')
assert state.state == 'not_home'

# state 1 is newer but ignored, keep tracker 2 state
hass.states.async_set(DEVICE_TRACKER, 'unknown')
await hass.async_block_till_done()

state = hass.states.get('person.tracked_person')
assert state.state == 'not_home'


async def test_restore_home_state(hass, hass_admin_user):
"""Test that the state is restored for a person on startup."""
user_id = hass_admin_user.id
Expand Down