[homematic] UoM support unexpected units (#6690)

* [homematic] UoM support unexpected units

* add support for "100%" and "À°C" which may be present in datapoints
* remove UoM for time (since hardly anybody will want to convert on those items)
* Extend unittests and improve logging

Not sure if it will fix #6612 but at least it will improve logging

Signed-off-by: Michael Reitler <michael.reitler@telekom.de>

extensions/binding/org.eclipse.smarthome.binding.homematic.test/src/test/java/org/eclipse/smarthome/binding/homematic/internal/converter/BaseConverterTest.java

@@ -0,0 +1,29 @@
+package org.eclipse.smarthome.binding.homematic.internal.converter;
+
+import org.eclipse.smarthome.binding.homematic.internal.model.HmChannel;
+import org.eclipse.smarthome.binding.homematic.internal.model.HmDatapoint;
+import org.eclipse.smarthome.binding.homematic.internal.model.HmDevice;
+import org.eclipse.smarthome.binding.homematic.internal.model.HmInterface;
+import org.eclipse.smarthome.binding.homematic.internal.model.HmParamsetType;
+import org.eclipse.smarthome.binding.homematic.internal.model.HmValueType;
+import org.junit.Before;
+
+public class BaseConverterTest {
+
+    protected final HmDatapoint floatDp = new HmDatapoint("floatDp", "", HmValueType.FLOAT, null, false,
+            HmParamsetType.VALUES);
+    protected final HmDatapoint integerDp = new HmDatapoint("integerDp", "", HmValueType.INTEGER, null, false,
+            HmParamsetType.VALUES);
+    protected final HmDatapoint floatQuantityDp = new HmDatapoint("floatQuantityDp", "", HmValueType.FLOAT, null, false,
+            HmParamsetType.VALUES);
+    protected final HmDatapoint integerQuantityDp = new HmDatapoint("floatIntegerDp", "", HmValueType.INTEGER, null,
+            false, HmParamsetType.VALUES);
+
+    @Before
+    public void setup() {
+        HmChannel stubChannel = new HmChannel("stubChannel", 0);
+        stubChannel.setDevice(new HmDevice("LEQ123456", HmInterface.RF, "HM-STUB-DEVICE", "", "", ""));
+        floatDp.setChannel(stubChannel);
+    }
+
+}

extensions/binding/org.eclipse.smarthome.binding.homematic.test/src/test/java/org/eclipse/smarthome/binding/homematic/internal/converter/ConvertFromBindingTest.java

@@ -17,8 +17,6 @@
 
 import org.eclipse.smarthome.binding.homematic.internal.converter.type.AbstractTypeConverter;
 import org.eclipse.smarthome.binding.homematic.internal.model.HmDatapoint;
-import org.eclipse.smarthome.binding.homematic.internal.model.HmParamsetType;
-import org.eclipse.smarthome.binding.homematic.internal.model.HmValueType;
 import org.eclipse.smarthome.core.library.types.DecimalType;
 import org.eclipse.smarthome.core.library.types.QuantityType;
 import org.eclipse.smarthome.core.library.unit.ImperialUnits;
@@ -35,22 +33,16 @@
  * @author Michael Reitler - Initial Contribution
  *
  */
-public class ConvertFromBindingTest {
-
-    private final HmDatapoint floatDp = new HmDatapoint("floatDp", "", HmValueType.FLOAT, null, false,
-            HmParamsetType.VALUES);
-    private final HmDatapoint integerDp = new HmDatapoint("integerDp", "", HmValueType.INTEGER, null, false,
-            HmParamsetType.VALUES);
-    private final HmDatapoint floatQuantityDp = new HmDatapoint("floatQuantityDp", "", HmValueType.FLOAT, null, false,
-            HmParamsetType.VALUES);
-    private final HmDatapoint integerQuantityDp = new HmDatapoint("floatIntegerDp", "", HmValueType.INTEGER, null,
-            false, HmParamsetType.VALUES);
+public class ConvertFromBindingTest extends BaseConverterTest {
 
     @Test
     public void testDecimalTypeConverter() throws ConverterException {
         State convertedState;
         TypeConverter<?> decimalConverter = ConverterFactory.createConverter("Number");
 
+        // the binding is backwards compatible, so clients may still use DecimalType, even if a unit is used
+        floatDp.setUnit("%");
+
         floatDp.setValue(99.9);
         convertedState = decimalConverter.convertFromBinding(floatDp);
         assertThat(convertedState, instanceOf(DecimalType.class));
@@ -88,6 +80,10 @@ public void testQuantityTypeConverter() throws ConverterException {
         assertThat(((QuantityType<?>) convertedState).doubleValue(), is(10.5));
         assertThat(((QuantityType<?>) convertedState).toUnit(ImperialUnits.FAHRENHEIT).doubleValue(), is(50.9));
 
+        floatQuantityDp.setUnit("°C");
+        assertThat(((QuantityType<?>) convertedState).getDimension(), is(QuantityDimension.TEMPERATURE));
+        assertThat(((QuantityType<?>) convertedState).doubleValue(), is(10.5));
+
         integerQuantityDp.setValue(50000);
         integerQuantityDp.setUnit("mHz");
         convertedState = frequencyConverter.convertFromBinding(integerQuantityDp);
@@ -97,13 +93,14 @@ public void testQuantityTypeConverter() throws ConverterException {
         assertThat(((QuantityType<?>) convertedState).intValue(), is(50000));
         assertThat(((QuantityType<?>) convertedState).toUnit(SIUnits.HERTZ).intValue(), is(50));
 
-        floatQuantityDp.setValue(12);
-        floatQuantityDp.setUnit("month");
+        floatQuantityDp.setValue(0.7);
+        floatQuantityDp.setUnit("100%");
         convertedState = timeConverter.convertFromBinding(floatQuantityDp);
         assertThat(convertedState, instanceOf(QuantityType.class));
-        assertThat(((QuantityType<?>) convertedState).getDimension(), is(QuantityDimension.TIME));
-        assertThat(((QuantityType<?>) convertedState).doubleValue(), is(12.0));
-        assertThat(((QuantityType<?>) convertedState).toUnit(SmartHomeUnits.YEAR).doubleValue(), is(1.0));
+        assertThat(((QuantityType<?>) convertedState).getDimension(), is(QuantityDimension.NONE));
+        assertThat(((QuantityType<?>) convertedState).doubleValue(), is(70.0));
+        assertThat(((QuantityType<?>) convertedState).getUnit(), is(SmartHomeUnits.PERCENT));
+        assertThat(((QuantityType<?>) convertedState).toUnit(SmartHomeUnits.ONE).doubleValue(), is(0.7));
 
     }
 

extensions/binding/org.eclipse.smarthome.binding.homematic.test/src/test/java/org/eclipse/smarthome/binding/homematic/internal/converter/ConvertToBindingTest.java

@@ -23,15 +23,9 @@
 import org.eclipse.smarthome.binding.homematic.internal.converter.type.AbstractTypeConverter;
 import org.eclipse.smarthome.binding.homematic.internal.converter.type.DecimalTypeConverter;
 import org.eclipse.smarthome.binding.homematic.internal.converter.type.QuantityTypeConverter;
-import org.eclipse.smarthome.binding.homematic.internal.model.HmChannel;
 import org.eclipse.smarthome.binding.homematic.internal.model.HmDatapoint;
-import org.eclipse.smarthome.binding.homematic.internal.model.HmDevice;
-import org.eclipse.smarthome.binding.homematic.internal.model.HmInterface;
-import org.eclipse.smarthome.binding.homematic.internal.model.HmParamsetType;
-import org.eclipse.smarthome.binding.homematic.internal.model.HmValueType;
 import org.eclipse.smarthome.core.library.types.DecimalType;
 import org.eclipse.smarthome.core.library.types.QuantityType;
-import org.junit.Before;
 import org.junit.Test;
 
 /**
@@ -40,28 +34,15 @@
  * @author Michael Reitler - Initial Contribution
  *
  */
-public class ConvertToBindingTest {
-
-    private final HmDatapoint floatDp = new HmDatapoint("floatDp", "", HmValueType.FLOAT, null, false,
-            HmParamsetType.VALUES);
-    private final HmDatapoint integerDp = new HmDatapoint("integerDp", "", HmValueType.INTEGER, null, false,
-            HmParamsetType.VALUES);
-    private final HmDatapoint floatQuantityDp = new HmDatapoint("floatQuantityDp", "", HmValueType.FLOAT, null, false,
-            HmParamsetType.VALUES);
-    private final HmDatapoint integerQuantityDp = new HmDatapoint("floatIntegerDp", "", HmValueType.INTEGER, null,
-            false, HmParamsetType.VALUES);
-
-    @Before
-    public void setup() {
-        HmChannel stubChannel = new HmChannel("stubChannel", 0);
-        stubChannel.setDevice(new HmDevice("LEQ123456", HmInterface.RF, "HM-STUB-DEVICE", "", "", ""));
-        floatDp.setChannel(stubChannel);
-    }
+public class ConvertToBindingTest extends BaseConverterTest {
 
     @Test
     public void testDecimalTypeConverter() throws ConverterException {
         Object convertedValue;
         TypeConverter<?> dTypeConverter = new DecimalTypeConverter();
+
+        // the binding is backwards compatible, so clients may still use DecimalType, even if a unit is used
+        floatDp.setUnit("°C");
 
         convertedValue = dTypeConverter.convertToBinding(new DecimalType(99.9), floatDp);
         assertThat(convertedValue, is(99.9));
@@ -84,6 +65,8 @@ public void testQuantityTypeConverter() throws ConverterException {
 
         convertedValue = qTypeConverter.convertToBinding(new QuantityType<Temperature>("99.9 °C"), floatQuantityDp);
         assertThat(convertedValue, is(99.9));
+
+        floatQuantityDp.setUnit("°C"); // at some points datapoints come with such unit instead of °C
 
         convertedValue = qTypeConverter.convertToBinding(new QuantityType<Temperature>("451 °F"), floatQuantityDp);
         assertThat(convertedValue, is(232.777778));
@@ -104,6 +87,17 @@ public void testQuantityTypeConverter() throws ConverterException {
         convertedValue = qTypeConverter.convertToBinding(new QuantityType<Dimensionless>("1"), integerQuantityDp);
         assertThat(convertedValue, is(100));
 
+        floatQuantityDp.setUnit("100%"); // not really a unit, but it occurs in homematic datapoints
+
+        convertedValue = qTypeConverter.convertToBinding(new QuantityType<Dimensionless>("99.0 %"), floatQuantityDp);
+        assertThat(convertedValue, is(0.99));
+
+        convertedValue = qTypeConverter.convertToBinding(new QuantityType<Dimensionless>("99.9 %"), floatQuantityDp);
+        assertThat(convertedValue, is(0.999));
+
+        convertedValue = qTypeConverter.convertToBinding(new QuantityType<Dimensionless>("1"), floatQuantityDp);
+        assertThat(convertedValue, is(1.0));
+
         integerQuantityDp.setUnit("Lux");
 
         convertedValue = qTypeConverter.convertToBinding(new QuantityType<Illuminance>("42 lx"), integerQuantityDp);

extensions/binding/org.eclipse.smarthome.binding.homematic/src/main/java/org/eclipse/smarthome/binding/homematic/internal/common/HomematicConfig.java

@@ -356,7 +356,7 @@ public boolean hasRfPort() {
     }
 
     /**
-     * Returns the encoding of a Homematic gateway.
+     * Returns the encoding that is suitable on requests to & responds from the Homematic gateway.
      */
     public String getEncoding() {
         if (gatewayInfo != null && gatewayInfo.isHomegear()) {

extensions/binding/org.eclipse.smarthome.binding.homematic/src/main/java/org/eclipse/smarthome/binding/homematic/internal/communicator/client/XmlRpcClient.java

@@ -83,12 +83,12 @@ private synchronized Object[] sendMessage(int port, RpcRequest<String> request,
                     .timeout(config.getTimeout(), TimeUnit.SECONDS)
                     .header(HttpHeader.CONTENT_TYPE, "text/xml;charset=" + config.getEncoding()).send();
 
-            String result = new String(response.getContent(), config.getEncoding());
             if (logger.isTraceEnabled()) {
+                String result = new String(response.getContent(), config.getEncoding());
                 logger.trace("Client XmlRpcResponse (port {}):\n{}", port, result);
             }
 
-            Object[] data = new XmlRpcResponse(new ByteArrayInputStream(result.getBytes(config.getEncoding())),
+            Object[] data = new XmlRpcResponse(new ByteArrayInputStream(response.getContent()),
                     config.getEncoding()).getResponseData();
             return new RpcResponseParser(request).parse(data);
         } catch (UnknownRpcFailureException | UnknownParameterSetException ex) {

extensions/binding/org.eclipse.smarthome.binding.homematic/src/main/java/org/eclipse/smarthome/binding/homematic/internal/converter/type/AbstractTypeConverter.java

@@ -81,10 +81,11 @@ protected BigDecimal round(Double number) {
     @SuppressWarnings("unchecked")
     @Override
     public Object convertToBinding(Type type, HmDatapoint dp) throws ConverterException {
-        if (logger.isTraceEnabled()) {
-            logger.trace("Converting type {} with value '{}' to {} value with {} for '{}'",
-                    type.getClass().getSimpleName(), type.toString(), dp.getType(), this.getClass().getSimpleName(),
-                    new HmDatapointInfo(dp));
+        if (isLoggingRequired()) {
+            logAtDefaultLevel(
+                    "Converting type {} with value '{}' using {} to datapoint '{}' (dpType='{}', dpUnit='{}')",
+                    type.getClass().getSimpleName(), type.toString(), this.getClass().getSimpleName(),
+                    new HmDatapointInfo(dp), dp.getType(), dp.getUnit());
         }
 
         if (type == UnDefType.NULL) {
@@ -105,9 +106,10 @@ public Object convertToBinding(Type type, HmDatapoint dp) throws ConverterExcept
     @SuppressWarnings("unchecked")
     @Override
     public T convertFromBinding(HmDatapoint dp) throws ConverterException {
-        if (logger.isTraceEnabled()) {
-            logger.trace("Converting {} value '{}' with {} for '{}'", dp.getType(), dp.getValue(),
-                    this.getClass().getSimpleName(), new HmDatapointInfo(dp));
+        if (isLoggingRequired()) {
+            logAtDefaultLevel("Converting datapoint '{}' (dpType='{}', dpUnit='{}', dpValue='{}') with {}",
+                    new HmDatapointInfo(dp), dp.getType(), dp.getUnit(), dp.getValue(),
+                    this.getClass().getSimpleName());
         }
 
         if (dp.getValue() == null) {
@@ -121,6 +123,41 @@ public T convertFromBinding(HmDatapoint dp) throws ConverterException {
         return fromBinding(dp);
     }
 
+    /**
+     * By default, instances of {@link AbstractTypeConverter} log in level TRACE.
+     * May be overridden to increase logging verbosity of a converter.
+     *
+     * @return desired LogLevel
+     */
+    protected LogLevel getDefaultLogLevelForTypeConverter() {
+        return LogLevel.TRACE;
+    }
+
+    private boolean isLoggingRequired() {
+        if (getDefaultLogLevelForTypeConverter() == LogLevel.TRACE) {
+            return logger.isTraceEnabled();
+        }
+        if (getDefaultLogLevelForTypeConverter() == LogLevel.DEBUG) {
+            return logger.isDebugEnabled();
+        }
+        return true;
+    }
+
+    private void logAtDefaultLevel(String format, Object... arguments) {
+        switch (getDefaultLogLevelForTypeConverter()) {
+            case TRACE:
+                logger.trace(format, arguments);
+                break;
+            case DEBUG:
+                logger.debug(format, arguments);
+                break;
+            case INFO:
+            default:
+                logger.info(format, arguments);
+                break;
+        }
+    }
+
     /**
      * Converts a openHAB command to a Homematic value.
      */
@@ -149,4 +186,10 @@ protected Object commandToBinding(Command command, HmDatapoint dp) throws Conver
      */
     protected abstract T fromBinding(HmDatapoint dp) throws ConverterException;
 
+    protected enum LogLevel {
+        TRACE,
+        INFO,
+        DEBUG
+    }
+
 }

extensions/binding/org.eclipse.smarthome.binding.homematic/src/main/java/org/eclipse/smarthome/binding/homematic/internal/converter/type/QuantityTypeConverter.java

@@ -26,7 +26,6 @@
 import javax.measure.quantity.Pressure;
 import javax.measure.quantity.Speed;
 import javax.measure.quantity.Temperature;
-import javax.measure.quantity.Time;
 import javax.measure.quantity.Volume;
 
 import org.eclipse.smarthome.binding.homematic.internal.converter.ConverterException;
@@ -44,6 +43,14 @@
  * @author Michael Reitler - Initial contribution
  */
 public class QuantityTypeConverter extends AbstractTypeConverter<QuantityType<? extends Quantity<?>>> {
+
+    // this literal is required because some gateway types are mixing up encodings in their XML-RPC responses
+    private final String UNCORRECT_ENCODED_CELSIUS = "°C";
+
+    // "100%" is a commonly used "unit" in datapoints. Generated channel-type is of DecimalType,
+    // but clients may define a QuantityType if preferred
+    private final String HUNDRED_PERCENT = "100%";
+
     @Override
     protected boolean toBindingValidation(HmDatapoint dp, Class<? extends Type> typeClass) {
         return dp.isNumberType() && typeClass.isAssignableFrom(QuantityType.class);
@@ -68,22 +75,26 @@ private QuantityType<? extends Quantity<?>> toUnitFromDatapoint(QuantityType<? e
 
         // convert the given QuantityType to a QuantityType with the unit of the target datapoint
         switch (dp.getUnit()) {
-            case "minutes":
-                return type.toUnit(SmartHomeUnits.MINUTE);
-            case "day":
-                return type.toUnit(SmartHomeUnits.DAY);
-            case "month":
-                return type.toUnit(SmartHomeUnits.YEAR.divide(12));
-            case "year":
-                return type.toUnit(SmartHomeUnits.YEAR);
             case "Lux":
                 return type.toUnit(SmartHomeUnits.LUX);
             case "degree":
                 return type.toUnit(SmartHomeUnits.DEGREE_ANGLE);
+            case HUNDRED_PERCENT:
+                return type.toUnit(SmartHomeUnits.ONE);
+            case UNCORRECT_ENCODED_CELSIUS:
+                return type.toUnit(SIUnits.CELSIUS);
+            case "dBm":
+            case "minutes":
+            case "day":
+            case "month":
+            case "year":
+            case "":
+                return type;
+            default:
+                // According to datapoint documentation, the following values are remaining
+                // °C, V, %, s, min, mHz, Hz, hPa, km/h, mm, W, m3
+                return type.toUnit(dp.getUnit());
         }
-        // According to datapoint documentation, the following values are remaining
-        // °C, V, %, s, min, mHz, Hz, hPa, km/h, mm, W, m3
-        return type.toUnit(dp.getUnit());
     }
 
     @Override
@@ -103,23 +114,13 @@ protected QuantityType<? extends Quantity<?>> fromBinding(HmDatapoint dp) throws
         // create a QuantityType from the datapoint's value based on the datapoint's unit
         String unit = dp.getUnit() != null ? dp.getUnit() : "";
         switch (unit) {
+            case UNCORRECT_ENCODED_CELSIUS:
             case "°C":
                 return new QuantityType<Temperature>(number, SIUnits.CELSIUS);
             case "V":
                 return new QuantityType<ElectricPotential>(number, SmartHomeUnits.VOLT);
             case "%":
                 return new QuantityType<Dimensionless>(number, SmartHomeUnits.PERCENT);
-            case "s":
-                return new QuantityType<Time>(number, SmartHomeUnits.SECOND);
-            case "min":
-            case "minutes":
-                return new QuantityType<Time>(number, SmartHomeUnits.MINUTE);
-            case "day":
-                return new QuantityType<Time>(number, SmartHomeUnits.DAY);
-            case "month":
-                return new QuantityType<Time>(number, SmartHomeUnits.YEAR.divide(12));
-            case "year":
-                return new QuantityType<Time>(number, SmartHomeUnits.YEAR);
             case "mHz":
                 return new QuantityType<Frequency>(number, MetricPrefix.MILLI(SmartHomeUnits.HERTZ));
             case "Hz":
@@ -140,8 +141,25 @@ protected QuantityType<? extends Quantity<?>> fromBinding(HmDatapoint dp) throws
                 return new QuantityType<Energy>(number, SmartHomeUnits.WATT_HOUR);
             case "m3":
                 return new QuantityType<Volume>(number, SIUnits.CUBIC_METRE);
+            case HUNDRED_PERCENT:
+                return new QuantityType<Dimensionless>(number.doubleValue() * 100.0, SmartHomeUnits.PERCENT);
+            case "dBm":
+            case "s":
+            case "min":
+            case "minutes":
+            case "day":
+            case "month":
+            case "year":
+            case "":
+            default:
+                return new QuantityType<Dimensionless>(number, SmartHomeUnits.ONE);
         }
-        return new QuantityType<Dimensionless>(number, SmartHomeUnits.ONE);
+    }
+
+    @Override
+    protected LogLevel getDefaultLogLevelForTypeConverter() {
+        // increase logging verbosity for this type of converter
+        return LogLevel.DEBUG;
     }
 
 }