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

[meta] Add introspectable interfaces #1495

Closed
8 tasks done
rytilahti opened this issue Aug 13, 2022 · 7 comments
Closed
8 tasks done

[meta] Add introspectable interfaces #1495

rytilahti opened this issue Aug 13, 2022 · 7 comments

Comments

@rytilahti
Copy link
Owner

rytilahti commented Aug 13, 2022

This is a meta task to keep track on what interfaces needs to be added to allow defining all the necessary information inside python-miio to make interfacing with it from homeassistant (and other downstreams) as straightforward as possible.

This issue serves three purposes:

  1. As a single point check list to keep track of the progress.
  2. As a call for help as I have access only to very limited set of devices.
  3. As something that can be linked to homeassistant contributors submitting PRs to enhance support for individual devices.

If you are reading this, and you have a Xiaomi device that is supported in homeassistant by this library, please consider contributing to this project by adding some simple metadata to the status containers of the devices you own (see "call for help" below for details).

The end goal of this journey is that this library provides enough information directly to avoid hard-coding any device-specific details–available features, actions, sensors, settings etc.–inside the downstream applications like homeassistant.
This would make this library the single source of truth when it comes to the devices supported by it, and relieves downstream maintainers from the painstaking process of adapting their implementation whenever this library gets extended.

Call for help

If you possess any devices besides the 1st gen Roborock vacuum and a zhimi powerstrip which I have, please consider creating a pull request to add the @sensor, @setting and @action descriptor decorators so that homeassistant (and other potential downstreams) can create entities for your device automatically in the future. https://python-miio.readthedocs.io/en/latest/contributing.html#status-containers contains some documentation on the topic, and the issues linked below in the checklist show an example of how they look like in homeassistant.

The homeassistant fork at https://github.com/rytilahti/home-assistant/tree/xiaomi_miio/feat/entities_from_upstream allows testing your changes. When we get feature parity (or close to it) with the existing homeassistant support, the plan is to create smaller PRs (per platform, most likely) based on the fork to first enable the additional entities, and sunset the hard-coded ones afterward.

When adding the descriptors to the status classes, you can provide the following extra kwargs that are currently used by the fork:

  • device_class would be great to have besides the name and the unit
  • icon for defining custom icons when no device_class is set
  • state_class is not a priority at the moment, the default value should be chosen inside homeassistant and overridden only when needed

If you want to contribute information on several devices, please create a single pull request for each separate integration.
Feel free to ping me on the homeassistant discord or via e-mail if you have any questions!

Checklist

Any help and contributions are welcome!

@starkillerOG
Copy link
Contributor

@rytilahti I am getting my new Roborock S7 maxV plus today.
I will be away coming week, but after that I am going to help cleaning up and moving over the vacuum platform to this new way of setting up the entities in HomeAssistant.

@rytilahti
Copy link
Owner Author

rytilahti commented Sep 8, 2022

@starkillerOG The roborock integration has already been converted to use dynamic sensors and my homeassistant branch has already converted vacuum platform to present the exposed sensors but what is still needed is to find sane a way to embed other status containers (e.g., for timers and consumables, that require separate I/O) into the main status container.

@starkillerOG
Copy link
Contributor

@rytilahti I started testing the dynamic vacuum sensors on my Roborock S7MaxV and have finished polishing the sensors, added some sensors and fixed some issues. The PR for it is here: #1543
Would you mind taking a look?

@rytilahti
Copy link
Owner Author

@starkillerOG great! I'll do a quick review take a deeper look at it when I return from travels.

@starkillerOG
Copy link
Contributor

@rytilahti thanks, that would be great!
When do you expect to return from travels (and have time to publish a new python-miio version)?

@rytilahti
Copy link
Owner Author

@starkillerOG I'll be back some time next week, but I'll have to see when I have some time to work on getting the last larger missing piece done. I want to have a way to initialize devices given only the host and the token, so that we have no blockers from python-miio's side to start removing hard-coded models from the homeassistant integration.

Getting a new release done and shipped with the 2022.11 like you mentioned elsewhere would be a good target!

rytilahti added a commit that referenced this issue Oct 25, 2022
This provides a common, vendor agnostic API for obtaining vacuum state
information,
to be used by downstreams like homeassistant.
This also converts roborock vacuum to use these common facilities.

Added common API:
* `VacuumDeviceStatus` is an interface extending `DeviceStatus` to
enforce common API for devices implementing `VacuumInterface`.
* `VacuumState` provides a generic interface to map different vacuum
statuses.

These interfaces are bound to change and are introduced to make it
simpler to make other vacuum integrations available to downstream users.
This is related to #1495.
@rytilahti
Copy link
Owner Author

As the interfaces are already there, I'm closing this. The conversion of existing integrations to have descriptors is tracked in #1617.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants