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

Rework OnOff output cluster handler logic #145

Open
elupus opened this issue Aug 6, 2024 · 4 comments
Open

Rework OnOff output cluster handler logic #145

elupus opened this issue Aug 6, 2024 · 4 comments
Labels
breaking change PR is a breaking change enhancement New feature or request

Comments

@elupus
Copy link
Contributor

elupus commented Aug 6, 2024

Currently there is a special logic at

def handle_on_off_output_cluster_exception(self, endpoint: Endpoint) -> None:
that converts a OnOff output (client) cluster into a virtual server cluster through a handler. It will generate attribute updates based on the commands generated by the client cluster.

I assume this logic stems from before we had event entities in home assistant and was some type of attempt to create a binary_input like entity from an OnOff client cluster.

We should add support for event entities for output clusters, and drop this rather confusing special logic for OnOff clusters.

@elupus elupus changed the title The special casing of OnOff client clusters should likely be removed The special casing of OnOff client clusters should likely be removed in preference to event entities Aug 6, 2024
@puddly
Copy link
Contributor

puddly commented Aug 6, 2024

Any sort of virtual clusters need to go, in my opinion. They were created to re-use the existing cluster handlers to create entities. With the ZHA library allowing direct entity creation, we need to start replacing virtual clusters with those.

@elupus
Copy link
Contributor Author

elupus commented Aug 6, 2024

In this case it's not virtual clusters, it's a virtual entity linked to a handler. The clusters are real here.

@elupus
Copy link
Contributor Author

elupus commented Aug 6, 2024

The attributes on the cluster is virtual thou.

@dmulcahey dmulcahey added enhancement New feature or request breaking change PR is a breaking change labels Aug 8, 2024
@TheJulianJES
Copy link
Contributor

TheJulianJES commented Nov 25, 2024

It doesn't make sense to convert a binary sensor for open/close or motion/no_motion to an event entity IMO.

The logic regarding OnOff entity discovery needs to be improved, but we should not use event entities in this case.
The OnOff case is only special, as it's an output cluster and we're somewhat misusing the cluster handler.

Other than that, it's similar to IasZone. Both the OnOff (output) cluster and the IasZone (input) cluster only send commands and we convert them to "attribute reports" locally. We can't actually set up attribute reporting for OnOff nor IasZone.

We should, however, consider implementing event entities for remotes with device automation triggers provided by quirks.
And we should make sure the OnOff entity is not displayed for remotes, but only for door/window, motion sensors, and so on. That's a separate issue though: #289

@TheJulianJES TheJulianJES changed the title The special casing of OnOff client clusters should likely be removed in preference to event entities Rework OnOff output cluster handler logic Nov 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking change PR is a breaking change enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

4 participants