-
Notifications
You must be signed in to change notification settings - Fork 463
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
base: main
Are you sure you want to change the base?
Changes from 5 commits
07b834d
47487a8
0d43440
431caab
4588815
76a09ff
a809cd9
d59def6
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -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" | ||||||
|
@@ -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 | ||||||
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 | ||||||
|
||||||
|
@@ -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 | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same idea as above:
Suggested change
|
||||||
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 | ||||||
|
@@ -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) | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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? There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 |
||||||
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 | ||||||
|
@@ -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 | ||||||
|
@@ -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, | ||||||
|
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.
This might be more clear just changing this to
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.
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)