Capability Color Control "color" Attribute & Command ambiguous, incorrectly documented, and/or improperly used in DTHs

Devices & Integrations
, , ,
1

Tagging @Jim; but encouraging color light developers to chime in.

Capability “Color Control” is defined in the Developer Documentation: http://docs.smartthings.com/en/latest/capabilities-reference.html#color-control

560×653 25.9 KB

###This definition seems effed-up to me … because:

  1. Attributes hue & saturation and Commands setHue(number) & setSaturation(number) are completely redundant with the “hue:” and “saturation:” keys available to the “color[]” Map.

Furthermore, map keys level and switch are obviously and painfully redundant with the distinct Capabilities (Attributes and Commands) of Switch Level and Switch.

This type of redundancy causes non-trivial risk of inconsistencies in DTH implementation and usage by SmartApps. WTF? :confounded:
.

  1. The “color[]” Map is described as “Color Options”.

Whenever elements of an Attribute are optional, it causes extra work for the DTH developer, since they have to determine what “options” (grrrr!!!) to support and not-support.

Capabilities should not have “optional” claims… otherwise, how the heck is a SmartApp supposed to ensure the Device responds appropriately?

###I’ve looked at several DTHs from the SmartThings Community Public GitHub…

A) LIFX accepts these keys for Command setColor(): hue, saturation and colorTemperature (WTF? That is not one of the “options” for color[] map:

B) The same DTH returns a String (not a Map) for Attribute color:

Actually… Observation (B) seems typical. Looking at the current values of one of my Hue Bulbs, for example, Attribute color is obviously being set to a String, not a Map.

I don’t have have an Osram Gardenspot, but its code also clearly shows it setting the color Attribute to a single hex value, not a Map:

And an example of the consequences of this confusion:

##Arrrgh! :confounded:

@jim, @slagle, @jody.albritton (et al.):

  • Is the Documentation for Capability “Color Control” incorrect?
  • or, @tyler, are the various color bulb DTHs that I’ve browsed non-compliant? If so, then (a) do you have an example of a fully compliant DTH, and (b) is anything being done to bring other published color lights into compliance?

Thanks! :confused:

4 Likes
2

The ZLL-RGBW-Bulb DTH by @workmonk totally ignores the the key “color.hex” to setColor.

I can’t test, but it appears to never set any value (whether it be a Map or a String) for the standard Attribute “color” (unless it is buried in a default parse message, but I don’t think so, since parse() explicitly sendEvent for “hue” and “saturation” Attributes:

This is a 2016 DTH written by a SmartThings employee (@workmonk). If they can’t follow their own specs, how can we?

3

There are two issues here:

  1. Command: Only hue and sat value is being used and the hex value is ignored. This is in line with what ZigBee and ZLL specs. There are various reasons on why hue/sat or x/y values are preferred over hex values. A quick example is the color 0x000000 (black) and those types of color do not map well when being represented by a light bulb.
    Having said that, based on the capability spec, the right thing to do in the DTH would have been to check if hue/sat or x/y values are null and then use hex to hue/sat conversion. That way the SmartApps using hex values for color control would function properly.

2.Parsing: You are right that this DTH does not set the value of the color attribute. In addition it also maps hue values to traditional 360 degree values rather than 0-100. Fix for both is coming either this week or next week.

2 Likes
4

Thanks @workmonk Vinay!

  1. If you have any influence on other published DTH’s that may be using setColor inconsistently, that would be appreciated.

  2. By the quote below, are you confirming that Attribute color is not a Map, but rather, always just the hex string “#rrggbb”? If not, what value are you proposing to set it to?

5

I’ve taken “hex”, “level”, and “switch” out of the supported values in the color_map for color control. These values are not inherently supported by the capability, although depending on your device may be supported in the Device Handler.
So while it is not appropriate for these values to be supported by the capability, Device Handlers are still able to implement the color map however the developer chooses, which is not only hard to document, but gets confusing for community developers.

6

  • You’re just arbitrarily changing the Capability Definition? :confused:

  • Does this change both the Attribute and Command?

  • Nearly every DTH I browsed set the value of color to a string (ie, not to a map, but to “#rrggbb”). There may be “many” SmartApps expecting this string.

  • Most SmartApps I browsed (well… I didn’t look at very many), called `setColor( hex: “#rrggbb”), and nearly every DTH implemented it.

In other words, while your proposed changes to the Capability are an improvement in making it simpler and more consistent, you are changing the “prevailing” de facto implementations / usage… no??

7

No. I’m not changing the capability definition at all. The capability has been implemented this way forever. The dths that allow you to pass a string instead of a map (the value hex: “…” is a map by the way) will continue to work. Capabilities are not interfaces. There is nothing that forces a developer of a device type to implement any part of a capability the way it is defined other than the fact that it is good programming practice. It’s one thing about capabilities that I do not like.

In short, I’ve only updated the documentation to reflect the capability as it is defined. I did not update any code.

8

###Yowza! That’s a huge surprise to me! :open_mouth:

  • The entire “swap & replace” (i.e., swap out any Thing assigned to a SmartApp of one model or manufacturer with another as long as they both implement the same Capability) is a foundational, fundamental underpinning of SmartThings’s magic!

I thought that the “formal review process” of Device Types submitted for publication (@tyler?) would ensure that they are “forced” to implement the Capability as defined!

We’re brothers in arms, bud … brothers in arms. There’s a lot more about Capabilities that I love than hate, but, seriously man, what is keeping SmartThings from making them … rigorous?

I know that the Command setColor uses a map (where “hex:” was defined as one possible, and very commonly support, key). However, I do not recall a single instance where the Attribute color is being set / returned as a map … When set, I always see it returned as a single String with the hex value of RGB color.

So if you’re going to update the docs and change the domain of color; can we try to get it 100% correct? or close?

Isn’t the following definition preferable (and easiest to deprecate the old version to…):

Three Attributes:

  • hue (0-100% map of the 360 degrees color circle)
  • saturation (0-100%)
  • color (RGB in the form of a radix 16 hex string “##rrggbb”)

And three setter Commands:

  • setHue, setSaturation, setColor

Eliminate the “map” to setColor … it’s not necessary.

To reflect the most common current usage and avoid deprecation, I’m fine with:

  • Attribute color = (RGB in the form of a radix 16 hex string “##rrggbb”) – i.e., Hex Triplet.
  • Command setColor( color_map ) where color_map supports keys: ( hue, saturation ) redundantly, and ( color ) defined as per the above Attribute. Any other keys (level, switch, colorTemperature) are out of spec.

?

9

The most concise and unambiguous API would have

Attribute color - String in the format of Hex Triplet
Command setColor(hex: value, ...) - accepting Hex Triplet and any number of optional (device specific) arguments.

Everything else in the API is redundant and confusing. It’s up to DTH to handle the input and output correctly.

1 Like
10

@unixbeast

Do the Hue and Saturation values correspond to HSL or HSV color models?

1 Like
11

Darn good question…

Since RGB does not have this significant ambiguity, it sure seems like RGB would be the preferred method of specifying light color internally, even if an HSV or HSL model is used as the GUI for the color picker.

Another thing that also confuses me is the relationship between Bulb Color (Capability Color) and Bulb Brightness (Capability Switch Level … i.e., dimmer control).

Luminosity (HSL) is affected by Brightness … so you can’t really set an RGB color and expect it to look the same if the Switch Level is changed.

12

And why oh why was the hue used in %? Who else uses hue in %? Use any photo editing app to get your Hue/Sat values and it’ll give you degrees, not percent… I’ve seen some official DTHs that accept % as input for hue, but return it as ° in the hue attribute… not helpful :smiley:

1 Like
13

Because the Slider Control Tile in the SmartThings mobile App was limited to reporting values 0 to 100.

That “0 to 100” limitation seriously messes (messed?) up any attempts to use the slider for thermostat control or many other possible use cases made hopelessly impractical.

Seriously.

I frankly really don’t know if it was ever fixed (the docs say that a range is allowed, but I think it said that for a long time while the limitation still existed in reality) … never saw it fixed in any release notes…

1 Like
14

Well, if it’s meant to do 0…100 then it fails at that too. Have numerous devices that only work 0…99 through the slider, though I can programmatically set them to 100… :smiley:

1 Like
15

Hi guys, I am currently working on OSRAM and Philips Hue to make setColor behave consistently. Philips Hue is already updated.

-Hue and Saturation should be treated as HSV and NOT HSL. As previously pointed out above, HSL requires brightness as well which we do not include in our API.
-Color hex string should always be in the format of “#aabbcc”
-When hue, saturation or hex color is changed. DTH should send events for all three since there is no way to know what smart apps rely on (I haven’t seen any that actually listen to hex yet though). Ideally, events should also come in a predictable order.
-DTH should support both hue/sat and hex input and convert as necessary (for example in the case of zigbee lights that only use hue/sat or xy)
-DTH should support level and switch in setColor as well (there are currently smart apps relying on this) Switch should take precedence over level. Please note though that changing color, level and setting switch to off or level to 0 might produce different results on different hardware due to implementation. E.g. sending update of color and turn off to a hue light will immediately turn it off but not update color.

Above should obviously apply to LIFX as well.

OSRAM hue bug for 0-360 has been fixed and it now expects 0-100 and delivers events in that range. 0-360 would be more universal but changing that now would obviously not be backwards compatible. We could potentially add a new parameter/method but at this stage it would increase confusion.

2 Likes
16

Yeyyy! That was a problem with OSRAMs, the hue was set in % and read in °… Thank you.

I for one would prefer that ST changes hue to °s… Maybe have a vote? I know it’s a big hurdle, but it may fix a lot of future issues - provided @tgauchat’s claim that the slider can only do 0-100 is corrected to support custom ranges - which would help thermostats too…

17

Changing a Capability definition is essentially impossible. SmartThings has no mechanism for putting new Capabilities through a review process let alone revising existing ones.

Reference: About the Capability Type Suggestions category - #7 by tgauchat

Capabilities are supposed to be an abstraction of a certain piece of common “Thing” Attributes and/or Commands so that it be unambiguously used by many SmartApps and many Things.

Each Capability does not need to be comprehensive, but there are qualities of a Capablity that help suggest how big or small (i.e., how granular) they should be. Reference: DRAFT: Guidelines for Capabilities (under construction...)

Now if you actually visited the two posts I reference above, you’ll quickly realize that anything that can simplify (i.e., minimize) a Capability definition is a good thing. What is important is that the Attributes and Commands are strictly specified and DTH’s implement them per that specification.

Obviously, this Topic here shows that “Capability Color Control” does not live up to that expectation.

To improve this Capability, the best thing we could do is reduce the number of redundant options in the documented specification, with everything being “use at your own risk”.

Ergo: Since RGB in the form of a hex triplet (“#rrggbb”) is completely sufficient to accurately describe a light color (in 24-bits), this capability should be reduced to that.

OK: Many SmartApps use hue and saturation instead (but we still don’t know if it is HSV or HSL). These can be converted in SmartApps to RGB. But because they already exist in the Capability, I’m sure they will continue to exist, with the existing ranges (0 to 100, 0 to 100).

What’s a good way to allow a Capability to support this type of expansion (i.e., taking degrees instead of percent for hue)? All Commands and Attributes should have a “units” portion … i.e., all Commands and Attributes should use Maps: [ value, units ], so that if a new units is introduced, the SmartApps and DTH’s can slowly adopt the new units. It is up to each SmartApp to check if the Device Instance supports the desired units before using those units.

1 Like
18

Please, please, please! Can we just close this can of worms? I don’t want to see what’s inside!!!
:scream: :head_bandage: :speak_no_evil:

1 Like
19

3 Likes
20

Can of worms aside, If values are HSL 0-100, how is the W diode in RGBW getting assigned? It seems to me that we should be able to control RGBW across all four values 0-100 independently… Hue as a color value designator is a poor means of achieving a value that is replicate-able across various brands but I do see how it works well for crude color wheel use for the consumer. Difference between composite and component color values. Component is many times more accurate. Plus, why are they/we working in 100th value increments? shouldnt it be at least 0-255 across each color diode independently? (I dont mean pixel addressable, but more increments per channel.) Working with LED color assignments should be addressed in much more accurate terms than what is currently happening in the SmartThings world.

1 Like