[lutron] Minor code and doc updates (#8795)

* [lutron] Minor code and doc updates
* [lutron] Address review comments

Signed-off-by: Bob Adair <bob.github@att.net>

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

@@ -1,18 +1,18 @@
 # Lutron Binding
 
 This binding integrates with [Lutron](http://www.lutron.com) lighting control and home automation systems.
-It contains separate binding support for four different types of Lutron systems:
+It contains support for four different types of Lutron systems via different bridge things:
 
-* RadioRA 2, HomeWorks QS, and other systems that can be controlled by Lutron Integration Protocol (LIP) or LEAP, such as RA2 Select, and Caseta
+* RadioRA 2, HomeWorks QS, Caseta, RA2 Select, and other current systems that can be controlled via Lutron Integration Protocol (LIP) or LEAP
 * The original RadioRA system, referred to here as RadioRA Classic
 * Legacy HomeWorks RS232 Processors
 * Grafik Eye 3x/4x systems with GRX-PRG or GRX-CI-PRG control interfaces
 
 Each is described in a separate section below.
 
-# Lutron RadioRA 2/HomeWorks QS/Caseta Binding
+# Lutron RadioRA 2/HomeWorks QS/RA2 Select/Caseta Binding
 
-**Note:** While the Lutron Integration Protocol used by this binding should largely be compatible with other current Lutron systems, this binding has only been fully tested with RadioRA 2, HomeWorks QS, and Caseta with Smart Bridge Pro.
+**Note:** While the Lutron Integration Protocol used by ipbridge in this binding should largely be compatible with other current Lutron systems, it has only been fully tested with RadioRA 2, HomeWorks QS, and Caseta with Smart Bridge Pro.
 Homeworks QS support is still a work in progress, since not all features/devices are supported yet.
 RA2 Select systems work with the binding, but full support for all devices still needs to be confirmed.
 Caseta Smart Bridge (non-Pro model) support and support for Caseta occupancy sensors is available only through the experimental leapbridge thing.
@@ -49,16 +49,20 @@ This binding currently supports the following thing types:
 ## Discovery
 
 Full discovery is supported for RadioRA 2 and HomeWorks QS systems.
-Both the main repeaters/processors themselves and the devices connected to them can be automatically discovered.
+Both the main repeaters/processors themselves and the devices connected to them will be automatically discovered.
 Discovered repeaters/processors will be accessed using the default integration credentials.
 These can be changed in the bridge thing configuration.
-Discovered keypad devices should now have their model parameters automatically set to the correct value.
+Discovered keypad devices should have their model parameters automatically set to the correct value.
 
-Caseta Smart Bridge hubs, Smart Bridge Pro 2 hubs, and RA2 Select main repeaters should now be discovered automatically via mDNS.
+Caseta Smart Bridge hubs, Smart Bridge Pro 2 hubs, and RA2 Select main repeaters should be discovered automatically via mDNS.
-Devices attached to them still need to be configured manually unless the experimental leapbridge is used.
+Devices attached to them need to be configured manually when using ipbridge.
+The experimental leapbridge supports full automated discovery of these systems, but authentication information must be manually entered.
 
 Other supported Lutron systems must be configured manually.
 
+**Note:** Discovery selects ipbridge for HomeWorks QS, RadioRA 2, RA2 Select, and Caseta Smart Bridge Pro.
+It select leapbridge for Caseta Smart Bridge, since only LEAP protocol is supported by this system.
+
 ## Binding Configuration
 
 This binding does not require any special configuration.
@@ -71,9 +75,9 @@ If a thing will not come online, but instead has the status "UNKNOWN: Awaiting i
 
 ### Bridges
 
-Two different bridges are now supported by the binding, ipbridge and leapbridge.
+Two different bridges are now supported by the binding for current Lutron systems, ipbridge and leapbridge.
 The LIP protocol is supported by ipbridge while the LEAP protocol is supported by leapbridge.
-Current Lutron systems support one or both protocols as shown below.
+Current systems support one or both protocols as shown below.
 
 |Bridge Device           | LIP | LEAP |
 |------------------------|-----|------|

bundles/org.openhab.binding.lutron/doc/leapnotes.md

@@ -3,8 +3,8 @@
 Unlike LIP, which was designed to use a simple serial or telnet connection and authenticates using a username/password, LEAP uses a SSL connection and authenticates using certificates.
 This necessarily makes configuration more complicated.
 There are several open source utilities available for generating the certificate files necessary to access your Caseta or RA2 Select hub.
-One good choice is the get_lutron_cert.py script included with the pylutron library which is available on Github at https://github.com/gurumitts/pylutron-caseta .
+One good choice is the get_lutron_cert.py script included with the popular pylutron library which is available on Github at https://github.com/gurumitts/pylutron-caseta .
-On a unix system, you can easily retrieve it using curl with a command like:
+On a unix-like system, you can easily retrieve it using curl with a command like:
 
 ```
 curl https://raw.githubusercontent.com/gurumitts/pylutron-caseta/dev/get_lutron_cert.py >get_lutron_cert.py
@@ -30,11 +30,10 @@ keytool -importcert -file caseta-bridge.crt -keystore lutron.keystore -alias cas
 ```
 
 Respond to the password prompt(s) with a password, and then use that password in the -srcstorepass parameter of the keytool command and in the keystorePassword parameter for leapbridge.
-In the example above, the pkcs12 store password was set to “secret”, but hopefully you can think of a better one.
+In the example above, the pkcs12 store password was set to “secret”, but hopefully you can think of a better one!
 The lutron.keystore file that you end up with is the one you’ll need to give the binding access to.
 The caseta.p12 file is just an intermediate file that you can delete later.
 
 Finally you’ll then need to set the ipAddress, keystore, and keystorePassword parameters of the leapbridge thing.
 The ipAddress will be set for you if you used discovery to detect a Caseta Smart Bridge.
-
+This should also work with DHCP, although setting a static IP address for your bridge is still recommended.
-

bundles/org.openhab.binding.lutron/src/main/java/org/openhab/binding/lutron/internal/StringUtils.java

@@ -0,0 +1,36 @@
+/**
+ * Copyright (c) 2010-2020 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.lutron.internal;
+
+import java.util.Objects;
+
+import org.eclipse.jdt.annotation.NonNullByDefault;
+import org.eclipse.jdt.annotation.Nullable;
+
+/**
+ * Supply some string utility methods formerly provided by org.apache.commons.lang.StringUtils.
+ *
+ * @author Bob Adair - Initial contribution
+ *
+ */
+@NonNullByDefault
+public class StringUtils {
+
+    public static boolean equals(@Nullable String s1, @Nullable String s2) {
+        return Objects.equals(s1, s2);
+    }
+
+    public static boolean isEmpty(@Nullable String s1) {
+        return (s1 == null || s1.isEmpty());
+    }
+}

bundles/org.openhab.binding.lutron/src/main/java/org/openhab/binding/lutron/internal/config/IPBridgeConfig.java

@@ -12,7 +12,7 @@
  */
 package org.openhab.binding.lutron.internal.config;
 
-import org.apache.commons.lang.StringUtils;
+import org.openhab.binding.lutron.internal.StringUtils;
 
 /**
  * Configuration settings for an {@link org.openhab.binding.lutron.internal.handler.IPBridgeHandler}.

bundles/org.openhab.binding.lutron/src/main/java/org/openhab/binding/lutron/internal/grxprg/GrafikEyeHandler.java

@@ -15,7 +15,6 @@ package org.openhab.binding.lutron.internal.grxprg;
 import java.util.concurrent.ScheduledFuture;
 import java.util.concurrent.TimeUnit;
 
-import org.apache.commons.lang.NullArgumentException;
 import org.openhab.core.library.types.DecimalType;
 import org.openhab.core.library.types.IncreaseDecreaseType;
 import org.openhab.core.library.types.OnOffType;
@@ -332,12 +331,12 @@ public class GrafikEyeHandler extends BaseThingHandler {
     private PrgProtocolHandler getProtocolHandler() {
         final Bridge bridge = getBridge();
         if (bridge == null || !(bridge.getHandler() instanceof PrgBridgeHandler)) {
-            throw new NullArgumentException("Cannot have a Grafix Eye thing outside of the PRG bridge");
+            throw new IllegalArgumentException("Cannot have a Grafix Eye thing outside of the PRG bridge");
         }
 
         final PrgProtocolHandler handler = ((PrgBridgeHandler) bridge.getHandler()).getProtocolHandler();
         if (handler == null) {
-            throw new NullArgumentException("No protocol handler set in the PrgBridgeHandler!");
+            throw new IllegalArgumentException("No protocol handler set in the PrgBridgeHandler!");
         }
         return handler;
     }

bundles/org.openhab.binding.lutron/src/main/java/org/openhab/binding/lutron/internal/grxprg/PrgProtocolHandler.java

@@ -24,7 +24,6 @@ import java.util.concurrent.TimeUnit;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 
-import org.apache.commons.lang.NullArgumentException;
 import org.openhab.core.library.types.DateTimeType;
 import org.openhab.core.library.types.DecimalType;
 import org.openhab.core.library.types.OnOffType;
@@ -629,11 +628,11 @@ class PrgProtocolHandler {
      * Sets the time on the PRG interface
      *
      * @param calendar a non-null calendar to set the time to
-     * @throws NullArgumentException if calendar is null
+     * @throws IllegalArgumentException if calendar is null
      */
     void setTime(Calendar calendar) {
         if (calendar == null) {
-            throw new NullArgumentException("calendar cannot be null");
+            throw new IllegalArgumentException("calendar cannot be null");
         }
         final String cmd = String.format("%1 %2$tk %2$tM %2$tm %2$te %2ty %3", CMD_SETTIME, calendar,
                 calendar.get(Calendar.DAY_OF_WEEK));

bundles/org.openhab.binding.lutron/src/main/java/org/openhab/binding/lutron/internal/handler/CcoHandler.java

@@ -15,8 +15,8 @@ package org.openhab.binding.lutron.internal.handler;
 import static org.openhab.binding.lutron.internal.LutronBindingConstants.*;
 
 import java.math.BigDecimal;
+import java.util.Arrays;
 
-import org.apache.commons.lang.StringUtils;
 import org.eclipse.jdt.annotation.NonNullByDefault;
 import org.eclipse.jdt.annotation.Nullable;
 import org.openhab.binding.lutron.internal.protocol.OutputCommand;
@@ -181,7 +181,7 @@ public class CcoHandler extends LutronHandler {
 
     @Override
     public void handleUpdate(LutronCommandType type, String... parameters) {
-        logger.debug("Update received for CCO: {} {}", type, StringUtils.join(parameters, ","));
+        logger.debug("Update received for CCO: {} {}", type, Arrays.asList(parameters));
 
         if (outputType == CcoOutputType.MAINTAINED) {
             if (type == LutronCommandType.OUTPUT && parameters.length > 1
@@ -193,7 +193,7 @@ public class CcoHandler extends LutronHandler {
                     BigDecimal state = new BigDecimal(parameters[1]);
                     updateState(CHANNEL_SWITCH, state.compareTo(BigDecimal.ZERO) == 0 ? OnOffType.OFF : OnOffType.ON);
                 } catch (NumberFormatException e) {
-                    logger.warn("Unable to parse update {} {} from CCO {}", type, StringUtils.join(parameters, ","),
+                    logger.warn("Unable to parse update {} {} from CCO {}", type, Arrays.asList(parameters),
                             integrationId);
                     return;
                 }

bundles/org.openhab.binding.lutron/src/main/java/org/openhab/binding/lutron/internal/handler/IPBridgeHandler.java

@@ -25,7 +25,7 @@ import java.util.regex.MatchResult;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 
-import org.apache.commons.lang.StringUtils;
+import org.openhab.binding.lutron.internal.StringUtils;
 import org.openhab.binding.lutron.internal.config.IPBridgeConfig;
 import org.openhab.binding.lutron.internal.discovery.LutronDeviceDiscoveryService;
 import org.openhab.binding.lutron.internal.net.TelnetSession;

bundles/org.openhab.binding.lutron/src/main/java/org/openhab/binding/lutron/internal/handler/LeapBridgeHandler.java

@@ -281,8 +281,8 @@ public class LeapBridgeHandler extends LutronBridgeHandler implements LeapMessag
             updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.CONFIGURATION_ERROR, "Invalid port number");
             return;
         } catch (InterruptedIOException e) {
-            Thread.currentThread().interrupt();
             logger.debug("Interrupted while establishing connection");
+            Thread.currentThread().interrupt();
             return;
         } catch (IOException e) {
             updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.COMMUNICATION_ERROR,
@@ -337,9 +337,6 @@ public class LeapBridgeHandler extends LutronBridgeHandler implements LeapMessag
     private synchronized void disconnect(boolean interruptAll) {
         logger.debug("Disconnecting");
 
-        Thread senderThread = this.senderThread;
-        Thread readerThread = this.readerThread;
-
         ScheduledFuture<?> connectRetryJob = this.connectRetryJob;
         if (connectRetryJob != null) {
             connectRetryJob.cancel(true);
@@ -351,9 +348,12 @@ public class LeapBridgeHandler extends LutronBridgeHandler implements LeapMessag
 
         reconnectTaskCancel(interruptAll); // May be called from keepAliveReconnectJob thread
 
+        Thread senderThread = this.senderThread;
         if (senderThread != null && senderThread.isAlive()) {
             senderThread.interrupt();
         }
+
+        Thread readerThread = this.readerThread;
         if (readerThread != null && readerThread.isAlive()) {
             readerThread.interrupt();
         }

bundles/org.openhab.binding.lutron/src/main/java/org/openhab/binding/lutron/internal/protocol/DeviceCommand.java

@@ -102,7 +102,10 @@ public class DeviceCommand extends LutronCommandNew {
         } else if (targetType == TargetType.VIRTUALKEYPAD) {
             if (action.equals(DeviceCommand.ACTION_PRESS)) {
                 return new LeapCommand(Request.virtualButtonCommand(component, CommandType.PRESSANDRELEASE));
-            } else if (!action.equals(DeviceCommand.ACTION_RELEASE)) {
+            } else if (action.equals(DeviceCommand.ACTION_RELEASE)) {
+                logger.trace("Ignoring release command for virtual keypad button.");
+                return null;
+            } else {
                 logger.debug("Ignoring device command with unsupported action.");
                 return null;
             }

bundles/org.openhab.binding.lutron/src/main/java/org/openhab/binding/lutron/internal/protocol/leap/LeapMessageParser.java

@@ -38,7 +38,11 @@ import com.google.gson.JsonParser;
 import com.google.gson.JsonSyntaxException;
 
 /**
- * Class responsible for parsing incoming LEAP messages
+ * Class responsible for parsing incoming LEAP messages. Calls back to an object implementing the
+ * LeapMessageParserCallbacks interface.
+ *
+ * Thanks to the authors of the pylutron-caseta Python API (github.com/gurumitts/pylutron-caseta), which I used as a
+ * reference when first researching the LEAP protocol.
  *
  * @author Bob Adair - Initial contribution
  */

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

@@ -6,7 +6,7 @@
 
 	<bridge-type id="ipbridge">
 		<label>Lutron IP Access Point</label>
-		<description>A Lutron controller using Lutron Integration Protocol (LIP) over TCP/IP</description>
+		<description>Lutron controller using Lutron Integration Protocol (LIP) over TCP/IP</description>
 		<properties>
 			<property name="vendor">Lutron</property>
 		</properties>
@@ -59,7 +59,7 @@
 
 	<bridge-type id="leapbridge">
 		<label>Lutron LEAP Access Point</label>
-		<description>A Lutron controller using LEAP protocol over TCP/IP</description>
+		<description>Lutron controller using LEAP protocol over TCP/IP</description>
 
 		<channels>
 			<channel id="command" typeId="command-type"/>
@@ -132,8 +132,9 @@
 			<bridge-type-ref id="leapbridge"/>
 		</supported-bridge-type-refs>
 
-		<label>Maestro Dimmer</label>
+		<label>Lutron Dimmer</label>
 		<description>Controls dimmable loads</description>
+		<category>Lightbulb</category>
 
 		<channels>
 			<channel id="lightlevel" typeId="lightDimmer"/>
@@ -202,8 +203,9 @@
 			<bridge-type-ref id="leapbridge"/>
 		</supported-bridge-type-refs>
 
-		<label>Sivoia QS Shade</label>
+		<label>Lutron Shade</label>
-		<description>Controls roller shades</description>
+		<description>Controls roller shades, drapes, and motor controllers</description>
+		<category>Blinds</category>
 
 		<channels>
 			<channel id="shadelevel" typeId="shadeControl"/>
@@ -258,8 +260,9 @@
 			<bridge-type-ref id="leapbridge"/>
 		</supported-bridge-type-refs>
 
-		<label>Maestro Switch</label>
+		<label>Lutron Switch</label>
 		<description>On/off switch</description>
+		<category>WallSwitch</category>
 
 		<channels>
 			<channel id="switchstatus" typeId="switchState"/>
@@ -368,6 +371,7 @@
 
 		<label>Radio Powr Savr Sensor</label>
 		<description>Motion sensor to detect occupancy status</description>
+		<category>MotionDetector</category>
 
 		<channels>
 			<channel id="occupancystatus" typeId="occupiedState"/>
@@ -411,6 +415,7 @@
 
 		<label>Occupancy Group</label>
 		<description>Shows state of occupancy sensor group</description>
+		<category>MotionDetector</category>
 
 		<channels>
 			<channel id="groupstate" typeId="groupState"/>
@@ -642,7 +647,7 @@
 			<bridge-type-ref id="ipbridge"/>
 		</supported-bridge-type-refs>
 
-		<label>GRAFIK Eye QS</label>
+		<label>GRAFIK Eye QS Keypad</label>
 		<description>Lutron GRAFIK Eye QS for RadioRA 2/HomeWorks QS</description>
 
 		<representation-property>integrationId</representation-property>
@@ -717,7 +722,6 @@
 		</config-description>
 	</thing-type>
 
-
 	<thing-type id="virtualkeypad">
 		<supported-bridge-type-refs>
 			<bridge-type-ref id="ipbridge"/>
@@ -838,7 +842,7 @@
 
 	<bridge-type id="prgbridge">
 		<label>Lutron GRX-PRG or GRX-CI-PRG Bridge</label>
-		<description>Ethernet access point to Lutron Grafik Eye 3x/4x Systems</description>
+		<description>Ethernet access point to Lutron GRAFIK Eye 3x/4x Systems</description>
 
 		<channels>
 			<channel id="buttonpress" typeId="buttonpress"/>
@@ -912,8 +916,8 @@
 			<bridge-type-ref id="prgbridge"/>
 		</supported-bridge-type-refs>
 
-		<label>Grafik Eye</label>
+		<label>GRAFIK Eye</label>
-		<description>Controls a Grafik Eye</description>
+		<description>Controls a GRAFIK Eye</description>
 
 		<channels>
 			<channel id="scene" typeId="scene"/>
@@ -1082,7 +1086,7 @@
 
 	<bridge-type id="hwserialbridge">
 		<label>Lutron HomeWorks RS232 Bridge</label>
-		<description>RS232 access point to Lutron HomeWorks lighting control system</description>
+		<description>RS232 access point to Legacy Lutron HomeWorks lighting control system</description>
 
 		<config-description>
 			<parameter name="serialPort" type="text" required="true">
@@ -1138,8 +1142,9 @@
 			<bridge-type-ref id="hwserialbridge"/>
 		</supported-bridge-type-refs>
 
-		<label>HomeWorks Dimmer</label>
+		<label>Lutron Dimmer (Legacy HomeWorks)</label>
-		<description>Controls dimmable loads</description>
+		<description>Controls dimmable loads for legacy HomeWorks systems</description>
+		<category>Lightbulb</category>
 
 		<channels>
 			<channel id="lightlevel" typeId="lightDimmer"/>
@@ -1165,7 +1170,7 @@
 
 	<bridge-type id="ra-rs232">
 		<label>Lutron RadioRA RS232</label>
-		<description>RS-232 access to Lutron RadioRA</description>
+		<description>RS-232 access to legacy Lutron RadioRA systems</description>
 
 		<config-description>
 			<parameter name="portName" type="text" required="true">
@@ -1185,8 +1190,9 @@
 		<supported-bridge-type-refs>
 			<bridge-type-ref id="ra-rs232"/>
 		</supported-bridge-type-refs>
-		<label>RadioRA Dimmer</label>
+		<label>Lutron Dimmer (Legacy RadioRA)</label>
-		<description>RadioRA Dimmer</description>
+		<description>Controls dimmable loads for legacy RadioRA systems</description>
+		<category>Lightbulb</category>
 
 		<channels>
 			<channel id="lightlevel" typeId="lightDimmer"/>
@@ -1212,8 +1218,9 @@
 		<supported-bridge-type-refs>
 			<bridge-type-ref id="ra-rs232"/>
 		</supported-bridge-type-refs>
-		<label>RadioRA Switch</label>
+		<label>Lutron Switch (Legacy RadioRA)</label>
-		<description>RadioRA Switch</description>
+		<description>On/off switch for Legacy RadioRA systems</description>
+		<category>WallSwitch</category>
 
 		<channels>
 			<channel id="switchstatus" typeId="switchState"/>
@@ -1231,8 +1238,8 @@
 		<supported-bridge-type-refs>
 			<bridge-type-ref id="ra-rs232"/>
 		</supported-bridge-type-refs>
-		<label>RadioRA Phantom Button</label>
+		<label>Phantom Button (Legacy RadioRA)</label>
-		<description>RadioRA Phantom Button</description>
+		<description>Phantom Button for Legacy RadioRA systems</description>
 
 		<channels>
 			<channel id="switchstatus" typeId="switchState"/>