[smartthings]Fix discovery service bug and enhancement to SmartApp for OH3 (#9889)

Signed-off-by: Bob Raker <rjraker@gmail.com>

bundles/org.openhab.binding.smartthings/contrib/smartthings/SmartApps/OpenHabAppV2.groovy

@@ -855,6 +855,8 @@ def actionAlarm(device, attribute, value) {
 }
 
 // This is the original color control
+// 1-19-2021 The color values of on and off were added in response to issue https://github.com/BobRak/OpenHAB-Smartthings/issues/102
+// These changes were made because OH 3.0 uses color values of on/off. OH 2 and 3.1 don't seem to need this.
 def actionColorControl(device, attribute, value) {
     log.debug "actionColor: attribute \"${attribute}\", value \"${value}\""
     switch (attribute) {
@@ -865,15 +867,23 @@ def actionColorControl(device, attribute, value) {
             device.setSaturation(value as int)
         break
         case "color":
-            def colormap = ["hue": value[0] as int, "saturation": value[1] as int]
+    		if (value == "off") {
-            // log.debug "actionColor: Setting device \"${device}\" with attribute \"${attribute}\" to colormap \"${colormap}\""
+        		device.off()
-            device.setColor(colormap)
+    		} else if (value == "on") {
-            device.setLevel(value[2] as int)
+        		device.on()
+    		} else {
+            	def colormap = ["hue": value[0] as int, "saturation": value[1] as int]
+            	log.debug "actionColorControl: Setting device \"${device}\" with attribute \"${attribute}\" to colormap \"${colormap}\""
+            	device.setColor(colormap)
+            	device.setLevel(value[2] as int)
+            }
         break
     }
 }
 
 // This is the new "proposed" color. Here hue is 0-360
+// 1-19-2021 The attributes of on and off were added in response to issue https://github.com/BobRak/OpenHAB-Smartthings/issues/102
+// These changes were made because OH 3.0 uses color values of on/off. OH 2 and 3.1 don't seem to need this.
 def actionColor(device, attribute, value) {
     log.debug "actionColor: attribute \"${attribute}\", value \"${value}\""
     switch (attribute) {
@@ -889,6 +899,14 @@ def actionColor(device, attribute, value) {
             device.setColor(colormap)
             device.setLevel(value[2] as int)
         break
+        case "off":
+            // log.debug "actionColor: Setting device \"${device}\" with attribute \"${attribute}\" to off"
+            device.off()
+        break
+        case "on":
+            // log.debug "actionColor: Setting device \"${device}\" with attribute \"${attribute}\" to on"
+            device.on()        
+        break
     }
 }
 

bundles/org.openhab.binding.smartthings/src/main/java/org/openhab/binding/smartthings/internal/SmartthingsHandlerFactory.java

@@ -55,9 +55,10 @@ import com.google.gson.Gson;
  * @author Bob Raker - Initial contribution
  */
 @NonNullByDefault
-@Component(service = { ThingHandlerFactory.class,
+@Component(service = { ThingHandlerFactory.class, SmartthingsHubCommand.class,
         EventHandler.class }, configurationPid = "binding.smarthings", property = "event.topics=org/openhab/binding/smartthings/state")
-public class SmartthingsHandlerFactory extends BaseThingHandlerFactory implements ThingHandlerFactory, EventHandler {
+public class SmartthingsHandlerFactory extends BaseThingHandlerFactory
+        implements ThingHandlerFactory, EventHandler, SmartthingsHubCommand {
 
     private final Logger logger = LoggerFactory.getLogger(SmartthingsHandlerFactory.class);
 
@@ -195,4 +196,10 @@ public class SmartthingsHandlerFactory extends BaseThingHandlerFactory implement
     public SmartthingsBridgeHandler getBridgeHandler() {
         return bridgeHandler;
     }
+
+    @Override
+    @Nullable
+    public ThingUID getBridgeUID() {
+        return bridgeHandler.getThing().getUID();
+    }
 }

bundles/org.openhab.binding.smartthings/src/main/java/org/openhab/binding/smartthings/internal/SmartthingsHubCommand.java

@@ -0,0 +1,43 @@
+/**
+ * Copyright (c) 2010-2021 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.smartthings.internal;
+
+import java.util.concurrent.ExecutionException;
+import java.util.concurrent.TimeoutException;
+
+import org.eclipse.jdt.annotation.NonNull;
+import org.openhab.core.thing.ThingUID;
+
+/**
+ * This interface is responsible sending commands to the Smartthings Hub
+ * handlers.
+ *
+ * @author Bob Raker - Initial contribution
+ */
+public interface SmartthingsHubCommand {
+
+    /**
+     * Send a command to the Smartthings Hub
+     *
+     * @param path http path which tells Smartthings what to execute
+     * @param data data to send
+     * @return Response from Smartthings
+     * @throws InterruptedException
+     * @throws TimeoutException
+     * @throws ExecutionException
+     */
+    public void sendDeviceCommand(@NonNull String path, int timeout, @NonNull String data)
+            throws InterruptedException, TimeoutException, ExecutionException;
+
+    public ThingUID getBridgeUID();
+}

bundles/org.openhab.binding.smartthings/src/main/java/org/openhab/binding/smartthings/internal/discovery/SmartthingsDiscoveryService.java

@@ -22,14 +22,13 @@ import java.util.regex.Pattern;
 import org.eclipse.jdt.annotation.NonNullByDefault;
 import org.eclipse.jdt.annotation.Nullable;
 import org.openhab.binding.smartthings.internal.SmartthingsBindingConstants;
-import org.openhab.binding.smartthings.internal.SmartthingsHandlerFactory;
+import org.openhab.binding.smartthings.internal.SmartthingsHubCommand;
 import org.openhab.binding.smartthings.internal.dto.SmartthingsDeviceData;
 import org.openhab.core.config.discovery.AbstractDiscoveryService;
 import org.openhab.core.config.discovery.DiscoveryResult;
 import org.openhab.core.config.discovery.DiscoveryResultBuilder;
 import org.openhab.core.config.discovery.DiscoveryService;
 import org.openhab.core.thing.ThingUID;
-import org.openhab.core.thing.binding.ThingHandlerFactory;
 import org.osgi.service.component.annotations.Component;
 import org.osgi.service.component.annotations.Reference;
 import org.osgi.service.event.Event;
@@ -58,7 +57,7 @@ public class SmartthingsDiscoveryService extends AbstractDiscoveryService implem
 
     private final Gson gson;
 
-    private @Nullable SmartthingsHandlerFactory smartthingsHandlerFactory;
+    private @Nullable SmartthingsHubCommand smartthingsHubCommand;
 
     private @Nullable ScheduledFuture<?> scanningJob;
 
@@ -71,16 +70,14 @@ public class SmartthingsDiscoveryService extends AbstractDiscoveryService implem
     }
 
     @Reference
-    protected void setThingHandlerFactory(ThingHandlerFactory handlerFactory) {
+    protected void setSmartthingsHubCommand(SmartthingsHubCommand hubCommand) {
-        if (handlerFactory instanceof SmartthingsHandlerFactory) {
+        smartthingsHubCommand = hubCommand;
-            smartthingsHandlerFactory = (SmartthingsHandlerFactory) handlerFactory;
-        }
     }
 
-    protected void unsetThingHandlerFactory(ThingHandlerFactory handlerFactory) {
+    protected void unsetSmartthingsHubCommand(SmartthingsHubCommand hubCommand) {
         // Make sure it is this handleFactory that should be unset
-        if (handlerFactory == smartthingsHandlerFactory) {
+        if (hubCommand == smartthingsHubCommand) {
-            this.smartthingsHandlerFactory = null;
+            this.smartthingsHubCommand = null;
         }
     }
 
@@ -129,11 +126,10 @@ public class SmartthingsDiscoveryService extends AbstractDiscoveryService implem
      * Start the discovery process by sending a discovery request to the Smartthings Hub
      */
     private void sendSmartthingsDiscoveryRequest() {
-        final SmartthingsHandlerFactory currentSmartthingsHandlerFactory = smartthingsHandlerFactory;
+        if (smartthingsHubCommand != null) {
-        if (currentSmartthingsHandlerFactory != null) {
             try {
                 String discoveryMsg = "{\"discovery\": \"yes\"}";
-                currentSmartthingsHandlerFactory.sendDeviceCommand("/discovery", 5, discoveryMsg);
+                smartthingsHubCommand.sendDeviceCommand("/discovery", 5, discoveryMsg);
                 // Smartthings will not return a response to this message but will send it's response message
                 // which will get picked up by the SmartthingBridgeHandler.receivedPushMessage handler
             } catch (InterruptedException | TimeoutException | ExecutionException e) {
@@ -188,14 +184,11 @@ public class SmartthingsDiscoveryService extends AbstractDiscoveryService implem
         }
         String deviceNameNoSpaces = name.replaceAll("\\s", "_");
         String smartthingsDeviceName = findIllegalChars.matcher(deviceNameNoSpaces).replaceAll("");
-        final SmartthingsHandlerFactory currentSmartthingsHandlerFactory = smartthingsHandlerFactory;
+        if (smartthingsHubCommand == null) {
-        if (currentSmartthingsHandlerFactory == null) {
+            logger.info("SmartthingsHubCommand is unexpectedly null, could not create device {}", deviceData);
-            logger.info(
-                    "SmartthingsDiscoveryService: smartthingshandlerfactory is unexpectedly null, could not create device {}",
-                    deviceData);
             return;
         }
-        ThingUID bridgeUid = currentSmartthingsHandlerFactory.getBridgeHandler().getThing().getUID();
+        ThingUID bridgeUid = smartthingsHubCommand.getBridgeUID();
         String bridgeId = bridgeUid.getId();
         String uidStr = String.format("smartthings:%s:%s:%s", deviceData.capability, bridgeId, smartthingsDeviceName);