From 826fc9e8d70dfb8e5d9bb80cb383480a6dcf08ba Mon Sep 17 00:00:00 2001 From: Cody Cutrer Date: Mon, 28 Nov 2022 15:11:40 -0700 Subject: [PATCH] [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 --- .../homekit/internal/HomekitTaggedItem.java | 28 +++++++++-- .../HomekitCharacteristicFactory.java | 50 +++++++------------ .../HomekitTemperatureSensorImpl.java | 5 +- .../accessories/HomekitThermostatImpl.java | 10 ++-- 4 files changed, 51 insertions(+), 42 deletions(-) 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 72c668d33..e479b7833 100644 --- 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 823be226d..8fa992526 100644 --- 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> 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 2be10469c..594d05a98 100644 --- 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 a227fef7b..27c9617d5 100644 --- 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