From f43c4d51e18ce4d26e7d0487e0e6e70adcb51d35 Mon Sep 17 00:00:00 2001 From: Jonathan Keslin Date: Wed, 12 Jan 2022 15:38:39 -0800 Subject: [PATCH] Address late review of VeSync (#63945) * Fast follow improvements to VeSync * Apply suggestions to other platforms, use async_on_unload * Rename dev_list to entities --- homeassistant/components/vesync/__init__.py | 11 +++------ homeassistant/components/vesync/const.py | 1 - homeassistant/components/vesync/fan.py | 22 +++++++++-------- homeassistant/components/vesync/light.py | 16 ++++++------ homeassistant/components/vesync/sensor.py | 27 ++++++++++----------- homeassistant/components/vesync/switch.py | 24 +++++++++--------- 6 files changed, 49 insertions(+), 52 deletions(-) diff --git a/homeassistant/components/vesync/__init__.py b/homeassistant/components/vesync/__init__.py index 3dd30b96f40..eb18a1229ca 100644 --- a/homeassistant/components/vesync/__init__.py +++ b/homeassistant/components/vesync/__init__.py @@ -14,7 +14,6 @@ from .const import ( DOMAIN, SERVICE_UPDATE_DEVS, VS_DISCOVERY, - VS_DISPATCHERS, VS_FANS, VS_LIGHTS, VS_MANAGER, @@ -56,8 +55,6 @@ async def async_setup_entry(hass: HomeAssistant, config_entry: ConfigEntry) -> b lights = hass.data[DOMAIN][VS_LIGHTS] = [] sensors = hass.data[DOMAIN][VS_SENSORS] = [] - hass.data[DOMAIN][VS_DISPATCHERS] = [] - if device_dict[VS_SWITCHES]: switches.extend(device_dict[VS_SWITCHES]) hass.async_create_task(forward_setup(config_entry, Platform.SWITCH)) @@ -96,7 +93,7 @@ async def async_setup_entry(hass: HomeAssistant, config_entry: ConfigEntry) -> b return if new_switches and not switches: switches.extend(new_switches) - hass.async_create_task(forward_setup(config_entry, "switch")) + hass.async_create_task(forward_setup(config_entry, Platform.SWITCH)) fan_set = set(fan_devs) new_fans = list(fan_set.difference(fans)) @@ -106,7 +103,7 @@ async def async_setup_entry(hass: HomeAssistant, config_entry: ConfigEntry) -> b return if new_fans and not fans: fans.extend(new_fans) - hass.async_create_task(forward_setup(config_entry, "fan")) + hass.async_create_task(forward_setup(config_entry, Platform.FAN)) light_set = set(light_devs) new_lights = list(light_set.difference(lights)) @@ -116,7 +113,7 @@ async def async_setup_entry(hass: HomeAssistant, config_entry: ConfigEntry) -> b return if new_lights and not lights: lights.extend(new_lights) - hass.async_create_task(forward_setup(config_entry, "light")) + hass.async_create_task(forward_setup(config_entry, Platform.LIGHT)) sensor_set = set(sensor_devs) new_sensors = list(sensor_set.difference(sensors)) @@ -126,7 +123,7 @@ async def async_setup_entry(hass: HomeAssistant, config_entry: ConfigEntry) -> b return if new_sensors and not sensors: sensors.extend(new_sensors) - hass.async_create_task(forward_setup(config_entry, "sensor")) + hass.async_create_task(forward_setup(config_entry, Platform.SENSOR)) hass.services.async_register( DOMAIN, SERVICE_UPDATE_DEVS, async_new_device_discovery diff --git a/homeassistant/components/vesync/const.py b/homeassistant/components/vesync/const.py index 9a2cb2a1281..fceeff81ae4 100644 --- a/homeassistant/components/vesync/const.py +++ b/homeassistant/components/vesync/const.py @@ -1,7 +1,6 @@ """Constants for VeSync Component.""" DOMAIN = "vesync" -VS_DISPATCHERS = "vesync_dispatchers" VS_DISCOVERY = "vesync_discovery_{}" SERVICE_UPDATE_DEVS = "update_devices" diff --git a/homeassistant/components/vesync/fan.py b/homeassistant/components/vesync/fan.py index b2682b67c32..2dee94f58e6 100644 --- a/homeassistant/components/vesync/fan.py +++ b/homeassistant/components/vesync/fan.py @@ -14,7 +14,7 @@ from homeassistant.util.percentage import ( ) from .common import VeSyncDevice -from .const import DOMAIN, VS_DISCOVERY, VS_DISPATCHERS, VS_FANS +from .const import DOMAIN, VS_DISCOVERY, VS_FANS _LOGGER = logging.getLogger(__name__) @@ -44,30 +44,32 @@ async def async_setup_entry( ) -> None: """Set up the VeSync fan platform.""" - async def async_discover(devices): + @callback + def discover(devices): """Add new devices to platform.""" - _async_setup_entities(devices, async_add_entities) + _setup_entities(devices, async_add_entities) - disp = async_dispatcher_connect(hass, VS_DISCOVERY.format(VS_FANS), async_discover) - hass.data[DOMAIN][VS_DISPATCHERS].append(disp) + config_entry.async_on_unload( + async_dispatcher_connect(hass, VS_DISCOVERY.format(VS_FANS), discover) + ) - _async_setup_entities(hass.data[DOMAIN][VS_FANS], async_add_entities) + _setup_entities(hass.data[DOMAIN][VS_FANS], async_add_entities) @callback -def _async_setup_entities(devices, async_add_entities): +def _setup_entities(devices, async_add_entities): """Check if device is online and add entity.""" - dev_list = [] + entities = [] for dev in devices: if DEV_TYPE_TO_HA.get(dev.device_type) == "fan": - dev_list.append(VeSyncFanHA(dev)) + entities.append(VeSyncFanHA(dev)) else: _LOGGER.warning( "%s - Unknown device type - %s", dev.device_name, dev.device_type ) continue - async_add_entities(dev_list, update_before_add=True) + async_add_entities(entities, update_before_add=True) class VeSyncFanHA(VeSyncDevice, FanEntity): diff --git a/homeassistant/components/vesync/light.py b/homeassistant/components/vesync/light.py index 00ca371a47b..7fd682e1943 100644 --- a/homeassistant/components/vesync/light.py +++ b/homeassistant/components/vesync/light.py @@ -14,7 +14,7 @@ from homeassistant.helpers.dispatcher import async_dispatcher_connect from homeassistant.helpers.entity_platform import AddEntitiesCallback from .common import VeSyncDevice -from .const import DOMAIN, VS_DISCOVERY, VS_DISPATCHERS, VS_LIGHTS +from .const import DOMAIN, VS_DISCOVERY, VS_LIGHTS _LOGGER = logging.getLogger(__name__) @@ -33,20 +33,20 @@ async def async_setup_entry( ) -> None: """Set up lights.""" - async def async_discover(devices): + @callback + def discover(devices): """Add new devices to platform.""" - _async_setup_entities(devices, async_add_entities) + _setup_entities(devices, async_add_entities) - disp = async_dispatcher_connect( - hass, VS_DISCOVERY.format(VS_LIGHTS), async_discover + config_entry.async_on_unload( + async_dispatcher_connect(hass, VS_DISCOVERY.format(VS_LIGHTS), discover) ) - hass.data[DOMAIN][VS_DISPATCHERS].append(disp) - _async_setup_entities(hass.data[DOMAIN][VS_LIGHTS], async_add_entities) + _setup_entities(hass.data[DOMAIN][VS_LIGHTS], async_add_entities) @callback -def _async_setup_entities(devices, async_add_entities): +def _setup_entities(devices, async_add_entities): """Check if device is online and add entity.""" entities = [] for dev in devices: diff --git a/homeassistant/components/vesync/sensor.py b/homeassistant/components/vesync/sensor.py index 1ed851b1dd3..cc69bf36fa6 100644 --- a/homeassistant/components/vesync/sensor.py +++ b/homeassistant/components/vesync/sensor.py @@ -14,7 +14,7 @@ from homeassistant.helpers.entity import EntityCategory from homeassistant.helpers.entity_platform import AddEntitiesCallback from .common import VeSyncBaseEntity -from .const import DOMAIN, VS_DISCOVERY, VS_DISPATCHERS, VS_SENSORS +from .const import DOMAIN, VS_DISCOVERY, VS_SENSORS from .switch import DEV_TYPE_TO_HA _LOGGER = logging.getLogger(__name__) @@ -27,31 +27,30 @@ async def async_setup_entry( ) -> None: """Set up switches.""" - async def async_discover(devices): + @callback + def discover(devices): """Add new devices to platform.""" - _async_setup_entities(devices, async_add_entities) + _setup_entities(devices, async_add_entities) - disp = async_dispatcher_connect( - hass, VS_DISCOVERY.format(VS_SENSORS), async_discover + config_entry.async_on_unload( + async_dispatcher_connect(hass, VS_DISCOVERY.format(VS_SENSORS), discover) ) - hass.data[DOMAIN][VS_DISPATCHERS].append(disp) - _async_setup_entities(hass.data[DOMAIN][VS_SENSORS], async_add_entities) + _setup_entities(hass.data[DOMAIN][VS_SENSORS], async_add_entities) @callback -def _async_setup_entities(devices, async_add_entities): +def _setup_entities(devices, async_add_entities): """Check if device is online and add entity.""" - dev_list = [] + entities = [] for dev in devices: - if DEV_TYPE_TO_HA.get(dev.device_type) == "outlet": - dev_list.append(VeSyncPowerSensor(dev)) - dev_list.append(VeSyncEnergySensor(dev)) - else: + if DEV_TYPE_TO_HA.get(dev.device_type) != "outlet": # Not an outlet that supports energy/power, so do not create sensor entities continue + entities.append(VeSyncPowerSensor(dev)) + entities.append(VeSyncEnergySensor(dev)) - async_add_entities(dev_list, update_before_add=True) + async_add_entities(entities, update_before_add=True) class VeSyncSensorEntity(VeSyncBaseEntity, SensorEntity): diff --git a/homeassistant/components/vesync/switch.py b/homeassistant/components/vesync/switch.py index 1894ad8eff5..282f8d99817 100644 --- a/homeassistant/components/vesync/switch.py +++ b/homeassistant/components/vesync/switch.py @@ -8,7 +8,7 @@ from homeassistant.helpers.dispatcher import async_dispatcher_connect from homeassistant.helpers.entity_platform import AddEntitiesCallback from .common import VeSyncDevice -from .const import DOMAIN, VS_DISCOVERY, VS_DISPATCHERS, VS_SWITCHES +from .const import DOMAIN, VS_DISCOVERY, VS_SWITCHES _LOGGER = logging.getLogger(__name__) @@ -30,34 +30,34 @@ async def async_setup_entry( ) -> None: """Set up switches.""" - async def async_discover(devices): + @callback + def discover(devices): """Add new devices to platform.""" - _async_setup_entities(devices, async_add_entities) + _setup_entities(devices, async_add_entities) - disp = async_dispatcher_connect( - hass, VS_DISCOVERY.format(VS_SWITCHES), async_discover + config_entry.async_on_unload( + async_dispatcher_connect(hass, VS_DISCOVERY.format(VS_SWITCHES), discover) ) - hass.data[DOMAIN][VS_DISPATCHERS].append(disp) - _async_setup_entities(hass.data[DOMAIN][VS_SWITCHES], async_add_entities) + _setup_entities(hass.data[DOMAIN][VS_SWITCHES], async_add_entities) @callback -def _async_setup_entities(devices, async_add_entities): +def _setup_entities(devices, async_add_entities): """Check if device is online and add entity.""" - dev_list = [] + entities = [] for dev in devices: if DEV_TYPE_TO_HA.get(dev.device_type) == "outlet": - dev_list.append(VeSyncSwitchHA(dev)) + entities.append(VeSyncSwitchHA(dev)) elif DEV_TYPE_TO_HA.get(dev.device_type) == "switch": - dev_list.append(VeSyncLightSwitch(dev)) + entities.append(VeSyncLightSwitch(dev)) else: _LOGGER.warning( "%s - Unknown device type - %s", dev.device_name, dev.device_type ) continue - async_add_entities(dev_list, update_before_add=True) + async_add_entities(entities, update_before_add=True) class VeSyncBaseSwitch(VeSyncDevice, SwitchEntity):