Skip to content

Conversation

@hcarter-775
Copy link
Contributor

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.

@github-actions
Copy link

github-actions bot commented Nov 6, 2025

@github-actions
Copy link

github-actions bot commented Nov 6, 2025

Test Results

   71 files    466 suites   0s ⏱️
2 416 tests 2 416 ✅ 0 💤 0 ❌
4 115 runs  4 115 ✅ 0 💤 0 ❌

Results for commit 1a962ef.

♻️ This comment has been updated with latest results.

@github-actions
Copy link

github-actions bot commented Nov 6, 2025

File Coverage
All files 92%
/home/runner/work/SmartThingsEdgeDrivers/SmartThingsEdgeDrivers/drivers/SmartThings/matter-switch/src/sub_drivers/aqara_cube/init.lua 96%
/home/runner/work/SmartThingsEdgeDrivers/SmartThingsEdgeDrivers/drivers/SmartThings/matter-switch/src/switch_utils/embedded_cluster_utils.lua 38%
/home/runner/work/SmartThingsEdgeDrivers/SmartThingsEdgeDrivers/drivers/SmartThings/matter-switch/src/switch_utils/utils.lua 90%
/home/runner/work/SmartThingsEdgeDrivers/SmartThingsEdgeDrivers/drivers/SmartThings/matter-switch/src/switch_utils/device_configuration.lua 97%
/home/runner/work/SmartThingsEdgeDrivers/SmartThingsEdgeDrivers/drivers/SmartThings/matter-switch/src/switch_utils/fields.lua 98%
/home/runner/work/SmartThingsEdgeDrivers/SmartThingsEdgeDrivers/drivers/SmartThings/matter-switch/src/switch_handlers/event_handlers.lua 97%
/home/runner/work/SmartThingsEdgeDrivers/SmartThingsEdgeDrivers/drivers/SmartThings/matter-switch/src/switch_handlers/attribute_handlers.lua 83%
/home/runner/work/SmartThingsEdgeDrivers/SmartThingsEdgeDrivers/drivers/SmartThings/matter-switch/src/switch_handlers/capability_handlers.lua 88%
/home/runner/work/SmartThingsEdgeDrivers/SmartThingsEdgeDrivers/drivers/SmartThings/matter-switch/src/sub_drivers/third_reality_mk1/init.lua 95%
/home/runner/work/SmartThingsEdgeDrivers/SmartThingsEdgeDrivers/drivers/SmartThings/matter-switch/src/sub_drivers/eve_energy/init.lua 92%
/home/runner/work/SmartThingsEdgeDrivers/SmartThingsEdgeDrivers/drivers/SmartThings/matter-switch/src/init.lua 96%

Minimum allowed coverage is 90%

Generated by 🐒 cobertura-action against 1a962ef

@mocelet
Copy link

mocelet commented Nov 6, 2025

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.

@ldeora
Copy link

ldeora commented Nov 6, 2025

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)
-- })
Copy link
Contributor

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?

Copy link
Contributor Author

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)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

spacing

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants