From ce1dd822a30fd62072c4a6bd0786b2b18725704d Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Thu, 2 Apr 2020 02:26:58 +0000 Subject: [PATCH 01/14] Use registry to find linked batteries for homekit --- homeassistant/components/homekit/__init__.py | 82 ++++++++++++-- .../components/homekit/accessories.py | 104 +++++++++++++----- homeassistant/components/homekit/const.py | 1 + tests/components/homekit/test_accessories.py | 37 +++++++ tests/components/homekit/test_homekit.py | 37 ++++--- 5 files changed, 212 insertions(+), 49 deletions(-) diff --git a/homeassistant/components/homekit/__init__.py b/homeassistant/components/homekit/__init__.py index 6021dc082a5bc7..d1970ead2c8f62 100644 --- a/homeassistant/components/homekit/__init__.py +++ b/homeassistant/components/homekit/__init__.py @@ -11,6 +11,8 @@ from homeassistant.components.http import HomeAssistantView from homeassistant.components.media_player import DEVICE_CLASS_TV from homeassistant.const import ( + ATTR_BATTERY_CHARGING, + ATTR_BATTERY_LEVEL, ATTR_DEVICE_CLASS, ATTR_ENTITY_ID, ATTR_SERVICE, @@ -20,6 +22,7 @@ CONF_NAME, CONF_PORT, CONF_TYPE, + DEVICE_CLASS_BATTERY, DEVICE_CLASS_HUMIDITY, DEVICE_CLASS_ILLUMINANCE, DEVICE_CLASS_TEMPERATURE, @@ -31,6 +34,7 @@ ) from homeassistant.core import callback from homeassistant.exceptions import Unauthorized +from homeassistant.helpers import entity_registry import homeassistant.helpers.config_validation as cv from homeassistant.helpers.entityfilter import FILTER_SCHEMA from homeassistant.util import get_local_ip @@ -47,6 +51,8 @@ CONF_ENTITY_CONFIG, CONF_FEATURE_LIST, CONF_FILTER, + CONF_LINKED_BATTERY_CHARGING_SENSOR, + CONF_LINKED_BATTERY_SENSOR, CONF_SAFE_MODE, CONF_ZEROCONF_DEFAULT_INTERFACE, DEFAULT_AUTO_START, @@ -76,6 +82,11 @@ validate_media_player_features, ) +# TODO: switch the below once done testing outside of tree +# from homeassistant.components.binary_sensor import DEVICE_CLASS_BATTERY_CHARGING +DEVICE_CLASS_BATTERY_CHARGING = "battery_charging" + + _LOGGER = logging.getLogger(__name__) MAX_DEVICES = 150 @@ -202,10 +213,10 @@ def async_describe_logbook_event(event): ) if auto_start: - hass.bus.async_listen_once(EVENT_HOMEASSISTANT_START, homekit.start) + hass.bus.async_listen_once(EVENT_HOMEASSISTANT_START, homekit.async_start) return True - def handle_homekit_service_start(service): + async def async_handle_homekit_service_start(service): """Handle start HomeKit service call.""" if homekit.status != STATUS_READY: _LOGGER.warning( @@ -213,10 +224,10 @@ def handle_homekit_service_start(service): "been stopped." ) return - homekit.start() + await homekit.async_start() hass.services.async_register( - DOMAIN, SERVICE_HOMEKIT_START, handle_homekit_service_start + DOMAIN, SERVICE_HOMEKIT_START, async_handle_homekit_service_start ) return True @@ -355,7 +366,7 @@ def setup(self): # pylint: disable=import-outside-toplevel from .accessories import HomeBridge, HomeDriver - self.hass.bus.async_listen_once(EVENT_HOMEASSISTANT_STOP, self.stop) + self.hass.bus.async_listen_once(EVENT_HOMEASSISTANT_STOP, self.async_stop) ip_addr = self._ip_address or get_local_ip() path = self.hass.config.path(HOMEKIT_FILE) @@ -428,12 +439,24 @@ def remove_bridge_accessory(self, aid): acc = self.bridge.accessories.pop(aid) return acc - def start(self, *args): + async def async_start(self, *args): """Start the accessory driver.""" if self.status != STATUS_READY: return self.status = STATUS_WAIT + ent_reg = await entity_registry.async_get_registry(self.hass) + + bridged_states = [] + for state in self.hass.states.async_all(): + if not state or not self._filter(state.entity_id): + continue + _async_configure_linked_battery_sensors(ent_reg, self._config, state) + bridged_states.append(state) + + await self.hass.async_add_executor_job(self._start, bridged_states) + + def _start(self, bridged_states): from . import ( # noqa: F401 pylint: disable=unused-import, import-outside-toplevel type_covers, type_fans, @@ -446,7 +469,7 @@ def start(self, *args): type_thermostats, ) - for state in self.hass.states.all(): + for state in bridged_states: self.add_bridge_accessory(state) self.driver.add_accessory(self.bridge) @@ -460,7 +483,7 @@ def start(self, *args): self.hass.add_job(self.driver.start) self.status = STATUS_RUNNING - def stop(self, *args): + async def async_stop(self, *args): """Stop the accessory driver.""" if self.status != STATUS_RUNNING: return @@ -486,3 +509,46 @@ async def get(self, request): body=request.app["hass"].data[HOMEKIT_PAIRING_QR], content_type="image/svg+xml", ) + + +@callback +def _async_configure_linked_battery_sensors(ent_reg, config, state): + entry = ent_reg.async_get(state.entity_id) + + if entry is None or entry.device_id is None: + return + + entries = entity_registry.async_entries_for_device(ent_reg, entry.device_id) + + for entry in entries: + if entry.device_class in ( + DEVICE_CLASS_BATTERY_CHARGING, + CONF_LINKED_BATTERY_SENSOR, + ): + continue + if ( + entry.domain == "binary_sensor" + and entry.device_class == DEVICE_CLASS_BATTERY_CHARGING + and ATTR_BATTERY_LEVEL not in state.attributes + ): + _LOGGER.debug( + "Found linked charging sensor for: %s: %s", + state.entity_id, + entry.entity_id, + ) + config.setdefault(state.entity_id, {}).setdefault( + CONF_LINKED_BATTERY_CHARGING_SENSOR, entry.entity_id + ) + if ( + entry.domain == "sensor" + and entry.device_class == DEVICE_CLASS_BATTERY + and ATTR_BATTERY_CHARGING not in state.attributes + ): + _LOGGER.debug( + "Found linked battery sensor for: %s: %s", + state.entity_id, + entry.entity_id, + ) + config.setdefault(state.entity_id, {}).setdefault( + CONF_LINKED_BATTERY_SENSOR, entry.entity_id + ) diff --git a/homeassistant/components/homekit/accessories.py b/homeassistant/components/homekit/accessories.py index 06a643c9f616ff..192980db6fa1fd 100644 --- a/homeassistant/components/homekit/accessories.py +++ b/homeassistant/components/homekit/accessories.py @@ -13,6 +13,7 @@ ATTR_BATTERY_LEVEL, ATTR_ENTITY_ID, ATTR_SERVICE, + STATE_ON, __version__, ) from homeassistant.core import callback as ha_callback, split_entity_id @@ -30,6 +31,7 @@ CHAR_BATTERY_LEVEL, CHAR_CHARGING_STATE, CHAR_STATUS_LOW_BATTERY, + CONF_LINKED_BATTERY_CHARGING_SENSOR, CONF_LINKED_BATTERY_SENSOR, CONF_LOW_BATTERY_THRESHOLD, DEBOUNCE_TIMEOUT, @@ -94,17 +96,17 @@ def __init__( self.entity_id = entity_id self.hass = hass self.debounce = {} - self._support_battery_level = False - self._support_battery_charging = True self.linked_battery_sensor = self.config.get(CONF_LINKED_BATTERY_SENSOR) + self.linked_battery_charging_sensor = self.config.get( + CONF_LINKED_BATTERY_CHARGING_SENSOR + ) self.low_battery_threshold = self.config.get( CONF_LOW_BATTERY_THRESHOLD, DEFAULT_LOW_BATTERY_THRESHOLD ) """Add battery service if available""" - battery_found = self.hass.states.get(self.entity_id).attributes.get( - ATTR_BATTERY_LEVEL - ) + entity_attributes = self.hass.states.get(self.entity_id).attributes + battery_found = entity_attributes.get(ATTR_BATTERY_LEVEL) if self.linked_battery_sensor: state = self.hass.states.get(self.linked_battery_sensor) @@ -118,10 +120,21 @@ def __init__( self.linked_battery_sensor, ) - if battery_found is None: + if not battery_found: return + _LOGGER.debug("%s: Found battery level", self.entity_id) - self._support_battery_level = True + + if self.linked_battery_charging_sensor: + state = self.hass.states.get(self.linked_battery_charging_sensor) + if state is None: + self.linked_battery_charging_sensor = None + _LOGGER.warning( + "%s: Battery charging binary_sensor state missing: %s", + self.entity_id, + self.linked_battery_charging_sensor, + ) + serv_battery = self.add_preload_service(SERV_BATTERY_SERVICE) self._char_battery = serv_battery.configure_char(CHAR_BATTERY_LEVEL, value=0) self._char_charging = serv_battery.configure_char(CHAR_CHARGING_STATE, value=2) @@ -147,12 +160,22 @@ async def run_handler(self): if self.linked_battery_sensor: battery_state = self.hass.states.get(self.linked_battery_sensor) - self.hass.async_add_job( - self.update_linked_battery, None, None, battery_state - ) + self.update_linked_battery(None, None, battery_state) async_track_state_change( self.hass, self.linked_battery_sensor, self.update_linked_battery ) + if self.linked_battery_charging_sensor: + battery_charging_state = self.hass.states.get( + self.linked_battery_charging_sensor + ) + self.hass.async_add_executor_job( + self.update_battery, None, battery_charging_state.state + ) + async_track_state_change( + self.hass, + self.linked_battery_charging_sensor, + self.update_linked_battery_charging, + ) @ha_callback def update_state_callback(self, entity_id=None, old_state=None, new_state=None): @@ -160,35 +183,60 @@ def update_state_callback(self, entity_id=None, old_state=None, new_state=None): _LOGGER.debug("New_state: %s", new_state) if new_state is None: return - if self._support_battery_level and not self.linked_battery_sensor: - self.hass.async_add_executor_job(self.update_battery, new_state) + battery_state = None + battery_charging_state = None + if ( + not self.linked_battery_sensor + and ATTR_BATTERY_LEVEL in new_state.attributes + ): + battery_state = new_state.attributes.get(ATTR_BATTERY_LEVEL) + if ( + not self.linked_battery_charging_sensor + and ATTR_BATTERY_CHARGING in new_state.attributes + ): + battery_charging_state = new_state.attributes.get(ATTR_BATTERY_CHARGING) + if battery_state is not None or battery_charging_state is not None: + self.hass.async_add_executor_job( + self.update_battery, battery_state, battery_charging_state + ) self.hass.async_add_executor_job(self.update_state, new_state) @ha_callback def update_linked_battery(self, entity_id=None, old_state=None, new_state=None): """Handle linked battery sensor state change listener callback.""" - self.hass.async_add_executor_job(self.update_battery, new_state) + if self.linked_battery_charging_sensor: + battery_charging_state = None + else: + battery_charging_state = new_state.attributes.get(ATTR_BATTERY_CHARGING) + self.hass.async_add_executor_job( + self.update_battery, new_state.state, battery_charging_state, + ) + + @ha_callback + def update_linked_battery_charging( + self, entity_id=None, old_state=None, new_state=None + ): + """Handle linked battery charging sensor state change listener callback.""" + self.hass.async_add_executor_job( + self.update_battery, None, new_state.state == STATE_ON + ) - def update_battery(self, new_state): + def update_battery(self, battery_level_state, battery_charging_state): """Update battery service if available. Only call this function if self._support_battery_level is True. """ - battery_level = convert_to_float(new_state.attributes.get(ATTR_BATTERY_LEVEL)) - if self.linked_battery_sensor: - battery_level = convert_to_float(new_state.state) - if battery_level is None: - return - self._char_battery.set_value(battery_level) - self._char_low_battery.set_value(battery_level < self.low_battery_threshold) - _LOGGER.debug("%s: Updated battery level to %d", self.entity_id, battery_level) - if not self._support_battery_charging: - return - charging = new_state.attributes.get(ATTR_BATTERY_CHARGING) - if charging is None: - self._support_battery_charging = False + battery_level = convert_to_float(battery_level_state) + if battery_level is not None: + self._char_battery.set_value(battery_level) + self._char_low_battery.set_value(battery_level < self.low_battery_threshold) + _LOGGER.debug( + "%s: Updated battery level to %d", self.entity_id, battery_level + ) + + if battery_charging_state is None: return - hk_charging = 1 if charging is True else 0 + hk_charging = 1 if battery_charging_state else 0 self._char_charging.set_value(hk_charging) _LOGGER.debug("%s: Updated battery charging to %d", self.entity_id, hk_charging) diff --git a/homeassistant/components/homekit/const.py b/homeassistant/components/homekit/const.py index 026cdc60e74e0d..94d3122d393b5f 100644 --- a/homeassistant/components/homekit/const.py +++ b/homeassistant/components/homekit/const.py @@ -21,6 +21,7 @@ CONF_FEATURE_LIST = "feature_list" CONF_FILTER = "filter" CONF_LINKED_BATTERY_SENSOR = "linked_battery_sensor" +CONF_LINKED_BATTERY_CHARGING_SENSOR = "linked_battery_charging_sensor" CONF_LOW_BATTERY_THRESHOLD = "low_battery_threshold" CONF_SAFE_MODE = "safe_mode" CONF_ZEROCONF_DEFAULT_INTERFACE = "zeroconf_default_interface" diff --git a/tests/components/homekit/test_accessories.py b/tests/components/homekit/test_accessories.py index ab6f58ebab86f6..1f0f88f6503c55 100644 --- a/tests/components/homekit/test_accessories.py +++ b/tests/components/homekit/test_accessories.py @@ -24,6 +24,7 @@ CHAR_MODEL, CHAR_NAME, CHAR_SERIAL_NUMBER, + CONF_LINKED_BATTERY_CHARGING_SENSOR, CONF_LINKED_BATTERY_SENSOR, CONF_LOW_BATTERY_THRESHOLD, MANUFACTURER, @@ -36,6 +37,8 @@ ATTR_NOW, ATTR_SERVICE, EVENT_TIME_CHANGED, + STATE_OFF, + STATE_ON, __version__, ) import homeassistant.util.dt as dt_util @@ -245,6 +248,40 @@ async def test_linked_battery_sensor(hass, hk_driver, caplog): assert acc._char_charging.value == 0 +async def test_linked_battery_charging_sensor(hass, hk_driver, caplog): + """Test battery service with linked_battery_charging_sensor.""" + entity_id = "homekit.accessory" + linked_battery_charging_sensor = "binary_sensor.battery_charging" + hass.states.async_set(entity_id, "open", {ATTR_BATTERY_LEVEL: 100}) + hass.states.async_set(linked_battery_charging_sensor, STATE_ON, None) + await hass.async_block_till_done() + + acc = HomeAccessory( + hass, + hk_driver, + "Battery Service", + entity_id, + 2, + {CONF_LINKED_BATTERY_CHARGING_SENSOR: linked_battery_charging_sensor}, + ) + acc.update_state = lambda x: None + assert acc.linked_battery_charging_sensor == linked_battery_charging_sensor + + await hass.async_add_job(acc.run) + await hass.async_block_till_done() + assert acc._char_battery.value == 100 + assert acc._char_low_battery.value == 0 + assert acc._char_charging.value == 1 + + hass.states.async_set(linked_battery_charging_sensor, STATE_OFF, None) + await hass.async_block_till_done() + assert acc._char_charging.value == 0 + + hass.states.async_set(linked_battery_charging_sensor, STATE_ON, None) + await hass.async_block_till_done() + assert acc._char_charging.value == 1 + + async def test_missing_linked_battery_sensor(hass, hk_driver, caplog): """Test battery service with missing linked_battery_sensor.""" entity_id = "homekit.accessory" diff --git a/tests/components/homekit/test_homekit.py b/tests/components/homekit/test_homekit.py index a7b46f57fd5874..89d96c3922d0b9 100644 --- a/tests/components/homekit/test_homekit.py +++ b/tests/components/homekit/test_homekit.py @@ -1,6 +1,7 @@ """Tests for the HomeKit component.""" from unittest.mock import ANY, Mock, patch +from asynctest import CoroutineMock import pytest from zeroconf import InterfaceChoice @@ -66,7 +67,7 @@ async def test_setup_min(hass): hass.bus.async_fire(EVENT_HOMEASSISTANT_START) await hass.async_block_till_done() - mock_homekit().start.assert_called_with(ANY) + mock_homekit().async_start.assert_called_with(ANY) async def test_setup_auto_start_disabled(hass): @@ -83,6 +84,7 @@ async def test_setup_auto_start_disabled(hass): with patch(f"{PATH_HOMEKIT}.HomeKit") as mock_homekit: mock_homekit.return_value = homekit = Mock() + type(homekit).async_start = CoroutineMock() assert await setup.async_setup_component(hass, DOMAIN, config) mock_homekit.assert_any_call( @@ -92,23 +94,26 @@ async def test_setup_auto_start_disabled(hass): # Test auto_start disabled homekit.reset_mock() + homekit.async_start.reset_mock() hass.bus.async_fire(EVENT_HOMEASSISTANT_START) await hass.async_block_till_done() - assert homekit.start.called is False + assert homekit.async_start.called is False # Test start call with driver is ready homekit.reset_mock() + homekit.async_start.reset_mock() homekit.status = STATUS_READY await hass.services.async_call(DOMAIN, SERVICE_HOMEKIT_START, blocking=True) - assert homekit.start.called is True + assert homekit.async_start.called is True # Test start call with driver started homekit.reset_mock() + homekit.async_start.reset_mock() homekit.status = STATUS_STOPPED await hass.services.async_call(DOMAIN, SERVICE_HOMEKIT_START, blocking=True) - assert homekit.start.called is False + assert homekit.async_start.called is False async def test_homekit_setup(hass, hk_driver): @@ -279,6 +284,7 @@ async def test_homekit_start(hass, hk_driver, debounce_patcher): homekit.bridge = Mock() homekit.bridge.accessories = [] homekit.driver = hk_driver + homekit._filter = Mock(return_value=True) hass.states.async_set("light.demo", "on") state = hass.states.async_all()[0] @@ -290,7 +296,7 @@ async def test_homekit_start(hass, hk_driver, debounce_patcher): ) as hk_driver_add_acc, patch( "pyhap.accessory_driver.AccessoryDriver.start" ) as hk_driver_start: - await hass.async_add_executor_job(homekit.start) + await homekit.async_start() mock_add_acc.assert_called_with(state) mock_setup_msg.assert_called_with(hass, pin, ANY) @@ -300,7 +306,7 @@ async def test_homekit_start(hass, hk_driver, debounce_patcher): # Test start() if already started hk_driver_start.reset_mock() - await hass.async_add_executor_job(homekit.start) + await homekit.async_start() assert not hk_driver_start.called @@ -326,7 +332,7 @@ async def test_homekit_start_with_a_broken_accessory(hass, hk_driver, debounce_p ) as hk_driver_add_acc, patch( "pyhap.accessory_driver.AccessoryDriver.start" ) as hk_driver_start: - await hass.async_add_executor_job(homekit.start) + await homekit.async_start() mock_setup_msg.assert_called_with(hass, pin, ANY) hk_driver_add_acc.assert_called_with(homekit.bridge) @@ -335,7 +341,7 @@ async def test_homekit_start_with_a_broken_accessory(hass, hk_driver, debounce_p # Test start() if already started hk_driver_start.reset_mock() - await hass.async_add_executor_job(homekit.start) + await homekit.async_start() assert not hk_driver_start.called @@ -345,16 +351,20 @@ async def test_homekit_stop(hass): homekit.driver = Mock() assert homekit.status == STATUS_READY - await hass.async_add_executor_job(homekit.stop) + await homekit.async_stop() + await hass.async_block_till_done() homekit.status = STATUS_WAIT - await hass.async_add_executor_job(homekit.stop) + await homekit.async_stop() + await hass.async_block_till_done() homekit.status = STATUS_STOPPED - await hass.async_add_executor_job(homekit.stop) + await homekit.async_stop() + await hass.async_block_till_done() assert homekit.driver.stop.called is False # Test if driver is started homekit.status = STATUS_RUNNING - await hass.async_add_executor_job(homekit.stop) + await homekit.async_stop() + await hass.async_block_till_done() assert homekit.driver.stop.called is True @@ -408,5 +418,6 @@ async def test_homekit_too_many_accessories(hass, hk_driver): ), patch("homeassistant.components.homekit._LOGGER.warning") as mock_warn, patch( f"{PATH_HOMEKIT}.show_setup_message" ): - await hass.async_add_executor_job(homekit.start) + await homekit.async_start() + await hass.async_block_till_done() assert mock_warn.called is True From a1638f16390549d3bc62e0259ef1c0324fbb6f5f Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Thu, 16 Apr 2020 19:54:56 +0000 Subject: [PATCH 02/14] Fix conflict --- homeassistant/components/homekit/__init__.py | 99 +++++++++---------- .../components/homekit/accessories.py | 10 +- homeassistant/helpers/entity_registry.py | 28 +++++- 3 files changed, 81 insertions(+), 56 deletions(-) diff --git a/homeassistant/components/homekit/__init__.py b/homeassistant/components/homekit/__init__.py index d1970ead2c8f62..5310240cc13a7e 100644 --- a/homeassistant/components/homekit/__init__.py +++ b/homeassistant/components/homekit/__init__.py @@ -7,6 +7,7 @@ from zeroconf import InterfaceChoice from homeassistant.components import cover, vacuum +from homeassistant.components.binary_sensor import DEVICE_CLASS_BATTERY_CHARGING from homeassistant.components.cover import DEVICE_CLASS_GARAGE, DEVICE_CLASS_GATE from homeassistant.components.http import HomeAssistantView from homeassistant.components.media_player import DEVICE_CLASS_TV @@ -82,11 +83,6 @@ validate_media_player_features, ) -# TODO: switch the below once done testing outside of tree -# from homeassistant.components.binary_sensor import DEVICE_CLASS_BATTERY_CHARGING -DEVICE_CLASS_BATTERY_CHARGING = "battery_charging" - - _LOGGER = logging.getLogger(__name__) MAX_DEVICES = 150 @@ -447,11 +443,21 @@ async def async_start(self, *args): ent_reg = await entity_registry.async_get_registry(self.hass) + domain_device_classes = { + ("binary_sensor", DEVICE_CLASS_BATTERY_CHARGING), + ("sensor", DEVICE_CLASS_BATTERY), + } + + device_lookup = ent_reg.async_get_device_class_lookup(domain_device_classes) + + _LOGGER.debug("device_lookup: %s", device_lookup) + bridged_states = [] for state in self.hass.states.async_all(): - if not state or not self._filter(state.entity_id): + if not self._filter(state.entity_id): continue - _async_configure_linked_battery_sensors(ent_reg, self._config, state) + + self._async_configure_linked_battery_sensors(ent_reg, device_lookup, state) bridged_states.append(state) await self.hass.async_add_executor_job(self._start, bridged_states) @@ -480,7 +486,7 @@ def _start(self, bridged_states): ) _LOGGER.debug("Driver start") - self.hass.add_job(self.driver.start) + self.hass.async_add_executor_job(self.driver.start) self.status = STATUS_RUNNING async def async_stop(self, *args): @@ -490,7 +496,39 @@ async def async_stop(self, *args): self.status = STATUS_STOPPED _LOGGER.debug("Driver stop") - self.hass.add_job(self.driver.stop) + self.hass.async_add_executor_job(self.driver.stop) + + @callback + def _async_configure_linked_battery_sensors(self, ent_reg, device_lookup, state): + entry = ent_reg.async_get(state.entity_id) + + if ( + entry is None + or entry.device_id is None + or entry.device_id not in device_lookup + or entry.device_class + in (DEVICE_CLASS_BATTERY_CHARGING, DEVICE_CLASS_BATTERY) + ): + return + + if ATTR_BATTERY_CHARGING not in state.attributes: + battery_charging_binary_sensor_entity_id = device_lookup[ + entry.device_id + ].get(("binary_sensor", DEVICE_CLASS_BATTERY_CHARGING)) + if battery_charging_binary_sensor_entity_id: + self._config.setdefault(state.entity_id, {}).setdefault( + CONF_LINKED_BATTERY_CHARGING_SENSOR, + battery_charging_binary_sensor_entity_id, + ) + + if ATTR_BATTERY_LEVEL not in state.attributes: + battery_sensor_entity_id = device_lookup[entry.device_id].get( + ("sensor", DEVICE_CLASS_BATTERY) + ) + if battery_sensor_entity_id: + self._config.setdefault(state.entity_id, {}).setdefault( + CONF_LINKED_BATTERY_SENSOR, battery_sensor_entity_id + ) class HomeKitPairingQRView(HomeAssistantView): @@ -509,46 +547,3 @@ async def get(self, request): body=request.app["hass"].data[HOMEKIT_PAIRING_QR], content_type="image/svg+xml", ) - - -@callback -def _async_configure_linked_battery_sensors(ent_reg, config, state): - entry = ent_reg.async_get(state.entity_id) - - if entry is None or entry.device_id is None: - return - - entries = entity_registry.async_entries_for_device(ent_reg, entry.device_id) - - for entry in entries: - if entry.device_class in ( - DEVICE_CLASS_BATTERY_CHARGING, - CONF_LINKED_BATTERY_SENSOR, - ): - continue - if ( - entry.domain == "binary_sensor" - and entry.device_class == DEVICE_CLASS_BATTERY_CHARGING - and ATTR_BATTERY_LEVEL not in state.attributes - ): - _LOGGER.debug( - "Found linked charging sensor for: %s: %s", - state.entity_id, - entry.entity_id, - ) - config.setdefault(state.entity_id, {}).setdefault( - CONF_LINKED_BATTERY_CHARGING_SENSOR, entry.entity_id - ) - if ( - entry.domain == "sensor" - and entry.device_class == DEVICE_CLASS_BATTERY - and ATTR_BATTERY_CHARGING not in state.attributes - ): - _LOGGER.debug( - "Found linked battery sensor for: %s: %s", - state.entity_id, - entry.entity_id, - ) - config.setdefault(state.entity_id, {}).setdefault( - CONF_LINKED_BATTERY_SENSOR, entry.entity_id - ) diff --git a/homeassistant/components/homekit/accessories.py b/homeassistant/components/homekit/accessories.py index 192980db6fa1fd..2b97cf4557a749 100644 --- a/homeassistant/components/homekit/accessories.py +++ b/homeassistant/components/homekit/accessories.py @@ -228,8 +228,11 @@ def update_battery(self, battery_level_state, battery_charging_state): """ battery_level = convert_to_float(battery_level_state) if battery_level is not None: - self._char_battery.set_value(battery_level) - self._char_low_battery.set_value(battery_level < self.low_battery_threshold) + if self._char_battery.value != battery_level: + self._char_battery.set_value(battery_level) + is_low_battery = 1 if battery_level < self.low_battery_threshold else 0 + if self._char_low_battery.value != is_low_battery: + self._char_low_battery.set_value(is_low_battery) _LOGGER.debug( "%s: Updated battery level to %d", self.entity_id, battery_level ) @@ -237,7 +240,8 @@ def update_battery(self, battery_level_state, battery_charging_state): if battery_charging_state is None: return hk_charging = 1 if battery_charging_state else 0 - self._char_charging.set_value(hk_charging) + if self._char_charging.value != hk_charging: + self._char_charging.set_value(hk_charging) _LOGGER.debug("%s: Updated battery charging to %d", self.entity_id, hk_charging) def update_state(self, new_state): diff --git a/homeassistant/helpers/entity_registry.py b/homeassistant/helpers/entity_registry.py index 3f08ccc218aa4d..d73812c207b8ab 100644 --- a/homeassistant/helpers/entity_registry.py +++ b/homeassistant/helpers/entity_registry.py @@ -10,7 +10,17 @@ import asyncio from collections import OrderedDict import logging -from typing import TYPE_CHECKING, Any, Callable, Dict, Iterable, List, Optional, cast +from typing import ( + TYPE_CHECKING, + Any, + Callable, + Dict, + Iterable, + List, + Optional, + Tuple, + cast, +) import attr @@ -109,6 +119,22 @@ def __init__(self, hass: HomeAssistantType): EVENT_DEVICE_REGISTRY_UPDATED, self.async_device_removed ) + @callback + def async_get_device_class_lookup(self, domain_device_classes: set) -> dict: + """Return a lookup for the device class by domain.""" + lookup: Dict[str, Dict[Tuple[Any, Any], str]] = {} + for entity in self.entities.values(): + if not entity.device_id: + continue + domain_device_class = (entity.domain, entity.device_class) + if domain_device_class not in domain_device_classes: + continue + if entity.device_id not in lookup: + lookup[entity.device_id] = {domain_device_class: entity.entity_id} + else: + lookup[entity.device_id][domain_device_class] = entity.entity_id + return lookup + @callback def async_is_registered(self, entity_id: str) -> bool: """Check if an entity_id is currently registered.""" From 23322bec96f89e08dd66974d046ace7b6e00badd Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Thu, 16 Apr 2020 20:01:06 +0000 Subject: [PATCH 03/14] freshen test --- tests/components/homekit/test_accessories.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/components/homekit/test_accessories.py b/tests/components/homekit/test_accessories.py index 1f0f88f6503c55..01dfdd902ebe41 100644 --- a/tests/components/homekit/test_accessories.py +++ b/tests/components/homekit/test_accessories.py @@ -267,7 +267,7 @@ async def test_linked_battery_charging_sensor(hass, hk_driver, caplog): acc.update_state = lambda x: None assert acc.linked_battery_charging_sensor == linked_battery_charging_sensor - await hass.async_add_job(acc.run) + await acc.run_handler() await hass.async_block_till_done() assert acc._char_battery.value == 100 assert acc._char_low_battery.value == 0 From 57f112d622c1373abd155f52fb909a4fd31b962d Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Thu, 16 Apr 2020 21:05:54 +0000 Subject: [PATCH 04/14] more cleanup --- homeassistant/components/homekit/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/homeassistant/components/homekit/__init__.py b/homeassistant/components/homekit/__init__.py index 5310240cc13a7e..788eb919c2bcdd 100644 --- a/homeassistant/components/homekit/__init__.py +++ b/homeassistant/components/homekit/__init__.py @@ -400,7 +400,7 @@ def reset_accessories(self, entity_ids): def add_bridge_accessory(self, state): """Try adding accessory to bridge if configured beforehand.""" - if not state or not self._filter(state.entity_id): + if not self._filter(state.entity_id): return # The bridge itself counts as an accessory From 0b4a4d168e00192267c1ce50b46e40d535aed10a Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sat, 18 Apr 2020 16:10:30 +0000 Subject: [PATCH 05/14] Increase battery test coverage --- .../components/homekit/accessories.py | 2 +- tests/components/homekit/test_accessories.py | 57 +++++++++++++++++++ tests/components/homekit/test_type_covers.py | 4 ++ 3 files changed, 62 insertions(+), 1 deletion(-) diff --git a/homeassistant/components/homekit/accessories.py b/homeassistant/components/homekit/accessories.py index 2b97cf4557a749..e7542634fbc0e8 100644 --- a/homeassistant/components/homekit/accessories.py +++ b/homeassistant/components/homekit/accessories.py @@ -155,7 +155,7 @@ async def run_handler(self): Run inside the Home Assistant event loop. """ state = self.hass.states.get(self.entity_id) - self.hass.async_add_job(self.update_state_callback, None, None, state) + self.hass.async_add_executor_job(self.update_state_callback, None, None, state) async_track_state_change(self.hass, self.entity_id, self.update_state_callback) if self.linked_battery_sensor: diff --git a/tests/components/homekit/test_accessories.py b/tests/components/homekit/test_accessories.py index 01dfdd902ebe41..2595dfe06a25f1 100644 --- a/tests/components/homekit/test_accessories.py +++ b/tests/components/homekit/test_accessories.py @@ -282,6 +282,63 @@ async def test_linked_battery_charging_sensor(hass, hk_driver, caplog): assert acc._char_charging.value == 1 +async def test_linked_battery_sensor_and_linked_battery_charging_sensor( + hass, hk_driver, caplog +): + """Test battery service with linked_battery_sensor and a linked_battery_charging_sensor.""" + entity_id = "homekit.accessory" + linked_battery = "sensor.battery" + linked_battery_charging_sensor = "binary_sensor.battery_charging" + hass.states.async_set(entity_id, "open", {ATTR_BATTERY_LEVEL: 100}) + hass.states.async_set(linked_battery, 50, None) + hass.states.async_set(linked_battery_charging_sensor, STATE_ON, None) + await hass.async_block_till_done() + + acc = HomeAccessory( + hass, + hk_driver, + "Battery Service", + entity_id, + 2, + { + CONF_LINKED_BATTERY_SENSOR: linked_battery, + CONF_LINKED_BATTERY_CHARGING_SENSOR: linked_battery_charging_sensor, + }, + ) + acc.update_state = lambda x: None + assert acc.linked_battery_sensor == linked_battery + + await acc.run_handler() + await hass.async_block_till_done() + assert acc._char_battery.value == 50 + assert acc._char_low_battery.value == 0 + assert acc._char_charging.value == 1 + + hass.states.async_set(linked_battery_charging_sensor, STATE_OFF, None) + await hass.async_block_till_done() + assert acc._char_battery.value == 50 + assert acc._char_low_battery.value == 0 + assert acc._char_charging.value == 0 + + +async def test_missing_linked_battery_charging_sensor(hass, hk_driver, caplog): + """Test battery service with linked_battery_charging_sensor that is mapping to a missing entity.""" + entity_id = "homekit.accessory" + linked_battery_charging_sensor = "binary_sensor.battery_charging" + hass.states.async_set(entity_id, "open", {ATTR_BATTERY_LEVEL: 100}) + await hass.async_block_till_done() + + acc = HomeAccessory( + hass, + hk_driver, + "Battery Service", + entity_id, + 2, + {CONF_LINKED_BATTERY_CHARGING_SENSOR: linked_battery_charging_sensor}, + ) + assert acc.linked_battery_charging_sensor is None + + async def test_missing_linked_battery_sensor(hass, hk_driver, caplog): """Test battery service with missing linked_battery_sensor.""" entity_id = "homekit.accessory" diff --git a/tests/components/homekit/test_type_covers.py b/tests/components/homekit/test_type_covers.py index c97703f3813e49..6be0acece4c7c6 100644 --- a/tests/components/homekit/test_type_covers.py +++ b/tests/components/homekit/test_type_covers.py @@ -411,6 +411,10 @@ async def test_window_open_close_with_position_and_stop(hass, hk_driver, cls, ev # Set from HomeKit call_stop_cover = async_mock_service(hass, DOMAIN, "stop_cover") + await hass.async_add_executor_job(acc.char_hold_position.client_update_value, 0) + await hass.async_block_till_done() + assert not call_stop_cover + await hass.async_add_executor_job(acc.char_hold_position.client_update_value, 1) await hass.async_block_till_done() assert call_stop_cover From ad7321b638125c77d1e9697895606a7b90b59af0 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sat, 18 Apr 2020 17:05:24 +0000 Subject: [PATCH 06/14] test to ensure we can find the linked battery sensors --- tests/components/homekit/test_homekit.py | 84 ++++++++++++++++++++++++ 1 file changed, 84 insertions(+) diff --git a/tests/components/homekit/test_homekit.py b/tests/components/homekit/test_homekit.py index 89d96c3922d0b9..5222cbf1e7628a 100644 --- a/tests/components/homekit/test_homekit.py +++ b/tests/components/homekit/test_homekit.py @@ -6,6 +6,7 @@ from zeroconf import InterfaceChoice from homeassistant import setup +from homeassistant.components.binary_sensor import DEVICE_CLASS_BATTERY_CHARGING from homeassistant.components.homekit import ( MAX_DEVICES, STATUS_READY, @@ -28,22 +29,39 @@ SERVICE_HOMEKIT_START, ) from homeassistant.const import ( + ATTR_DEVICE_CLASS, ATTR_ENTITY_ID, CONF_IP_ADDRESS, CONF_NAME, CONF_PORT, + DEVICE_CLASS_BATTERY, EVENT_HOMEASSISTANT_START, EVENT_HOMEASSISTANT_STOP, + STATE_ON, ) from homeassistant.core import State +from homeassistant.helpers import device_registry from homeassistant.helpers.entityfilter import generate_filter +from tests.common import MockConfigEntry, mock_device_registry, mock_registry from tests.components.homekit.common import patch_debounce IP_ADDRESS = "127.0.0.1" PATH_HOMEKIT = "homeassistant.components.homekit" +@pytest.fixture +def device_reg(hass): + """Return an empty, loaded, registry.""" + return mock_device_registry(hass) + + +@pytest.fixture +def entity_reg(hass): + """Return an empty, loaded, registry.""" + return mock_registry(hass) + + @pytest.fixture(scope="module") def debounce_patcher(): """Patch debounce method.""" @@ -421,3 +439,69 @@ async def test_homekit_too_many_accessories(hass, hk_driver): await homekit.async_start() await hass.async_block_till_done() assert mock_warn.called is True + + +async def test_homekit_finds_linked_batteries( + hass, hk_driver, debounce_patcher, device_reg, entity_reg +): + """Test HomeKit start method.""" + homekit = HomeKit(hass, None, None, None, {}, {"light.demo": {}}, None, None) + homekit.driver = hk_driver + homekit._filter = Mock(return_value=True) + homekit.bridge = HomeBridge(hass, hk_driver, "mock_bridge") + + config_entry = MockConfigEntry(domain="test", data={}) + config_entry.add_to_hass(hass) + device_entry = device_reg.async_get_or_create( + config_entry_id=config_entry.entry_id, + connections={(device_registry.CONNECTION_NETWORK_MAC, "12:34:56:AB:CD:EF")}, + ) + + binary_charging_sensor = entity_reg.async_get_or_create( + "binary_sensor", + "light", + "battery_charging", + device_id=device_entry.id, + device_class=DEVICE_CLASS_BATTERY_CHARGING, + ) + battery_sensor = entity_reg.async_get_or_create( + "sensor", + "light", + "battery", + device_id=device_entry.id, + device_class=DEVICE_CLASS_BATTERY, + ) + light = entity_reg.async_get_or_create( + "light", "light", "demo", device_id=device_entry.id + ) + + hass.states.async_set( + binary_charging_sensor.entity_id, + STATE_ON, + {ATTR_DEVICE_CLASS: DEVICE_CLASS_BATTERY_CHARGING}, + ) + hass.states.async_set( + battery_sensor.entity_id, 30, {ATTR_DEVICE_CLASS: DEVICE_CLASS_BATTERY} + ) + hass.states.async_set(light.entity_id, STATE_ON) + + def _mock_get_accessory(*args, **kwargs): + return [None, "acc", None] + + with patch.object(homekit.bridge, "add_accessory"), patch( + f"{PATH_HOMEKIT}.show_setup_message" + ), patch(f"{PATH_HOMEKIT}.get_accessory") as mock_get_acc, patch( + "pyhap.accessory_driver.AccessoryDriver.start" + ): + await homekit.async_start() + + mock_get_acc.assert_called_with( + hass, + hk_driver, + ANY, + ANY, + { + "linked_battery_charging_sensor": "binary_sensor.light_battery_charging", + "linked_battery_sensor": "sensor.light_battery", + }, + ) From 23778f119c9007453110e46b1d6872ff54d9996b Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sat, 18 Apr 2020 17:09:31 +0000 Subject: [PATCH 07/14] Remove debug --- homeassistant/components/homekit/__init__.py | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/homeassistant/components/homekit/__init__.py b/homeassistant/components/homekit/__init__.py index 788eb919c2bcdd..d114e02ef06643 100644 --- a/homeassistant/components/homekit/__init__.py +++ b/homeassistant/components/homekit/__init__.py @@ -443,14 +443,12 @@ async def async_start(self, *args): ent_reg = await entity_registry.async_get_registry(self.hass) - domain_device_classes = { - ("binary_sensor", DEVICE_CLASS_BATTERY_CHARGING), - ("sensor", DEVICE_CLASS_BATTERY), - } - - device_lookup = ent_reg.async_get_device_class_lookup(domain_device_classes) - - _LOGGER.debug("device_lookup: %s", device_lookup) + device_lookup = ent_reg.async_get_device_class_lookup( + { + ("binary_sensor", DEVICE_CLASS_BATTERY_CHARGING), + ("sensor", DEVICE_CLASS_BATTERY), + } + ) bridged_states = [] for state in self.hass.states.async_all(): From b88bcad8b68971f6ebb9cac19241c673ad89acfd Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sat, 18 Apr 2020 18:34:53 +0000 Subject: [PATCH 08/14] tests for async_get_device_class_lookup --- tests/helpers/test_entity_registry.py | 79 +++++++++++++++++++++++++++ 1 file changed, 79 insertions(+) diff --git a/tests/helpers/test_entity_registry.py b/tests/helpers/test_entity_registry.py index e7a7b856da2ed6..cda2f1245fb347 100644 --- a/tests/helpers/test_entity_registry.py +++ b/tests/helpers/test_entity_registry.py @@ -561,3 +561,82 @@ async def test_restore_states(hass): assert hass.states.get("light.simple") is None assert hass.states.get("light.disabled") is None assert hass.states.get("light.all_info_set") is None + + +async def test_async_get_device_class_lookup(hass): + """Test registry device class lookup.""" + hass.state = CoreState.not_running + + ent_reg = await entity_registry.async_get_registry(hass) + + ent_reg.async_get_or_create( + "binary_sensor", + "light", + "battery_charging", + device_id="light_device_entry_id", + device_class="battery_charging", + ) + ent_reg.async_get_or_create( + "sensor", + "light", + "battery", + device_id="light_device_entry_id", + device_class="battery", + ) + ent_reg.async_get_or_create( + "light", "light", "demo", device_id="light_device_entry_id" + ) + ent_reg.async_get_or_create( + "binary_sensor", + "vacuum", + "battery_charging", + device_id="vacuum_device_entry_id", + device_class="battery_charging", + ) + ent_reg.async_get_or_create( + "sensor", + "vacuum", + "battery", + device_id="vacuum_device_entry_id", + device_class="battery", + ) + ent_reg.async_get_or_create( + "vacuum", "vacuum", "demo", device_id="vacuum_device_entry_id" + ) + ent_reg.async_get_or_create( + "binary_sensor", + "remote", + "battery_charging", + device_id="remote_device_entry_id", + device_class="battery_charging", + ) + ent_reg.async_get_or_create( + "remote", "remote", "demo", device_id="remote_device_entry_id" + ) + + device_lookup = ent_reg.async_get_device_class_lookup( + {("binary_sensor", "battery_charging"), ("sensor", "battery")} + ) + + assert device_lookup == { + "remote_device_entry_id": { + ( + "binary_sensor", + "battery_charging", + ): "binary_sensor.remote_battery_charging" + }, + "light_device_entry_id": { + ( + "binary_sensor", + "battery_charging", + ): "binary_sensor.light_battery_charging", + ("sensor", "battery"): "sensor.light_battery", + }, + "vacuum_device_entry_id": { + ( + "binary_sensor", + "battery_charging", + ): "binary_sensor.vacuum_battery_charging", + ("sensor", "battery"): "sensor.vacuum_battery", + }, + } From 4837a83452fcf33aa45e0cfe758ef0e164c56183 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sat, 18 Apr 2020 20:43:34 +0000 Subject: [PATCH 09/14] reduce number of calls to async_add_executor_job, add constants --- .../components/homekit/accessories.py | 47 ++++++++++++++----- homeassistant/components/homekit/const.py | 5 ++ tests/components/homekit/test_accessories.py | 2 + 3 files changed, 42 insertions(+), 12 deletions(-) diff --git a/homeassistant/components/homekit/accessories.py b/homeassistant/components/homekit/accessories.py index e7542634fbc0e8..8f7c42cb7a9133 100644 --- a/homeassistant/components/homekit/accessories.py +++ b/homeassistant/components/homekit/accessories.py @@ -37,6 +37,9 @@ DEBOUNCE_TIMEOUT, DEFAULT_LOW_BATTERY_THRESHOLD, EVENT_HOMEKIT_CHANGED, + HK_CHARGING, + HK_NOT_CHARGABLE, + HK_NOT_CHARGING, MANUFACTURER, SERV_BATTERY_SERVICE, ) @@ -134,10 +137,14 @@ def __init__( self.entity_id, self.linked_battery_charging_sensor, ) + else: + _LOGGER.debug("%s: Found battery charging", self.entity_id) serv_battery = self.add_preload_service(SERV_BATTERY_SERVICE) self._char_battery = serv_battery.configure_char(CHAR_BATTERY_LEVEL, value=0) - self._char_charging = serv_battery.configure_char(CHAR_CHARGING_STATE, value=2) + self._char_charging = serv_battery.configure_char( + CHAR_CHARGING_STATE, value=HK_NOT_CHARGABLE + ) self._char_low_battery = serv_battery.configure_char( CHAR_STATUS_LOW_BATTERY, value=0 ) @@ -158,24 +165,37 @@ async def run_handler(self): self.hass.async_add_executor_job(self.update_state_callback, None, None, state) async_track_state_change(self.hass, self.entity_id, self.update_state_callback) + battery_charging_state = None + battery_state = None if self.linked_battery_sensor: - battery_state = self.hass.states.get(self.linked_battery_sensor) - self.update_linked_battery(None, None, battery_state) + linked_battery_sensor_state = self.hass.states.get( + self.linked_battery_sensor + ) + battery_state = linked_battery_sensor_state.state + battery_charging_state = linked_battery_sensor_state.attributes.get( + ATTR_BATTERY_CHARGING + ) async_track_state_change( self.hass, self.linked_battery_sensor, self.update_linked_battery ) + else: + battery_state = state.attributes.get(ATTR_BATTERY_LEVEL) if self.linked_battery_charging_sensor: battery_charging_state = self.hass.states.get( self.linked_battery_charging_sensor - ) - self.hass.async_add_executor_job( - self.update_battery, None, battery_charging_state.state - ) + ).state async_track_state_change( self.hass, self.linked_battery_charging_sensor, self.update_linked_battery_charging, ) + elif battery_charging_state is None: + battery_charging_state = state.attributes.get(ATTR_BATTERY_CHARGING) + + if battery_state is not None or battery_charging_state is not None: + self.hass.async_add_executor_job( + self.update_battery, battery_state, battery_charging_state + ) @ha_callback def update_state_callback(self, entity_id=None, old_state=None, new_state=None): @@ -233,16 +253,19 @@ def update_battery(self, battery_level_state, battery_charging_state): is_low_battery = 1 if battery_level < self.low_battery_threshold else 0 if self._char_low_battery.value != is_low_battery: self._char_low_battery.set_value(is_low_battery) - _LOGGER.debug( - "%s: Updated battery level to %d", self.entity_id, battery_level - ) + _LOGGER.debug( + "%s: Updated battery level to %d", self.entity_id, battery_level + ) if battery_charging_state is None: return - hk_charging = 1 if battery_charging_state else 0 + + hk_charging = HK_CHARGING if battery_charging_state else HK_NOT_CHARGING if self._char_charging.value != hk_charging: self._char_charging.set_value(hk_charging) - _LOGGER.debug("%s: Updated battery charging to %d", self.entity_id, hk_charging) + _LOGGER.debug( + "%s: Updated battery charging to %d", self.entity_id, hk_charging + ) def update_state(self, new_state): """Handle state change to update HomeKit value. diff --git a/homeassistant/components/homekit/const.py b/homeassistant/components/homekit/const.py index 94d3122d393b5f..f0224ce71f40ae 100644 --- a/homeassistant/components/homekit/const.py +++ b/homeassistant/components/homekit/const.py @@ -198,3 +198,8 @@ HK_POSITION_GOING_TO_MIN = 0 HK_POSITION_GOING_TO_MAX = 1 HK_POSITION_STOPPED = 2 + +# ### Charging State ### +HK_NOT_CHARGING = 0 +HK_CHARGING = 1 +HK_NOT_CHARGABLE = 2 diff --git a/tests/components/homekit/test_accessories.py b/tests/components/homekit/test_accessories.py index 2595dfe06a25f1..becbcb2d6d442d 100644 --- a/tests/components/homekit/test_accessories.py +++ b/tests/components/homekit/test_accessories.py @@ -274,10 +274,12 @@ async def test_linked_battery_charging_sensor(hass, hk_driver, caplog): assert acc._char_charging.value == 1 hass.states.async_set(linked_battery_charging_sensor, STATE_OFF, None) + await acc.run_handler() await hass.async_block_till_done() assert acc._char_charging.value == 0 hass.states.async_set(linked_battery_charging_sensor, STATE_ON, None) + await acc.run_handler() await hass.async_block_till_done() assert acc._char_charging.value == 1 From cc62b59551531436f780fa7dc41272ba2dfdb5bc Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sat, 18 Apr 2020 20:48:59 +0000 Subject: [PATCH 10/14] fix failing test --- homeassistant/components/homekit/accessories.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/homeassistant/components/homekit/accessories.py b/homeassistant/components/homekit/accessories.py index 8f7c42cb7a9133..6771be0cd57c57 100644 --- a/homeassistant/components/homekit/accessories.py +++ b/homeassistant/components/homekit/accessories.py @@ -181,9 +181,10 @@ async def run_handler(self): else: battery_state = state.attributes.get(ATTR_BATTERY_LEVEL) if self.linked_battery_charging_sensor: - battery_charging_state = self.hass.states.get( - self.linked_battery_charging_sensor - ).state + battery_charging_state = ( + self.hass.states.get(self.linked_battery_charging_sensor).state + == STATE_ON + ) async_track_state_change( self.hass, self.linked_battery_charging_sensor, From 3e0d7148a08f8919bf3598aef1115283fecd19fb Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sat, 18 Apr 2020 20:52:16 +0000 Subject: [PATCH 11/14] naming --- homeassistant/components/homekit/accessories.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/homeassistant/components/homekit/accessories.py b/homeassistant/components/homekit/accessories.py index 6771be0cd57c57..a18f42e76b60f9 100644 --- a/homeassistant/components/homekit/accessories.py +++ b/homeassistant/components/homekit/accessories.py @@ -242,12 +242,12 @@ def update_linked_battery_charging( self.update_battery, None, new_state.state == STATE_ON ) - def update_battery(self, battery_level_state, battery_charging_state): + def update_battery(self, battery_level, battery_charging): """Update battery service if available. Only call this function if self._support_battery_level is True. """ - battery_level = convert_to_float(battery_level_state) + battery_level = convert_to_float(battery_level) if battery_level is not None: if self._char_battery.value != battery_level: self._char_battery.set_value(battery_level) @@ -258,10 +258,10 @@ def update_battery(self, battery_level_state, battery_charging_state): "%s: Updated battery level to %d", self.entity_id, battery_level ) - if battery_charging_state is None: + if battery_charging is None: return - hk_charging = HK_CHARGING if battery_charging_state else HK_NOT_CHARGING + hk_charging = HK_CHARGING if battery_charging else HK_NOT_CHARGING if self._char_charging.value != hk_charging: self._char_charging.set_value(hk_charging) _LOGGER.debug( From 5a2911c7490893c6b05fea9f7a2c359dc2015d12 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sun, 19 Apr 2020 19:03:01 +0000 Subject: [PATCH 12/14] Fix tests that broke due to merge conflict --- tests/components/homekit/test_homekit.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tests/components/homekit/test_homekit.py b/tests/components/homekit/test_homekit.py index 5222cbf1e7628a..30652f58924604 100644 --- a/tests/components/homekit/test_homekit.py +++ b/tests/components/homekit/test_homekit.py @@ -368,6 +368,8 @@ async def test_homekit_stop(hass): homekit = HomeKit(hass, None, None, None, None, None, None) homekit.driver = Mock() + assert await setup.async_setup_component(hass, DOMAIN, {DOMAIN: {}}) + assert homekit.status == STATUS_READY await homekit.async_stop() await hass.async_block_till_done() @@ -445,6 +447,8 @@ async def test_homekit_finds_linked_batteries( hass, hk_driver, debounce_patcher, device_reg, entity_reg ): """Test HomeKit start method.""" + assert await setup.async_setup_component(hass, DOMAIN, {DOMAIN: {}}) + homekit = HomeKit(hass, None, None, None, {}, {"light.demo": {}}, None, None) homekit.driver = hk_driver homekit._filter = Mock(return_value=True) From 5fa6a7ef793fb8a730a682d29966bc2e732be9e6 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Mon, 20 Apr 2020 01:24:32 +0000 Subject: [PATCH 13/14] Remove flags that were being used to suppress homekit state updates after changing state from homekit as unexpected event was fixed in HAP-python 2.7.0+ --- .../components/homekit/type_media_players.py | 103 ++++++------------ 1 file changed, 33 insertions(+), 70 deletions(-) diff --git a/homeassistant/components/homekit/type_media_players.py b/homeassistant/components/homekit/type_media_players.py index 794bf671abc67b..78c11fc41f9ea4 100644 --- a/homeassistant/components/homekit/type_media_players.py +++ b/homeassistant/components/homekit/type_media_players.py @@ -100,12 +100,6 @@ def __init__(self, *args): """Initialize a Switch accessory object.""" super().__init__(*args, category=CATEGORY_SWITCH) state = self.hass.states.get(self.entity_id) - self._flag = { - FEATURE_ON_OFF: False, - FEATURE_PLAY_PAUSE: False, - FEATURE_PLAY_STOP: False, - FEATURE_TOGGLE_MUTE: False, - } self.chars = { FEATURE_ON_OFF: None, FEATURE_PLAY_PAUSE: None, @@ -154,7 +148,6 @@ def generate_service_name(self, mode): def set_on_off(self, value): """Move switch state to value if call came from HomeKit.""" _LOGGER.debug('%s: Set switch state for "on_off" to %s', self.entity_id, value) - self._flag[FEATURE_ON_OFF] = True service = SERVICE_TURN_ON if value else SERVICE_TURN_OFF params = {ATTR_ENTITY_ID: self.entity_id} self.call_service(DOMAIN, service, params) @@ -164,7 +157,6 @@ def set_play_pause(self, value): _LOGGER.debug( '%s: Set switch state for "play_pause" to %s', self.entity_id, value ) - self._flag[FEATURE_PLAY_PAUSE] = True service = SERVICE_MEDIA_PLAY if value else SERVICE_MEDIA_PAUSE params = {ATTR_ENTITY_ID: self.entity_id} self.call_service(DOMAIN, service, params) @@ -174,7 +166,6 @@ def set_play_stop(self, value): _LOGGER.debug( '%s: Set switch state for "play_stop" to %s', self.entity_id, value ) - self._flag[FEATURE_PLAY_STOP] = True service = SERVICE_MEDIA_PLAY if value else SERVICE_MEDIA_STOP params = {ATTR_ENTITY_ID: self.entity_id} self.call_service(DOMAIN, service, params) @@ -184,7 +175,6 @@ def set_toggle_mute(self, value): _LOGGER.debug( '%s: Set switch state for "toggle_mute" to %s', self.entity_id, value ) - self._flag[FEATURE_TOGGLE_MUTE] = True params = {ATTR_ENTITY_ID: self.entity_id, ATTR_MEDIA_VOLUME_MUTED: value} self.call_service(DOMAIN, SERVICE_VOLUME_MUTE, params) @@ -199,49 +189,39 @@ def update_state(self, new_state): STATE_STANDBY, "None", ) - if not self._flag[FEATURE_ON_OFF]: - _LOGGER.debug( - '%s: Set current state for "on_off" to %s', self.entity_id, hk_state - ) - if self.chars[FEATURE_ON_OFF].value != hk_state: - self.chars[FEATURE_ON_OFF].set_value(hk_state) - self._flag[FEATURE_ON_OFF] = False + _LOGGER.debug( + '%s: Set current state for "on_off" to %s', self.entity_id, hk_state + ) + if self.chars[FEATURE_ON_OFF].value != hk_state: + self.chars[FEATURE_ON_OFF].set_value(hk_state) if self.chars[FEATURE_PLAY_PAUSE]: hk_state = current_state == STATE_PLAYING - if not self._flag[FEATURE_PLAY_PAUSE]: - _LOGGER.debug( - '%s: Set current state for "play_pause" to %s', - self.entity_id, - hk_state, - ) - if self.chars[FEATURE_PLAY_PAUSE].value != hk_state: - self.chars[FEATURE_PLAY_PAUSE].set_value(hk_state) - self._flag[FEATURE_PLAY_PAUSE] = False + _LOGGER.debug( + '%s: Set current state for "play_pause" to %s', + self.entity_id, + hk_state, + ) + if self.chars[FEATURE_PLAY_PAUSE].value != hk_state: + self.chars[FEATURE_PLAY_PAUSE].set_value(hk_state) if self.chars[FEATURE_PLAY_STOP]: hk_state = current_state == STATE_PLAYING - if not self._flag[FEATURE_PLAY_STOP]: - _LOGGER.debug( - '%s: Set current state for "play_stop" to %s', - self.entity_id, - hk_state, - ) - if self.chars[FEATURE_PLAY_STOP].value != hk_state: - self.chars[FEATURE_PLAY_STOP].set_value(hk_state) - self._flag[FEATURE_PLAY_STOP] = False + _LOGGER.debug( + '%s: Set current state for "play_stop" to %s', self.entity_id, hk_state, + ) + if self.chars[FEATURE_PLAY_STOP].value != hk_state: + self.chars[FEATURE_PLAY_STOP].set_value(hk_state) if self.chars[FEATURE_TOGGLE_MUTE]: current_state = new_state.attributes.get(ATTR_MEDIA_VOLUME_MUTED) - if not self._flag[FEATURE_TOGGLE_MUTE]: - _LOGGER.debug( - '%s: Set current state for "toggle_mute" to %s', - self.entity_id, - current_state, - ) - if self.chars[FEATURE_TOGGLE_MUTE].value != current_state: - self.chars[FEATURE_TOGGLE_MUTE].set_value(current_state) - self._flag[FEATURE_TOGGLE_MUTE] = False + _LOGGER.debug( + '%s: Set current state for "toggle_mute" to %s', + self.entity_id, + current_state, + ) + if self.chars[FEATURE_TOGGLE_MUTE].value != current_state: + self.chars[FEATURE_TOGGLE_MUTE].set_value(current_state) @TYPES.register("TelevisionMediaPlayer") @@ -253,11 +233,6 @@ def __init__(self, *args): super().__init__(*args, category=CATEGORY_TELEVISION) state = self.hass.states.get(self.entity_id) - self._flag = { - CHAR_ACTIVE: False, - CHAR_ACTIVE_IDENTIFIER: False, - CHAR_MUTE: False, - } self.support_select_source = False self.sources = [] @@ -348,7 +323,6 @@ def __init__(self, *args): def set_on_off(self, value): """Move switch state to value if call came from HomeKit.""" _LOGGER.debug('%s: Set switch state for "on_off" to %s', self.entity_id, value) - self._flag[CHAR_ACTIVE] = True service = SERVICE_TURN_ON if value else SERVICE_TURN_OFF params = {ATTR_ENTITY_ID: self.entity_id} self.call_service(DOMAIN, service, params) @@ -358,7 +332,6 @@ def set_mute(self, value): _LOGGER.debug( '%s: Set switch state for "toggle_mute" to %s', self.entity_id, value ) - self._flag[CHAR_MUTE] = True params = {ATTR_ENTITY_ID: self.entity_id, ATTR_MEDIA_VOLUME_MUTED: value} self.call_service(DOMAIN, SERVICE_VOLUME_MUTE, params) @@ -379,7 +352,6 @@ def set_input_source(self, value): """Send input set value if call came from HomeKit.""" _LOGGER.debug("%s: Set current input to %s", self.entity_id, value) source = self.sources[value] - self._flag[CHAR_ACTIVE_IDENTIFIER] = True params = {ATTR_ENTITY_ID: self.entity_id, ATTR_INPUT_SOURCE: source} self.call_service(DOMAIN, SERVICE_SELECT_SOURCE, params) @@ -409,31 +381,23 @@ def update_state(self, new_state): if current_state not in ("None", STATE_OFF, STATE_UNKNOWN): hk_state = 1 - if not self._flag[CHAR_ACTIVE]: - _LOGGER.debug( - "%s: Set current active state to %s", self.entity_id, hk_state - ) - if self.char_active.value != hk_state: - self.char_active.set_value(hk_state) - self._flag[CHAR_ACTIVE] = False + _LOGGER.debug("%s: Set current active state to %s", self.entity_id, hk_state) + if self.char_active.value != hk_state: + self.char_active.set_value(hk_state) # Set mute state if CHAR_VOLUME_SELECTOR in self.chars_speaker: current_mute_state = new_state.attributes.get(ATTR_MEDIA_VOLUME_MUTED) - if not self._flag[CHAR_MUTE]: - _LOGGER.debug( - "%s: Set current mute state to %s", - self.entity_id, - current_mute_state, - ) - if self.char_mute.value != current_mute_state: - self.char_mute.set_value(current_mute_state) - self._flag[CHAR_MUTE] = False + _LOGGER.debug( + "%s: Set current mute state to %s", self.entity_id, current_mute_state, + ) + if self.char_mute.value != current_mute_state: + self.char_mute.set_value(current_mute_state) # Set active input if self.support_select_source: source_name = new_state.attributes.get(ATTR_INPUT_SOURCE) - if self.sources and not self._flag[CHAR_ACTIVE_IDENTIFIER]: + if self.sources: _LOGGER.debug( "%s: Set current input to %s", self.entity_id, source_name ) @@ -448,4 +412,3 @@ def update_state(self, new_state): ) if self.char_input_source.value != 0: self.char_input_source.set_value(0) - self._flag[CHAR_ACTIVE_IDENTIFIER] = False From 74c47af95ae8f8e8648ac91d45abbe73965bea4f Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Mon, 20 Apr 2020 20:04:44 +0000 Subject: [PATCH 14/14] fix flakey homekit tests --- tests/components/homekit/test_type_covers.py | 6 ++++++ tests/components/homekit/test_type_fans.py | 4 ++++ tests/components/homekit/test_type_media_players.py | 2 ++ tests/components/homekit/test_type_sensors.py | 8 ++++++++ 4 files changed, 20 insertions(+) diff --git a/tests/components/homekit/test_type_covers.py b/tests/components/homekit/test_type_covers.py index 6be0acece4c7c6..806aa60ee50787 100644 --- a/tests/components/homekit/test_type_covers.py +++ b/tests/components/homekit/test_type_covers.py @@ -65,6 +65,7 @@ async def test_garage_door_open_close(hass, hk_driver, cls, events): await hass.async_block_till_done() acc = cls.garage(hass, hk_driver, "Garage Door", entity_id, 2, None) await acc.run_handler() + await hass.async_block_till_done() assert acc.aid == 2 assert acc.category == 4 # GarageDoorOpener @@ -143,6 +144,7 @@ async def test_window_set_cover_position(hass, hk_driver, cls, events): await hass.async_block_till_done() acc = cls.window(hass, hk_driver, "Cover", entity_id, 2, None) await acc.run_handler() + await hass.async_block_till_done() assert acc.aid == 2 assert acc.category == 14 # WindowCovering @@ -214,6 +216,7 @@ async def test_window_cover_set_tilt(hass, hk_driver, cls, events): await hass.async_block_till_done() acc = cls.window(hass, hk_driver, "Cover", entity_id, 2, None) await acc.run_handler() + await hass.async_block_till_done() assert acc.aid == 2 assert acc.category == 14 # CATEGORY_WINDOW_COVERING @@ -277,6 +280,7 @@ async def test_window_open_close(hass, hk_driver, cls, events): hass.states.async_set(entity_id, STATE_UNKNOWN, {ATTR_SUPPORTED_FEATURES: 0}) acc = cls.window_basic(hass, hk_driver, "Cover", entity_id, 2, None) await acc.run_handler() + await hass.async_block_till_done() assert acc.aid == 2 assert acc.category == 14 # WindowCovering @@ -359,6 +363,7 @@ async def test_window_open_close_stop(hass, hk_driver, cls, events): ) acc = cls.window_basic(hass, hk_driver, "Cover", entity_id, 2, None) await acc.run_handler() + await hass.async_block_till_done() # Set from HomeKit call_close_cover = async_mock_service(hass, DOMAIN, "close_cover") @@ -407,6 +412,7 @@ async def test_window_open_close_with_position_and_stop(hass, hk_driver, cls, ev ) acc = cls.window(hass, hk_driver, "Cover", entity_id, 2, None) await acc.run_handler() + await hass.async_block_till_done() # Set from HomeKit call_stop_cover = async_mock_service(hass, DOMAIN, "stop_cover") diff --git a/tests/components/homekit/test_type_fans.py b/tests/components/homekit/test_type_fans.py index b77b799900a777..915e7c59d7c91a 100644 --- a/tests/components/homekit/test_type_fans.py +++ b/tests/components/homekit/test_type_fans.py @@ -286,6 +286,8 @@ async def test_fan_speed(hass, hk_driver, cls, events): assert acc.char_speed.value != 0 await acc.run_handler() + await hass.async_block_till_done() + assert ( acc.speed_mapping.speed_ranges == HomeKitSpeedMapping(speed_list).speed_ranges ) @@ -351,6 +353,8 @@ async def test_fan_set_all_one_shot(hass, hk_driver, cls, events): # speed to 100 when turning on a fan on a freshly booted up server. assert acc.char_speed.value != 0 await acc.run_handler() + await hass.async_block_till_done() + assert ( acc.speed_mapping.speed_ranges == HomeKitSpeedMapping(speed_list).speed_ranges ) diff --git a/tests/components/homekit/test_type_media_players.py b/tests/components/homekit/test_type_media_players.py index 7248b73afc8e39..5fe8c438ca1b8f 100644 --- a/tests/components/homekit/test_type_media_players.py +++ b/tests/components/homekit/test_type_media_players.py @@ -58,6 +58,7 @@ async def test_media_player_set_state(hass, hk_driver, events): await hass.async_block_till_done() acc = MediaPlayer(hass, hk_driver, "MediaPlayer", entity_id, 2, config) await acc.run_handler() + await hass.async_block_till_done() assert acc.aid == 2 assert acc.category == 8 # Switch @@ -199,6 +200,7 @@ async def test_media_player_television(hass, hk_driver, events, caplog): await hass.async_block_till_done() acc = TelevisionMediaPlayer(hass, hk_driver, "MediaPlayer", entity_id, 2, None) await acc.run_handler() + await hass.async_block_till_done() assert acc.aid == 2 assert acc.category == 31 # Television diff --git a/tests/components/homekit/test_type_sensors.py b/tests/components/homekit/test_type_sensors.py index 8eb993ab6f703c..ec746dec5b9092 100644 --- a/tests/components/homekit/test_type_sensors.py +++ b/tests/components/homekit/test_type_sensors.py @@ -41,6 +41,7 @@ async def test_temperature(hass, hk_driver): await hass.async_block_till_done() acc = TemperatureSensor(hass, hk_driver, "Temperature", entity_id, 2, None) await acc.run_handler() + await hass.async_block_till_done() assert acc.aid == 2 assert acc.category == 10 # Sensor @@ -74,6 +75,7 @@ async def test_humidity(hass, hk_driver): await hass.async_block_till_done() acc = HumiditySensor(hass, hk_driver, "Humidity", entity_id, 2, None) await acc.run_handler() + await hass.async_block_till_done() assert acc.aid == 2 assert acc.category == 10 # Sensor @@ -97,6 +99,7 @@ async def test_air_quality(hass, hk_driver): await hass.async_block_till_done() acc = AirQualitySensor(hass, hk_driver, "Air Quality", entity_id, 2, None) await acc.run_handler() + await hass.async_block_till_done() assert acc.aid == 2 assert acc.category == 10 # Sensor @@ -128,6 +131,7 @@ async def test_co(hass, hk_driver): await hass.async_block_till_done() acc = CarbonMonoxideSensor(hass, hk_driver, "CO", entity_id, 2, None) await acc.run_handler() + await hass.async_block_till_done() assert acc.aid == 2 assert acc.category == 10 # Sensor @@ -167,6 +171,7 @@ async def test_co2(hass, hk_driver): await hass.async_block_till_done() acc = CarbonDioxideSensor(hass, hk_driver, "CO2", entity_id, 2, None) await acc.run_handler() + await hass.async_block_till_done() assert acc.aid == 2 assert acc.category == 10 # Sensor @@ -206,6 +211,7 @@ async def test_light(hass, hk_driver): await hass.async_block_till_done() acc = LightSensor(hass, hk_driver, "Light", entity_id, 2, None) await acc.run_handler() + await hass.async_block_till_done() assert acc.aid == 2 assert acc.category == 10 # Sensor @@ -230,6 +236,7 @@ async def test_binary(hass, hk_driver): acc = BinarySensor(hass, hk_driver, "Window Opening", entity_id, 2, None) await acc.run_handler() + await hass.async_block_till_done() assert acc.aid == 2 assert acc.category == 10 # Sensor @@ -268,6 +275,7 @@ async def test_motion_uses_bool(hass, hk_driver): acc = BinarySensor(hass, hk_driver, "Motion Sensor", entity_id, 2, None) await acc.run_handler() + await hass.async_block_till_done() assert acc.aid == 2 assert acc.category == 10 # Sensor