-
-
Notifications
You must be signed in to change notification settings - Fork 32.2k
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
vesync new platform of binary sensor #134221
Open
cdnninja
wants to merge
29
commits into
home-assistant:dev
Choose a base branch
from
cdnninja:binary-sensor
base: dev
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+132
−0
Open
Changes from all commits
Commits
Show all changes
29 commits
Select commit
Hold shift + click to select a range
fab323f
Added the basics for binary sensor support.
cdnninja c4d2533
Binary sensor first draft
cdnninja de6f8da
Update tests
cdnninja 9c44916
Remove unneeded code to improve test coverage.
cdnninja 02dc884
Adjust what sensors are displayed.
cdnninja a8cab84
Final corrections to handle dict. Clean up too.
cdnninja cefac79
Align order to sensor for consistancy
cdnninja de6f5b7
Merge branch 'dev' into binary-sensor
cdnninja 25c5958
unload binary sensor
cdnninja 7e7c0d5
Update homeassistant/components/vesync/binary_sensor.py
cdnninja faccf26
Update strings.json
cdnninja 66fb73c
Wrong type
cdnninja 33ab7be
Add call backs.
cdnninja 0b50f48
Merge branch 'home-assistant:dev' into binary-sensor
cdnninja c010230
Align to coordinator
cdnninja 8eaa370
Update homeassistant/components/vesync/binary_sensor.py
cdnninja 412e1ad
Update common.py
cdnninja 0a9331a
Update common.py
cdnninja 3a01836
Remove warning as this mostly hits humidifers
cdnninja 9410726
Merge branch 'dev' into binary-sensor
cdnninja a7e50e7
Correct ruff.
cdnninja 330457c
format
cdnninja 0699a22
Merge branch 'dev' into binary-sensor
cdnninja 8d9b4ef
Refactor based on parent branch
cdnninja b4695c9
Merge branch 'dev' into binary-sensor
cdnninja 27d775e
Correct merge issues and initial test corrections
cdnninja 163d18b
Feedback corrections.
cdnninja b3211ff
Remove is_on value
cdnninja 8eeec71
Move typing as per other feedback
cdnninja File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,96 @@ | ||
"""Binary Sensor for VeSync.""" | ||
|
||
from __future__ import annotations | ||
|
||
import logging | ||
|
||
from pyvesync.vesyncbasedevice import VeSyncBaseDevice | ||
|
||
from homeassistant.components.binary_sensor import ( | ||
BinarySensorDeviceClass, | ||
BinarySensorEntity, | ||
BinarySensorEntityDescription, | ||
) | ||
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 .common import rgetattr | ||
from .const import DOMAIN, VS_COORDINATOR, VS_DEVICES, VS_DISCOVERY | ||
from .coordinator import VeSyncDataCoordinator | ||
from .entity import VeSyncBaseEntity | ||
|
||
_LOGGER = logging.getLogger(__name__) | ||
|
||
|
||
SENSOR_DESCRIPTIONS: tuple[BinarySensorEntityDescription, ...] = ( | ||
BinarySensorEntityDescription( | ||
key="water_lacks", | ||
translation_key="water_lacks", | ||
device_class=BinarySensorDeviceClass.PROBLEM, | ||
), | ||
BinarySensorEntityDescription( | ||
key="details.water_tank_lifted", | ||
translation_key="water_tank_lifted", | ||
device_class=BinarySensorDeviceClass.PROBLEM, | ||
), | ||
) | ||
|
||
|
||
async def async_setup_entry( | ||
hass: HomeAssistant, | ||
config_entry: ConfigEntry, | ||
async_add_entities: AddEntitiesCallback, | ||
) -> None: | ||
"""Set up binary_sensor platform.""" | ||
|
||
coordinator = hass.data[DOMAIN][VS_COORDINATOR] | ||
|
||
@callback | ||
def discover(devices): | ||
"""Add new devices to platform.""" | ||
_setup_entities(devices, async_add_entities) | ||
|
||
config_entry.async_on_unload( | ||
async_dispatcher_connect(hass, VS_DISCOVERY.format(VS_DEVICES), discover) | ||
) | ||
|
||
_setup_entities(hass.data[DOMAIN][VS_DEVICES], async_add_entities, coordinator) | ||
|
||
|
||
@callback | ||
def _setup_entities(devices, async_add_entities, coordinator): | ||
"""Add entity.""" | ||
async_add_entities( | ||
( | ||
VeSyncBinarySensor(dev, description, coordinator) | ||
for dev in devices | ||
for description in SENSOR_DESCRIPTIONS | ||
if rgetattr(dev, description.key) is not None | ||
), | ||
) | ||
|
||
|
||
class VeSyncBinarySensor(BinarySensorEntity, VeSyncBaseEntity): | ||
"""Vesync binary sensor class.""" | ||
|
||
entity_description: BinarySensorEntityDescription | ||
|
||
def __init__( | ||
self, | ||
device: VeSyncBaseDevice, | ||
description: BinarySensorEntityDescription, | ||
coordinator: VeSyncDataCoordinator, | ||
) -> None: | ||
"""Initialize the sensor.""" | ||
super().__init__(device, coordinator) | ||
self.entity_description = description | ||
self._attr_unique_id = f"{super().unique_id}-{description.key}" | ||
|
||
@property | ||
def is_on(self) -> bool: | ||
"""Return true if the binary sensor is on.""" | ||
_LOGGER.debug(rgetattr(self.device, self.entity_description.key)) | ||
return rgetattr(self.device, self.entity_description.key) | ||
# return self.entity_description.is_on(self.device) |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I find the use of key as means to check attribute presence cryptic. Why not use device type based check so that we generate sensors for known cases?
It looks like a lot more attributes are populated in build_humid_dict with default value but I definitely know that my humidifier
Classic 200
does not support any of them.However it could be tat some attributes are common but someone would need to verify that fact.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My theory on this types of data and sensor.py is that datapoints vary by device. Different humidifiers have different data points for example. By checking for the datapoint we don't need to check against every potential child class defined. We are agnostic and data focused so if a new humidifier type is added to the child library no update needed beyond bumping the library. Thoughts?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thinking more on this - maybe I move that same logic to a exists_fn? Still with the reget but within the exists lambda. That way it is flexible depending on what is needed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The datapoint approach is what we would want but I don't have other devices to confirm or deny my theory. Let us go with it. The custom vesync integration also seems to do something similar so there must some prior validation.
As for the key, isn't that what is being read in the value lambda. What if we called it
data_path
or something and got rid of is_on lambda?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is_on is binary sensor native name: https://developers.home-assistant.io/docs/core/entity/binary-sensor
Now that I look at it again I think I leave as is. Not sure an easier way exists to use the is on value to safely check it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To confirm do you have changes requested or are you okay with approving as is?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would loved to see key and value lambda be combined since they seem to be using the same data path but I am okay with what we have.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Got it. So combine in description and use the same value programmatically for "is_on". I will give it a go.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It was just an idea. If we can have cases where key is one thing but the data returned is something, then we would want to keep what we have.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is possible that occurs but quick to make that change when/if it occurs. VScode is uploading the changes now. Should be up shortly - it takes forever.