From 95bcb8a3cdba7ce640df8c3c72925565b9b4647c Mon Sep 17 00:00:00 2001
From: rschoene <rene.schoene@tu-dresden.de>
Date: Wed, 30 Oct 2019 14:38:44 +0100
Subject: [PATCH] TupleHSB actually has different value ranges.

- Hue goes from 0..360 (and wraps)
- Saturation and brightness go from 0..100
- Ensure those bounds during creation
- Added test for TupleHSB
---
 .../st/eraser/commons/color/ColorUtils.java   |  67 +++++------
 .../inf/st/eraser/jastadd/model/TupleHSB.java |  10 +-
 .../de/tudresden/inf/st/eraser/ItemTests.java |  15 ++-
 .../jastadd_test/core/TupleHSBTest.java       | 109 ++++++++++++++++++
 .../st/eraser/feedbackloop/api/Execute.java   |   4 +-
 .../feedbackloop/execute/ExecuteImpl.java     |  14 +--
 .../skywriter_hue_integration/Main.java       |   8 +-
 7 files changed, 177 insertions(+), 50 deletions(-)
 create mode 100644 eraser-base/src/test/java/de/tudresden/inf/st/eraser/jastadd_test/core/TupleHSBTest.java

diff --git a/commons.color/src/main/java/de/tudresden/inf/st/eraser/commons/color/ColorUtils.java b/commons.color/src/main/java/de/tudresden/inf/st/eraser/commons/color/ColorUtils.java
index 236f5d38..21d53dab 100644
--- a/commons.color/src/main/java/de/tudresden/inf/st/eraser/commons/color/ColorUtils.java
+++ b/commons.color/src/main/java/de/tudresden/inf/st/eraser/commons/color/ColorUtils.java
@@ -11,16 +11,17 @@ import java.util.function.Consumer;
  *
  * @author rschoene - Initial contribution
  */
+@SuppressWarnings("WeakerAccess")
 public class ColorUtils {
 
   /**
    * Value class comprising three double values X, Y, Z ranging from 0 to 1.
    * The values represent coordinates.
    */
-  public static class XYZvalues {
+  public static class ValuesXYZ {
     double x, y, z;
-    public static XYZvalues of(double x, double y, double z) {
-      XYZvalues result = new XYZvalues();
+    public static ValuesXYZ of(double x, double y, double z) {
+      ValuesXYZ result = new ValuesXYZ();
       result.x = x;
       result.y = y;
       result.z = z;
@@ -32,22 +33,22 @@ public class ColorUtils {
    * Value class comprising three integral values R, G, B ranging from 0 to 255.
    * The values represent red, green and blue.
    */
-  public static class RGBvalues {
+  public static class ValuesRGB {
     public int red;
     public int green;
     public int blue;
-    public static RGBvalues of(int red, int green, int blue) {
-      RGBvalues result = new RGBvalues();
+    public static ValuesRGB of(int red, int green, int blue) {
+      ValuesRGB result = new ValuesRGB();
       result.red = red;
       result.green = green;
       result.blue = blue;
       return result;
     }
-    public static RGBvalues of(int[] rgb) {
+    public static ValuesRGB of(int[] rgb) {
       return of(rgb[0], rgb[1], rgb[2]);
     }
     /** Set all values to a minimum of zero. */
-    public RGBvalues minZero() {
+    public ValuesRGB minZero() {
       red = Math.max(0, red);
       green = Math.max(0, green);
       blue = Math.max(0, blue);
@@ -59,52 +60,52 @@ public class ColorUtils {
    * Value class comprising three double values H, S, B ranging from 0 to 1.
    * The values represent hue, saturation and brightness.
    */
-  public static class HSBvalues {
+  public static class ValuesHSB {
     public double hue;
     public double saturation;
     public double brightness;
-    public static HSBvalues of(double hue, double saturation, double brightness) {
-      HSBvalues result = new HSBvalues();
+    public static ValuesHSB of(double hue, double saturation, double brightness) {
+      ValuesHSB result = new ValuesHSB();
       result.hue = hue;
       result.saturation = saturation;
       result.brightness = brightness;
       return result;
     }
     /** Set all values to a minimum of zero. */
-    public HSBvalues minZero() {
+    public ValuesHSB minZero() {
       hue = Math.max(0, hue);
       saturation = Math.max(0, saturation);
       brightness = Math.max(0, brightness);
       return this;
     }
-    /** Convert to HSB with value range of 0..255 */
-    public HSBvalues255 toIntegral() {
-      return HSBvalues255.of(
-          (int) Math.round(255 * hue),
-          (int) Math.round(255 * saturation),
-          (int) Math.round(255 * brightness)
+    /** Convert to the same HSB with integral values */
+    public ValuesIntegralHSB toIntegral() {
+      return ValuesIntegralHSB.of(
+          (int) Math.round(360 * hue),
+          (int) Math.round(100 * saturation),
+          (int) Math.round(100 * brightness)
       );
     }
   }
 
   /**
-   * Value class comprising three integral values H, S, B ranging from 0 to 255.
+   * Value class comprising three integral values H, S, B ranging from 0 either to 360 (H) or 100 (S and B).
    * The values represent hue, saturation and brightness.
    */
-  public static class HSBvalues255 {
+  public static class ValuesIntegralHSB {
     public int hue;
     public int saturation;
     public int brightness;
-    public static HSBvalues255 of(int hue, int saturation, int brightness) {
-      HSBvalues255 result = new HSBvalues255();
+    public static ValuesIntegralHSB of(int hue, int saturation, int brightness) {
+      ValuesIntegralHSB result = new ValuesIntegralHSB();
       result.hue = hue;
       result.saturation = saturation;
       result.brightness = brightness;
       return result;
     }
-    /** Set all values to a minimum of zero, and a maximum of 255. */
-    public HSBvalues255 ensureBounds() {
-      hue = Math.min(Math.max(0, hue), 255);
+    /** Set all values to a minimum of zero, and a maximum of either 360 (for H) or 100 (for S and B). */
+    public ValuesIntegralHSB ensureBounds() {
+      hue = Math.min(Math.max(0, hue), 360);
       saturation = Math.min(Math.max(0, saturation), 100);
       brightness = Math.min(Math.max(0, brightness), 100);
       return this;
@@ -122,15 +123,15 @@ public class ColorUtils {
       { 0.0134474, -0.1183897,  1.0154096}};
   private static RealMatrix mInverted = MatrixUtils.createRealMatrix(matrixData);
 
-  public static RGBvalues convertXYtoRGB(XYZvalues values) {
+  public static ValuesRGB convertXYtoRGB(ValuesXYZ values) {
     return convertXYtoRGB(values.x, values.y, values.z);
   }
 
-  public static RGBvalues convertXYtoRGB(double x, double y, double z) {
+  public static ValuesRGB convertXYtoRGB(double x, double y, double z) {
     RealMatrix xyz = MatrixUtils.createColumnRealMatrix(new double[] {x, y, z});
     RealMatrix result = mInverted.multiply(xyz);
     double[][] rgb = result.getData();
-    return RGBvalues.of(
+    return ValuesRGB.of(
         (int) Math.round(255 * rgb[0][0]),  // red
         (int) Math.round(255 * rgb[1][0]),  // green
         (int) Math.round(255 * rgb[2][0])); // blue
@@ -138,24 +139,24 @@ public class ColorUtils {
 
   public static void convertXYtoRGB(double x, double y, double z, Consumer<Integer> setRed,
                                     Consumer<Integer> setGreen, Consumer<Integer> setBlue) {
-    RGBvalues result = convertXYtoRGB(x, y, z);
+    ValuesRGB result = convertXYtoRGB(x, y, z);
     setRed.accept(result.red);
     setGreen.accept(result.green);
     setBlue.accept(result.blue);
   }
 
-  public static HSBvalues convertRGBtoHSB(RGBvalues values) {
+  public static ValuesHSB convertRGBtoHSB(ValuesRGB values) {
     return convertRGBtoHSB(values.red, values.green, values.blue);
   }
 
-  public static HSBvalues convertRGBtoHSB(int red, int green, int blue) {
+  public static ValuesHSB convertRGBtoHSB(int red, int green, int blue) {
     float[] hsb = Color.RGBtoHSB(red, green, blue, null);
-    return HSBvalues.of(hsb[0], hsb[1], hsb[2]);
+    return ValuesHSB.of(hsb[0], hsb[1], hsb[2]);
   }
 
   public static void convertRGBtoHSB(int red, int green, int blue, Consumer<Double> setHue,
                                      Consumer<Double> setSaturation, Consumer<Double> setBrightness) {
-    HSBvalues values = convertRGBtoHSB(red, green, blue);
+    ValuesHSB values = convertRGBtoHSB(red, green, blue);
     setHue.accept(values.hue);
     setSaturation.accept(values.saturation);
     setBrightness.accept(values.brightness);
diff --git a/eraser-base/src/main/java/de/tudresden/inf/st/eraser/jastadd/model/TupleHSB.java b/eraser-base/src/main/java/de/tudresden/inf/st/eraser/jastadd/model/TupleHSB.java
index 8e9ee2ad..3caddf85 100644
--- a/eraser-base/src/main/java/de/tudresden/inf/st/eraser/jastadd/model/TupleHSB.java
+++ b/eraser-base/src/main/java/de/tudresden/inf/st/eraser/jastadd/model/TupleHSB.java
@@ -13,12 +13,16 @@ public class TupleHSB implements Cloneable {
   private int brightness;
   public static TupleHSB of(int hue, int saturation, int brightness) {
     TupleHSB result = new TupleHSB();
-    result.hue = hue;
-    result.saturation = saturation;
-    result.brightness = brightness;
+    result.hue = hue % 360;
+    result.saturation = ensureBetweenZeroAndHundred(saturation);
+    result.brightness = ensureBetweenZeroAndHundred(brightness);
     return result;
   }
 
+  private static int ensureBetweenZeroAndHundred(int value) {
+    return Math.max(0, Math.min(value, 100));
+  }
+
   public int getHue() {
     return hue;
   }
diff --git a/eraser-base/src/test/java/de/tudresden/inf/st/eraser/ItemTests.java b/eraser-base/src/test/java/de/tudresden/inf/st/eraser/ItemTests.java
index ffda213b..2da798c2 100644
--- a/eraser-base/src/test/java/de/tudresden/inf/st/eraser/ItemTests.java
+++ b/eraser-base/src/test/java/de/tudresden/inf/st/eraser/ItemTests.java
@@ -7,7 +7,6 @@ import org.junit.Assert;
 import org.junit.Test;
 
 import java.time.Instant;
-import java.util.Date;
 
 /**
  * Testing wrapper methods of {@link Item}.
@@ -70,6 +69,20 @@ public class ItemTests {
         sut.stateEquals(TupleHSB.of(99, 99, 3)));
     Assert.assertFalse("State 'TupleHSB(1,2,3)' should not match 'TupleHSB(5,5,5)'",
         sut.stateEquals(TupleHSB.of(5, 5, 5)));
+
+    sut.setState(TupleHSB.of(347, 80, 95));
+    Assert.assertTrue("State 'TupleHSB(357,80,95)' should match 'TupleHSB(357,80,95)'",
+        sut.stateEquals(TupleHSB.of(1, 2, 3)));
+    Assert.assertFalse("State 'TupleHSB(357,80,95)' should not match 'TupleHSB(1,2,4)'",
+        sut.stateEquals(TupleHSB.of(1, 2, 4)));
+    Assert.assertFalse("State 'TupleHSB(357,80,95)' should not match 'TupleHSB(9,2,3)'",
+        sut.stateEquals(TupleHSB.of(9, 2, 3)));
+    Assert.assertFalse("State 'TupleHSB(357,80,95)' should not match 'TupleHSB(1,17,3)'",
+        sut.stateEquals(TupleHSB.of(1, 17, 3)));
+    Assert.assertFalse("State 'TupleHSB(357,80,95)' should not match 'TupleHSB(99,99,3)'",
+        sut.stateEquals(TupleHSB.of(99, 99, 3)));
+    Assert.assertFalse("State 'TupleHSB(357,80,95)' should not match 'TupleHSB(5,5,5)'",
+        sut.stateEquals(TupleHSB.of(5, 5, 5)));
   }
 
   @Test
diff --git a/eraser-base/src/test/java/de/tudresden/inf/st/eraser/jastadd_test/core/TupleHSBTest.java b/eraser-base/src/test/java/de/tudresden/inf/st/eraser/jastadd_test/core/TupleHSBTest.java
new file mode 100644
index 00000000..b5be25e5
--- /dev/null
+++ b/eraser-base/src/test/java/de/tudresden/inf/st/eraser/jastadd_test/core/TupleHSBTest.java
@@ -0,0 +1,109 @@
+package de.tudresden.inf.st.eraser.jastadd_test.core;
+
+import de.tudresden.inf.st.eraser.jastadd.model.TupleHSB;
+import org.junit.Assert;
+import org.junit.Test;
+
+import static org.hamcrest.Matchers.greaterThanOrEqualTo;
+import static org.hamcrest.Matchers.lessThanOrEqualTo;
+
+/**
+ * Testing the class {@link de.tudresden.inf.st.eraser.jastadd.model.TupleHSB}.
+ *
+ * @author rschoene - Initial contribution
+ */
+public class TupleHSBTest {
+
+  @Test
+  public void testBounds() {
+    checkWithinBounds(TupleHSB.of(1,2,3));
+    checkWithinBounds(TupleHSB.of(340,2,3));
+    checkWithinBounds(TupleHSB.of(999,2,3));
+    checkWithinBounds(TupleHSB.of(11,200,3));
+    checkWithinBounds(TupleHSB.of(240,200,3));
+    checkWithinBounds(TupleHSB.of(899,200,3));
+    checkWithinBounds(TupleHSB.of(21,2,300));
+    checkWithinBounds(TupleHSB.of(140,2,300));
+    checkWithinBounds(TupleHSB.of(799,2,300));
+  }
+
+  @Test
+  public void testEquals() {
+    TupleHSB one = TupleHSB.of(1,2,3);
+    TupleHSB two = TupleHSB.of(1,2,3);
+    TupleHSB three = TupleHSB.of(99,99,99);
+    Assert.assertEquals(one, two);
+    Assert.assertNotEquals(one, three);
+    Assert.assertNotEquals(two, three);
+
+    TupleHSB oneEqualModuloHue = TupleHSB.of(361,2,3);
+    Assert.assertEquals(one, oneEqualModuloHue);
+
+    TupleHSB bigSB = TupleHSB.of(50,100,100);
+    TupleHSB biggerS = TupleHSB.of(50,123,100);
+    TupleHSB biggerB = TupleHSB.of(50,123,6484);
+    Assert.assertEquals(bigSB, biggerS);
+    Assert.assertEquals(bigSB, biggerB);
+  }
+
+  @Test
+  public void testWithDifferent() {
+    TupleHSB one = TupleHSB.of(1,2,3);
+    TupleHSB oneDifferentHue = one.withDifferentHue(43);
+    TupleHSB oneDifferentSaturation = one.withDifferentSaturation(43);
+    TupleHSB oneDifferentBrightness = one.withDifferentBrightness(43);
+    TupleHSB oneEqualModuloHue = one.withDifferentHue(721);
+
+    Assert.assertNotEquals(one, oneDifferentHue);
+    Assert.assertNotEquals(one, oneDifferentSaturation);
+    Assert.assertNotEquals(one, oneDifferentBrightness);
+    Assert.assertNotEquals(oneDifferentHue, oneDifferentSaturation);
+    Assert.assertNotEquals(oneDifferentHue, oneDifferentBrightness);
+    Assert.assertNotEquals(oneDifferentSaturation, oneDifferentBrightness);
+
+    Assert.assertEquals(one, oneEqualModuloHue);
+  }
+
+  @Test
+  public void testPrint() {
+    TupleHSB one = TupleHSB.of(1,2,3);
+    String expectedForOne = "1,2,3";
+    Assert.assertEquals(expectedForOne, one.toString());
+
+    TupleHSB two = TupleHSB.of(341,92,555);
+    String expectedForTwo = "341,92,100";
+    Assert.assertEquals(expectedForTwo, two.toString());
+  }
+
+  @Test
+  public void testParse() {
+    String one = "3,2,1";
+    TupleHSB expectedForOne = TupleHSB.of(3, 2, 1);
+    Assert.assertEquals(expectedForOne, TupleHSB.parse(one));
+
+    String two = "399,201,17";
+    TupleHSB expectedForTwo = TupleHSB.of(39, 100, 17);
+    Assert.assertEquals(expectedForTwo, TupleHSB.parse(two));
+  }
+
+  @Test
+  public void testClone() {
+    TupleHSB one = TupleHSB.of(361,2,3);
+    TupleHSB two = TupleHSB.of(50,123,100);
+    TupleHSB clone = one.clone();
+
+    Assert.assertEquals(one, clone);
+    Assert.assertNotEquals(one, two);
+    Assert.assertNotEquals(clone, two);
+  }
+
+  private void checkWithinBounds(TupleHSB hsb) {
+    Assert.assertThat(hsb.getHue(), greaterThanOrEqualTo(0));
+    Assert.assertThat(hsb.getHue(), lessThanOrEqualTo(360));
+    Assert.assertThat(hsb.getSaturation(), greaterThanOrEqualTo(0));
+    Assert.assertThat(hsb.getSaturation(), lessThanOrEqualTo(100));
+    Assert.assertThat(hsb.getBrightness(), greaterThanOrEqualTo(0));
+    Assert.assertThat(hsb.getBrightness(), lessThanOrEqualTo(100));
+  }
+
+}
diff --git a/feedbackloop.api/src/main/java/de/tudresden/inf/st/eraser/feedbackloop/api/Execute.java b/feedbackloop.api/src/main/java/de/tudresden/inf/st/eraser/feedbackloop/api/Execute.java
index 4809ab73..3b3da24d 100644
--- a/feedbackloop.api/src/main/java/de/tudresden/inf/st/eraser/feedbackloop/api/Execute.java
+++ b/feedbackloop.api/src/main/java/de/tudresden/inf/st/eraser/feedbackloop/api/Execute.java
@@ -1,6 +1,6 @@
 package de.tudresden.inf.st.eraser.feedbackloop.api;
 
-import de.tudresden.inf.st.eraser.commons.color.ColorUtils.RGBvalues;
+import de.tudresden.inf.st.eraser.commons.color.ColorUtils.ValuesRGB;
 import de.tudresden.inf.st.eraser.jastadd.model.ItemUpdate;
 import de.tudresden.inf.st.eraser.jastadd.model.Root;
 import de.tudresden.inf.st.eraser.util.Tuple;
@@ -25,7 +25,7 @@ public interface Execute {
    * @param brightnessAndRgbForItems Map, keys are item names, values are RGB and brightness values
    */
   @Deprecated
-  void updateItems(Map<String, Tuple<Integer, RGBvalues>> brightnessAndRgbForItems);
+  void updateItems(Map<String, Tuple<Integer, ValuesRGB>> brightnessAndRgbForItems);
 
   /**
    * Updates items according to given updates
diff --git a/feedbackloop.execute/src/main/java/de/tudresden/inf/st/eraser/feedbackloop/execute/ExecuteImpl.java b/feedbackloop.execute/src/main/java/de/tudresden/inf/st/eraser/feedbackloop/execute/ExecuteImpl.java
index 1765c3e6..2b305dfd 100644
--- a/feedbackloop.execute/src/main/java/de/tudresden/inf/st/eraser/feedbackloop/execute/ExecuteImpl.java
+++ b/feedbackloop.execute/src/main/java/de/tudresden/inf/st/eraser/feedbackloop/execute/ExecuteImpl.java
@@ -1,8 +1,8 @@
 package de.tudresden.inf.st.eraser.feedbackloop.execute;
 
 import de.tudresden.inf.st.eraser.commons.color.ColorUtils;
-import de.tudresden.inf.st.eraser.commons.color.ColorUtils.HSBvalues255;
-import de.tudresden.inf.st.eraser.commons.color.ColorUtils.RGBvalues;
+import de.tudresden.inf.st.eraser.commons.color.ColorUtils.ValuesIntegralHSB;
+import de.tudresden.inf.st.eraser.commons.color.ColorUtils.ValuesRGB;
 import de.tudresden.inf.st.eraser.feedbackloop.api.Execute;
 import de.tudresden.inf.st.eraser.jastadd.model.*;
 import de.tudresden.inf.st.eraser.util.Tuple;
@@ -28,23 +28,23 @@ public class ExecuteImpl implements Execute {
   }
 
   @Override
-  public void updateItems(Map<String, Tuple<Integer, RGBvalues>> brightnessAndRgbForItems) {
+  public void updateItems(Map<String, Tuple<Integer, ValuesRGB>> brightnessAndRgbForItems) {
     List<ItemUpdate> updates = new ArrayList<>();
-    for (Map.Entry<String, Tuple<Integer, RGBvalues>> entry : brightnessAndRgbForItems.entrySet()) {
+    for (Map.Entry<String, Tuple<Integer, ValuesRGB>> entry : brightnessAndRgbForItems.entrySet()) {
       String itemId = entry.getKey();
       resolveOrLogError(itemId, item -> {
         if (entry.getValue() == null) {
           return;
         }
         Integer brightness = entry.getValue().x;
-        RGBvalues rgb = entry.getValue().y;
-        HSBvalues255 hsb;
+        ValuesRGB rgb = entry.getValue().y;
+        ValuesIntegralHSB hsb;
         if (rgb != null) {
           // also set rgb values
           hsb = ColorUtils.convertRGBtoHSB(rgb).toIntegral();
           hsb.brightness = brightness;
         } else {
-          hsb = HSBvalues255.of(0, 100, brightness);
+          hsb = ValuesIntegralHSB.of(0, 100, brightness);
         }
         hsb.ensureBounds();
         updates.add(new ItemUpdateColor(item, TupleHSB.of(hsb.hue, hsb.saturation, hsb.brightness)));
diff --git a/skywriter-hue-integration/src/main/java/de/tudresden/inf/st/eraser/skywriter_hue_integration/Main.java b/skywriter-hue-integration/src/main/java/de/tudresden/inf/st/eraser/skywriter_hue_integration/Main.java
index 43e229c1..04c3ffe4 100644
--- a/skywriter-hue-integration/src/main/java/de/tudresden/inf/st/eraser/skywriter_hue_integration/Main.java
+++ b/skywriter-hue-integration/src/main/java/de/tudresden/inf/st/eraser/skywriter_hue_integration/Main.java
@@ -2,7 +2,7 @@ package de.tudresden.inf.st.eraser.skywriter_hue_integration;
 
 import beaver.Parser;
 import de.tudresden.inf.st.eraser.commons.color.ColorUtils;
-import de.tudresden.inf.st.eraser.commons.color.ColorUtils.RGBvalues;
+import de.tudresden.inf.st.eraser.commons.color.ColorUtils.ValuesRGB;
 import de.tudresden.inf.st.eraser.jastadd.model.*;
 import de.tudresden.inf.st.eraser.openhab2.mqtt.MQTTUpdater;
 import de.tudresden.inf.st.eraser.util.ParserUtils;
@@ -147,10 +147,10 @@ public class Main {
   }
 
   private static void updateStateHSB(Item skywriter1_x, Item skywriter1_y, Item irisItem) {
-    RGBvalues rgb = ColorUtils.convertXYtoRGB(Double.parseDouble(skywriter1_x.getStateAsString()),
+    ValuesRGB rgb = ColorUtils.convertXYtoRGB(Double.parseDouble(skywriter1_x.getStateAsString()),
         Double.parseDouble(skywriter1_y.getStateAsString()), 1.0);
 //    irisItem.setState(String.format("%d,%d,%d", rgb.red, rgb.green, rgb.blue));
-    ColorUtils.HSBvalues255 hsb = ColorUtils.convertRGBtoHSB(rgb).toIntegral().ensureBounds();
+    ColorUtils.ValuesIntegralHSB hsb = ColorUtils.convertRGBtoHSB(rgb).toIntegral().ensureBounds();
     irisItem.setStateFromString(String.format("%d,%d,%d", hsb.hue, hsb.saturation, hsb.brightness));
   }
 
@@ -161,7 +161,7 @@ public class Main {
       double xyzToken = Double.parseDouble(tokens[i]);
       rgbArray[i] = (int) Math.round(255 * xyzToken);
     }
-    ColorUtils.HSBvalues255 hsb = ColorUtils.convertRGBtoHSB(RGBvalues.of(rgbArray))
+    ColorUtils.ValuesIntegralHSB hsb = ColorUtils.convertRGBtoHSB(ValuesRGB.of(rgbArray))
         .toIntegral().ensureBounds();
     return String.format("%d,%d,%d", hsb.hue, hsb.saturation, hsb.brightness);
   }
-- 
GitLab