Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor switch for vesync #134409

Open
wants to merge 19 commits into
base: dev
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 13 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 22 additions & 0 deletions homeassistant/components/vesync/common.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,28 @@
_LOGGER = logging.getLogger(__name__)


def rgetattr(obj, attr):
"""Return a string in the form word.1.2.3 and return the item as 3. Note that this last value could be in a dict as well."""
_this_func = rgetattr
sp = attr.split(".", 1)
if len(sp) == 1:
left, right = sp[0], ""
else:
left, right = sp

Check warning on line 22 in homeassistant/components/vesync/common.py

View check run for this annotation

Codecov / codecov/patch

homeassistant/components/vesync/common.py#L22

Added line #L22 was not covered by tests

if isinstance(obj, dict):
obj = obj.get(left)

Check warning on line 25 in homeassistant/components/vesync/common.py

View check run for this annotation

Codecov / codecov/patch

homeassistant/components/vesync/common.py#L25

Added line #L25 was not covered by tests
elif hasattr(obj, left):
obj = getattr(obj, left)
else:
return None

Check warning on line 29 in homeassistant/components/vesync/common.py

View check run for this annotation

Codecov / codecov/patch

homeassistant/components/vesync/common.py#L29

Added line #L29 was not covered by tests

if right:
obj = _this_func(obj, right)

Check warning on line 32 in homeassistant/components/vesync/common.py

View check run for this annotation

Codecov / codecov/patch

homeassistant/components/vesync/common.py#L32

Added line #L32 was not covered by tests

return obj


async def async_process_devices(
hass: HomeAssistant, manager: VeSync
) -> dict[str, list[VeSyncBaseDevice]]:
Expand Down
5 changes: 5 additions & 0 deletions homeassistant/components/vesync/strings.json
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,11 @@
"name": "Current voltage"
}
},
"switch": {
"on": {
"name": "On"
}
},
"fan": {
"vesync": {
"state_attributes": {
Expand Down
114 changes: 64 additions & 50 deletions homeassistant/components/vesync/switch.py
Original file line number Diff line number Diff line change
@@ -1,29 +1,54 @@
"""Support for VeSync switches."""

from collections.abc import Callable
from dataclasses import dataclass
import logging
from typing import Any
from typing import Any, Final

from pyvesync.vesyncbasedevice import VeSyncBaseDevice

from homeassistant.components.switch import SwitchEntity
from pyvesync.vesyncoutlet import VeSyncOutlet
from pyvesync.vesyncswitch import VeSyncSwitch

from homeassistant.components.switch import (
SwitchDeviceClass,
SwitchEntity,
SwitchEntityDescription,
)
from homeassistant.config_entries import ConfigEntry
from homeassistant.core import HomeAssistant, callback
from homeassistant.helpers.dispatcher import async_dispatcher_connect
from homeassistant.helpers.entity_platform import AddEntitiesCallback

from .const import DEV_TYPE_TO_HA, DOMAIN, VS_COORDINATOR, VS_DISCOVERY, VS_SWITCHES
from .common import rgetattr
from .const import DOMAIN, VS_COORDINATOR, VS_DISCOVERY, VS_SWITCHES
from .coordinator import VeSyncDataCoordinator
from .entity import VeSyncBaseEntity

_LOGGER = logging.getLogger(__name__)


@dataclass(frozen=True, kw_only=True)
class VeSyncSwitchEntityDescription(SwitchEntityDescription):
"""A class that describes custom switch entities."""

is_on: Callable[[VeSyncBaseDevice], bool]


SENSOR_DESCRIPTIONS: Final[tuple[VeSyncSwitchEntityDescription, ...]] = (
VeSyncSwitchEntityDescription(
key="device_status",
translation_key="on",
is_on=lambda device: device.device_status == "on",
),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we just use the power device class? We should either set the name to None (causing the entity to follow the device name, so switch.vesync_device) or if there already is another main feature of the device (say a climate entity or humidifier), we should use the device class translation for the POWER device class

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any harm in us not filtering if say it is a fan or humidifer? name would be switch.vesync_device and allow a toggle for power. This keeps the code simple as I don't need to filter if one of those.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Duplicate entity? Humidifier and fan both derive from ToggleEntity and expose the same actions.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will test this in conjunction with humidifier PR once that one is merged. Lets plan to merge this after that one.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry but why do we need this entity?

The device_status in pyvesync seems to reflect the on/off state which fan/humidifier entities will report as well via is_on. They also have turn_on/turn_off to change the status. I did not check but I think the same should be true for light entities.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They used to sell outlets and switches. As per: https://www.home-assistant.io/integrations/vesync/ so this would show on off for the light switch or outlet. I am thinking I should add a filter ability so it can be removed for select situations. In the future this switch platform can offer other items though, which is the driver for the refactor.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Switch and outlet would need that but I don't think they should be needed for fan/humidifiers.

My humidifier appears as following with off state.
image
image

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will add an additional check so we can adjust for that. I won't get to this right a way since I think humidifier merge and binary_sensor take priority.

)


async def async_setup_entry(
hass: HomeAssistant,
config_entry: ConfigEntry,
async_add_entities: AddEntitiesCallback,
) -> None:
"""Set up switches."""
"""Set up switch platform."""

coordinator = hass.data[DOMAIN][VS_COORDINATOR]

Expand All @@ -46,57 +71,46 @@
coordinator: VeSyncDataCoordinator,
):
"""Check if device is online and add entity."""
entities: list[VeSyncBaseSwitch] = []
for dev in devices:
if DEV_TYPE_TO_HA.get(dev.device_type) == "outlet":
entities.append(VeSyncSwitchHA(dev, coordinator))
elif DEV_TYPE_TO_HA.get(dev.device_type) == "switch":
entities.append(VeSyncLightSwitch(dev, coordinator))
else:
_LOGGER.warning(
"%s - Unknown device type - %s", dev.device_name, dev.device_type
)
continue

async_add_entities(entities, update_before_add=True)

async_add_entities(
(
VeSyncSwitchEntity(dev, description, coordinator)
for dev in devices
for description in SENSOR_DESCRIPTIONS
if rgetattr(dev, description.key) is not None
),
update_before_add=True,
cdnninja marked this conversation as resolved.
Show resolved Hide resolved
)

class VeSyncBaseSwitch(VeSyncBaseEntity, SwitchEntity):
"""Base class for VeSync switch Device Representations."""

_attr_name = None
class VeSyncSwitchEntity(SwitchEntity, VeSyncBaseEntity):
"""VeSync sensor class."""
cdnninja marked this conversation as resolved.
Show resolved Hide resolved

def turn_on(self, **kwargs: Any) -> None:
"""Turn the device on."""
self.device.turn_on()
def __init__(
self,
device: VeSyncBaseDevice,
description: VeSyncSwitchEntityDescription,
coordinator: VeSyncDataCoordinator,
) -> None:
"""Initialize the sensor."""
super().__init__(device, coordinator)
self.entity_description: VeSyncSwitchEntityDescription = description
if isinstance(self.device, VeSyncOutlet):
self._attr_device_class = SwitchDeviceClass.OUTLET
if isinstance(self.device, VeSyncSwitch):
self._attr_device_class = SwitchDeviceClass.SWITCH
self._attr_name = None

@property
def is_on(self) -> bool:
"""Return True if device is on."""
return self.device.device_status == "on"
def is_on(self) -> bool | None:
"""Return the entity value to represent the entity state."""
if self.entity_description.is_on is not None:
return self.entity_description.is_on(self.device)
return None

Check warning on line 108 in homeassistant/components/vesync/switch.py

View check run for this annotation

Codecov / codecov/patch

homeassistant/components/vesync/switch.py#L108

Added line #L108 was not covered by tests
cdnninja marked this conversation as resolved.
Show resolved Hide resolved

def turn_off(self, **kwargs: Any) -> None:
"""Turn the device off."""
"""Turn the entity off."""
self.device.turn_off()


class VeSyncSwitchHA(VeSyncBaseSwitch, SwitchEntity):
"""Representation of a VeSync switch."""

def __init__(
self, plug: VeSyncBaseDevice, coordinator: VeSyncDataCoordinator
) -> None:
"""Initialize the VeSync switch device."""
super().__init__(plug, coordinator)
self.smartplug = plug


class VeSyncLightSwitch(VeSyncBaseSwitch, SwitchEntity):
"""Handle representation of VeSync Light Switch."""

def __init__(
self, switch: VeSyncBaseDevice, coordinator: VeSyncDataCoordinator
) -> None:
"""Initialize Light Switch device class."""
super().__init__(switch, coordinator)
self.switch = switch
def turn_on(self, **kwargs: Any) -> None:
"""Turn the entity on."""
self.device.turn_on()

Check warning on line 116 in homeassistant/components/vesync/switch.py

View check run for this annotation

Codecov / codecov/patch

homeassistant/components/vesync/switch.py#L116

Added line #L116 was not covered by tests
10 changes: 6 additions & 4 deletions tests/components/vesync/snapshots/test_switch.ambr
Original file line number Diff line number Diff line change
Expand Up @@ -300,13 +300,13 @@
'name': None,
'options': dict({
}),
'original_device_class': None,
'original_device_class': <SwitchDeviceClass.OUTLET: 'outlet'>,
'original_icon': None,
'original_name': None,
'platform': 'vesync',
'previous_unique_id': None,
'supported_features': 0,
'translation_key': None,
'translation_key': 'on',
'unique_id': 'outlet',
'unit_of_measurement': None,
}),
Expand All @@ -315,6 +315,7 @@
# name: test_switch_state[Outlet][switch.outlet]
StateSnapshot({
'attributes': ReadOnlyDict({
'device_class': 'outlet',
'friendly_name': 'Outlet',
}),
'context': <ANY>,
Expand Down Expand Up @@ -420,13 +421,13 @@
'name': None,
'options': dict({
}),
'original_device_class': None,
'original_device_class': <SwitchDeviceClass.SWITCH: 'switch'>,
'original_icon': None,
'original_name': None,
'platform': 'vesync',
'previous_unique_id': None,
'supported_features': 0,
'translation_key': None,
'translation_key': 'on',
'unique_id': 'switch',
'unit_of_measurement': None,
}),
Expand All @@ -435,6 +436,7 @@
# name: test_switch_state[Wall Switch][switch.wall_switch]
StateSnapshot({
'attributes': ReadOnlyDict({
'device_class': 'switch',
'friendly_name': 'Wall Switch',
}),
'context': <ANY>,
Expand Down