[Accepted with Revisions] SDL 0227 - Add Supported RGB Colors to Light Capabilities

[theresalech]

Hello SDL community,

The review of "SDL 0227 - Add Supported RGB Colors to Light Capabilities" begins now and runs through May 14, 2019. The proposal is available here:

https://github.com/smartdevicelink/sdl_evolution/blob/master/proposals/0227-add-supported-rgb-colors.md

Reviews are an important part of the SDL evolution process. All reviews should be sent to the associated Github issue at:

#720

What goes into a review?

The goal of the review process is to improve the proposal under review through constructive criticism and, eventually, determine the direction of SDL. When writing your review, here are some questions you might want to answer in your review:

• Is the problem being addressed significant enough to warrant a change to SDL?
• Does this proposal fit well with the feel and direction of SDL?
• If you have used competitors with a similar feature, how do you feel that this proposal compares to those?
• How much effort did you put into your review? A glance, a quick reading, or an in-depth study?
  Please state explicitly whether you believe that the proposal should be accepted into SDL.

More information about the SDL evolution process is available at

https://github.com/smartdevicelink/sdl_evolution/blob/master/process.md

Thank you,
Theresa Lech

Program Manager - Livio
[email protected]

[joeljfischer]

I think there's a very obvious problem with this proposal: to declare that you support all but one color, you would have to add over 16 million entries to the array. Because of that simple fact, this proposal is untenable and should be revised or rejected.

I would suggest the following:

<struct name="LightCapabilities" since="5.0">
    <param name="name" type="LightName" mandatory="true" />
    <param name="statusAvailable" type="Boolean" mandatory="false">
      <description>
        Indicates if the status (ON/OFF) can be set remotely. App shall not use read-only values (RAMP_UP/RAMP_DOWN/UNKNOWN/INVALID) in a setInteriorVehicleData request.
      </description>
    </param>
    <param name="densityAvailable" type="Boolean" mandatory="false">
        <description>
            Indicates if the light's density can be set remotely (similar to a dimmer).
        </description>
    </param>
    <param name="rgbColorSpaceAvailable" type="Boolean" mandatory="false">
        <description>
            Indicates if the light's color can be set remotely by using the RGB color space.
        </description>
    </param>
+    <param name="supportedRgbColors" type="RGBColorsSupported" mandatory="false" since="x.x">
+        <description>
+            Whether red, green, blue, and / or white colors are supported in the vehicle.
+        </description>
+    </param>
</struct>

<struct name="RGBColorsSupported" since="5.0">
    <param name="red" type="Boolean" mandatory="true" />
    <param name="green" type="Boolean" mandatory="true" />
    <param name="blue" type="Boolean" mandatory="true" />
    <param name="white" type="Boolean" mandatory="true" />
</struct>

This change allows us to declare if red / green / blue / and or white are supported. If, for example, red and blue are supported, then it can be assumed that purple is supported, and any combination 0-255 of that RGB color is supported. White was added because non-colored lights should also be supported, and the only way to do that without declaring a separate white parameter would be to declare the red, green, and blue are supported, but the constituent colors may not be.

[Jack-Byrne]

Regarding Joels suggestion, I would like to suggest bit depth as a parameter to be added to RGBColorsSupported.

Using available color channels and the bit depth should be enough to explain the full spectrum of colors that are available to be displayed.

<struct name="RGBColorsSupported" since="5.0">
    <param name="red" type="Boolean" mandatory="true" />
    <param name="green" type="Boolean" mandatory="true" />
    <param name="blue" type="Boolean" mandatory="true" />
    <param name="white" type="Boolean" mandatory="true" />
+  <param name="bitDepth" type="Integer" mandatory="true" />
</struct>

Note: bitDepth: 8 would mean available color channels can each represent a decimal value from 0-255

[joeygrover]

What about combining the two for a more granular control?

<struct name="RGBColorsSupported" since="5.x">
    <param name="redMin" type="Integer" mandatory="true" defvalue="0"/>
    <param name="redMax" type="Integer" mandatory="true" defvalue="255"/>
    <param name="greenMin" type="Integer" mandatory="true" defvalue="0"/>
    <param name="greenMax" type="Integer" mandatory="true" defvalue="255"/>
    <param name="blueMin" type="Integer" mandatory="true" defvalue="0"/>
    <param name="blueMax" type="Integer" mandatory="true" defvalue="255"/>
</struct>

This would remove the white RGB support as it could be implied through the min values supported from each color.

[joeljfischer]

The only concern I would have is that often LEDs like this don't simply combine red, green, and blue LEDs to create white, they have a separate white channel. Also, I would be concerned about if an OEM has lights to can do red, or blue, or green, but can't mix them.

[yang1070]

@joeljfischer @JackLivio
Thank you very much for the good suggestions about RGBColorsSupported. I like the min-max range version. However, it still does not solve our problem.
The situation here is an OEM may only support a very few number of colors (<20) , and the majority of r,g,b, combinations are not supported. This also apply to an OEM that can do white, red, blue or green but cannot mix them.

if an OEM supports the majority of r,g,b colors but a few not supported, it can just not include the parameter as supportedRgbColors is optional.

[joeljfischer]

I don't think it's a good option to say that if an OEM supports the majority of colors they can just not include the parameter. The number of values that need to be added to say that an OEM supports many but not all colors quickly reaches the thousands and millions. Another solution must be reached.

[joeljfischer]

<struct name="RGBColorsSupported" since="5.x">
    <param name="redMin" type="Integer" mandatory="true" defvalue="0"/>
    <param name="redMax" type="Integer" mandatory="true" defvalue="255"/>
    <param name="greenMin" type="Integer" mandatory="true" defvalue="0"/>
    <param name="greenMax" type="Integer" mandatory="true" defvalue="255"/>
    <param name="blueMin" type="Integer" mandatory="true" defvalue="0"/>
    <param name="blueMax" type="Integer" mandatory="true" defvalue="255"/>
    <param name="whiteMin" type="Integer" mandatory="true" defvalue="0"/>
    <param name="whiteMax" type="Integer" mandatory="true" defvalue="255"/>
    <param name="supportsMixing" type="Boolean" mandatory="true" />
</struct>

What about something like this?

[yang1070]

The min-max range still cannot solve the problem that an OEM only supports, for example 10 colors (r1,g1,b1), (r2,g2,b2) .... (r10,g10,b10). if red_min<= r_i i=1,2,..10 <= red_max), additional values within the range for red and additional combinations with green/blue/white is included, but that is actually not supported.

[joeygrover]

I don't really like this, but what about something like this that can contain ranges and discrete colors?

<struct name="LightCapabilities" since="5.0">
: 
: 
   <param name="supportedRgbColors" type="RGBColorsSupported" array="true" minsize="1" maxSize="999999" mandatory="false" since="x.x"/>

</struct>


<struct name="RGBColorsSupported" since="5.x">
    <param name="redMin" type="Integer" mandatory="true" defvalue="0"/>
    <param name="redMax" type="Integer" mandatory="true" defvalue="255"/>
    <param name="greenMin" type="Integer" mandatory="true" defvalue="0"/>
    <param name="greenMax" type="Integer" mandatory="true" defvalue="255"/>
    <param name="blueMin" type="Integer" mandatory="true" defvalue="0"/>
    <param name="blueMax" type="Integer" mandatory="true" defvalue="255"/>
    <param name="supportsMixing" type="Boolean" mandatory="true" />
</struct>

[yang1070]

I'm not quite understand the need for parameter supportsMixing. If a color has r,g,b three elements, it is already mixed. For the LED with white color case, we can directly say WHITE (255,255,255) is supported?

We have a use case to support a list of discrete colors. To support it using above methold, for one color we need to have an item in the array with redMin=redMax=red_value1, greenMin=greenMax=green_value1, blueMin=blueMax=blue_value1 for all colors.

For non-discrete colors, I can imagine that if a light can mix r,g,b, and each r/g/b has its own range, it is hard to think the range is not a full range 0-255 and it can have multiple ranges that are not adjacent.

[joeygrover]

How about making each color min/max non-mandatory? Single color capabilities would only include the color ranges/points for that specific color. If multiple colors are included in a single LightCapabilities it could be assumed that each color's ranges could be mixed together.

<struct name="LightCapabilities" since="5.0">
: 
: 
   <param name="supportedRgbColors" type="RGBColorsSupported" array="true" minsize="1" maxSize="999999" mandatory="false" since="x.x"/>

</struct>

<struct name="RGBColorsSupported" since="5.x">
    <param name="redMin" type="Integer" mandatory="false" defvalue="0"/>
    <param name="redMax" type="Integer" mandatory="false" defvalue="255"/>
    <param name="greenMin" type="Integer" mandatory="false" defvalue="0"/>
    <param name="greenMax" type="Integer" mandatory="false" defvalue="255"/>
    <param name="blueMin" type="Integer" mandatory="false" defvalue="0"/>
    <param name="blueMax" type="Integer" mandatory="false" defvalue="255"/>
</struct>

*Edit: The defvalue tag will be removed as it is non-mandatory and be moved to the description tags.

[yang1070]

Yes, can we have something like

<struct name="LightCapabilities" since="5.0">
: 
: 
   <param name="supportedRgbColors" type="RGBColorsSupported" array="true" minsize="1" maxSize="999999" mandatory="false" since="x.x"/>

</struct>

<struct name="RGBColorsSupported" since="5.x">
    <param name="redMin" type="Integer" mandatory="true" />
    <param name="redMax" type="Integer" mandatory="false" />
    <param name="greenMin" type="Integer" mandatory="true" />
    <param name="greenMax" type="Integer" mandatory="false" />
    <param name="blueMin" type="Integer" mandatory="true" />
    <param name="blueMax" type="Integer" mandatory="false" />
</struct>

min value is mandatory,
max value is not, without default, if missing it is the same as min value.

[yang1070]

<struct name="LightCapabilities" since="5.0">
: 
: 
   <param name="supportedRgbColorRanges" type="RGBColorRanges" array="true" minsize="1" maxSize="999999" mandatory="false" since="x.x"/>

</struct>


<struct name="RGBColorRanges" since="5.x">
	<description>
	Indicate a color or a range of colors
	case1: to support any combination of r,g,b color, use a full range redMin=0,redMax=255,greenMin=0,greenMax=255,blueMin=0,blueMax=255
	case2: to support a single discrete r,g,b color, use redMin=r,geenMin=g,blueMin=b without any max values or set optional max=min
	case3: to support a range of r,g,b color elements, use both min and max with proper values for r,g,b as needed
	</description>
    <param name="redMin" type="Integer" minvalue="0" maxvalue="255" mandatory="true" />
    <param name="redMax" type="Integer" minvalue="0" maxvalue="255" mandatory="false" />
    <param name="greenMin" type="Integer" minvalue="0" maxvalue="255" mandatory="true" />
    <param name="greenMax" type="Integer" minvalue="0" maxvalue="255" mandatory="false" />
    <param name="blueMin" type="Integer" minvalue="0" maxvalue="255" mandatory="true" />
    <param name="blueMax" type="Integer" minvalue="0" maxvalue="255" mandatory="false" />
</struct>

[theresalech]

The Steering Committee voted to accept this proposal with the following revisions:

• Change RGBColorsSupported to RGBColorRange
• Change supportedRGBColors to supportedRGBColorRanges
• Update RGBColorRanges struct description to include the following cases:
  • If only minimums are included for R, G, and B, it is a single color
  • If a max is included on any R, G, B color, it is a range of color
  • If two or more colors contain both min and max, those two colors can be shaded together within the ranges supplied

[theresalech]

@yang1070 please advise when a new PR has been entered to update the proposal to reflect the agreed upon revisions. I'll then merge the PR so the proposal is up to date, and update issues in the respective repositories for implementation. Thanks!

[theresalech]

Proposal has been updated to reflect agreed upon revisions, and implementation issues have been entered:

• Core
• RPC Spec
• iOS
• Java Suite
• SDL HMI
• JavaScript Suite