-
Notifications
You must be signed in to change notification settings - Fork 517
Matter Switch: Write EXECUTE_IF_OFF bit to Control Cluster Options attribute #2535
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
base: main
Are you sure you want to change the base?
Conversation
|
Invitation URL: |
Test Results 71 files 466 suites 0s ⏱️ Results for commit 1a962ef. ♻️ This comment has been updated with latest results. |
|
Minimum allowed coverage is Generated by 🐒 cobertura-action against 1a962ef |
|
Out of curioisity, is there an scenario where it is needed? The driver already sets the Options Mask to override the ExecuteIfOff bit and set it to 1 when setting a colour or temperature and for setting the brightness level it uses the MoveToLevelWithOnOff command and ExecuteIfOff does not apply to WithOnOff commands. |
Multi-Admin? My interpretation is, that this ensures consistent behavior in multi-admin Matter setups, where the device may be controlled by more than one controller. Not all controllers reliably include OptionsMask = 1 and OptionsOverride = 1 in their command payloads for the Color Control or Level Control clusters. If the server’s Options attribute has the bit set to 0, those external commands may fail when the device is off. This can lead to inconsistent user experiences across platforms - for example, attempting to adjust hue or brightness from a third-party app may not take effect unless the device is manually turned on first. Setting the bit to 1 during commissioning ensures that commands execute by default across all clients, without depending on overrides in each command. Additionally, this helps future-proof the device against driver updates or new command patterns where overrides may not be applied, as well as scenarios that involve reading or adjusting level/color states while the device is off. If your setup uses only a single controller (SmartThings only), the driver’s overrides already handle the behavior, so the attribute change may be redundant, but it remains a low-cost improvement for broader compatibility. |
| -- test.socket.matter:__expect_send({ | ||
| -- mock_device_color_dimmer.id, | ||
| -- clusters.ColorControl.attributes.Options:write(mock_device_color_dimmer, 7, clusters.ColorControl.types.OptionsBitmap.EXECUTE_IF_OFF) | ||
| -- }) |
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.
Seeing this commented out makes me wonder if these expectations are working being set in the test init function. Is this device not supposed to set these options bits?
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.
whoops, gotta remove those. I had those lines originally but then made the code more stringent, to only do the write on devices supporting these clusters as server rather than client, so this test should (correctly) not have these writes occur.
| return (cluster.feature_map & checked_feature) == checked_feature | ||
| end | ||
| for _, cluster in ipairs(ep_info.clusters) do | ||
| if ((cluster.cluster_id == cluster_id) |
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.
spacing
Description of Change
During device configuration (and only during commissioning per spec), we can write the EXECUTE_IF_OFF bit to the Options attribute of the Level Control and Color Control clusters (that are supported as SERVER) to indicate to all devices that level controlability should be allowed even when the device is off.
This is the norm for most light devices, so for consistency across all user experiences on the ST platform, we should attempt to make all devices functional like this.
Summary of Completed Tests
Unit tests updated.