-
Notifications
You must be signed in to change notification settings - Fork 55
Rename quirk_id to exposes_features + allow multiple features
#575
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
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## dev #575 +/- ##
==========================================
- Coverage 97.02% 97.00% -0.03%
==========================================
Files 63 63
Lines 10535 10544 +9
==========================================
+ Hits 10222 10228 +6
- Misses 313 316 +3 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Custom quirks could make this mistake. Since it's easy to correct on our side, we do.
- `exposes_features` is used by the device. - `exposed_features` is used by the MatchRules to create discovery matches.
8ed730b to
11a6c1e
Compare
|
I think we could get this in as well. It does what the PR title says, but it only processes the (v1) Another PR would add the exposed features from the v2 quirks API, something like TheJulianJES/zha@tjj/multiple_quirk_id...TheJulianJES:zha:tjj/quirks_v2_exposed_features. For a diff of this PR without the regenerated diagnostics (so GitHub dies less), see: dev...TheJulianJES:zha:tjj/multiple_quirk_id_without_diagnostics |
|
This would require one small change in Core here: https://github.com/home-assistant/core/blob/9ebc6cbb23f470c5c371b1b5abb189513a9086af/homeassistant/components/zha/helpers.py#L344 You're good with this PR? |
puddly
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
Core patch for this PR included here: home-assistant/core@dev...TheJulianJES:tjj/zha-deps-bump-20251126 |
Proposed change
This renames
quirk_idtoexposes_featuresinternally and allows quirks to set multiple exposed features.Why
This will be useful for a number of issues, including Frient sensors that expose other IAS bits (alarm 1/2 + tamper + test bit), so we don't have to use custom quirks v2 entities for all devices using standard ZCL functionality. It's also useful to remove certain features we currently expose for all siren entities, like a lot of the siren specific parameters or the configuration entities.
In v1/v2 quirks, we can simply add a line that's something like
.exposes_feature(IAS_TAMPER_FEATURE)or.exposes_feature(SIREN_LIMITED_FEATURE)and ZHA will add or change the behavior of some entities for those devices.Other notes
For now, I've left the attribute we check for the exposed features in v1 quirks called
quirk_id. A previous iteration of this PR only renamed all internal references toquirk_ids, but I think we can just go ahead and rename it toexposes_featureslike done in this PR.A small follow-up PR would add support from specifying exposed features in v2 quirks.
quirk_idcan be aset[str]/list[str]orstras before, to make sure we don't break existing integrated v1 quirks or custom v1 quirks that may be used for some Tuya (plug) devices.Additional information
Addresses (see for more background info):
quirk_idtoexposes_featuresand allow multiple features in quirks #311Other related issues:
quirk_idorexposes_featuresvia quirks v2 backlog#62 (with future PRs)For a diff without the regenerated diagnostics, see: dev...TheJulianJES:zha:tjj/multiple_quirk_id_without_diagnostics