From 9f7ce23e80a749d4ff8a34e2a9d9326632ba1f79 Mon Sep 17 00:00:00 2001 From: pavoni Date: Tue, 16 Feb 2016 12:54:36 +0000 Subject: [PATCH 1/3] Fix suspect race condition in leave region. Add safely check for double beacon entry. Remove battery for beacons. Disable lint warning. --- .../components/device_tracker/owntracks.py | 47 +++++++++++-------- 1 file changed, 27 insertions(+), 20 deletions(-) diff --git a/homeassistant/components/device_tracker/owntracks.py b/homeassistant/components/device_tracker/owntracks.py index 2b8e612030b..3f430d798a4 100644 --- a/homeassistant/components/device_tracker/owntracks.py +++ b/homeassistant/components/device_tracker/owntracks.py @@ -62,7 +62,7 @@ def setup_scanner(hass, config, see): see_beacons(dev_id, kwargs) def owntracks_event_update(topic, payload, qos): - # pylint: disable=too-many-branches + # pylint: disable=too-many-branches, too-many-statements """ MQTT event (geofences) received. """ # Docs on available data: @@ -92,7 +92,10 @@ def setup_scanner(hass, config, see): if zone is None: if data['t'] == 'b': # Not a HA zone, and a beacon so assume mobile - MOBILE_BEACONS_ACTIVE[dev_id].append(location) + beacons = MOBILE_BEACONS_ACTIVE[dev_id] + if location not in beacons: + beacons.append(location) + _LOGGER.info("Added beacon %s", location) else: # Normal region if not zone.attributes.get('passive'): @@ -108,28 +111,30 @@ def setup_scanner(hass, config, see): see_beacons(dev_id, kwargs) elif data['event'] == 'leave': - regions = REGIONS_ENTERED[dev_id] - if location in regions: - regions.remove(location) - new_region = regions[-1] if regions else None + with LOCK: + regions = REGIONS_ENTERED[dev_id] + if location in regions: + regions.remove(location) + new_region = regions[-1] if regions else None - if new_region: - # Exit to previous region - zone = hass.states.get("zone.{}".format(new_region)) - if not zone.attributes.get('passive'): - kwargs['location_name'] = new_region - _set_gps_from_zone(kwargs, zone) - _LOGGER.info("Exit from to %s", new_region) + if new_region: + # Exit to previous region + zone = hass.states.get("zone.{}".format(new_region)) + if not zone.attributes.get('passive'): + kwargs['location_name'] = new_region + _set_gps_from_zone(kwargs, zone) + _LOGGER.info("Exit to %s", new_region) - else: - _LOGGER.info("Exit to GPS") + else: + _LOGGER.info("Exit to GPS") - see(**kwargs) - see_beacons(dev_id, kwargs) + see(**kwargs) + see_beacons(dev_id, kwargs) - beacons = MOBILE_BEACONS_ACTIVE[dev_id] - if location in beacons: - beacons.remove(location) + beacons = MOBILE_BEACONS_ACTIVE[dev_id] + if location in beacons: + beacons.remove(location) + _LOGGER.info("Remove beacon %s", location) else: _LOGGER.error( @@ -141,6 +146,8 @@ def setup_scanner(hass, config, see): """ Set active beacons to the current location """ kwargs = kwargs_param.copy() + # the battery state applies to the tracking device, not the beacon + kwargs.pop('battery', None) for beacon in MOBILE_BEACONS_ACTIVE[dev_id]: kwargs['dev_id'] = "{}_{}".format(BEACON_DEV_ID, beacon) kwargs['host_name'] = beacon From 7bd4e58b9dabc65a7b68d4da99bbb7c4451d92a0 Mon Sep 17 00:00:00 2001 From: pavoni Date: Fri, 19 Feb 2016 09:43:59 +0000 Subject: [PATCH 2/3] Add tests for race condition. --- .../device_tracker/test_owntracks.py | 32 +++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/tests/components/device_tracker/test_owntracks.py b/tests/components/device_tracker/test_owntracks.py index ac9243ae5f8..6f35fe0e2de 100644 --- a/tests/components/device_tracker/test_owntracks.py +++ b/tests/components/device_tracker/test_owntracks.py @@ -379,3 +379,35 @@ class TestDeviceTrackerOwnTracks(unittest.TestCase): message['lat'] = "4.0" self.send_message(LOCATION_TOPIC, LOCATION_MESSAGE) self.assert_tracker_latitude(3.0) + + def test_mobile_multiple_async_enter_exit(self): + # Test race condition + enter_message = REGION_ENTER_MESSAGE.copy() + enter_message['desc'] = IBEACON_DEVICE + exit_message = REGION_LEAVE_MESSAGE.copy() + exit_message['desc'] = IBEACON_DEVICE + + fire_mqtt_message( + self.hass, EVENT_TOPIC, json.dumps(enter_message)) + fire_mqtt_message( + self.hass, EVENT_TOPIC, json.dumps(exit_message)) + fire_mqtt_message( + self.hass, EVENT_TOPIC, json.dumps(enter_message)) + fire_mqtt_message( + self.hass, EVENT_TOPIC, json.dumps(exit_message)) + + self.hass.pool.block_till_done() + self.assertEqual(owntracks.MOBILE_BEACONS_ACTIVE['greg_phone'], []) + + def test_mobile_multiple_enter_exit(self): + # Should only happen if the iphone dies + enter_message = REGION_ENTER_MESSAGE.copy() + enter_message['desc'] = IBEACON_DEVICE + exit_message = REGION_LEAVE_MESSAGE.copy() + exit_message['desc'] = IBEACON_DEVICE + + self.send_message(EVENT_TOPIC, enter_message) + self.send_message(EVENT_TOPIC, enter_message) + self.send_message(EVENT_TOPIC, exit_message) + + self.assertEqual(owntracks.MOBILE_BEACONS_ACTIVE['greg_phone'], []) From ee62120fe59d0fc996cb025c48e6820a56355d56 Mon Sep 17 00:00:00 2001 From: pavoni Date: Fri, 19 Feb 2016 10:19:14 +0000 Subject: [PATCH 3/3] Revise race condition test. --- tests/components/device_tracker/test_owntracks.py | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/tests/components/device_tracker/test_owntracks.py b/tests/components/device_tracker/test_owntracks.py index 6f35fe0e2de..39914e704a3 100644 --- a/tests/components/device_tracker/test_owntracks.py +++ b/tests/components/device_tracker/test_owntracks.py @@ -387,16 +387,17 @@ class TestDeviceTrackerOwnTracks(unittest.TestCase): exit_message = REGION_LEAVE_MESSAGE.copy() exit_message['desc'] = IBEACON_DEVICE + for i in range(0, 20): + fire_mqtt_message( + self.hass, EVENT_TOPIC, json.dumps(enter_message)) + fire_mqtt_message( + self.hass, EVENT_TOPIC, json.dumps(exit_message)) + fire_mqtt_message( self.hass, EVENT_TOPIC, json.dumps(enter_message)) - fire_mqtt_message( - self.hass, EVENT_TOPIC, json.dumps(exit_message)) - fire_mqtt_message( - self.hass, EVENT_TOPIC, json.dumps(enter_message)) - fire_mqtt_message( - self.hass, EVENT_TOPIC, json.dumps(exit_message)) self.hass.pool.block_till_done() + self.send_message(EVENT_TOPIC, exit_message) self.assertEqual(owntracks.MOBILE_BEACONS_ACTIVE['greg_phone'], []) def test_mobile_multiple_enter_exit(self):