There doesn't seem to be a good way to assert using `RGBColor` that a `LightCommand` colour has been set correctly.

[ghuntley]

Background

I'm using your [amazing] library to build out my home office automation as part of the http://ghuntley.com/live series. I'm in the midst of writing unit tests to Assert that my business logic sends the correct RGB colour for the particular state where OnAir = red and OffAir = green.

Here's my business logic:

private async Task ActivateOfficeDoorLight(bool isOnAir, IEnumerable<Light> lights)
{
    var command = new LightCommand();
    if (isOnAir)
    {
        command.TurnOn().SetColor(new RGBColor("#FF0000")); // red
    }
    else
    {
        command.TurnOn().SetColor(new RGBColor("#008000")); // green 
    }

    var officeDoorLight = lights.Single(x => x.Name == "Office Door");

    await _phillipsHueService.SendCommand(command, new List<string> { officeDoorLight.Id }).ConfigureAwait(false);
}

Heres my tests:

[Theory]
// processes
[InlineData("vMix64", "")]
// window titles
[InlineData("", "Zoom Participant")]
public void TheLightIsChangedWhenThereIsAMatch(string processName, string windowName)
{
    var activeProcessesService = Substitute.For<IActiveProcessesService>();
    activeProcessesService.ProcessNames.Returns(new ReactiveList<string>() { processName });
    activeProcessesService.WindowTitles.Returns(new ReactiveList<string>() { windowName });

    var phillipsHueService = Substitute.For<IPhillipsHueService>();

    var light = new Light()
    {
        Id = "5",
        Name = "Office Door",
    };
    phillipsHueService.Lights.Returns(new ReactiveList<Light>() { light });

    var sut = new OnAirLightViewModelBuilder()
        .WithActiveProcessesService(activeProcessesService)
        .WithPhillipsHueService(phillipsHueService)
        .Build();

    // user interface is updated
    sut.IsOnAir.ShouldBeTrue();

    // the correct light is turned on
    phillipsHueService.Received().SendCommand(
        Arg.Is<LightCommand>(x => x.On.Value == true),
        Arg.Is<IEnumerable<string>>(x => x.Contains(light.Id)));
}

I think I've found a limitation with the current API surface - maybe I've missed something but there doesn't seem to be a good way to assert using RGBColor that a LightCommand colour has been set correctly.

One must manually break it down into HSB or RgbToXY which is harder than it should be because https://github.com/Q42/Q42.HueApi/blob/master/src/Q42.HueApi.ColorConverters/HSB/RGBExtensions.cs#L12 is all internal.

Thoughts

What about exposing an RGBColor field on LightCommand or making RGBExtensions public? I've got no strong feelings as to which way would best. If you have better ideas (tm) or I've missed something lemme know. Thanks ;-)

Goals

To be able to assert this:

RGBColor expected = new RGBColor("#008000");
phillipsHueService.Received().SendCommand(
    Arg.Is<LightCommand>(x => x.RGBColor == expected),
    Arg.Is<IEnumerable<string>>(x => x.Contains(light.Id)));

or

RGBColor color = new RGBColor("#008000");
HSB expected = color.ToHSB();
phillipsHueService.Received().SendCommand(
    Arg.Is<LightCommand>(x => x.[Hue|Sat|Etc] == expected),
    Arg.Is<IEnumerable<string>>(x => x.Contains(light.Id)));

Finally

This is all going to be open-source over at https://github.com/ghuntley/live ;-)

[michielpost]

Great to hear you're using the library.

Philips Hue lights and RGB Colors is not really a match. For us, working with RGB is easy, but the Philips lights don't really know about RGB, they work with the HSB or XY values. Converting from RGB to HSB/XY is also not straightforward, there is no one on one mapping, that's why the ColorConverters package contains 3 different implementations.

So RGB value X may convert to HSB value Y But HSB value Y may not convert to RGB value X

That's why I think asserting on the HSB value is better.

There is currently a way to convert HSB back to RGB by creating your own State object: https://github.com/Q42/Q42.HueApi/blob/master/src/Q42.HueApi.ColorConverters.Tests/HSB/StateExtensionsTests.cs#L40-L47

Let me know if this changes anything. If you still would like to have the internal extensions public, I can do that, no problem. Or send a PR.

[ghuntley]

Howdy,

Ah thanks for sharing those insights that the lights internally use HSB/XY values. I could move to asserting those values. If you think it's worth adding the conversion extension to the public API surface then for my specific use case changing

internal static HSB GetHSB(this RGBColor rgb)

to

public static HSB GetHSB(this RGBColor rgb)

would suffice. I don't have a need for these methods to be publicly exposed

internal static double GetHue(this RGBColor rgb) internal static double GetSaturation(this RGBColor rgb) internal static double GetBrightness(this RGBColor rgb)

At this stage thinking they could be a separate PR when the usecase arises. Happy to send in a PR, if you want to also update the accessibility of GetHue, GetSaturation, GetBrightness whilst I'm in there then I can do that. If you think adding RGB conversion to the public API surface is a bad idea - let's not do it.

[michielpost]

I made the extensions public, available in NuGet package Q42.HueApi.ColorConverters version 3.6.2

[ghuntley]

Thank-you! <3