[gardena] Improve and fix UoM support (#15523)

* [gardena] Improve and fix UoM support

Properly convert incoming UoM values for command durations, and output
measurements as UoM values where possible.

* [gardena] Fix signal strength channel value

Previously the binding sent 0..100, but the system expects 0..4 for the
system.signal-strength channel.

* [gardena] Update README
* [gardena] Use actual units in state description where appropriate

Signed-off-by: Danny Baumann <dannybaumann@web.de>

bundles/org.openhab.binding.gardena/README.md

@@ -88,10 +88,10 @@ Sensor refresh commands are not yet supported by the Gardena smart system integr
 ```java
 // smart Water Control
 String  WC_Valve_Activity                 "Valve Activity" { channel="gardena:water_control:home:myWateringComputer:valve#activity" }
-Number  WC_Valve_Duration                 "Last Watering Duration [%d min]" { channel="gardena:water_control:home:myWateringComputer:valve#duration" }
+Number:Time WC_Valve_Duration             "Last Watering Duration [%d min]" { channel="gardena:water_control:home:myWateringComputer:valve#duration" }
 
-Number  WC_Valve_cmd_Duration             "Command Duration [%d min]" { channel="gardena:water_control:home:myWateringComputer:valve_commands#commandDuration" }
+Number:Time WC_Valve_cmd_Duration         "Command Duration [%d min]" { channel="gardena:water_control:home:myWateringComputer:valve_commands#commandDuration" }
-Switch  WC_Valve_cmd_OpenWithDuration     "Watering Timer [%d min]" { channel="gardena:water_control:home:myWateringComputer:valve_commands#start_seconds_to_override" }
+Switch  WC_Valve_cmd_OpenWithDuration     "Start Watering Timer" { channel="gardena:water_control:home:myWateringComputer:valve_commands#start_seconds_to_override" }
 Switch  WC_Valve_cmd_CloseValve           "Stop Switch" { channel="gardena:water_control:home:myWateringComputer:valve_commands#stop_until_next_task" }
 
 openhab:status WC_Valve_Duration // returns the duration of the last watering request if still active, or 0
@@ -101,7 +101,7 @@ openhab:status WC_Valve_Activity // returns the current valve activity  (CLOSED|
 All channels are read-only, except the command group and the lastUpdate timestamp
 
 ```shell
-openhab:send WC_Valve_cmd_Duration.sendCommand(10) // set the duration for the command to 10min
+openhab:send WC_Valve_cmd_Duration.sendCommand(600) // set the duration for the command to 10min
 openhab:send WC_Valve_cmd_OpenWithDuration.sendCommand(ON) // start watering
 openhab:send WC_Valve_cmd_CloseValve.sendCommand(ON) // stop any active watering
 ```

bundles/org.openhab.binding.gardena/src/main/java/org/openhab/binding/gardena/internal/handler/GardenaThingHandler.java

@@ -16,10 +16,13 @@ import static org.openhab.binding.gardena.internal.GardenaBindingConstants.*;
 
 import java.time.ZonedDateTime;
 import java.util.Date;
+import java.util.HashMap;
 import java.util.Map;
 import java.util.concurrent.ScheduledFuture;
 import java.util.concurrent.TimeUnit;
 
+import javax.measure.Unit;
+
 import org.eclipse.jdt.annotation.NonNullByDefault;
 import org.eclipse.jdt.annotation.Nullable;
 import org.openhab.binding.gardena.internal.GardenaSmart;
@@ -47,6 +50,7 @@ import org.openhab.core.library.types.DecimalType;
 import org.openhab.core.library.types.OnOffType;
 import org.openhab.core.library.types.QuantityType;
 import org.openhab.core.library.types.StringType;
+import org.openhab.core.library.unit.Units;
 import org.openhab.core.thing.Bridge;
 import org.openhab.core.thing.Channel;
 import org.openhab.core.thing.ChannelUID;
@@ -72,6 +76,7 @@ public class GardenaThingHandler extends BaseThingHandler {
     private final Logger logger = LoggerFactory.getLogger(GardenaThingHandler.class);
     private TimeZoneProvider timeZoneProvider;
     private @Nullable ScheduledFuture<?> commandResetFuture;
+    private Map<String, Integer> commandDurations = new HashMap<>();
 
     public GardenaThingHandler(Thing thing, TimeZoneProvider timeZoneProvider) {
         super(thing);
@@ -132,14 +137,17 @@ public class GardenaThingHandler extends BaseThingHandler {
      */
     protected void updateChannel(ChannelUID channelUID) throws GardenaException, AccountHandlerNotAvailableException {
         String groupId = channelUID.getGroupId();
-        if (groupId != null) {
+        if (groupId == null) {
-            boolean isCommand = groupId.endsWith("_commands");
+            return;
-            if (!isCommand || (isCommand && isLocalDurationCommand(channelUID))) {
+        }
-                Device device = getDevice();
+        if (isLocalDurationCommand(channelUID)) {
-                State state = convertToState(device, channelUID);
+            int commandDuration = getCommandDurationSeconds(getDeviceDataItemProperty(channelUID));
-                if (state != null) {
+            updateState(channelUID, new QuantityType<>(commandDuration, Units.SECOND));
-                    updateState(channelUID, state);
+        } else if (!groupId.endsWith("_commands")) {
-                }
+            Device device = getDevice();
+            State state = convertToState(device, channelUID);
+            if (state != null) {
+                updateState(channelUID, state);
             }
         }
     }
@@ -148,60 +156,66 @@ public class GardenaThingHandler extends BaseThingHandler {
      * Converts a Gardena property value to an openHAB state.
      */
     private @Nullable State convertToState(Device device, ChannelUID channelUID) throws GardenaException {
-        if (isLocalDurationCommand(channelUID)) {
-            String dataItemProperty = getDeviceDataItemProperty(channelUID);
-            return new DecimalType(Math.round(device.getLocalService(dataItemProperty).commandDuration / 60.0));
-        }
-
         String propertyPath = channelUID.getGroupId() + ".attributes.";
         String propertyName = channelUID.getIdWithoutGroup();
+        String unitPropertyPath = propertyPath;
 
         if (propertyName.endsWith("_timestamp")) {
             propertyPath += propertyName.replace("_", ".");
         } else {
             propertyPath += propertyName + ".value";
+            unitPropertyPath += propertyName + "Unit";
+        }
+
+        Channel channel = getThing().getChannel(channelUID.getId());
+        String acceptedItemType = channel != null ? channel.getAcceptedItemType() : null;
+        String baseItemType = StringUtils.substringBefore(acceptedItemType, ":");
+
+        boolean isNullPropertyValue = PropertyUtils.isNull(device, propertyPath);
+
+        if (isNullPropertyValue) {
+            return UnDefType.NULL;
+        }
+        if (baseItemType == null || acceptedItemType == null) {
+            return null;
         }
 
-        String acceptedItemType = null;
         try {
-            Channel channel = getThing().getChannel(channelUID.getId());
+            switch (baseItemType) {
-            if (channel != null) {
+                case "String":
-                acceptedItemType = StringUtils.substringBefore(channel.getAcceptedItemType(), ":");
+                    return new StringType(PropertyUtils.getPropertyValue(device, propertyPath, String.class));
-
+                case "Number":
-                if (acceptedItemType != null) {
+                    if (isNullPropertyValue) {
-                    boolean isNullPropertyValue = PropertyUtils.isNull(device, propertyPath);
+                        return new DecimalType(0);
-                    boolean isDurationProperty = "duration".equals(propertyName);
+                    } else {
-
+                        Number value = PropertyUtils.getPropertyValue(device, propertyPath, Number.class);
-                    if (isNullPropertyValue && !isDurationProperty) {
+                        Unit<?> unit = PropertyUtils.getPropertyValue(device, unitPropertyPath, Unit.class);
-                        return UnDefType.NULL;
+                        if (value == null) {
-                    }
-                    switch (acceptedItemType) {
-                        case "String":
-                            return new StringType(PropertyUtils.getPropertyValue(device, propertyPath, String.class));
-                        case "Number":
-                            if (isNullPropertyValue) {
-                                return new DecimalType(0);
-                            } else {
-                                Number value = PropertyUtils.getPropertyValue(device, propertyPath, Number.class);
-                                // convert duration from seconds to minutes
-                                if (value != null) {
-                                    if (isDurationProperty) {
-                                        value = Math.round(value.longValue() / 60.0);
-                                    }
-                                    return new DecimalType(value.longValue());
-                                }
-                                return UnDefType.NULL;
-                            }
-                        case "DateTime":
-                            Date date = PropertyUtils.getPropertyValue(device, propertyPath, Date.class);
-                            if (date != null) {
-                                ZonedDateTime zdt = ZonedDateTime.ofInstant(date.toInstant(),
-                                        timeZoneProvider.getTimeZone());
-                                return new DateTimeType(zdt);
-                            }
                             return UnDefType.NULL;
+                        } else {
+                            if ("rfLinkLevel".equals(propertyName)) {
+                                // Gardena gives us link level as 0..100%, while the system.signal-strength
+                                // channel type wants a 0..4 enum
+                                int percent = value.intValue();
+                                value = percent == 100 ? 4 : percent / 20;
+                                unit = null;
+                            }
+                            if (acceptedItemType.equals(baseItemType) || unit == null) {
+                                // No UoM or no unit found
+                                return new DecimalType(value);
+                            } else {
+                                return new QuantityType<>(value, unit);
+                            }
+                        }
+                    }
+                case "DateTime":
+                    Date date = PropertyUtils.getPropertyValue(device, propertyPath, Date.class);
+                    if (date == null) {
+                        return UnDefType.NULL;
+                    } else {
+                        ZonedDateTime zdt = ZonedDateTime.ofInstant(date.toInstant(), timeZoneProvider.getTimeZone());
+                        return new DateTimeType(zdt);
                     }
-                }
             }
         } catch (GardenaException e) {
             logger.warn("Channel '{}' cannot be updated as device does not contain propertyPath '{}'", channelUID,
@@ -223,8 +237,15 @@ public class GardenaThingHandler extends BaseThingHandler {
                 logger.debug("Refreshing Gardena connection");
                 getGardenaSmart().restartWebsockets();
             } else if (isLocalDurationCommand(channelUID)) {
-                QuantityType<?> quantityType = (QuantityType<?>) command;
+                QuantityType<?> commandInSeconds = null;
-                getDevice().getLocalService(dataItemProperty).commandDuration = quantityType.intValue() * 60;
+                if (command instanceof QuantityType<?> timeCommand) {
+                    commandInSeconds = timeCommand.toUnit(Units.SECOND);
+                }
+                if (commandInSeconds != null) {
+                    commandDurations.put(dataItemProperty, commandInSeconds.intValue());
+                } else {
+                    logger.info("Invalid command '{}' for command duration channel, ignoring.", command);
+                }
             } else if (isOnCommand) {
                 GardenaCommand gardenaCommand = getGardenaCommand(dataItemProperty, channelUID);
                 logger.debug("Received Gardena command: {}, {}", gardenaCommand.getClass().getSimpleName(),
@@ -261,17 +282,15 @@ public class GardenaThingHandler extends BaseThingHandler {
         String commandName = channelUID.getIdWithoutGroup().toUpperCase();
         String groupId = channelUID.getGroupId();
         if (groupId != null) {
+            int commandDuration = getCommandDurationSeconds(dataItemProperty);
             if ("valveSet_commands".equals(groupId)) {
                 return new ValveSetCommand(ValveSetControl.valueOf(commandName));
             } else if (groupId.startsWith("valve") && groupId.endsWith("_commands")) {
-                return new ValveCommand(ValveControl.valueOf(commandName),
+                return new ValveCommand(ValveControl.valueOf(commandName), commandDuration);
-                        getDevice().getLocalService(dataItemProperty).commandDuration);
             } else if ("mower_commands".equals(groupId)) {
-                return new MowerCommand(MowerControl.valueOf(commandName),
+                return new MowerCommand(MowerControl.valueOf(commandName), commandDuration);
-                        getDevice().getLocalService(dataItemProperty).commandDuration);
             } else if ("powerSocket_commands".equals(groupId)) {
-                return new PowerSocketCommand(PowerSocketControl.valueOf(commandName),
+                return new PowerSocketCommand(PowerSocketControl.valueOf(commandName), commandDuration);
-                        getDevice().getLocalService(dataItemProperty).commandDuration);
             }
         }
         throw new GardenaException("Command " + channelUID.getId() + " not found or groupId null");
@@ -308,6 +327,11 @@ public class GardenaThingHandler extends BaseThingHandler {
         throw new GardenaException("Can't extract dataItemProperty from channel group " + channelUID.getGroupId());
     }
 
+    private int getCommandDurationSeconds(String dataItemProperty) {
+        Integer duration = commandDurations.get(dataItemProperty);
+        return duration != null ? duration : 3600;
+    }
+
     /**
      * Returns true, if the channel is the duration command.
      */

bundles/org.openhab.binding.gardena/src/main/java/org/openhab/binding/gardena/internal/model/dto/Device.java

@@ -15,8 +15,6 @@ package org.openhab.binding.gardena.internal.model.dto;
 import static org.openhab.binding.gardena.internal.GardenaBindingConstants.*;
 
 import java.util.Date;
-import java.util.HashMap;
-import java.util.Map;
 
 import org.openhab.binding.gardena.internal.exception.GardenaException;
 import org.openhab.binding.gardena.internal.model.dto.api.CommonService;
@@ -60,25 +58,10 @@ public class Device {
     public ValveServiceDataItem valveSix;
     public ValveSetServiceDataItem valveSet;
 
-    private Map<String, LocalService> localServices = new HashMap<>();
-
     public Device(String id) {
         this.id = id;
     }
 
-    /**
-     * Returns the local service or creates one if it does not exist.
-     */
-    public LocalService getLocalService(String key) {
-        LocalService localService = localServices.get(key);
-        if (localService == null) {
-            localService = new LocalService();
-            localServices.put(key, localService);
-            localService.commandDuration = 3600;
-        }
-        return localService;
-    }
-
     /**
      * Evaluates the device type.
      */

bundles/org.openhab.binding.gardena/src/main/java/org/openhab/binding/gardena/internal/model/dto/LocalService.java

@@ -1,23 +0,0 @@
-/**
- * Copyright (c) 2010-2023 Contributors to the openHAB project
- *
- * See the NOTICE file(s) distributed with this work for additional
- * information.
- *
- * This program and the accompanying materials are made available under the
- * terms of the Eclipse Public License 2.0 which is available at
- * http://www.eclipse.org/legal/epl-2.0
- *
- * SPDX-License-Identifier: EPL-2.0
- */
-package org.openhab.binding.gardena.internal.model.dto;
-
-/**
- * A local service exists only in openHAB and the state is not saved on restarts.
- *
- * @author Gerhard Riegler - Initial contribution
- */
-
-public class LocalService {
-    public Integer commandDuration;
-}

bundles/org.openhab.binding.gardena/src/main/java/org/openhab/binding/gardena/internal/model/dto/api/CommonService.java

@@ -12,6 +12,12 @@
  */
 package org.openhab.binding.gardena.internal.model.dto.api;
 
+import javax.measure.Unit;
+import javax.measure.quantity.Dimensionless;
+
+import org.eclipse.jdt.annotation.NonNull;
+import org.openhab.core.library.unit.Units;
+
 /**
  * Represents a Gardena object that is sent via the Gardena API.
  *
@@ -20,8 +26,10 @@ package org.openhab.binding.gardena.internal.model.dto.api;
 public class CommonService {
     public UserDefinedNameWrapper name;
     public TimestampedIntegerValue batteryLevel;
+    public @NonNull Unit<@NonNull Dimensionless> batteryLevelUnit = Units.PERCENT;
     public TimestampedStringValue batteryState;
     public TimestampedIntegerValue rfLinkLevel;
+    public @NonNull Unit<@NonNull Dimensionless> rfLinkLevelUnit = Units.PERCENT;
     public StringValue serial;
     public StringValue modelType;
     public TimestampedStringValue rfLinkState;

bundles/org.openhab.binding.gardena/src/main/java/org/openhab/binding/gardena/internal/model/dto/api/MowerService.java

@@ -12,6 +12,12 @@
  */
 package org.openhab.binding.gardena.internal.model.dto.api;
 
+import javax.measure.Unit;
+import javax.measure.quantity.Time;
+
+import org.eclipse.jdt.annotation.NonNull;
+import org.openhab.core.library.unit.Units;
+
 /**
  * Represents a Gardena object that is sent via the Gardena API.
  *
@@ -23,4 +29,5 @@ public class MowerService {
     public TimestampedStringValue activity;
     public TimestampedStringValue lastErrorCode;
     public IntegerValue operatingHours;
+    public @NonNull Unit<@NonNull Time> operatingHoursUnit = Units.HOUR;
 }

bundles/org.openhab.binding.gardena/src/main/java/org/openhab/binding/gardena/internal/model/dto/api/PowerSocketService.java

@@ -12,6 +12,12 @@
  */
 package org.openhab.binding.gardena.internal.model.dto.api;
 
+import javax.measure.Unit;
+import javax.measure.quantity.Time;
+
+import org.eclipse.jdt.annotation.NonNull;
+import org.openhab.core.library.unit.Units;
+
 /**
  * Represents a Gardena object that is sent via the Gardena API.
  *
@@ -23,4 +29,5 @@ public class PowerSocketService {
     public TimestampedStringValue state;
     public TimestampedStringValue lastErrorCode;
     public TimestampedIntegerValue duration;
+    public @NonNull Unit<@NonNull Time> durationUnit = Units.SECOND;
 }

bundles/org.openhab.binding.gardena/src/main/java/org/openhab/binding/gardena/internal/model/dto/api/SensorService.java

@@ -12,6 +12,15 @@
  */
 package org.openhab.binding.gardena.internal.model.dto.api;
 
+import javax.measure.Unit;
+import javax.measure.quantity.Dimensionless;
+import javax.measure.quantity.Illuminance;
+import javax.measure.quantity.Temperature;
+
+import org.eclipse.jdt.annotation.NonNull;
+import org.openhab.core.library.unit.SIUnits;
+import org.openhab.core.library.unit.Units;
+
 /**
  * Represents a Gardena object that is sent via the Gardena API.
  *
@@ -19,7 +28,11 @@ package org.openhab.binding.gardena.internal.model.dto.api;
  */
 public class SensorService {
     public TimestampedIntegerValue soilHumidity;
+    public @NonNull Unit<@NonNull Dimensionless> soilHumidityUnit = Units.PERCENT;
     public TimestampedIntegerValue soilTemperature;
+    public @NonNull Unit<@NonNull Temperature> soilTemperatureUnit = SIUnits.CELSIUS;
     public TimestampedIntegerValue ambientTemperature;
+    public @NonNull Unit<@NonNull Temperature> ambientTemperatureUnit = SIUnits.CELSIUS;
     public TimestampedIntegerValue lightIntensity;
+    public @NonNull Unit<@NonNull Illuminance> lightIntensityUnit = Units.LUX;
 }

bundles/org.openhab.binding.gardena/src/main/java/org/openhab/binding/gardena/internal/model/dto/api/ValveService.java

@@ -12,6 +12,12 @@
  */
 package org.openhab.binding.gardena.internal.model.dto.api;
 
+import javax.measure.Unit;
+import javax.measure.quantity.Time;
+
+import org.eclipse.jdt.annotation.NonNull;
+import org.openhab.core.library.unit.Units;
+
 /**
  * Represents a Gardena object that is sent via the Gardena API.
  *
@@ -23,4 +29,5 @@ public class ValveService {
     public TimestampedStringValue state;
     public TimestampedStringValue lastErrorCode;
     public TimestampedIntegerValue duration;
+    public @NonNull Unit<@NonNull Time> durationUnit = Units.SECOND;
 }

bundles/org.openhab.binding.gardena/src/main/resources/OH-INF/thing/thing-types.xml

@@ -592,22 +592,22 @@
 	<channel-type id="duration">
 		<item-type>Number:Time</item-type>
 		<label>Duration</label>
-		<description>Duration in minutes</description>
+		<description>Duration</description>
 		<state readOnly="true" pattern="%d min"/>
 	</channel-type>
 
 	<channel-type id="soilHumidity">
 		<item-type>Number:Dimensionless</item-type>
 		<label>Soil Humidity</label>
-		<description>Soil humidity in percent</description>
+		<description>Soil humidity</description>
-		<state readOnly="true" pattern="%d %unit%"/>
+		<state readOnly="true" pattern="%d %%"/>
 	</channel-type>
 
 	<channel-type id="lightIntensity">
 		<item-type>Number:Illuminance</item-type>
 		<label>Light Intensity</label>
-		<description>Light intensity in Lux</description>
+		<description>Light intensity</description>
-		<state readOnly="true" pattern="%d lux"/>
+		<state readOnly="true" pattern="%d %unit%"/>
 	</channel-type>
 
 	<channel-type id="temperature">
@@ -621,7 +621,7 @@
 		<item-type>Number:Time</item-type>
 		<label>Operating Hours</label>
 		<description>The operating hours</description>
-		<state readOnly="true" pattern="%d %unit%"/>
+		<state readOnly="true" pattern="%d h"/>
 	</channel-type>
 
 	<channel-type id="batteryState">