Address Switcher late review comments (#56264)

* Address Switcher late review comments

* Rename wrapper to coordinator
This commit is contained in:
Shay Levy 2021-09-16 18:06:58 +03:00 committed by GitHub
parent 1609d069bb
commit 8418d4ade2
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 54 additions and 45 deletions

View File

@ -1,6 +1,7 @@
"""The Switcher integration.""" """The Switcher integration."""
from __future__ import annotations from __future__ import annotations
import asyncio
from datetime import timedelta from datetime import timedelta
import logging import logging
@ -75,10 +76,10 @@ async def async_setup_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool:
# Existing device update device data # Existing device update device data
if device.device_id in hass.data[DOMAIN][DATA_DEVICE]: if device.device_id in hass.data[DOMAIN][DATA_DEVICE]:
wrapper: SwitcherDeviceWrapper = hass.data[DOMAIN][DATA_DEVICE][ coordinator: SwitcherDataUpdateCoordinator = hass.data[DOMAIN][DATA_DEVICE][
device.device_id device.device_id
] ]
wrapper.async_set_updated_data(device) coordinator.async_set_updated_data(device)
return return
# New device - create device # New device - create device
@ -90,15 +91,19 @@ async def async_setup_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool:
device.device_type.hex_rep, device.device_type.hex_rep,
) )
wrapper = hass.data[DOMAIN][DATA_DEVICE][ coordinator = hass.data[DOMAIN][DATA_DEVICE][
device.device_id device.device_id
] = SwitcherDeviceWrapper(hass, entry, device) ] = SwitcherDataUpdateCoordinator(hass, entry, device)
hass.async_create_task(wrapper.async_setup()) coordinator.async_setup()
async def platforms_setup_task() -> None: async def platforms_setup_task() -> None:
# Must be ready before dispatcher is called # Must be ready before dispatcher is called
for platform in PLATFORMS: await asyncio.gather(
await hass.config_entries.async_forward_entry_setup(entry, platform) *(
hass.config_entries.async_forward_entry_setup(entry, platform)
for platform in PLATFORMS
)
)
discovery_task = hass.data[DOMAIN].pop(DATA_DISCOVERY, None) discovery_task = hass.data[DOMAIN].pop(DATA_DISCOVERY, None)
if discovery_task is not None: if discovery_task is not None:
@ -114,25 +119,26 @@ async def async_setup_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool:
async def stop_bridge(event: Event) -> None: async def stop_bridge(event: Event) -> None:
await async_stop_bridge(hass) await async_stop_bridge(hass)
hass.bus.async_listen_once(EVENT_HOMEASSISTANT_STOP, stop_bridge) entry.async_on_unload(
hass.bus.async_listen_once(EVENT_HOMEASSISTANT_STOP, stop_bridge)
)
return True return True
class SwitcherDeviceWrapper(update_coordinator.DataUpdateCoordinator): class SwitcherDataUpdateCoordinator(update_coordinator.DataUpdateCoordinator):
"""Wrapper for a Switcher device with Home Assistant specific functions.""" """Switcher device data update coordinator."""
def __init__( def __init__(
self, hass: HomeAssistant, entry: ConfigEntry, device: SwitcherBase self, hass: HomeAssistant, entry: ConfigEntry, device: SwitcherBase
) -> None: ) -> None:
"""Initialize the Switcher device wrapper.""" """Initialize the Switcher device coordinator."""
super().__init__( super().__init__(
hass, hass,
_LOGGER, _LOGGER,
name=device.name, name=device.name,
update_interval=timedelta(seconds=MAX_UPDATE_INTERVAL_SEC), update_interval=timedelta(seconds=MAX_UPDATE_INTERVAL_SEC),
) )
self.hass = hass
self.entry = entry self.entry = entry
self.data = device self.data = device
@ -157,9 +163,10 @@ class SwitcherDeviceWrapper(update_coordinator.DataUpdateCoordinator):
"""Switcher device mac address.""" """Switcher device mac address."""
return self.data.mac_address # type: ignore[no-any-return] return self.data.mac_address # type: ignore[no-any-return]
async def async_setup(self) -> None: @callback
"""Set up the wrapper.""" def async_setup(self) -> None:
dev_reg = await device_registry.async_get_registry(self.hass) """Set up the coordinator."""
dev_reg = device_registry.async_get(self.hass)
dev_reg.async_get_or_create( dev_reg.async_get_or_create(
config_entry_id=self.entry.entry_id, config_entry_id=self.entry.entry_id,
connections={(device_registry.CONNECTION_NETWORK_MAC, self.mac_address)}, connections={(device_registry.CONNECTION_NETWORK_MAC, self.mac_address)},

View File

@ -20,7 +20,7 @@ from homeassistant.helpers.entity_platform import AddEntitiesCallback
from homeassistant.helpers.typing import StateType from homeassistant.helpers.typing import StateType
from homeassistant.helpers.update_coordinator import CoordinatorEntity from homeassistant.helpers.update_coordinator import CoordinatorEntity
from . import SwitcherDeviceWrapper from . import SwitcherDataUpdateCoordinator
from .const import SIGNAL_DEVICE_ADD from .const import SIGNAL_DEVICE_ADD
@ -75,16 +75,16 @@ async def async_setup_entry(
"""Set up Switcher sensor from config entry.""" """Set up Switcher sensor from config entry."""
@callback @callback
def async_add_sensors(wrapper: SwitcherDeviceWrapper) -> None: def async_add_sensors(coordinator: SwitcherDataUpdateCoordinator) -> None:
"""Add sensors from Switcher device.""" """Add sensors from Switcher device."""
if wrapper.data.device_type.category == DeviceCategory.POWER_PLUG: if coordinator.data.device_type.category == DeviceCategory.POWER_PLUG:
async_add_entities( async_add_entities(
SwitcherSensorEntity(wrapper, attribute, info) SwitcherSensorEntity(coordinator, attribute, info)
for attribute, info in POWER_PLUG_SENSORS.items() for attribute, info in POWER_PLUG_SENSORS.items()
) )
elif wrapper.data.device_type.category == DeviceCategory.WATER_HEATER: elif coordinator.data.device_type.category == DeviceCategory.WATER_HEATER:
async_add_entities( async_add_entities(
SwitcherSensorEntity(wrapper, attribute, info) SwitcherSensorEntity(coordinator, attribute, info)
for attribute, info in WATER_HEATER_SENSORS.items() for attribute, info in WATER_HEATER_SENSORS.items()
) )
@ -98,30 +98,31 @@ class SwitcherSensorEntity(CoordinatorEntity, SensorEntity):
def __init__( def __init__(
self, self,
wrapper: SwitcherDeviceWrapper, coordinator: SwitcherDataUpdateCoordinator,
attribute: str, attribute: str,
description: AttributeDescription, description: AttributeDescription,
) -> None: ) -> None:
"""Initialize the entity.""" """Initialize the entity."""
super().__init__(wrapper) super().__init__(coordinator)
self.wrapper = wrapper
self.attribute = attribute self.attribute = attribute
# Entity class attributes # Entity class attributes
self._attr_name = f"{wrapper.name} {description.name}" self._attr_name = f"{coordinator.name} {description.name}"
self._attr_icon = description.icon self._attr_icon = description.icon
self._attr_native_unit_of_measurement = description.unit self._attr_native_unit_of_measurement = description.unit
self._attr_device_class = description.device_class self._attr_device_class = description.device_class
self._attr_entity_registry_enabled_default = description.default_enabled self._attr_entity_registry_enabled_default = description.default_enabled
self._attr_unique_id = f"{wrapper.device_id}-{wrapper.mac_address}-{attribute}" self._attr_unique_id = (
f"{coordinator.device_id}-{coordinator.mac_address}-{attribute}"
)
self._attr_device_info = { self._attr_device_info = {
"connections": { "connections": {
(device_registry.CONNECTION_NETWORK_MAC, wrapper.mac_address) (device_registry.CONNECTION_NETWORK_MAC, coordinator.mac_address)
} }
} }
@property @property
def native_value(self) -> StateType: def native_value(self) -> StateType:
"""Return value of sensor.""" """Return value of sensor."""
return getattr(self.wrapper.data, self.attribute) # type: ignore[no-any-return] return getattr(self.coordinator.data, self.attribute) # type: ignore[no-any-return]

View File

@ -26,7 +26,7 @@ from homeassistant.helpers.dispatcher import async_dispatcher_connect
from homeassistant.helpers.entity_platform import AddEntitiesCallback from homeassistant.helpers.entity_platform import AddEntitiesCallback
from homeassistant.helpers.update_coordinator import CoordinatorEntity from homeassistant.helpers.update_coordinator import CoordinatorEntity
from . import SwitcherDeviceWrapper from . import SwitcherDataUpdateCoordinator
from .const import ( from .const import (
CONF_AUTO_OFF, CONF_AUTO_OFF,
CONF_TIMER_MINUTES, CONF_TIMER_MINUTES,
@ -69,12 +69,12 @@ async def async_setup_entry(
) )
@callback @callback
def async_add_switch(wrapper: SwitcherDeviceWrapper) -> None: def async_add_switch(coordinator: SwitcherDataUpdateCoordinator) -> None:
"""Add switch from Switcher device.""" """Add switch from Switcher device."""
if wrapper.data.device_type.category == DeviceCategory.POWER_PLUG: if coordinator.data.device_type.category == DeviceCategory.POWER_PLUG:
async_add_entities([SwitcherPowerPlugSwitchEntity(wrapper)]) async_add_entities([SwitcherPowerPlugSwitchEntity(coordinator)])
elif wrapper.data.device_type.category == DeviceCategory.WATER_HEATER: elif coordinator.data.device_type.category == DeviceCategory.WATER_HEATER:
async_add_entities([SwitcherWaterHeaterSwitchEntity(wrapper)]) async_add_entities([SwitcherWaterHeaterSwitchEntity(coordinator)])
config_entry.async_on_unload( config_entry.async_on_unload(
async_dispatcher_connect(hass, SIGNAL_DEVICE_ADD, async_add_switch) async_dispatcher_connect(hass, SIGNAL_DEVICE_ADD, async_add_switch)
@ -84,18 +84,17 @@ async def async_setup_entry(
class SwitcherBaseSwitchEntity(CoordinatorEntity, SwitchEntity): class SwitcherBaseSwitchEntity(CoordinatorEntity, SwitchEntity):
"""Representation of a Switcher switch entity.""" """Representation of a Switcher switch entity."""
def __init__(self, wrapper: SwitcherDeviceWrapper) -> None: def __init__(self, coordinator: SwitcherDataUpdateCoordinator) -> None:
"""Initialize the entity.""" """Initialize the entity."""
super().__init__(wrapper) super().__init__(coordinator)
self.wrapper = wrapper
self.control_result: bool | None = None self.control_result: bool | None = None
# Entity class attributes # Entity class attributes
self._attr_name = wrapper.name self._attr_name = coordinator.name
self._attr_unique_id = f"{wrapper.device_id}-{wrapper.mac_address}" self._attr_unique_id = f"{coordinator.device_id}-{coordinator.mac_address}"
self._attr_device_info = { self._attr_device_info = {
"connections": { "connections": {
(device_registry.CONNECTION_NETWORK_MAC, wrapper.mac_address) (device_registry.CONNECTION_NETWORK_MAC, coordinator.mac_address)
} }
} }
@ -113,7 +112,7 @@ class SwitcherBaseSwitchEntity(CoordinatorEntity, SwitchEntity):
try: try:
async with SwitcherApi( async with SwitcherApi(
self.wrapper.data.ip_address, self.wrapper.device_id self.coordinator.data.ip_address, self.coordinator.data.device_id
) as swapi: ) as swapi:
response = await getattr(swapi, api)(*args) response = await getattr(swapi, api)(*args)
except (asyncio.TimeoutError, OSError, RuntimeError) as err: except (asyncio.TimeoutError, OSError, RuntimeError) as err:
@ -127,7 +126,7 @@ class SwitcherBaseSwitchEntity(CoordinatorEntity, SwitchEntity):
args, args,
response or error, response or error,
) )
self.wrapper.last_update_success = False self.coordinator.last_update_success = False
@property @property
def is_on(self) -> bool: def is_on(self) -> bool:
@ -135,7 +134,7 @@ class SwitcherBaseSwitchEntity(CoordinatorEntity, SwitchEntity):
if self.control_result is not None: if self.control_result is not None:
return self.control_result return self.control_result
return bool(self.wrapper.data.device_state == DeviceState.ON) return bool(self.coordinator.data.device_state == DeviceState.ON)
async def async_turn_on(self, **kwargs: Any) -> None: async def async_turn_on(self, **kwargs: Any) -> None:
"""Turn the entity on.""" """Turn the entity on."""

View File

@ -44,10 +44,11 @@ async def test_import(hass):
) )
async def test_user_setup(hass, mock_bridge): async def test_user_setup(hass, mock_bridge):
"""Test we can finish a config flow.""" """Test we can finish a config flow."""
with patch("homeassistant.components.switcher_kis.utils.asyncio.sleep"): with patch("homeassistant.components.switcher_kis.utils.DISCOVERY_TIME_SEC", 0):
result = await hass.config_entries.flow.async_init( result = await hass.config_entries.flow.async_init(
DOMAIN, context={"source": config_entries.SOURCE_USER} DOMAIN, context={"source": config_entries.SOURCE_USER}
) )
await hass.async_block_till_done()
assert mock_bridge.is_running is False assert mock_bridge.is_running is False
assert len(hass.data[DOMAIN][DATA_DISCOVERY].result()) == 2 assert len(hass.data[DOMAIN][DATA_DISCOVERY].result()) == 2
@ -68,10 +69,11 @@ async def test_user_setup(hass, mock_bridge):
async def test_user_setup_abort_no_devices_found(hass, mock_bridge): async def test_user_setup_abort_no_devices_found(hass, mock_bridge):
"""Test we abort a config flow if no devices found.""" """Test we abort a config flow if no devices found."""
with patch("homeassistant.components.switcher_kis.utils.asyncio.sleep"): with patch("homeassistant.components.switcher_kis.utils.DISCOVERY_TIME_SEC", 0):
result = await hass.config_entries.flow.async_init( result = await hass.config_entries.flow.async_init(
DOMAIN, context={"source": config_entries.SOURCE_USER} DOMAIN, context={"source": config_entries.SOURCE_USER}
) )
await hass.async_block_till_done()
assert mock_bridge.is_running is False assert mock_bridge.is_running is False
assert len(hass.data[DOMAIN][DATA_DISCOVERY].result()) == 0 assert len(hass.data[DOMAIN][DATA_DISCOVERY].result()) == 0