Add support for the color mode attribute #1772
Conversation
|
Invitation URL: |
Test Results 64 files 402 suites 0s ⏱️ Results for commit d59def6. ♻️ This comment has been updated with latest results. |
|
Minimum allowed coverage is Generated by 🐒 cobertura-action against d59def6 |
…SmartThingsCommunity/SmartThingsEdgeDrivers into CHAD-13851-support-color-mode-attribute
| 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 |
There was a problem hiding this comment.
This might be more clear just changing this to
| if current_color_mode == 1 or current_color_mode == 2 then | |
| if current_color_mode != 0 then |
There was a problem hiding this comment.
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)
| 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.
Same idea as above:
| if current_color_mode == 0 or current_color_mode == 2 then | |
| if current_color_mode != 1 then |
| 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.
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.
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.
…SmartThingsCommunity/SmartThingsEdgeDrivers into CHAD-13851-support-color-mode-attribute
nickolas-deboom
force-pushed
the
CHAD-13851-support-color-mode-attribute
branch
from
4bffb76 to
a809cd9
Compare
Type of Change
Checklist
Description of Change
CHAD-13851
In certain situations such as after a driver restart, the hue and saturation attributes of the
colorControlcapability may be overwritten by device attributes that are not currently controlling the color of the device. This change utilizes theColorModeattribute to check the current color mode before emitting thecolorControlcapability to prevent this from happening.Summary of Completed Tests
Tested using a Zemismart Smart Bulb. Without these changes, restarting hubcore results in the
CurrentXandCurrentYattributes overriding thecolorControlcapability which results in the ST app displaying a different color than what the device shows. With the changes, the capability is only emitted after theCurrentHueandCurrentSaturationattributes are received, which results in the ST app matching the color of the device.