[Aqara] Aqara Dual Relay Module T2 - #3146
Conversation
|
Duplicate profile check: Passed - no duplicate profiles detected. |
|
Invitation URL: |
|
zigbee-switch_coverage.xml
Minimum allowed coverage is Generated by 🐒 cobertura-action against 8b3470c |
Test Results 73 files 541 suites 0s ⏱️ Results for commit 8b3470c. ♻️ This comment has been updated with latest results. |
| data_type = data_types.Uint8, | ||
| value_map = { rocker = 0x01, rebound = 0x02 }, | ||
| }, | ||
| -- External switch wiring type (same attribute as stse.electricSwitchType, with an extra "nc" |
There was a problem hiding this comment.
Why is this needed? Can we add the nc variant t othe existing electricSwitchType preference?
Since they map to the same attribute, this could get confusing in the future.
There was a problem hiding this comment.
If we add the 'nc' variant to the existing electricSwitchType preference, I am wondering if there will be any impact or side effects on devices that previously did not support the 'nc' variant.
I would appreciate it if you could share your thoughts on whether this might cause any issues regarding backward compatibility.
There was a problem hiding this comment.
Your concern is valid; adding the variant to the preference will expose it for all devices that have the preference even if they do not support it. IMO this is probably okay, but would depend on how existing devices that do not support it handle the nc option being set in the attribute write. If the attribute write is rejected and it is essentially a no-op (what I would expect), then we can just add text in the preference definition that the option may not be supported.
IMO, we should combine them, but I am curious if @hcarter-775 or @tpmanley have any thoughts
There was a problem hiding this comment.
@tpmanley do you have any thoughts here? I dont love the duplicate named preference, but it may be warranted here.
|
Profile category check: ✅ Passed - all profiles have a category defined. |
|
This is looking good to me. It needs to be updated to resolve conflicts and have tests passing. |
Check all that apply
Type of Change
Checklist
Description of Change
Summary of Completed Tests