Skip to content
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

Add support for the color mode attribute #1772

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
45 changes: 39 additions & 6 deletions drivers/SmartThings/matter-switch/src/init.lua
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ local COMPONENT_TO_ENDPOINT_MAP = "__component_to_endpoint_map"
-- rather than COMPONENT_TO_ENDPOINT_MAP.
local COMPONENT_TO_ENDPOINT_MAP_BUTTON = "__component_to_endpoint_map_button"
local IS_PARENT_CHILD_DEVICE = "__is_parent_child_device"
local COLOR_MODE = "__color_mode"
local COLOR_TEMP_BOUND_RECEIVED = "__colorTemp_bound_received"
local COLOR_TEMP_MIN = "__color_temp_min"
local COLOR_TEMP_MAX = "__color_temp_max"
Expand Down Expand Up @@ -840,14 +841,26 @@ end
local function hue_attr_handler(driver, device, ib, response)
if ib.data.value ~= nil then
local hue = math.floor((ib.data.value / 0xFE * 100) + 0.5)
device:emit_event_for_endpoint(ib.endpoint_id, capabilities.colorControl.hue(hue))
local current_color_mode = get_field_for_endpoint(device, COLOR_MODE, ib.endpoint_id)
-- don't send capability events if the color of the device is being determined by CurrentX and CurrentY (1) or ColorTemperatureMireds (2)
if current_color_mode == 1 or current_color_mode == 2 then
Copy link
Contributor

Choose a reason for hiding this comment

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

This might be more clear just changing this to

Suggested change
if current_color_mode == 1 or current_color_mode == 2 then
if current_color_mode != 0 then

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I did it this way in case a device doesn't implement the ColorMode attribute. In that case, COLOR_MODE will be nil in the driver, so doing the logic this way maintains the same behavior (we only want to prevent capabilities from being emitted if ColorMode is actually implemented)

log.info_with({hub_logs=true}, string.format("CurrentHue and CurrentSaturation are not currently determining the color of the device. ColorMode is %d", current_color_mode))
else
device:emit_event_for_endpoint(ib.endpoint_id, capabilities.colorControl.hue(hue))
end
end
end

local function sat_attr_handler(driver, device, ib, response)
if ib.data.value ~= nil then
local sat = math.floor((ib.data.value / 0xFE * 100) + 0.5)
device:emit_event_for_endpoint(ib.endpoint_id, capabilities.colorControl.saturation(sat))
local current_color_mode = get_field_for_endpoint(device, COLOR_MODE, ib.endpoint_id)
-- don't send capability events if the color of the device is being determined by CurrentX and CurrentY (1) or ColorTemperatureMireds (2)
if current_color_mode == 1 or current_color_mode == 2 then
log.info_with({hub_logs=true}, string.format("CurrentHue and CurrentSaturation are not currently determining the color of the device. ColorMode is %d", current_color_mode))
else
device:emit_event_for_endpoint(ib.endpoint_id, capabilities.colorControl.saturation(sat))
end
end
end

Expand Down Expand Up @@ -942,8 +955,14 @@ local function x_attr_handler(driver, device, ib, response)
else
local x = ib.data.value
local h, s, _ = color_utils.safe_xy_to_hsv(x, y)
device:emit_event_for_endpoint(ib.endpoint_id, capabilities.colorControl.hue(h))
device:emit_event_for_endpoint(ib.endpoint_id, capabilities.colorControl.saturation(s))
local current_color_mode = get_field_for_endpoint(device, COLOR_MODE, ib.endpoint_id)
-- don't send capability events if the color of the device is being determined by CurrentHue and CurrentSaturation (0) or ColorTemperatureMireds (2)
if current_color_mode == 0 or current_color_mode == 2 then
Copy link
Contributor

Choose a reason for hiding this comment

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

Same idea as above:

Suggested change
if current_color_mode == 0 or current_color_mode == 2 then
if current_color_mode != 1 then

log.info_with({hub_logs=true}, string.format("CurrentX and CurrentY are not currently determining the color of the device. ColorMode is %d", current_color_mode))
else
device:emit_event_for_endpoint(ib.endpoint_id, capabilities.colorControl.hue(h))
device:emit_event_for_endpoint(ib.endpoint_id, capabilities.colorControl.saturation(s))
end
device:set_field(RECEIVED_Y, nil)
end
end
Expand All @@ -955,12 +974,24 @@ local function y_attr_handler(driver, device, ib, response)
else
local y = ib.data.value
local h, s, _ = color_utils.safe_xy_to_hsv(x, y)
device:emit_event_for_endpoint(ib.endpoint_id, capabilities.colorControl.hue(h))
device:emit_event_for_endpoint(ib.endpoint_id, capabilities.colorControl.saturation(s))
local current_color_mode = get_field_for_endpoint(device, COLOR_MODE, ib.endpoint_id)
-- don't send capability events if the color of the device is being determined by CurrentHue and CurrentSaturation (0) or ColorTemperatureMireds (2)
if current_color_mode == 0 or current_color_mode == 2 then
log.info_with({hub_logs=true}, string.format("CurrentX and CurrentY are not currently determining the color of the device. ColorMode is %d", current_color_mode))
else
device:emit_event_for_endpoint(ib.endpoint_id, capabilities.colorControl.hue(h))
device:emit_event_for_endpoint(ib.endpoint_id, capabilities.colorControl.saturation(s))
end
device:set_field(RECEIVED_X, nil)
end
end

local function color_mode_attr_handler(driver, device, ib, response)
Copy link
Contributor

Choose a reason for hiding this comment

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

Are we guaranteed to receive the color mode attribute update before the actually x/y or hue/sat attribute update if it is changed on the device? Do you see this attribute actually change on the device?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That is a really good point, and no that is not guaranteed. The ColorMode attribute is always sent by the device if it changes however.

I added new logic to color_mode_attr_handler to account for ColorMode not necessarily being received first. When the attribute handlers for Hue, Sat, X, and Y run, they now store the computed Hue and Saturation values, and then emit the hue and sat capabilities depending on the ColorMode. The capabilities are not emitted if ColorMode is set to one of the other 3 possible values. In that case, either the ColorMode is currently set to one of those other values, or it was just changed to a new value but the ColorMode attribute handler hasn't run yet. In the latter case, the ColorMode attribute handler will emit the capabilities using the stored values.

if ib.data.value ~= nil then
set_field_for_endpoint(device, COLOR_MODE, ib.endpoint_id, tonumber(ib.data.value), {persist = true})
end
end

--TODO setup configure handler to read this attribute.
local function color_cap_attr_handler(driver, device, ib, response)
if ib.data.value ~= nil then
Expand Down Expand Up @@ -1160,6 +1191,7 @@ local matter_driver_template = {
[clusters.ColorControl.attributes.ColorTemperatureMireds.ID] = temp_attr_handler,
[clusters.ColorControl.attributes.CurrentX.ID] = x_attr_handler,
[clusters.ColorControl.attributes.CurrentY.ID] = y_attr_handler,
[clusters.ColorControl.attributes.ColorMode.ID] = color_mode_attr_handler,
[clusters.ColorControl.attributes.ColorCapabilities.ID] = color_cap_attr_handler,
[clusters.ColorControl.attributes.ColorTempPhysicalMaxMireds.ID] = mired_bounds_handler_factory(COLOR_TEMP_MIN), -- max mireds = min kelvin
[clusters.ColorControl.attributes.ColorTempPhysicalMinMireds.ID] = mired_bounds_handler_factory(COLOR_TEMP_MAX), -- min mireds = max kelvin
Expand Down Expand Up @@ -1212,6 +1244,7 @@ local matter_driver_template = {
clusters.ColorControl.attributes.CurrentSaturation,
clusters.ColorControl.attributes.CurrentX,
clusters.ColorControl.attributes.CurrentY,
clusters.ColorControl.attributes.ColorMode,
},
[capabilities.colorTemperature.ID] = {
clusters.ColorControl.attributes.ColorTemperatureMireds,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@ local function test_init()
clusters.ColorControl.attributes.CurrentSaturation,
clusters.ColorControl.attributes.CurrentX,
clusters.ColorControl.attributes.CurrentY,
clusters.ColorControl.attributes.ColorMode,
clusters.ColorControl.attributes.ColorTemperatureMireds,
clusters.ColorControl.attributes.ColorTempPhysicalMaxMireds,
clusters.ColorControl.attributes.ColorTempPhysicalMinMireds,
Expand Down
113 changes: 113 additions & 0 deletions drivers/SmartThings/matter-switch/src/test/test_matter_switch.lua
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@ local cluster_subscribe_list = {
clusters.ColorControl.attributes.CurrentSaturation,
clusters.ColorControl.attributes.CurrentX,
clusters.ColorControl.attributes.CurrentY,
clusters.ColorControl.attributes.ColorMode,
clusters.ColorControl.attributes.ColorTemperatureMireds,
clusters.ColorControl.attributes.ColorTempPhysicalMaxMireds,
clusters.ColorControl.attributes.ColorTempPhysicalMinMireds,
Expand Down Expand Up @@ -792,4 +793,116 @@ test.register_message_test(
}
)

test.register_message_test(
"colorControl capability sent based on CurrentHue and CurrentSaturation but not CurrentX and CurrentY due to ColorMode",
{
{
channel = "matter",
direction = "receive",
message = {
mock_device.id,
clusters.ColorControl.attributes.ColorMode:build_test_report_data(mock_device, 1, 0)
}
},
{
channel = "matter",
direction = "receive",
message = {
mock_device.id,
clusters.ColorControl.attributes.CurrentHue:build_test_report_data(mock_device, 1, 0xFE)
}
},
{
channel = "capability",
direction = "send",
message = mock_device:generate_test_message("main", capabilities.colorControl.hue(100))
},
{
channel = "matter",
direction = "receive",
message = {
mock_device.id,
clusters.ColorControl.attributes.CurrentSaturation:build_test_report_data(mock_device, 1, 0xFE)
}
},
{
channel = "capability",
direction = "send",
message = mock_device:generate_test_message("main", capabilities.colorControl.saturation(100))
},
{
channel = "matter",
direction = "receive",
message = {
mock_device.id,
clusters.ColorControl.attributes.CurrentX:build_test_report_data(mock_device, 1, 15091)
}
},
{
channel = "matter",
direction = "receive",
message = {
mock_device.id,
clusters.ColorControl.attributes.CurrentY:build_test_report_data(mock_device, 1, 21547)
}
},
}
)

test.register_message_test(
"colorControl capability sent based on CurrentX and CurrentY but not CurrentHue and CurrentSaturation due to ColorMode",
{
{
channel = "matter",
direction = "receive",
message = {
mock_device.id,
clusters.ColorControl.attributes.ColorMode:build_test_report_data(mock_device, 1, 1)
}
},
{
channel = "matter",
direction = "receive",
message = {
mock_device.id,
clusters.ColorControl.attributes.CurrentHue:build_test_report_data(mock_device, 1, 0xFE)
}
},
{
channel = "matter",
direction = "receive",
message = {
mock_device.id,
clusters.ColorControl.attributes.CurrentSaturation:build_test_report_data(mock_device, 1, 0xFE)
}
},
{
channel = "matter",
direction = "receive",
message = {
mock_device.id,
clusters.ColorControl.attributes.CurrentX:build_test_report_data(mock_device, 1, 15091)
}
},
{
channel = "matter",
direction = "receive",
message = {
mock_device.id,
clusters.ColorControl.attributes.CurrentY:build_test_report_data(mock_device, 1, 21547)
}
},
{
channel = "capability",
direction = "send",
message = mock_device:generate_test_message("main", capabilities.colorControl.hue(50))
},
{
channel = "capability",
direction = "send",
message = mock_device:generate_test_message("main", capabilities.colorControl.saturation(72))
},
}
)

test.run_registered_tests()
Loading