vesync new platform of binary sensor

[cdnninja]

Proposed change

Added binary sensor platform, with items around humidifiers.

Type of change

• Dependency upgrade
• Bugfix (non-breaking change which fixes an issue)
• New integration (thank you!)
• New feature (which adds functionality to an existing integration)
• Deprecation (breaking change to happen in the future)
• Breaking change (fix/feature causing existing functionality to break)
• Code quality improvements to existing code or addition of tests

Additional information

• This PR fixes or closes issue: fixes #
• This PR is related to issue:
• Link to documentation pull request: Vesync Binary Sensor home-assistant.io#37004

Checklist

• The code change is tested and works locally.
• Local tests pass. Your PR cannot be merged unless tests pass
• There is no commented out code in this PR.
• I have followed the development checklist
• I have followed the perfect PR recommendations
• The code has been formatted using Ruff (ruff format homeassistant tests)
• Tests have been added to verify that the new code works.

If user exposed functionality or configuration variables are added/changed:

• Documentation added/updated for www.home-assistant.io
  Vesync Binary Sensor home-assistant.io#37004
  If the code communicates with devices, web services, or third-party tools:

• The manifest file has all fields filled out correctly.
  Updated and included derived files by running: python3 -m script.hassfest.

• New or updated dependencies have been added to requirements_all.txt.
  Updated by running python3 -m script.gen_requirements_all.

• For the updated dependencies - a link to the changelog, or at minimum a diff between library versions is added to the PR description.

To help with the load of incoming pull requests:

• I have reviewed two other open pull requests in this repository.

[home-assistant]

Hey there @markperdue, @webdjoe, @TheGardenMonkey, mind taking a look at this pull request as it has been labeled with an integration (vesync) you are listed as a code owner for? Thanks!

Code owner commands

Code owners of vesync can trigger bot actions by commenting:

• @home-assistant close Closes the pull request.
• @home-assistant rename Awesome new title Renames the pull request.
• @home-assistant reopen Reopen the pull request.
• @home-assistant unassign vesync Removes the current integration label and assignees on the pull request, add the integration domain after the command.
• @home-assistant add-label needs-more-information Add a label (needs-more-information, problem in dependency, problem in custom component) to the pull request.
• @home-assistant remove-label needs-more-information Remove a label (needs-more-information, problem in dependency, problem in custom component) on the pull request.

[cdnninja]

@iprak I am interested in your feedback on the approach of this platform type. As well if you can spot why online sensor goes unavailable I am open to it! I have to add some debug lines to figure that out.

[iprak]

@iprak I am interested in your feedback on the approach of this platform type. As well if you can spot why online sensor goes unavailable I am open to it! I have to add some debug lines to figure that out.

Looks good, some comments:

• We should remove the comment about Kia.
• I am not sure if the on_icon and off_icon are useful, as it stands the one sensor is not defining them.
  What if we used device_class on VeSyncBinarySensorEntityDescription? These are the available device_classes and they should automatically provide the icon? https://www.home-assistant.io/integrations/binary_sensor/#device-class
• I implemented a version of binary_sensor and number but wanted to resolve the dataCoordinator stuff before making it official -https://github.com/iprak/core-fork/blob/add-humidifier/homeassistant/components/vesync/binary_sensor.py
• I have been continuously running with debugging enabled in the python package with this change manager = VeSync(username, password, time_zone, debug=True) and debug logging and all the entities have been chugging fine for past week. The entity will go unavailable if devicedevice.connection_status == "online" which means that maybe the Vesync server was not able to reach the device or did not hear back from the device in timely manner. Is your update interval still the same 30 seconds?

[cdnninja]

1. Good catch, I missed one spot.
2. I removed the code for icons for now, it wasn't used anyway. Device class is already implemented as shown in the descriptions.
3. That is good to know! We should find a way to coordinate to reduce efforts.
4. I figured it out. This code here causes it to go unavailable when offline. This means all sensors including useful ones like an "Online" sensor are gone.

   core/homeassistant/components/vesync/entity.py

   Lines 33 to 36 in 9804e8a

   	@property
   	def available(self) -> bool:
   	"""Return True if device is available."""
   	return self.device.connection_status == "online"

   Leaving that for now.

[cdnninja]

Putting this ready for review, while I would like coordinator to go first that is a quick update to this PR once that is merged. In the meantime it can get some eyes.

[home-assistant]

Please take a look at the requested changes, and use the Ready for review button when you are done, thanks 👍

Learn more about our pull request process.

[iprak]

I like this approach of adding the sensor based on presence of description.key.

Can water_tank_lifted be exposed as a property on VeSyncHumid200300S as water_lacks? I did not initially understand why rgetattr was needed. This definitely does not need to be addressed here.

[joostlek]

Would it maybe be nice to have a group on discord to chat on?

[cdnninja]

Happy to have you join. iprak and I have been chatting directly on discord.

[joostlek]

there's a merge conflict

[iprak]

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.

[cdnninja]

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?

[cdnninja]

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.

[iprak]

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?

[cdnninja]

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?

[cdnninja]

To confirm do you have changes requested or are you okay with approving as is?

[iprak]

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.

[cdnninja]

Got it. So combine in description and use the same value programmatically for "is_on". I will give it a go.

[iprak]

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.

[cdnninja]

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.

[iprak]

A few comments

[cdnninja]

Ready for another round of review.