Conversation
Co-authored-by: puddly <32534428+puddly@users.noreply.github.com>
I'm in favor of ZCL base class entities inheriting from the abstract entity. There are few platforms where it actually helps (
This is an interesting API gap. We need entities whose discovery depends on the existence of another entity, right? This requires some discovery restructuring, because we right now assume all entities will be discovered in a single pass and thus they are assumed to be order-independent. Right now, we treat the entity itself as the smallest unit for encompassed functionality but I guess we could also think of this as a special case of entity "groups": where groups of entities are created by discovery, not just one? That way, the group of entities has shared discovery logic?
We currently have only a single |
With the current approach, we have the abstract
Hmm, I didn't even think about linking the select entities to the presence of the
I get that but I also feel like it's essentially a cleaner Let's assume we want to do the following:
|
DRAFT. Steals abstract base class siren changes from #662.
It's easier to look at the diffs from the individual commits.
TODO: Move
"siren_basic"string as constant tozha-quirks, so we can later import it.TODO: Regenerate diagnostics (mostly due to class name change in latest commit).
TODO: Regenerate diagnostics with quirks bump.
Proposed change
This adds a "basic siren" to ZHA, which does not expose tone + volume + strobe features. The matching is done via an "exposed feature" (formerly known as
quirk_id).Weirdly enough, all Zigbee sirens I've encountered so far do not expose these features. I'm wondering if it makes sense to flip this around, so have a "basic siren" by default, and "advanced siren" with a quirk. I'll need to look at diagnostics for that.
EDIT: It does look like some sirens we have in our diagnostics are intended to expose the more advanced options (looking at Z2M), so I probably wouldn't risk flipping it around now and feature hinting existing advanced sirens.
Additional information
Implementation
The latest commit
466eb6b(this PR) also introduces aBaseZclSiren(based onBaseSiren).Both
AdvancedSirenandBasicSirenthen inherit from theBaseZclSiren. This avoids some code duplication, but I'm not sure if it's fully worth it. If not, I can just revert the commit. Thoughts on this are appreciated.What it does
So far, this PR should remove these options:

Select entities
But not the useless non-ZCL select entities below. This would be easier to do with the exposed feature matching if we assume "basic" is the default, not "advanced". If we don't do that, we'll have to create separate entity classes and override
is_supportedOR checkis_supportedin the normal classes. (I think both should work?)Alternatively, we could also add
not_exposed_featuresto our discovery. It should be a very small change. Would also like opinions on this.Example of the useless select entities:

I think it makes the most sense to have these entities be removed in a follow-up PR, since it's a different platform. EDIT: See: