From 1e9bc278e04ca22d9c6b5bdcb68e5a444aeebfef Mon Sep 17 00:00:00 2001 From: Eugene Prystupa Date: Fri, 10 Jul 2020 21:20:50 -0400 Subject: [PATCH] Refactor Bond integration to remove duplication (#37740) * Refactor Bond integration to remove duplication in Entity code and unit tests * Refactor Bond integration to remove duplication in Entity code and unit tests (PR feedback) --- homeassistant/components/bond/cover.py | 30 ++--------- homeassistant/components/bond/entity.py | 38 ++++++++++++++ homeassistant/components/bond/fan.py | 29 ++--------- tests/components/bond/common.py | 18 +++++-- tests/components/bond/test_cover.py | 66 ++++--------------------- tests/components/bond/test_fan.py | 64 +++++------------------- 6 files changed, 85 insertions(+), 160 deletions(-) create mode 100644 homeassistant/components/bond/entity.py diff --git a/homeassistant/components/bond/cover.py b/homeassistant/components/bond/cover.py index 82db8bb4f27..0e4a1dbc727 100644 --- a/homeassistant/components/bond/cover.py +++ b/homeassistant/components/bond/cover.py @@ -1,15 +1,15 @@ """Support for Bond covers.""" -from typing import Any, Callable, Dict, List, Optional +from typing import Any, Callable, List, Optional from bond import BOND_DEVICE_TYPE_MOTORIZED_SHADES, Bond from homeassistant.components.cover import DEVICE_CLASS_SHADE, CoverEntity from homeassistant.config_entries import ConfigEntry -from homeassistant.const import ATTR_NAME from homeassistant.core import HomeAssistant from homeassistant.helpers.entity import Entity from .const import DOMAIN +from .entity import BondEntity from .utils import BondDevice, get_bond_devices @@ -32,13 +32,13 @@ async def async_setup_entry( async_add_entities(covers, True) -class BondCover(CoverEntity): +class BondCover(BondEntity, CoverEntity): """Representation of a Bond cover.""" def __init__(self, bond: Bond, device: BondDevice): """Create HA entity representing Bond cover.""" - self._bond = bond - self._device = device + super().__init__(bond, device) + self._closed: Optional[bool] = None @property @@ -46,26 +46,6 @@ class BondCover(CoverEntity): """Get device class.""" return DEVICE_CLASS_SHADE - @property - def unique_id(self) -> Optional[str]: - """Get unique ID for the entity.""" - return self._device.device_id - - @property - def name(self) -> Optional[str]: - """Get entity name.""" - return self._device.name - - @property - def device_info(self) -> Optional[Dict[str, Any]]: - """Get a an HA device representing this cover.""" - return {ATTR_NAME: self.name, "identifiers": {(DOMAIN, self._device.device_id)}} - - @property - def assumed_state(self) -> bool: - """Let HA know this entity relies on an assumed state tracked by Bond.""" - return True - def update(self): """Fetch assumed state of the cover from the hub using API.""" state: dict = self._bond.getDeviceState(self._device.device_id) diff --git a/homeassistant/components/bond/entity.py b/homeassistant/components/bond/entity.py new file mode 100644 index 00000000000..aa4b86b2acf --- /dev/null +++ b/homeassistant/components/bond/entity.py @@ -0,0 +1,38 @@ +"""An abstract class common to all Bond entities.""" +from typing import Any, Dict, Optional + +from bond import Bond + +from homeassistant.components.bond.utils import BondDevice +from homeassistant.const import ATTR_NAME + +from .const import DOMAIN + + +class BondEntity: + """Generic Bond entity encapsulating common features of any Bond controlled device.""" + + def __init__(self, bond: Bond, device: BondDevice): + """Initialize entity with API and device info.""" + self._bond = bond + self._device = device + + @property + def unique_id(self) -> Optional[str]: + """Get unique ID for the entity.""" + return self._device.device_id + + @property + def name(self) -> Optional[str]: + """Get entity name.""" + return self._device.name + + @property + def device_info(self) -> Optional[Dict[str, Any]]: + """Get a an HA device representing this Bond controlled device.""" + return {ATTR_NAME: self.name, "identifiers": {(DOMAIN, self._device.device_id)}} + + @property + def assumed_state(self) -> bool: + """Let HA know this entity relies on an assumed state tracked by Bond.""" + return True diff --git a/homeassistant/components/bond/fan.py b/homeassistant/components/bond/fan.py index 170a72bbfe9..b048154ba01 100644 --- a/homeassistant/components/bond/fan.py +++ b/homeassistant/components/bond/fan.py @@ -1,5 +1,5 @@ """Support for Bond fans.""" -from typing import Any, Callable, Dict, List, Optional +from typing import Any, Callable, List, Optional from bond import BOND_DEVICE_TYPE_CEILING_FAN, Bond @@ -15,8 +15,8 @@ from homeassistant.config_entries import ConfigEntry from homeassistant.core import HomeAssistant from homeassistant.helpers.entity import Entity -from ...const import ATTR_NAME from .const import DOMAIN +from .entity import BondEntity from .utils import BondDevice, get_bond_devices @@ -39,37 +39,16 @@ async def async_setup_entry( async_add_entities(fans, True) -class BondFan(FanEntity): +class BondFan(BondEntity, FanEntity): """Representation of a Bond fan.""" def __init__(self, bond: Bond, device: BondDevice): """Create HA entity representing Bond fan.""" - self._bond = bond - self._device = device + super().__init__(bond, device) self._power: Optional[bool] = None self._speed: Optional[int] = None - @property - def unique_id(self) -> Optional[str]: - """Get unique ID for the entity.""" - return self._device.device_id - - @property - def name(self) -> Optional[str]: - """Get entity name.""" - return self._device.name - - @property - def device_info(self) -> Optional[Dict[str, Any]]: - """Get a an HA device representing this fan.""" - return {ATTR_NAME: self.name, "identifiers": {(DOMAIN, self._device.device_id)}} - - @property - def assumed_state(self) -> bool: - """Let HA know this entity relies on an assumed state tracked by Bond.""" - return True - @property def supported_features(self) -> int: """Flag supported features.""" diff --git a/tests/components/bond/common.py b/tests/components/bond/common.py index b315f7e86fd..975efaf12c5 100644 --- a/tests/components/bond/common.py +++ b/tests/components/bond/common.py @@ -1,4 +1,7 @@ """Common methods used across tests for Bond.""" +from typing import Any, Dict + +from homeassistant import core from homeassistant.components.bond.const import DOMAIN as BOND_DOMAIN from homeassistant.const import CONF_ACCESS_TOKEN, CONF_HOST from homeassistant.setup import async_setup_component @@ -7,7 +10,9 @@ from tests.async_mock import patch from tests.common import MockConfigEntry -async def setup_platform(hass, platform): +async def setup_platform( + hass: core.HomeAssistant, platform: str, discovered_device: Dict[str, Any] +): """Set up the specified Bond platform.""" mock_entry = MockConfigEntry( domain=BOND_DOMAIN, @@ -15,8 +20,15 @@ async def setup_platform(hass, platform): ) mock_entry.add_to_hass(hass) - with patch("homeassistant.components.bond.PLATFORMS", [platform]): + with patch("homeassistant.components.bond.PLATFORMS", [platform]), patch( + "homeassistant.components.bond.Bond.getDeviceIds", + return_value=["bond-device-id"], + ), patch( + "homeassistant.components.bond.Bond.getDevice", return_value=discovered_device + ), patch( + "homeassistant.components.bond.Bond.getDeviceState", return_value={} + ): assert await async_setup_component(hass, BOND_DOMAIN, {}) - await hass.async_block_till_done() + await hass.async_block_till_done() return mock_entry diff --git a/tests/components/bond/test_cover.py b/tests/components/bond/test_cover.py index da81e3ad27f..b76d2e9165d 100644 --- a/tests/components/bond/test_cover.py +++ b/tests/components/bond/test_cover.py @@ -22,21 +22,15 @@ from tests.common import async_fire_time_changed _LOGGER = logging.getLogger(__name__) -TEST_DEVICE_IDS = ["device-1"] -TEST_COVER_DEVICE = {"name": "name-1", "type": BOND_DEVICE_TYPE_MOTORIZED_SHADES} + +def shades(name: str): + """Create motorized shades with given name.""" + return {"name": name, "type": BOND_DEVICE_TYPE_MOTORIZED_SHADES} async def test_entity_registry(hass: core.HomeAssistant): """Tests that the devices are registered in the entity registry.""" - - with patch( - "homeassistant.components.bond.Bond.getDeviceIds", return_value=TEST_DEVICE_IDS - ), patch( - "homeassistant.components.bond.Bond.getDevice", return_value=TEST_COVER_DEVICE - ), patch( - "homeassistant.components.bond.Bond.getDeviceState", return_value={} - ): - await setup_platform(hass, COVER_DOMAIN) + await setup_platform(hass, COVER_DOMAIN, shades("name-1")) registry: EntityRegistry = await hass.helpers.entity_registry.async_get_registry() assert [key for key in registry.entities.keys()] == ["cover.name_1"] @@ -44,15 +38,7 @@ async def test_entity_registry(hass: core.HomeAssistant): async def test_open_cover(hass: core.HomeAssistant): """Tests that open cover command delegates to API.""" - - with patch( - "homeassistant.components.bond.Bond.getDeviceIds", return_value=TEST_DEVICE_IDS - ), patch( - "homeassistant.components.bond.Bond.getDevice", return_value=TEST_COVER_DEVICE - ), patch( - "homeassistant.components.bond.Bond.getDeviceState", return_value={} - ): - await setup_platform(hass, COVER_DOMAIN) + await setup_platform(hass, COVER_DOMAIN, shades("name-1")) with patch("homeassistant.components.bond.Bond.open") as mock_open: await hass.services.async_call( @@ -67,15 +53,7 @@ async def test_open_cover(hass: core.HomeAssistant): async def test_close_cover(hass: core.HomeAssistant): """Tests that close cover command delegates to API.""" - - with patch( - "homeassistant.components.bond.Bond.getDeviceIds", return_value=TEST_DEVICE_IDS - ), patch( - "homeassistant.components.bond.Bond.getDevice", return_value=TEST_COVER_DEVICE - ), patch( - "homeassistant.components.bond.Bond.getDeviceState", return_value={} - ): - await setup_platform(hass, COVER_DOMAIN) + await setup_platform(hass, COVER_DOMAIN, shades("name-1")) with patch("homeassistant.components.bond.Bond.close") as mock_close: await hass.services.async_call( @@ -90,15 +68,7 @@ async def test_close_cover(hass: core.HomeAssistant): async def test_stop_cover(hass: core.HomeAssistant): """Tests that stop cover command delegates to API.""" - - with patch( - "homeassistant.components.bond.Bond.getDeviceIds", return_value=TEST_DEVICE_IDS - ), patch( - "homeassistant.components.bond.Bond.getDevice", return_value=TEST_COVER_DEVICE - ), patch( - "homeassistant.components.bond.Bond.getDeviceState", return_value={} - ): - await setup_platform(hass, COVER_DOMAIN) + await setup_platform(hass, COVER_DOMAIN, shades("name-1")) with patch("homeassistant.components.bond.Bond.hold") as mock_hold: await hass.services.async_call( @@ -113,15 +83,7 @@ async def test_stop_cover(hass: core.HomeAssistant): async def test_update_reports_open_cover(hass: core.HomeAssistant): """Tests that update command sets correct state when Bond API reports cover is open.""" - - with patch( - "homeassistant.components.bond.Bond.getDeviceIds", return_value=TEST_DEVICE_IDS - ), patch( - "homeassistant.components.bond.Bond.getDevice", return_value=TEST_COVER_DEVICE - ), patch( - "homeassistant.components.bond.Bond.getDeviceState", return_value={} - ): - await setup_platform(hass, COVER_DOMAIN) + await setup_platform(hass, COVER_DOMAIN, shades("name-1")) with patch( "homeassistant.components.bond.Bond.getDeviceState", return_value={"open": 1} @@ -134,15 +96,7 @@ async def test_update_reports_open_cover(hass: core.HomeAssistant): async def test_update_reports_closed_cover(hass: core.HomeAssistant): """Tests that update command sets correct state when Bond API reports cover is closed.""" - - with patch( - "homeassistant.components.bond.Bond.getDeviceIds", return_value=TEST_DEVICE_IDS - ), patch( - "homeassistant.components.bond.Bond.getDevice", return_value=TEST_COVER_DEVICE - ), patch( - "homeassistant.components.bond.Bond.getDeviceState", return_value={} - ): - await setup_platform(hass, COVER_DOMAIN) + await setup_platform(hass, COVER_DOMAIN, shades("name-1")) with patch( "homeassistant.components.bond.Bond.getDeviceState", return_value={"open": 0} diff --git a/tests/components/bond/test_fan.py b/tests/components/bond/test_fan.py index e83c1f86511..eab05657b77 100644 --- a/tests/components/bond/test_fan.py +++ b/tests/components/bond/test_fan.py @@ -15,25 +15,19 @@ from .common import setup_platform from tests.async_mock import patch -TEST_DEVICE_IDS = ["device-1"] -TEST_FAN_DEVICE = { - "name": "name-1", - "type": BOND_DEVICE_TYPE_CEILING_FAN, - "actions": ["SetSpeed"], -} + +def ceiling_fan(name: str): + """Create a ceiling fan with given name.""" + return { + "name": name, + "type": BOND_DEVICE_TYPE_CEILING_FAN, + "actions": ["SetSpeed"], + } async def test_entity_registry(hass: core.HomeAssistant): """Tests that the devices are registered in the entity registry.""" - - with patch( - "homeassistant.components.bond.Bond.getDeviceIds", return_value=TEST_DEVICE_IDS - ), patch( - "homeassistant.components.bond.Bond.getDevice", return_value=TEST_FAN_DEVICE - ), patch( - "homeassistant.components.bond.Bond.getDeviceState", return_value={} - ): - await setup_platform(hass, FAN_DOMAIN) + await setup_platform(hass, FAN_DOMAIN, ceiling_fan("name-1")) registry: EntityRegistry = await hass.helpers.entity_registry.async_get_registry() assert [key for key in registry.entities.keys()] == ["fan.name_1"] @@ -41,15 +35,7 @@ async def test_entity_registry(hass: core.HomeAssistant): async def test_turn_on_fan(hass: core.HomeAssistant): """Tests that turn on command delegates to API.""" - - with patch( - "homeassistant.components.bond.Bond.getDeviceIds", return_value=TEST_DEVICE_IDS - ), patch( - "homeassistant.components.bond.Bond.getDevice", return_value=TEST_FAN_DEVICE - ), patch( - "homeassistant.components.bond.Bond.getDeviceState", return_value={} - ): - await setup_platform(hass, FAN_DOMAIN) + await setup_platform(hass, FAN_DOMAIN, ceiling_fan("name-1")) with patch("homeassistant.components.bond.Bond.turnOn") as mock_turn_on, patch( "homeassistant.components.bond.Bond.setSpeed" @@ -68,15 +54,7 @@ async def test_turn_on_fan(hass: core.HomeAssistant): async def test_turn_off_fan(hass: core.HomeAssistant): """Tests that turn off command delegates to API.""" - - with patch( - "homeassistant.components.bond.Bond.getDeviceIds", return_value=TEST_DEVICE_IDS - ), patch( - "homeassistant.components.bond.Bond.getDevice", return_value=TEST_FAN_DEVICE - ), patch( - "homeassistant.components.bond.Bond.getDeviceState", return_value={} - ): - await setup_platform(hass, FAN_DOMAIN) + await setup_platform(hass, FAN_DOMAIN, ceiling_fan("name-1")) with patch("homeassistant.components.bond.Bond.turnOff") as mock_turn_off: await hass.services.async_call( @@ -88,15 +66,7 @@ async def test_turn_off_fan(hass: core.HomeAssistant): async def test_update_reports_fan_on(hass: core.HomeAssistant): """Tests that update command sets correct state when Bond API reports fan power is on.""" - - with patch( - "homeassistant.components.bond.Bond.getDeviceIds", return_value=TEST_DEVICE_IDS - ), patch( - "homeassistant.components.bond.Bond.getDevice", return_value=TEST_FAN_DEVICE - ), patch( - "homeassistant.components.bond.Bond.getDeviceState", return_value={} - ): - await setup_platform(hass, FAN_DOMAIN) + await setup_platform(hass, FAN_DOMAIN, ceiling_fan("name-1")) with patch( "homeassistant.components.bond.Bond.getDeviceState", @@ -110,15 +80,7 @@ async def test_update_reports_fan_on(hass: core.HomeAssistant): async def test_update_reports_fan_off(hass: core.HomeAssistant): """Tests that update command sets correct state when Bond API reports fan power is off.""" - - with patch( - "homeassistant.components.bond.Bond.getDeviceIds", return_value=TEST_DEVICE_IDS - ), patch( - "homeassistant.components.bond.Bond.getDevice", return_value=TEST_FAN_DEVICE - ), patch( - "homeassistant.components.bond.Bond.getDeviceState", return_value={} - ): - await setup_platform(hass, FAN_DOMAIN) + await setup_platform(hass, FAN_DOMAIN, ceiling_fan("name-1")) with patch( "homeassistant.components.bond.Bond.getDeviceState",