Hager rotary dimmer implementation - #3215
Conversation
|
Invitation URL: |
|
matter-switch_coverage.xml
Minimum allowed coverage is Generated by 🐒 cobertura-action against c19f612 |
Test Results 73 files ±0 541 suites ±0 0s ⏱️ ±0s For more details on these errors, see this check. Results for commit c19f612. ± Comparison against base commit 71bbd3d. ♻️ This comment has been updated with latest results. |
| device:send(cluster_base.subscribe(device, endpoint_id, cluster_id, attr_id, event_id)) | ||
| end | ||
|
|
||
| local function is_standalone_device (device) |
There was a problem hiding this comment.
what does this function imply?
There was a problem hiding this comment.
This function is created to separate standalone devices which are from WAASYS series but do not utilize the matter bridge device type. They differ in the endpoint architecture and have to be handled in a different way but they also support manually set functions like adding a child device responsible for remote light control.
This function allows to determine this type of device and eliminate/include them the driver behavior.
for example, standalone devices do not require mapping done in the info_changed function.
There was a problem hiding this comment.
So you are saying that these are just devices from Hager, rather than bridged devices from the Hager Bridge? I am curious, but I also think this would be a worthwhile thing to explain in a comment, since is_standalone does not express a lot (in my opinion) about what is being checked, especially when the internals are extremely device specific.
I also suggest that you add some commentary in the function itself to explain what is being checked, since the endpoint id checks are not understandable without Hager-specific context.
There was a problem hiding this comment.
Yes, they are just simple Hager devices without the bridge functionality.
I will change the function name to "is_non_bridge_type" and add comments for better clarity
and understanding of the architectural differences between them.
There was a problem hiding this comment.
Yeah, I'd maybe just check then if the device is 1. an Aggregator device type and 2. is a Matter (not EDGE child) device? Or something like that. If it isn't, then we'd know it is a non-bridge Matter device. I am not sure we need this extremely specific endpoint checking to know whether it is a regular device or not
b9d4915 to
c19f612
Compare
| local value = ib.data.elements | ||
| local reports_dimmable = false | ||
|
|
||
| for _, element in ipairs(value) do |
There was a problem hiding this comment.
| for _, element in ipairs(value) do | |
| for _, element in ipairs(value or {}) do |
2 participants
Check all that apply
Type of Change
Checklist
Description of Change
Adding rotary dimmer device support for Hager.
Summary of Completed Tests