chore: Add a high level direction for improving device and feature discovery #722
Conversation
…scovery This is meant to try to summarize the device features & discovery pieces that need to be improved to get to the next level of home assistant quality.
| | :--- | :--- | :--- | | ||
| | **Startup** | Waits for all devices to refresh. | Starts immediately after getting device list. | | ||
| | **Offline Device** | May cause setup failure or missing entities. | Device and entities are created as "Unavailable". | | ||
| | **Unknown Device** | Might crash or fail integration setup. | Gracefully handled using basic generic traits. | |
There was a problem hiding this comment.
by this i assume you mean unknown as in devices we have never seen - rather than 'new' devices i.e. the b01 devices
There was a problem hiding this comment.
I meant here an unsupported device. There was a bug where it would throw an exception and fail the whole integration, even if some devices were supported. I believe this may now be fixed though.
| - **Result**: The device is registered and visible in the UI immediately. | ||
| 3. **Connection & Recovery (Background)**: | ||
| - In the background, `DeviceManager` works to establish MQTT/Local connections. | ||
| - As devices come online, the `DeviceListener` triggers updates, and entities become "Available". |
There was a problem hiding this comment.
Ideally, DeviceListener utilizes the existing RoborockProtocol to automatically listen for devices.
Devices will ping us with their ip and their duid. We can then automatically add them.
Some instances will have these ports blocked (we should recommend opening them on the docs). If the ports are blocked, we should do a less often Home data call.
|
|
||
| To verify a device is supported, we'll need to move to the model of checking device features, or only relying on the product information `HomeData` instead of waiting for a live response. | ||
|
|
||
| #### Entity Registration |
There was a problem hiding this comment.
While less sophisticated, I wonder if utilizing the HA entity manager would be easier - to determine what entities have existed before
|
|
||
| #### Entity Registration | ||
| - **Current Model**: We try to communicate with the device. If it fails, we don't know what sensors to create, so we create nothing. | ||
| - **Proposed Model**: We check the account metadata or cached device features, and create the corresponding sensors immediately. |
There was a problem hiding this comment.
If there is a new device that we fail to communicate with - do we still create the device and just add no entities?
There was a problem hiding this comment.
Yeah, that would be the proposal.
| - *Rationale*: A timeout or connection failure is a *device state*, not a discovery failure. Once we get the list (from cache or cloud), discovery is successful. | ||
| - Update `DeviceManager` to optionally *not* auto-connect. The Home Assistant Coordinator will be responsible for calling `start_connect()` so that disabled devices remain disconnected. | ||
| - Ensure `device_ready` callbacks fire once feature discovery completes even if the initial connection fails | ||
| - Add a periodic task to re-run `discover_devices` every N minutes to fetch fresh `HomeData`. |
There was a problem hiding this comment.
We should be careful calling home_data often. This is a potential route to rate limiting.
There was a problem hiding this comment.
Want to propose a frequency? We could also do this:
(1) Never poll in the background
(2) Fetch on startup only if over X hours since last fetch (e.g. if you reload many times in a row it does nothing)
|
I wonder if we should follow this rule: If a feature is supported on only some devices - it should be split up into a separate trait. This may mean that things like 'Status' need to be split up. It is a lot easier for us to check/organize the logic if it were checking one trait rather than checking the attribute of a trait. But that may have some complexity to it. What did you have in mind for this part of the system? The actual entity/feature mapping. |
This sounds like a reasonable proposal to me. |
By this i mean: It sounds like it can work, i don't have any objection to it, and i don't have any other better suggestions. I like the idea of treating "device features" as an internal implementation detail. |
Although - some devices have more data for things like dnd than others. and we may end up with something like DnDTrait Then consumables as well can be different per devices, meaning we may need to have a seperate trait per consumable. I guess this may get pretty big pretty fast. What about a custom decorator for properties? If no decorator -> if the device ahs the trait it is supported. If decorator -> device must have specific trait(s) for this property to be supported. We can still abstract this up somehow, but then device features stay grouped with the specific thing that they are supporting. |
What would the caller API be to check if a feature is supported? An alternative I can think of is to have an enum with supported features on the trait (an enum value per optional field) and the convention would just be that it has a similar name to the specific fields. |
theoretically, it could just hit the property and get back some exception? Not sure. Or the property could dynamically pass the information upwards and create a kind of device feature manager. Or the property could set some value on the trait that we can then check.
That could be fine. entities would end up having a extra field on the description of which enum it needs for that entity to exist. How do you then map that enum to the actual device feature? i.e. how do we know that this device supports that enum value for that property? I think device features is likely too abstract of a concept to expose to HA, so I like where we are heading with this. |
2 participants
This is meant to try to summarize the device features progress and potential next steps, and discovery pieces that need to be improved to get to the next level of home assistant quality.