[homekit] make sure to convert step values to Celsius (#13796)

otherwise if your step is 1.0 in fahrenheit, then your
values will get rounded to 1.0 celsius, and you might not
even notice you've lost precision in the Home app.

Signed-off-by: Cody Cutrer <cody@cutrer.us>

diff --git a/bundles/org.openhab.io.homekit/src/main/java/org/openhab/io/homekit/internal/HomekitTaggedItem.java b/bundles/org.openhab.io.homekit/src/main/java/org/openhab/io/homekit/internal/HomekitTaggedItem.java
index 72c668d338f068e3d3e2e5a4fb5d97894c31f28b..e479b7833f648554607c7baa83a20b66d7b686e5 100644 (file)
--- a/bundles/org.openhab.io.homekit/src/main/java/org/openhab/io/homekit/internal/HomekitTaggedItem.java
+++ b/bundles/org.openhab.io.homekit/src/main/java/org/openhab/io/homekit/internal/HomekitTaggedItem.java
@@ -18,6 +18,8 @@ import java.math.BigDecimal;
 import java.util.Map;
 import java.util.concurrent.ConcurrentHashMap;
 
+import javax.measure.Unit;
+
 import org.eclipse.jdt.annotation.NonNullByDefault;
 import org.eclipse.jdt.annotation.Nullable;
 import org.openhab.core.items.GroupItem;
@@ -415,14 +417,34 @@ public class HomekitTaggedItem {
      * @param defaultValue default value
      * @return value
      */
-    public QuantityType getConfigurationAsQuantity(String key, QuantityType defaultValue) {
+    public QuantityType<?> getConfigurationAsQuantity(String key, QuantityType defaultValue,
+            boolean relativeConversion) {
         String stringValue = getConfiguration(key, new String());
         if (stringValue.isEmpty()) {
             return defaultValue;
         }
         var parsedValue = new QuantityType(stringValue);
-        var convertedValue = parsedValue.toInvertibleUnit(defaultValue.getUnit());
-        // not convertible? just assume it's in the expected unit
+        QuantityType<?> convertedValue;
+
+        if (relativeConversion) {
+            convertedValue = parsedValue.toUnitRelative(defaultValue.getUnit());
+        } else {
+            convertedValue = parsedValue.toInvertibleUnit(defaultValue.getUnit());
+        }
+        // not convertible? just assume it's in the item's unit
+        if (convertedValue == null) {
+            Unit unit;
+            if (getBaseItem() instanceof NumberItem && (unit = ((NumberItem) getBaseItem()).getUnit()) != null) {
+                var bdValue = new BigDecimal(stringValue);
+                parsedValue = new QuantityType(bdValue, unit);
+                if (relativeConversion) {
+                    convertedValue = parsedValue.toUnitRelative(defaultValue.getUnit());
+                } else {
+                    convertedValue = parsedValue.toInvertibleUnit(defaultValue.getUnit());
+                }
+            }
+        }
+        // still not convertible? just assume it's in the default's unit
         if (convertedValue == null) {
             return new QuantityType(parsedValue.toBigDecimal(), defaultValue.getUnit());
         }

diff --git a/bundles/org.openhab.io.homekit/src/main/java/org/openhab/io/homekit/internal/accessories/HomekitCharacteristicFactory.java b/bundles/org.openhab.io.homekit/src/main/java/org/openhab/io/homekit/internal/accessories/HomekitCharacteristicFactory.java
index 823be226dcb581cf4cb54003101dccf661049396..8fa99252685bde6f96a3443ede1e89fcb6ce7cc9 100644 (file)
--- a/bundles/org.openhab.io.homekit/src/main/java/org/openhab/io/homekit/internal/accessories/HomekitCharacteristicFactory.java
+++ b/bundles/org.openhab.io.homekit/src/main/java/org/openhab/io/homekit/internal/accessories/HomekitCharacteristicFactory.java
@@ -18,7 +18,6 @@ import java.math.BigDecimal;
 import java.math.RoundingMode;
 import java.util.HashMap;
 import java.util.Map;
-import java.util.Objects;
 import java.util.concurrent.CompletableFuture;
 import java.util.function.BiFunction;
 import java.util.function.Consumer;
@@ -303,6 +302,11 @@ public class HomekitCharacteristicFactory {
         return convertAndRound(degrees, SIUnits.CELSIUS, useFahrenheit() ? ImperialUnits.FAHRENHEIT : SIUnits.CELSIUS);
     }
 
+    public static double getTemperatureStep(HomekitTaggedItem taggedItem, double defaultValue) {
+        return taggedItem.getConfigurationAsQuantity(HomekitTaggedItem.STEP,
+                new QuantityType(defaultValue, SIUnits.CELSIUS), true).doubleValue();
+    }
+
     private static Supplier<CompletableFuture<Integer>> getAngleSupplier(HomekitTaggedItem taggedItem,
             int defaultValue) {
         return () -> CompletableFuture.completedFuture(getAngleFromItem(taggedItem, defaultValue));
@@ -616,34 +620,16 @@ public class HomekitCharacteristicFactory {
 
     private static ColorTemperatureCharacteristic createColorTemperatureCharacteristic(HomekitTaggedItem taggedItem,
             HomekitAccessoryUpdater updater) {
-        // Check if units are expressed in Kelvin, not mireds, and adjust
-        // the min/max appropriately
-        Unit unit = null;
-        var numberItem = taggedItem.getBaseItem();
-        if (numberItem instanceof NumberItem) {
-            unit = ((NumberItem) numberItem).getUnit();
-        }
-        if (unit == null) {
-            unit = Units.MIRED;
-        }
-        final Unit finalUnit = unit;
-
         final boolean inverted = taggedItem.isInverted();
 
-        if (!unit.equals(Units.KELVIN) && !unit.equals(Units.MIRED)) {
-            logger.warn("Item {} must be in either K or mired. Given {}.", taggedItem.getName(), unit);
-            return new ColorTemperatureCharacteristic(null, null, null, null);
-        }
-
-        var minValueQt = taggedItem.getConfigurationAsQuantity(HomekitTaggedItem.MIN_VALUE,
-                Objects.requireNonNull(new QuantityType(ColorTemperatureCharacteristic.DEFAULT_MIN_VALUE, Units.MIRED)
-                        .toInvertibleUnit(unit)));
-        var maxValueQt = taggedItem.getConfigurationAsQuantity(HomekitTaggedItem.MAX_VALUE,
-                Objects.requireNonNull(new QuantityType(ColorTemperatureCharacteristic.DEFAULT_MAX_VALUE, Units.MIRED)
-                        .toInvertibleUnit(unit)));
-
-        int minValue = minValueQt.toInvertibleUnit(Units.MIRED).intValue();
-        int maxValue = maxValueQt.toInvertibleUnit(Units.MIRED).intValue();
+        int minValue = taggedItem
+                .getConfigurationAsQuantity(HomekitTaggedItem.MIN_VALUE,
+                        new QuantityType(ColorTemperatureCharacteristic.DEFAULT_MIN_VALUE, Units.MIRED), false)
+                .intValue();
+        int maxValue = taggedItem
+                .getConfigurationAsQuantity(HomekitTaggedItem.MAX_VALUE,
+                        new QuantityType(ColorTemperatureCharacteristic.DEFAULT_MAX_VALUE, Units.MIRED), false)
+                .intValue();
 
         // It's common to swap these if you're providing in Kelvin instead of mired
         if (minValue > maxValue) {
@@ -804,9 +790,8 @@ public class HomekitCharacteristicFactory {
                 HomekitTaggedItem.MIN_VALUE, CoolingThresholdTemperatureCharacteristic.DEFAULT_MIN_VALUE));
         double maxValue = HomekitCharacteristicFactory.convertToCelsius(taggedItem.getConfigurationAsDouble(
                 HomekitTaggedItem.MAX_VALUE, CoolingThresholdTemperatureCharacteristic.DEFAULT_MAX_VALUE));
-        return new CoolingThresholdTemperatureCharacteristic(minValue, maxValue,
-                taggedItem.getConfigurationAsDouble(HomekitTaggedItem.STEP,
-                        CoolingThresholdTemperatureCharacteristic.DEFAULT_STEP),
+        double step = getTemperatureStep(taggedItem, CoolingThresholdTemperatureCharacteristic.DEFAULT_STEP);
+        return new CoolingThresholdTemperatureCharacteristic(minValue, maxValue, step,
                 getTemperatureSupplier(taggedItem, minValue), setTemperatureConsumer(taggedItem),
                 getSubscriber(taggedItem, COOLING_THRESHOLD_TEMPERATURE, updater),
                 getUnsubscriber(taggedItem, COOLING_THRESHOLD_TEMPERATURE, updater));
@@ -818,9 +803,8 @@ public class HomekitCharacteristicFactory {
                 HomekitTaggedItem.MIN_VALUE, HeatingThresholdTemperatureCharacteristic.DEFAULT_MIN_VALUE));
         double maxValue = HomekitCharacteristicFactory.convertToCelsius(taggedItem.getConfigurationAsDouble(
                 HomekitTaggedItem.MAX_VALUE, HeatingThresholdTemperatureCharacteristic.DEFAULT_MAX_VALUE));
-        return new HeatingThresholdTemperatureCharacteristic(minValue, maxValue,
-                taggedItem.getConfigurationAsDouble(HomekitTaggedItem.STEP,
-                        HeatingThresholdTemperatureCharacteristic.DEFAULT_STEP),
+        double step = getTemperatureStep(taggedItem, HeatingThresholdTemperatureCharacteristic.DEFAULT_STEP);
+        return new HeatingThresholdTemperatureCharacteristic(minValue, maxValue, step,
                 getTemperatureSupplier(taggedItem, minValue), setTemperatureConsumer(taggedItem),
                 getSubscriber(taggedItem, HEATING_THRESHOLD_TEMPERATURE, updater),
                 getUnsubscriber(taggedItem, HEATING_THRESHOLD_TEMPERATURE, updater));

diff --git a/bundles/org.openhab.io.homekit/src/main/java/org/openhab/io/homekit/internal/accessories/HomekitTemperatureSensorImpl.java b/bundles/org.openhab.io.homekit/src/main/java/org/openhab/io/homekit/internal/accessories/HomekitTemperatureSensorImpl.java
index 2be10469c1ec696fbbee6d653cd397c7e2bb40b9..594d05a98b953ada55073d9c22e1964a6250bebb 100644 (file)
--- a/bundles/org.openhab.io.homekit/src/main/java/org/openhab/io/homekit/internal/accessories/HomekitTemperatureSensorImpl.java
+++ b/bundles/org.openhab.io.homekit/src/main/java/org/openhab/io/homekit/internal/accessories/HomekitTemperatureSensorImpl.java
@@ -75,8 +75,9 @@ class HomekitTemperatureSensorImpl extends AbstractHomekitAccessoryImpl implemen
 
     @Override
     public double getMinStepCurrentTemperature() {
-        return getAccessoryConfiguration(HomekitCharacteristicType.CURRENT_TEMPERATURE, HomekitTaggedItem.STEP,
-                BigDecimal.valueOf(TargetTemperatureCharacteristic.DEFAULT_STEP)).doubleValue();
+        return HomekitCharacteristicFactory.getTemperatureStep(
+                getCharacteristic(HomekitCharacteristicType.CURRENT_TEMPERATURE).get(),
+                TargetTemperatureCharacteristic.DEFAULT_STEP);
     }
 
     @Override

diff --git a/bundles/org.openhab.io.homekit/src/main/java/org/openhab/io/homekit/internal/accessories/HomekitThermostatImpl.java b/bundles/org.openhab.io.homekit/src/main/java/org/openhab/io/homekit/internal/accessories/HomekitThermostatImpl.java
index a227fef7bf4e427e3c483181e19ac0ddbb80626e..27c9617d5aed3bd44e4a11df12f3d50c8142f3ab 100644 (file)
--- a/bundles/org.openhab.io.homekit/src/main/java/org/openhab/io/homekit/internal/accessories/HomekitThermostatImpl.java
+++ b/bundles/org.openhab.io.homekit/src/main/java/org/openhab/io/homekit/internal/accessories/HomekitThermostatImpl.java
@@ -134,8 +134,9 @@ class HomekitThermostatImpl extends AbstractHomekitAccessoryImpl implements Ther
 
     @Override
     public double getMinStepCurrentTemperature() {
-        return getAccessoryConfiguration(HomekitCharacteristicType.CURRENT_TEMPERATURE, HomekitTaggedItem.STEP,
-                BigDecimal.valueOf(TargetTemperatureCharacteristic.DEFAULT_STEP)).doubleValue();
+        return HomekitCharacteristicFactory.getTemperatureStep(
+                getCharacteristic(HomekitCharacteristicType.CURRENT_TEMPERATURE).get(),
+                TargetTemperatureCharacteristic.DEFAULT_STEP);
     }
 
     @Override
@@ -204,8 +205,9 @@ class HomekitThermostatImpl extends AbstractHomekitAccessoryImpl implements Ther
 
     @Override
     public double getMinStepTargetTemperature() {
-        return getAccessoryConfiguration(HomekitCharacteristicType.TARGET_TEMPERATURE, HomekitTaggedItem.STEP,
-                BigDecimal.valueOf(TargetTemperatureCharacteristic.DEFAULT_STEP)).doubleValue();
+        return HomekitCharacteristicFactory.getTemperatureStep(
+                getCharacteristic(HomekitCharacteristicType.TARGET_TEMPERATURE).get(),
+                TargetTemperatureCharacteristic.DEFAULT_STEP);
     }
 
     @Override