Fix UnitUtils.getDimensionName returns Angle instead of Dimensionless

An Angle is also Dimensionless and because it is the first compatible unit it is always returned instead of any other Dimensionless unit. With these changes `getDimensionName` will prefer the dimension of equal units over those of compatible units. Fixes #2880 Signed-off-by: Wouter Born <github@maindrain.net>
openhab · Apr 4, 2022 · 380626c · 380626c
1 parent 2a3e2e5
commit 380626c
Show file tree

Hide file tree

Showing 2 changed files with 25 additions and 15 deletions.
diff --git a/bundles/org.openhab.core/src/main/java/org/openhab/core/types/util/UnitUtils.java b/bundles/org.openhab.core/src/main/java/org/openhab/core/types/util/UnitUtils.java
@@ -110,20 +110,22 @@ public class UnitUtils {
      * @return The Dimension string or null if the unit can not be found in any of the SystemOfUnits.
      */
     public static @Nullable String getDimensionName(Unit<?> unit) {
+        String compatibleDimension = null;
         for (Class<? extends SystemOfUnits> system : ALL_SYSTEM_OF_UNITS) {
             for (Field field : system.getDeclaredFields()) {
                 if (field.getType().isAssignableFrom(Unit.class) && Modifier.isStatic(field.getModifiers())) {
                     Type genericType = field.getGenericType();
                     if (genericType instanceof ParameterizedType) {
                         String dimension = ((Class<?>) ((ParameterizedType) genericType).getActualTypeArguments()[0])
                                 .getSimpleName();
-                        Unit<?> systemUnit;
                         try {
-                            systemUnit = (Unit<?>) field.get(null);
+                            Unit<?> systemUnit = (Unit<?>) field.get(null);
                             if (systemUnit == null) {
                                 LOGGER.warn("Unit field points to a null value: {}", field);
-                            } else if (systemUnit.isCompatible(unit)) {
+                            } else if (systemUnit.equals(unit)) {
                                 return dimension;
+                            } else if (compatibleDimension == null && systemUnit.isCompatible(unit)) {
+                                compatibleDimension = dimension;
                             }
                         } catch (IllegalArgumentException | IllegalAccessException e) {
                             LOGGER.error("The unit field '{}' seems to be not accessible", field, e);
@@ -135,7 +137,7 @@ public class UnitUtils {
                 }
             }
         }
-        return null;
+        return compatibleDimension == null ? null : compatibleDimension;
     }
 
     /**

diff --git a/bundles/org.openhab.core/src/test/java/org/openhab/core/types/util/UnitUtilsTest.java b/bundles/org.openhab.core/src/test/java/org/openhab/core/types/util/UnitUtilsTest.java
@@ -19,8 +19,12 @@
 import static org.junit.jupiter.api.Assertions.*;
 import static org.openhab.core.library.unit.MetricPrefix.*;
 
+import java.util.Objects;
+
 import javax.measure.Quantity;
 import javax.measure.Unit;
+import javax.measure.quantity.Angle;
+import javax.measure.quantity.Dimensionless;
 import javax.measure.quantity.Energy;
 import javax.measure.quantity.Length;
 import javax.measure.quantity.Power;
@@ -103,22 +107,26 @@ public void testParseUnknownUnit() {
     }
 
     @Test
-    public void testGetDimensionName() {
+    public void testGetDimensionNameWithDimension() {
         assertThat(UnitUtils.getDimensionName(SIUnits.CELSIUS), is(Temperature.class.getSimpleName()));
+        assertThat(UnitUtils.getDimensionName(Units.DEGREE_ANGLE), is(Angle.class.getSimpleName()));
         assertThat(UnitUtils.getDimensionName(Units.KILOWATT_HOUR), is(Energy.class.getSimpleName()));
         assertThat(UnitUtils.getDimensionName(Units.WATT), is(Power.class.getSimpleName()));
         assertThat(UnitUtils.getDimensionName(MetricPrefix.MEGA(Units.KILOWATT_HOUR)),
                 is(Energy.class.getSimpleName()));
 
-        Unit<?> unit = UnitUtils.parseUnit("°F");
-        assertNotNull(unit);
-        if (unit != null) {
-            assertThat(UnitUtils.getDimensionName(unit), is(Temperature.class.getSimpleName()));
-        }
-        unit = UnitUtils.parseUnit("m");
-        assertNotNull(unit);
-        if (unit != null) {
-            assertThat(UnitUtils.getDimensionName(unit), is(Length.class.getSimpleName()));
-        }
+        Unit<?> unit = Objects.requireNonNull(UnitUtils.parseUnit("°F"));
+        assertThat(UnitUtils.getDimensionName(unit), is(Temperature.class.getSimpleName()));
+
+        unit = Objects.requireNonNull(UnitUtils.parseUnit("m"));
+        assertThat(UnitUtils.getDimensionName(unit), is(Length.class.getSimpleName()));
+    }
+
+    @Test
+    public void testGetDimensionNameWithoutDimension() {
+        assertThat(UnitUtils.getDimensionName(Units.DECIBEL), is(Dimensionless.class.getSimpleName()));
+        assertThat(UnitUtils.getDimensionName(Units.ONE), is(Dimensionless.class.getSimpleName()));
+        assertThat(UnitUtils.getDimensionName(Units.PARTS_PER_MILLION), is(Dimensionless.class.getSimpleName()));
+        assertThat(UnitUtils.getDimensionName(Units.PERCENT), is(Dimensionless.class.getSimpleName()));
     }
 }