From 38a5f25b595f92a7327b2d1ea339eea8bcb4fd40 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Mon, 11 Jan 2021 03:38:42 -1000 Subject: [PATCH] Fix vacuums that do not support start with homekit (#45030) * Fix vacuums that do not support start with homekit * fix tests --- .../components/homekit/accessories.py | 8 +- .../components/homekit/type_switches.py | 21 +++-- .../homekit/test_get_accessories.py | 4 +- .../components/homekit/test_type_switches.py | 87 ++++++++++++++++--- 4 files changed, 95 insertions(+), 25 deletions(-) diff --git a/homeassistant/components/homekit/accessories.py b/homeassistant/components/homekit/accessories.py index 6dc2e2364b6..51b6508149b 100644 --- a/homeassistant/components/homekit/accessories.py +++ b/homeassistant/components/homekit/accessories.py @@ -8,7 +8,7 @@ from pyhap.accessory import Accessory, Bridge from pyhap.accessory_driver import AccessoryDriver from pyhap.const import CATEGORY_OTHER -from homeassistant.components import cover, vacuum +from homeassistant.components import cover from homeassistant.components.cover import ( DEVICE_CLASS_GARAGE, DEVICE_CLASS_GATE, @@ -215,11 +215,7 @@ def get_accessory(hass, driver, state, aid, config): a_type = SWITCH_TYPES[switch_type] elif state.domain == "vacuum": - features = state.attributes.get(ATTR_SUPPORTED_FEATURES, 0) - if features & (vacuum.SUPPORT_START | vacuum.SUPPORT_RETURN_HOME): - a_type = "DockVacuum" - else: - a_type = "Switch" + a_type = "Vacuum" elif state.domain in ("automation", "input_boolean", "remote", "scene", "script"): a_type = "Switch" diff --git a/homeassistant/components/homekit/type_switches.py b/homeassistant/components/homekit/type_switches.py index beaccd1f3dc..b3ee8a06497 100644 --- a/homeassistant/components/homekit/type_switches.py +++ b/homeassistant/components/homekit/type_switches.py @@ -15,9 +15,12 @@ from homeassistant.components.vacuum import ( SERVICE_RETURN_TO_BASE, SERVICE_START, STATE_CLEANING, + SUPPORT_RETURN_HOME, + SUPPORT_START, ) from homeassistant.const import ( ATTR_ENTITY_ID, + ATTR_SUPPORTED_FEATURES, CONF_TYPE, SERVICE_TURN_OFF, SERVICE_TURN_ON, @@ -149,16 +152,24 @@ class Switch(HomeAccessory): self.char_on.set_value(current_state) -@TYPES.register("DockVacuum") -class DockVacuum(Switch): +@TYPES.register("Vacuum") +class Vacuum(Switch): """Generate a Switch accessory.""" def set_state(self, value): """Move switch state to value if call came from HomeKit.""" _LOGGER.debug("%s: Set switch state to %s", self.entity_id, value) - params = {ATTR_ENTITY_ID: self.entity_id} - service = SERVICE_START if value else SERVICE_RETURN_TO_BASE - self.call_service(VACUUM_DOMAIN, service, params) + state = self.hass.states.get(self.entity_id) + features = state.attributes.get(ATTR_SUPPORTED_FEATURES, 0) + + if value: + sup_start = features & SUPPORT_START + service = SERVICE_START if sup_start else SERVICE_TURN_ON + else: + sup_return_home = features & SUPPORT_RETURN_HOME + service = SERVICE_RETURN_TO_BASE if sup_return_home else SERVICE_TURN_OFF + + self.call_service(VACUUM_DOMAIN, service, {ATTR_ENTITY_ID: self.entity_id}) @callback def async_update_state(self, new_state): diff --git a/tests/components/homekit/test_get_accessories.py b/tests/components/homekit/test_get_accessories.py index 314d0516223..70d59408011 100644 --- a/tests/components/homekit/test_get_accessories.py +++ b/tests/components/homekit/test_get_accessories.py @@ -257,7 +257,7 @@ def test_type_switches(type_name, entity_id, state, attrs, config): "type_name, entity_id, state, attrs", [ ( - "DockVacuum", + "Vacuum", "vacuum.dock_vacuum", "docked", { @@ -265,7 +265,7 @@ def test_type_switches(type_name, entity_id, state, attrs, config): | vacuum.SUPPORT_RETURN_HOME }, ), - ("Switch", "vacuum.basic_vacuum", "off", {}), + ("Vacuum", "vacuum.basic_vacuum", "off", {}), ], ) def test_type_vacuum(type_name, entity_id, state, attrs): diff --git a/tests/components/homekit/test_type_switches.py b/tests/components/homekit/test_type_switches.py index 4f36adc99e1..5d218a6ef8a 100644 --- a/tests/components/homekit/test_type_switches.py +++ b/tests/components/homekit/test_type_switches.py @@ -10,20 +10,25 @@ from homeassistant.components.homekit.const import ( TYPE_SPRINKLER, TYPE_VALVE, ) -from homeassistant.components.homekit.type_switches import ( - DockVacuum, - Outlet, - Switch, - Valve, -) +from homeassistant.components.homekit.type_switches import Outlet, Switch, Vacuum, Valve from homeassistant.components.vacuum import ( DOMAIN as VACUUM_DOMAIN, SERVICE_RETURN_TO_BASE, SERVICE_START, + SERVICE_TURN_OFF, + SERVICE_TURN_ON, STATE_CLEANING, STATE_DOCKED, + SUPPORT_RETURN_HOME, + SUPPORT_START, +) +from homeassistant.const import ( + ATTR_ENTITY_ID, + ATTR_SUPPORTED_FEATURES, + CONF_TYPE, + STATE_OFF, + STATE_ON, ) -from homeassistant.const import ATTR_ENTITY_ID, CONF_TYPE, STATE_OFF, STATE_ON from homeassistant.core import split_entity_id import homeassistant.util.dt as dt_util @@ -193,14 +198,18 @@ async def test_valve_set_state(hass, hk_driver, events): assert events[-1].data[ATTR_VALUE] is None -async def test_vacuum_set_state(hass, hk_driver, events): +async def test_vacuum_set_state_with_returnhome_and_start_support( + hass, hk_driver, events +): """Test if Vacuum accessory and HA are updated accordingly.""" entity_id = "vacuum.roomba" - hass.states.async_set(entity_id, None) + hass.states.async_set( + entity_id, None, {ATTR_SUPPORTED_FEATURES: SUPPORT_RETURN_HOME | SUPPORT_START} + ) await hass.async_block_till_done() - acc = DockVacuum(hass, hk_driver, "DockVacuum", entity_id, 2, None) + acc = Vacuum(hass, hk_driver, "Vacuum", entity_id, 2, None) await acc.run_handler() await hass.async_block_till_done() assert acc.aid == 2 @@ -208,11 +217,19 @@ async def test_vacuum_set_state(hass, hk_driver, events): assert acc.char_on.value == 0 - hass.states.async_set(entity_id, STATE_CLEANING) + hass.states.async_set( + entity_id, + STATE_CLEANING, + {ATTR_SUPPORTED_FEATURES: SUPPORT_RETURN_HOME | SUPPORT_START}, + ) await hass.async_block_till_done() assert acc.char_on.value == 1 - hass.states.async_set(entity_id, STATE_DOCKED) + hass.states.async_set( + entity_id, + STATE_DOCKED, + {ATTR_SUPPORTED_FEATURES: SUPPORT_RETURN_HOME | SUPPORT_START}, + ) await hass.async_block_till_done() assert acc.char_on.value == 0 @@ -239,6 +256,52 @@ async def test_vacuum_set_state(hass, hk_driver, events): assert events[-1].data[ATTR_VALUE] is None +async def test_vacuum_set_state_without_returnhome_and_start_support( + hass, hk_driver, events +): + """Test if Vacuum accessory and HA are updated accordingly.""" + entity_id = "vacuum.roomba" + + hass.states.async_set(entity_id, None) + await hass.async_block_till_done() + + acc = Vacuum(hass, hk_driver, "Vacuum", entity_id, 2, None) + await acc.run_handler() + await hass.async_block_till_done() + assert acc.aid == 2 + assert acc.category == 8 # Switch + + assert acc.char_on.value == 0 + + hass.states.async_set(entity_id, STATE_ON) + await hass.async_block_till_done() + assert acc.char_on.value == 1 + + hass.states.async_set(entity_id, STATE_OFF) + await hass.async_block_till_done() + assert acc.char_on.value == 0 + + # Set from HomeKit + call_turn_on = async_mock_service(hass, VACUUM_DOMAIN, SERVICE_TURN_ON) + call_turn_off = async_mock_service(hass, VACUUM_DOMAIN, SERVICE_TURN_OFF) + + await hass.async_add_executor_job(acc.char_on.client_update_value, 1) + await hass.async_block_till_done() + assert acc.char_on.value == 1 + assert call_turn_on + assert call_turn_on[0].data[ATTR_ENTITY_ID] == entity_id + assert len(events) == 1 + assert events[-1].data[ATTR_VALUE] is None + + await hass.async_add_executor_job(acc.char_on.client_update_value, 0) + await hass.async_block_till_done() + assert acc.char_on.value == 0 + assert call_turn_off + assert call_turn_off[0].data[ATTR_ENTITY_ID] == entity_id + assert len(events) == 2 + assert events[-1].data[ATTR_VALUE] is None + + async def test_reset_switch(hass, hk_driver, events): """Test if switch accessory is reset correctly.""" domain = "scene"