From 5b0b01d727f8364229dafc443b7744abe7e2f32e Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Fri, 19 Feb 2021 21:06:43 -1000 Subject: [PATCH] Implement suggested areas in bond (#45942) Co-authored-by: Paulus Schoutsen --- homeassistant/components/bond/__init__.py | 4 +- homeassistant/components/bond/config_flow.py | 22 ++-- homeassistant/components/bond/entity.py | 3 +- homeassistant/components/bond/manifest.json | 2 +- homeassistant/components/bond/utils.py | 52 +++++--- requirements_all.txt | 2 +- requirements_test_all.txt | 2 +- tests/components/bond/common.py | 35 +++++- tests/components/bond/test_config_flow.py | 62 ++++++++-- tests/components/bond/test_init.py | 124 ++++++++++++++++--- tests/components/bond/test_light.py | 2 +- 11 files changed, 249 insertions(+), 61 deletions(-) diff --git a/homeassistant/components/bond/__init__.py b/homeassistant/components/bond/__init__.py index 4e6705cbe09..9d0a613000a 100644 --- a/homeassistant/components/bond/__init__.py +++ b/homeassistant/components/bond/__init__.py @@ -50,14 +50,16 @@ async def async_setup_entry(hass: HomeAssistant, entry: ConfigEntry): if not entry.unique_id: hass.config_entries.async_update_entry(entry, unique_id=hub.bond_id) + hub_name = hub.name or hub.bond_id device_registry = await dr.async_get_registry(hass) device_registry.async_get_or_create( config_entry_id=config_entry_id, identifiers={(DOMAIN, hub.bond_id)}, manufacturer=BRIDGE_MAKE, - name=hub.bond_id, + name=hub_name, model=hub.target, sw_version=hub.fw_ver, + suggested_area=hub.location, ) _async_remove_old_device_identifiers(config_entry_id, device_registry, hub) diff --git a/homeassistant/components/bond/config_flow.py b/homeassistant/components/bond/config_flow.py index 9298961269e..0132df486d3 100644 --- a/homeassistant/components/bond/config_flow.py +++ b/homeassistant/components/bond/config_flow.py @@ -1,6 +1,6 @@ """Config flow for Bond integration.""" import logging -from typing import Any, Dict, Optional +from typing import Any, Dict, Optional, Tuple from aiohttp import ClientConnectionError, ClientResponseError from bond_api import Bond @@ -16,6 +16,7 @@ from homeassistant.const import ( from .const import CONF_BOND_ID from .const import DOMAIN # pylint:disable=unused-import +from .utils import BondHub _LOGGER = logging.getLogger(__name__) @@ -25,14 +26,13 @@ DATA_SCHEMA_USER = vol.Schema( DATA_SCHEMA_DISCOVERY = vol.Schema({vol.Required(CONF_ACCESS_TOKEN): str}) -async def _validate_input(data: Dict[str, Any]) -> str: +async def _validate_input(data: Dict[str, Any]) -> Tuple[str, Optional[str]]: """Validate the user input allows us to connect.""" + bond = Bond(data[CONF_HOST], data[CONF_ACCESS_TOKEN]) try: - bond = Bond(data[CONF_HOST], data[CONF_ACCESS_TOKEN]) - version = await bond.version() - # call to non-version API is needed to validate authentication - await bond.devices() + hub = BondHub(bond) + await hub.setup(max_devices=1) except ClientConnectionError as error: raise InputValidationError("cannot_connect") from error except ClientResponseError as error: @@ -44,11 +44,10 @@ async def _validate_input(data: Dict[str, Any]) -> str: raise InputValidationError("unknown") from error # Return unique ID from the hub to be stored in the config entry. - bond_id = version.get("bondid") - if not bond_id: + if not hub.bond_id: raise InputValidationError("old_firmware") - return bond_id + return hub.bond_id, hub.name class ConfigFlow(config_entries.ConfigFlow, domain=DOMAIN): @@ -113,10 +112,11 @@ class ConfigFlow(config_entries.ConfigFlow, domain=DOMAIN): ) async def _try_create_entry(self, data: Dict[str, Any]) -> Dict[str, Any]: - bond_id = await _validate_input(data) + bond_id, name = await _validate_input(data) await self.async_set_unique_id(bond_id) self._abort_if_unique_id_configured() - return self.async_create_entry(title=bond_id, data=data) + hub_name = name or bond_id + return self.async_create_entry(title=hub_name, data=data) class InputValidationError(exceptions.HomeAssistantError): diff --git a/homeassistant/components/bond/entity.py b/homeassistant/components/bond/entity.py index 769794a31e8..f6165eb7890 100644 --- a/homeassistant/components/bond/entity.py +++ b/homeassistant/components/bond/entity.py @@ -65,7 +65,8 @@ class BondEntity(Entity): device_info = { ATTR_NAME: self.name, "manufacturer": self._hub.make, - "identifiers": {(DOMAIN, self._hub.bond_id, self._device_id)}, + "identifiers": {(DOMAIN, self._hub.bond_id, self._device.device_id)}, + "suggested_area": self._device.location, "via_device": (DOMAIN, self._hub.bond_id), } if not self._hub.is_bridge: diff --git a/homeassistant/components/bond/manifest.json b/homeassistant/components/bond/manifest.json index e1ec5e5dd46..cf009c11caa 100644 --- a/homeassistant/components/bond/manifest.json +++ b/homeassistant/components/bond/manifest.json @@ -3,7 +3,7 @@ "name": "Bond", "config_flow": true, "documentation": "https://www.home-assistant.io/integrations/bond", - "requirements": ["bond-api==0.1.9"], + "requirements": ["bond-api==0.1.10"], "zeroconf": ["_bond._tcp.local."], "codeowners": ["@prystupa"], "quality_scale": "platinum" diff --git a/homeassistant/components/bond/utils.py b/homeassistant/components/bond/utils.py index df3373ed7a1..6d3aacf5e42 100644 --- a/homeassistant/components/bond/utils.py +++ b/homeassistant/components/bond/utils.py @@ -1,7 +1,9 @@ """Reusable utilities for the Bond component.""" +import asyncio import logging from typing import List, Optional +from aiohttp import ClientResponseError from bond_api import Action, Bond from .const import BRIDGE_MAKE @@ -39,7 +41,7 @@ class BondDevice: @property def location(self) -> str: """Get the location of this device.""" - return self._attrs["location"] + return self._attrs.get("location") @property def template(self) -> str: @@ -89,31 +91,40 @@ class BondHub: def __init__(self, bond: Bond): """Initialize Bond Hub.""" self.bond: Bond = bond + self._bridge: Optional[dict] = None self._version: Optional[dict] = None self._devices: Optional[List[BondDevice]] = None - async def setup(self): + async def setup(self, max_devices=None): """Read hub version information.""" self._version = await self.bond.version() _LOGGER.debug("Bond reported the following version info: %s", self._version) - # Fetch all available devices using Bond API. device_ids = await self.bond.devices() - self._devices = [ - BondDevice( - device_id, - await self.bond.device(device_id), - await self.bond.device_properties(device_id), + self._devices = [] + for idx, device_id in enumerate(device_ids): + if max_devices is not None and idx >= max_devices: + break + + device, props = await asyncio.gather( + self.bond.device(device_id), self.bond.device_properties(device_id) ) - for device_id in device_ids - ] + + self._devices.append(BondDevice(device_id, device, props)) _LOGGER.debug("Discovered Bond devices: %s", self._devices) + try: + # Smart by bond devices do not have a bridge api call + self._bridge = await self.bond.bridge() + except ClientResponseError: + self._bridge = {} + _LOGGER.debug("Bond reported the following bridge info: %s", self._bridge) @property - def bond_id(self) -> str: + def bond_id(self) -> Optional[str]: """Return unique Bond ID for this hub.""" - return self._version["bondid"] + # Old firmwares are missing the bondid + return self._version.get("bondid") @property def target(self) -> str: @@ -130,6 +141,20 @@ class BondHub: """Return this hub make.""" return self._version.get("make", BRIDGE_MAKE) + @property + def name(self) -> Optional[str]: + """Get the name of this bridge.""" + if not self.is_bridge and self._devices: + return self._devices[0].name + return self._bridge.get("name") + + @property + def location(self) -> Optional[str]: + """Get the location of this bridge.""" + if not self.is_bridge and self._devices: + return self._devices[0].location + return self._bridge.get("location") + @property def fw_ver(self) -> str: """Return this hub firmware version.""" @@ -143,5 +168,4 @@ class BondHub: @property def is_bridge(self) -> bool: """Return if the Bond is a Bond Bridge.""" - # If False, it means that it is a Smart by Bond product. Assumes that it is if the model is not available. - return self._version.get("model", "BD-").startswith("BD-") + return bool(self._bridge) diff --git a/requirements_all.txt b/requirements_all.txt index 0e3c6117780..971b8ac9c92 100644 --- a/requirements_all.txt +++ b/requirements_all.txt @@ -371,7 +371,7 @@ blockchain==1.4.4 # bme680==1.0.5 # homeassistant.components.bond -bond-api==0.1.9 +bond-api==0.1.10 # homeassistant.components.amazon_polly # homeassistant.components.route53 diff --git a/requirements_test_all.txt b/requirements_test_all.txt index 317063ffffc..c552f8452c0 100644 --- a/requirements_test_all.txt +++ b/requirements_test_all.txt @@ -205,7 +205,7 @@ blebox_uniapi==1.3.2 blinkpy==0.17.0 # homeassistant.components.bond -bond-api==0.1.9 +bond-api==0.1.10 # homeassistant.components.braviatv bravia-tv==1.0.8 diff --git a/tests/components/bond/common.py b/tests/components/bond/common.py index ba4d10c8892..54d127832b5 100644 --- a/tests/components/bond/common.py +++ b/tests/components/bond/common.py @@ -29,13 +29,16 @@ async def setup_bond_entity( patch_version=False, patch_device_ids=False, patch_platforms=False, + patch_bridge=False, ): """Set up Bond entity.""" config_entry.add_to_hass(hass) - with patch_start_bpup(), patch_bond_version( - enabled=patch_version - ), patch_bond_device_ids(enabled=patch_device_ids), patch_setup_entry( + with patch_start_bpup(), patch_bond_bridge( + enabled=patch_bridge + ), patch_bond_version(enabled=patch_version), patch_bond_device_ids( + enabled=patch_device_ids + ), patch_setup_entry( "cover", enabled=patch_platforms ), patch_setup_entry( "fan", enabled=patch_platforms @@ -56,6 +59,7 @@ async def setup_platform( bond_version: Dict[str, Any] = None, props: Dict[str, Any] = None, state: Dict[str, Any] = None, + bridge: Dict[str, Any] = None, ): """Set up the specified Bond platform.""" mock_entry = MockConfigEntry( @@ -65,7 +69,9 @@ async def setup_platform( mock_entry.add_to_hass(hass) with patch("homeassistant.components.bond.PLATFORMS", [platform]): - with patch_bond_version(return_value=bond_version), patch_bond_device_ids( + with patch_bond_version(return_value=bond_version), patch_bond_bridge( + return_value=bridge + ), patch_bond_device_ids( return_value=[bond_device_id] ), patch_start_bpup(), patch_bond_device( return_value=discovered_device @@ -97,6 +103,27 @@ def patch_bond_version( ) +def patch_bond_bridge( + enabled: bool = True, return_value: Optional[dict] = None, side_effect=None +): + """Patch Bond API bridge endpoint.""" + if not enabled: + return nullcontext() + + if return_value is None: + return_value = { + "name": "bond-name", + "location": "bond-location", + "bluelight": 127, + } + + return patch( + "homeassistant.components.bond.Bond.bridge", + return_value=return_value, + side_effect=side_effect, + ) + + def patch_bond_device_ids(enabled: bool = True, return_value=None, side_effect=None): """Patch Bond API devices endpoint.""" if not enabled: diff --git a/tests/components/bond/test_config_flow.py b/tests/components/bond/test_config_flow.py index dba6c590641..2a76e1fa6a0 100644 --- a/tests/components/bond/test_config_flow.py +++ b/tests/components/bond/test_config_flow.py @@ -8,7 +8,13 @@ from homeassistant import config_entries, core, setup from homeassistant.components.bond.const import DOMAIN from homeassistant.const import CONF_ACCESS_TOKEN, CONF_HOST -from .common import patch_bond_device_ids, patch_bond_version +from .common import ( + patch_bond_bridge, + patch_bond_device, + patch_bond_device_ids, + patch_bond_device_properties, + patch_bond_version, +) from tests.common import MockConfigEntry @@ -24,7 +30,9 @@ async def test_user_form(hass: core.HomeAssistant): with patch_bond_version( return_value={"bondid": "test-bond-id"} - ), patch_bond_device_ids(), _patch_async_setup() as mock_setup, _patch_async_setup_entry() as mock_setup_entry: + ), patch_bond_device_ids( + return_value=["f6776c11", "f6776c12"] + ), patch_bond_bridge(), patch_bond_device_properties(), patch_bond_device(), _patch_async_setup() as mock_setup, _patch_async_setup_entry() as mock_setup_entry: result2 = await hass.config_entries.flow.async_configure( result["flow_id"], {CONF_HOST: "some host", CONF_ACCESS_TOKEN: "test-token"}, @@ -32,7 +40,43 @@ async def test_user_form(hass: core.HomeAssistant): await hass.async_block_till_done() assert result2["type"] == "create_entry" - assert result2["title"] == "test-bond-id" + assert result2["title"] == "bond-name" + assert result2["data"] == { + CONF_HOST: "some host", + CONF_ACCESS_TOKEN: "test-token", + } + assert len(mock_setup.mock_calls) == 1 + assert len(mock_setup_entry.mock_calls) == 1 + + +async def test_user_form_with_non_bridge(hass: core.HomeAssistant): + """Test setup a smart by bond fan.""" + await setup.async_setup_component(hass, "persistent_notification", {}) + result = await hass.config_entries.flow.async_init( + DOMAIN, context={"source": config_entries.SOURCE_USER} + ) + assert result["type"] == "form" + assert result["errors"] == {} + + with patch_bond_version( + return_value={"bondid": "test-bond-id"} + ), patch_bond_device_ids( + return_value=["f6776c11"] + ), patch_bond_device_properties(), patch_bond_device( + return_value={ + "name": "New Fan", + } + ), patch_bond_bridge( + return_value={} + ), _patch_async_setup() as mock_setup, _patch_async_setup_entry() as mock_setup_entry: + result2 = await hass.config_entries.flow.async_configure( + result["flow_id"], + {CONF_HOST: "some host", CONF_ACCESS_TOKEN: "test-token"}, + ) + await hass.async_block_till_done() + + assert result2["type"] == "create_entry" + assert result2["title"] == "New Fan" assert result2["data"] == { CONF_HOST: "some host", CONF_ACCESS_TOKEN: "test-token", @@ -49,7 +93,7 @@ async def test_user_form_invalid_auth(hass: core.HomeAssistant): with patch_bond_version( return_value={"bond_id": "test-bond-id"} - ), patch_bond_device_ids( + ), patch_bond_bridge(), patch_bond_device_ids( side_effect=ClientResponseError(Mock(), Mock(), status=401), ): result2 = await hass.config_entries.flow.async_configure( @@ -69,7 +113,7 @@ async def test_user_form_cannot_connect(hass: core.HomeAssistant): with patch_bond_version( side_effect=ClientConnectionError() - ), patch_bond_device_ids(): + ), patch_bond_bridge(), patch_bond_device_ids(): result2 = await hass.config_entries.flow.async_configure( result["flow_id"], {CONF_HOST: "some host", CONF_ACCESS_TOKEN: "test-token"}, @@ -87,7 +131,7 @@ async def test_user_form_old_firmware(hass: core.HomeAssistant): with patch_bond_version( return_value={"no_bond_id": "present"} - ), patch_bond_device_ids(): + ), patch_bond_bridge(), patch_bond_device_ids(): result2 = await hass.config_entries.flow.async_configure( result["flow_id"], {CONF_HOST: "some host", CONF_ACCESS_TOKEN: "test-token"}, @@ -133,7 +177,7 @@ async def test_user_form_one_entry_per_device_allowed(hass: core.HomeAssistant): with patch_bond_version( return_value={"bondid": "already-registered-bond-id"} - ), patch_bond_device_ids(), _patch_async_setup() as mock_setup, _patch_async_setup_entry() as mock_setup_entry: + ), patch_bond_bridge(), patch_bond_device_ids(), _patch_async_setup() as mock_setup, _patch_async_setup_entry() as mock_setup_entry: result2 = await hass.config_entries.flow.async_configure( result["flow_id"], {CONF_HOST: "some host", CONF_ACCESS_TOKEN: "test-token"}, @@ -160,7 +204,7 @@ async def test_zeroconf_form(hass: core.HomeAssistant): with patch_bond_version( return_value={"bondid": "test-bond-id"} - ), patch_bond_device_ids(), _patch_async_setup() as mock_setup, _patch_async_setup_entry() as mock_setup_entry: + ), patch_bond_bridge(), patch_bond_device_ids(), _patch_async_setup() as mock_setup, _patch_async_setup_entry() as mock_setup_entry: result2 = await hass.config_entries.flow.async_configure( result["flow_id"], {CONF_ACCESS_TOKEN: "test-token"}, @@ -168,7 +212,7 @@ async def test_zeroconf_form(hass: core.HomeAssistant): await hass.async_block_till_done() assert result2["type"] == "create_entry" - assert result2["title"] == "test-bond-id" + assert result2["title"] == "bond-name" assert result2["data"] == { CONF_HOST: "test-host", CONF_ACCESS_TOKEN: "test-token", diff --git a/tests/components/bond/test_init.py b/tests/components/bond/test_init.py index 4dc7ae5c8d4..7346acc5276 100644 --- a/tests/components/bond/test_init.py +++ b/tests/components/bond/test_init.py @@ -1,5 +1,7 @@ """Tests for the Bond module.""" -from aiohttp import ClientConnectionError +from unittest.mock import Mock + +from aiohttp import ClientConnectionError, ClientResponseError from bond_api import DeviceType from homeassistant.components.bond.const import DOMAIN @@ -14,6 +16,7 @@ from homeassistant.helpers import device_registry as dr from homeassistant.setup import async_setup_component from .common import ( + patch_bond_bridge, patch_bond_device, patch_bond_device_ids, patch_bond_device_properties, @@ -54,25 +57,22 @@ async def test_async_setup_entry_sets_up_hub_and_supported_domains(hass: HomeAss data={CONF_HOST: "some host", CONF_ACCESS_TOKEN: "test-token"}, ) - with patch_bond_version( + with patch_bond_bridge(), patch_bond_version( return_value={ "bondid": "test-bond-id", "target": "test-model", "fw_ver": "test-version", } - ): - with patch_setup_entry( - "cover" - ) as mock_cover_async_setup_entry, patch_setup_entry( - "fan" - ) as mock_fan_async_setup_entry, patch_setup_entry( - "light" - ) as mock_light_async_setup_entry, patch_setup_entry( - "switch" - ) as mock_switch_async_setup_entry: - result = await setup_bond_entity(hass, config_entry, patch_device_ids=True) - assert result is True - await hass.async_block_till_done() + ), patch_setup_entry("cover") as mock_cover_async_setup_entry, patch_setup_entry( + "fan" + ) as mock_fan_async_setup_entry, patch_setup_entry( + "light" + ) as mock_light_async_setup_entry, patch_setup_entry( + "switch" + ) as mock_switch_async_setup_entry: + result = await setup_bond_entity(hass, config_entry, patch_device_ids=True) + assert result is True + await hass.async_block_till_done() assert config_entry.entry_id in hass.data[DOMAIN] assert config_entry.state == ENTRY_STATE_LOADED @@ -81,7 +81,7 @@ async def test_async_setup_entry_sets_up_hub_and_supported_domains(hass: HomeAss # verify hub device is registered correctly device_registry = await dr.async_get_registry(hass) hub = device_registry.async_get_device(identifiers={(DOMAIN, "test-bond-id")}) - assert hub.name == "test-bond-id" + assert hub.name == "bond-name" assert hub.manufacturer == "Olibra" assert hub.model == "test-model" assert hub.sw_version == "test-version" @@ -106,6 +106,7 @@ async def test_unload_config_entry(hass: HomeAssistant): patch_version=True, patch_device_ids=True, patch_platforms=True, + patch_bridge=True, ) assert result is True await hass.async_block_till_done() @@ -136,7 +137,7 @@ async def test_old_identifiers_are_removed(hass: HomeAssistant): config_entry.add_to_hass(hass) - with patch_bond_version( + with patch_bond_bridge(), patch_bond_version( return_value={ "bondid": "test-bond-id", "target": "test-model", @@ -164,3 +165,92 @@ async def test_old_identifiers_are_removed(hass: HomeAssistant): # verify the device info is cleaned up assert device_registry.async_get_device(identifiers={old_identifers}) is None assert device_registry.async_get_device(identifiers={new_identifiers}) is not None + + +async def test_smart_by_bond_device_suggested_area(hass: HomeAssistant): + """Test we can setup a smart by bond device and get the suggested area.""" + config_entry = MockConfigEntry( + domain=DOMAIN, + data={CONF_HOST: "some host", CONF_ACCESS_TOKEN: "test-token"}, + ) + + config_entry.add_to_hass(hass) + + with patch_bond_bridge( + side_effect=ClientResponseError(Mock(), Mock(), status=404) + ), patch_bond_version( + return_value={ + "bondid": "test-bond-id", + "target": "test-model", + "fw_ver": "test-version", + } + ), patch_start_bpup(), patch_bond_device_ids( + return_value=["bond-device-id", "device_id"] + ), patch_bond_device( + return_value={ + "name": "test1", + "type": DeviceType.GENERIC_DEVICE, + "location": "Den", + } + ), patch_bond_device_properties( + return_value={} + ), patch_bond_device_state( + return_value={} + ): + assert await hass.config_entries.async_setup(config_entry.entry_id) is True + await hass.async_block_till_done() + + assert config_entry.entry_id in hass.data[DOMAIN] + assert config_entry.state == ENTRY_STATE_LOADED + assert config_entry.unique_id == "test-bond-id" + + device_registry = await hass.helpers.device_registry.async_get_registry() + device = device_registry.async_get_device(identifiers={(DOMAIN, "test-bond-id")}) + assert device is not None + assert device.suggested_area == "Den" + + +async def test_bridge_device_suggested_area(hass: HomeAssistant): + """Test we can setup a bridge bond device and get the suggested area.""" + config_entry = MockConfigEntry( + domain=DOMAIN, + data={CONF_HOST: "some host", CONF_ACCESS_TOKEN: "test-token"}, + ) + + config_entry.add_to_hass(hass) + + with patch_bond_bridge( + return_value={ + "name": "Office Bridge", + "location": "Office", + } + ), patch_bond_version( + return_value={ + "bondid": "test-bond-id", + "target": "test-model", + "fw_ver": "test-version", + } + ), patch_start_bpup(), patch_bond_device_ids( + return_value=["bond-device-id", "device_id"] + ), patch_bond_device( + return_value={ + "name": "test1", + "type": DeviceType.GENERIC_DEVICE, + "location": "Bathroom", + } + ), patch_bond_device_properties( + return_value={} + ), patch_bond_device_state( + return_value={} + ): + assert await hass.config_entries.async_setup(config_entry.entry_id) is True + await hass.async_block_till_done() + + assert config_entry.entry_id in hass.data[DOMAIN] + assert config_entry.state == ENTRY_STATE_LOADED + assert config_entry.unique_id == "test-bond-id" + + device_registry = await hass.helpers.device_registry.async_get_registry() + device = device_registry.async_get_device(identifiers={(DOMAIN, "test-bond-id")}) + assert device is not None + assert device.suggested_area == "Office" diff --git a/tests/components/bond/test_light.py b/tests/components/bond/test_light.py index 3f7a3ef62f9..e4cd4e4e3e9 100644 --- a/tests/components/bond/test_light.py +++ b/tests/components/bond/test_light.py @@ -148,7 +148,7 @@ async def test_sbb_trust_state(hass: core.HomeAssistant): "bondid": "test-bond-id", } await setup_platform( - hass, LIGHT_DOMAIN, ceiling_fan("name-1"), bond_version=version + hass, LIGHT_DOMAIN, ceiling_fan("name-1"), bond_version=version, bridge={} ) device = hass.states.get("light.name_1")