Handle black color conversion and fix transaction with brightness 0 flickering.

Jojo-1000 · Moritz Wirger
1 parent 74b51fb2
Showing 4 changed files with 25 additions and 4 deletions
src/ColorUnits.cpp
src/StateTransaction.cpp
test/test_ColorUnits.cpp
test/test_StateTransaction.cpp
@@ -107,6 +107,10 @@ XY ColorGamut::corrected(const XY&amp; xy) const
 XYBrightness RGB::toXY() const
 {
+    if (r == 0 && g == 0 && b == 0)
+    {
+        return XYBrightness {XY {0.f, 0.f}, 0.f};
+    }
     const float red = r / 255.f;
     const float green = g / 255.f;
     const float blue = b / 255.f;
@@ -39,8 +39,8 @@ bool StateTransaction::commit(bool trimRequest)
     // Check this before request is trimmed
     if (!request.count("on"))
     {
-        if (!stateJson.value("on", false)
-            && (request.value("bri", 0) != 0 || request.count("effect") || request.count("hue") || request.count("sat")
+        if (!stateJson.value("on", false) && request.value("bri", 254) != 0
+            && (request.count("bri") || request.count("effect") || request.count("hue") || request.count("sat")
                 || request.count("xy") || request.count("ct")))
         {
             // Turn on if it was turned off
@@ -105,6 +105,13 @@ TEST(RGB, toXY)
         EXPECT_FLOAT_EQ(xy.xy.y, 0.32902291f);
         EXPECT_FLOAT_EQ(xy.brightness, 0.99999905f);
     }
+    {
+        const RGB black{ 0,0,0 };
+        XYBrightness xy = black.toXY(gamut::maxGamut);
+        EXPECT_FLOAT_EQ(xy.xy.x, 0.0f);
+        EXPECT_FLOAT_EQ(xy.xy.y, 0.0f);
+        EXPECT_FLOAT_EQ(xy.brightness, 0.0f);
+    }
 }
 TEST(RGB, fromXY)
@@ -154,6 +154,16 @@ TEST(StateTransaction, setBrightness)
         EXPECT_TRUE(StateTransaction(commands, "/path", &state).setBrightness(bri).commit());
         Mock::VerifyAndClearExpectations(handler.get());
     }
+    // Fixed unexpected on/off flickering
+    {
+        const int hue = 32;
+        nlohmann::json state = {{"on", false}, {"bri", 20}};
+        nlohmann::json request = { {"hue", hue}, {"bri", 0} };
+        nlohmann::json response = {{{"success", {{"/path/hue", hue}}}}};
+        EXPECT_CALL(*handler, PUTJson(requestPath, request, getBridgeIp(), getBridgePort())).WillOnce(Return(response));
+        EXPECT_TRUE(StateTransaction(commands, "/path", &state).setBrightness(0).setColorHue(hue).commit());
+        Mock::VerifyAndClearExpectations(handler.get());
+    }
 }
 TEST(StateTransaction, setColorHue)
@@ -303,7 +313,7 @@ TEST(StateTransaction, setColorXY)
                 }},
             {"colormode", "hs"}};
         EXPECT_CALL(*handler, PUTJson(requestPath, request, getBridgeIp(), getBridgePort())).WillOnce(Return(response));
-        EXPECT_TRUE(StateTransaction(commands, "/path", &state).setColor(XY{ x, y }).commit());
+        EXPECT_TRUE(StateTransaction(commands, "/path", &state).setColor(XY {x, y}).commit());
         Mock::VerifyAndClearExpectations(handler.get());
     }
     // No request
@@ -318,7 +328,7 @@ TEST(StateTransaction, setColorXY)
                 }},
             {"colormode", "xy"}};
         EXPECT_CALL(*handler, PUTJson(_, _, getBridgeIp(), getBridgePort())).Times(0);
-        EXPECT_TRUE(StateTransaction(commands, "/path", &state).setColor(XY{ x, y }).commit());
+        EXPECT_TRUE(StateTransaction(commands, "/path", &state).setColor(XY {x, y}).commit());
         Mock::VerifyAndClearExpectations(handler.get());
     }
 }