[unifi] Various stability improvements

bundles/org.openhab.binding.unifi/src/main/java/org/openhab/binding/unifi/internal/api/UniFiCommunicationException.java

@@ -22,7 +22,11 @@
 @NonNullByDefault
 public class UniFiCommunicationException extends UniFiException {
 
-    private static final long serialVersionUID = -7261308872245069364L;
+    private static final long serialVersionUID = 1L;
+
+    public UniFiCommunicationException(final String message) {
+        super(message);
+    }
 
     public UniFiCommunicationException(final Throwable cause) {
         super(cause);

bundles/org.openhab.binding.unifi/src/main/java/org/openhab/binding/unifi/internal/api/UniFiControllerRequest.java

@@ -47,6 +47,7 @@
 import com.google.gson.Gson;
 import com.google.gson.GsonBuilder;
 import com.google.gson.JsonObject;
+import com.google.gson.JsonParseException;
 import com.google.gson.JsonParser;
 
 /**
@@ -59,6 +60,8 @@
 @NonNullByDefault
 class UniFiControllerRequest<T> {
 
+    private static final String CONTROLLER_PARSE_ERROR = "@text/error.controller.parse_error";
+
     private static final String CONTENT_TYPE_APPLICATION_JSON_UTF_8 = MimeTypes.Type.APPLICATION_JSON_UTF_8.asString();
 
     private static final long TIMEOUT_SECONDS = 5;
@@ -128,10 +131,20 @@ public void setQueryParameter(final String key, final Object value) {
         final String json = getContent();
         // mgb: only try and unmarshall non-void result types
         if (!Void.class.equals(resultType)) {
-            final JsonObject jsonObject = JsonParser.parseString(json).getAsJsonObject();
+            try {
+                final JsonObject jsonObject = JsonParser.parseString(json).getAsJsonObject();
 
-            if (jsonObject.has(PROPERTY_DATA) && jsonObject.get(PROPERTY_DATA).isJsonArray()) {
-                result = (T) gson.fromJson(jsonObject.getAsJsonArray(PROPERTY_DATA), resultType);
+                if (jsonObject.has(PROPERTY_DATA) && jsonObject.get(PROPERTY_DATA).isJsonArray()) {
+                    result = (T) gson.fromJson(jsonObject.getAsJsonArray(PROPERTY_DATA), resultType);
+                }
+            } catch (final JsonParseException e) {
+                logger.debug(
+                        "Could not parse content retreived from the server. Is the configuration poiting to the right server/port?, {}",
+                        e.getMessage());
+                if (logger.isTraceEnabled()) {
+                    prettyPrintJson(json);
+                }
+                throw new UniFiCommunicationException(CONTROLLER_PARSE_ERROR);
             }
         }
         return result;
@@ -182,7 +195,9 @@ private Response getContentResponse(final InputStreamResponseListener listener)
         } catch (final ExecutionException e) {
             // mgb: unwrap the cause and try to cleanly handle it
             final Throwable cause = e.getCause();
-            if (cause instanceof UnknownHostException) {
+            if (cause instanceof TimeoutException) {
+                throw new UniFiCommunicationException(e);
+            } else if (cause instanceof UnknownHostException) {
                 // invalid hostname
                 throw new UniFiInvalidHostException(cause);
             } else if (cause instanceof ConnectException) {
@@ -242,13 +257,14 @@ private Request newRequest() {
         return request;
     }
 
-    private static String prettyPrintJson(final String content) {
+    private String prettyPrintJson(final String content) {
         try {
             final JsonObject json = JsonParser.parseString(content).getAsJsonObject();
             final Gson prettyGson = new GsonBuilder().setPrettyPrinting().create();
 
             return prettyGson.toJson(json);
         } catch (final RuntimeException e) {
+            logger.debug("RuntimeException pretty printing json. Returning the raw content.", e);
             // If could not parse the string as json, just return the string
             return content;
         }

bundles/org.openhab.binding.unifi/src/main/java/org/openhab/binding/unifi/internal/api/cache/UniFiCache.java

@@ -16,7 +16,9 @@
 import java.util.HashMap;
 import java.util.Locale;
 import java.util.Map;
+import java.util.Objects;
 import java.util.stream.Collectors;
+import java.util.stream.Stream;
 
 import org.eclipse.jdt.annotation.NonNullByDefault;
 import org.eclipse.jdt.annotation.Nullable;
@@ -103,13 +105,12 @@ public void clear() {
 
     public final void putAll(final T @Nullable [] values) {
         if (values != null) {
-            logger.debug("Put #{} entries in {}: {}", values.length, getClass().getSimpleName(),
-                    lazyFormatAsList(values));
-            for (final T value : values) {
-                if (value != null) {
-                    put(value.getId(), value);
-                }
+            if (logger.isDebugEnabled()) {
+                logger.debug("Put #{} entries in {}: {}", values.length, getClass().getSimpleName(),
+                        Stream.of(values).filter(Objects::nonNull).map(Object::toString)
+                                .collect(Collectors.joining(System.lineSeparator() + " - ")));
             }
+            Stream.of(values).filter(Objects::nonNull).forEach(value -> put(value.getId(), value));
         }
     }
 
@@ -133,18 +134,4 @@ public final Collection<T> values() {
     }
 
     protected abstract @Nullable String getSuffix(T value, Prefix prefix);
-
-    private static Object lazyFormatAsList(final Object[] arr) {
-        return new Object() {
-
-            @Override
-            public String toString() {
-                String value = "";
-                for (final Object o : arr) {
-                    value += "\n - " + o.toString();
-                }
-                return value;
-            }
-        };
-    }
 }

bundles/org.openhab.binding.unifi/src/main/java/org/openhab/binding/unifi/internal/api/cache/UniFiControllerCache.java

@@ -17,7 +17,7 @@
 import java.util.Map;
 import java.util.Objects;
 import java.util.concurrent.ConcurrentHashMap;
-import java.util.function.Function;
+import java.util.function.Predicate;
 import java.util.stream.Stream;
 
 import org.eclipse.jdt.annotation.NonNullByDefault;
@@ -138,8 +138,8 @@ public Collection<UniFiClient> getClients() {
         return clientsCache.values();
     }
 
-    public long countClients(final UniFiSite site, final Function<UniFiClient, Boolean> filter) {
-        return getClients().stream().filter(c -> site.isSite(c.getSite())).filter(filter::apply).count();
+    public long countClients(final UniFiSite site, final Predicate<UniFiClient> filter) {
+        return getClients().stream().filter(c -> site.isSite(c.getSite())).filter(filter::test).count();
     }
 
     public @Nullable UniFiClient getClient(@Nullable final String cid) {

bundles/org.openhab.binding.unifi/src/main/java/org/openhab/binding/unifi/internal/api/dto/UniFiClient.java

@@ -101,10 +101,10 @@ public boolean isBlocked() {
         return blocked;
     }
 
-    public abstract Boolean isWired();
+    public abstract boolean isWired();
 
-    public final Boolean isWireless() {
-        return isWired() == null ? null : Boolean.FALSE.equals(isWired());
+    public final boolean isWireless() {
+        return !isWired();
     }
 
     protected abstract String getDeviceMac();

bundles/org.openhab.binding.unifi/src/main/java/org/openhab/binding/unifi/internal/api/dto/UniFiUnknownClient.java

@@ -29,8 +29,8 @@ public UniFiUnknownClient(final UniFiControllerCache cache) {
     }
 
     @Override
-    public Boolean isWired() {
-        return null; // mgb: no is_wired property in the json
+    public boolean isWired() {
+        return false; // mgb: no is_wired property in the json
     }
 
     @Override

bundles/org.openhab.binding.unifi/src/main/java/org/openhab/binding/unifi/internal/api/dto/UniFiWiredClient.java

@@ -30,7 +30,7 @@ public UniFiWiredClient(final UniFiControllerCache cache) {
     }
 
     @Override
-    public Boolean isWired() {
+    public boolean isWired() {
         return true;
     }
 

bundles/org.openhab.binding.unifi/src/main/java/org/openhab/binding/unifi/internal/api/dto/UniFiWirelessClient.java

@@ -38,7 +38,7 @@ public UniFiWirelessClient(final UniFiControllerCache cache) {
     }
 
     @Override
-    public Boolean isWired() {
+    public boolean isWired() {
         return false;
     }
 

bundles/org.openhab.binding.unifi/src/main/java/org/openhab/binding/unifi/internal/handler/UniFiClientThingHandler.java

@@ -165,7 +165,7 @@ private synchronized boolean isClientHome(final UniFiClient client) {
     protected State getChannelState(final UniFiClient client, final String channelId) {
         final boolean clientHome = isClientHome(client);
         final UniFiDevice device = client.getDevice();
-        final UniFiSite site = (device == null ? null : device.getSite());
+        final UniFiSite site = device == null ? null : device.getSite();
         State state = getDefaultState(channelId);
 
         switch (channelId) {

bundles/org.openhab.binding.unifi/src/main/java/org/openhab/binding/unifi/internal/handler/UniFiControllerThingHandler.java

@@ -55,10 +55,10 @@
 @NonNullByDefault
 public class UniFiControllerThingHandler extends BaseBridgeHandler {
 
-    private static final String STATUS_DESCRIPTION_COMMUNICATION_ERROR = "@text/error.bridge.offline.communication_error";
-    private static final String STATUS_DESCRIPTION_SSL_ERROR = "@text/error.bridge.offline.ssl_error";
-    private static final String STATUS_DESCRIPTION_INVALID_CREDENTIALS = "@text/error.bridge.offline.invalid_credentials";
-    private static final String STATUS_DESCRIPTION_INVALID_HOSTNAME = "@text/error.bridge.offline.invalid_hostname";
+    private static final String STATUS_DESCRIPTION_COMMUNICATION_ERROR = "@text/error.bridge.offline.communication_error ";
+    private static final String STATUS_DESCRIPTION_SSL_ERROR = "@text/error.bridge.offline.ssl_error ";
+    private static final String STATUS_DESCRIPTION_INVALID_CREDENTIALS = "@text/error.bridge.offline.invalid_credentials ";
+    private static final String STATUS_DESCRIPTION_INVALID_HOSTNAME = "@text/error.bridge.offline.invalid_hostname ";
 
     private final Logger logger = LoggerFactory.getLogger(UniFiControllerThingHandler.class);
 
@@ -138,14 +138,14 @@ private void start(final UniFiController uc) {
             uc.start();
             startRefresh = true;
         } catch (final UniFiCommunicationException e) {
-            updateStatus(OFFLINE, COMMUNICATION_ERROR, STATUS_DESCRIPTION_COMMUNICATION_ERROR);
+            updateStatus(OFFLINE, COMMUNICATION_ERROR, STATUS_DESCRIPTION_COMMUNICATION_ERROR + e.getMessage());
             startRefresh = true;
         } catch (final UniFiInvalidHostException e) {
-            updateStatus(OFFLINE, CONFIGURATION_ERROR, STATUS_DESCRIPTION_INVALID_HOSTNAME);
+            updateStatus(OFFLINE, CONFIGURATION_ERROR, STATUS_DESCRIPTION_INVALID_HOSTNAME + e.getMessage());
         } catch (final UniFiSSLException e) {
-            updateStatus(OFFLINE, CONFIGURATION_ERROR, STATUS_DESCRIPTION_SSL_ERROR);
+            updateStatus(OFFLINE, CONFIGURATION_ERROR, STATUS_DESCRIPTION_SSL_ERROR + e.getMessage());
         } catch (final UniFiInvalidCredentialsException e) {
-            updateStatus(OFFLINE, CONFIGURATION_ERROR, STATUS_DESCRIPTION_INVALID_CREDENTIALS);
+            updateStatus(OFFLINE, CONFIGURATION_ERROR, STATUS_DESCRIPTION_INVALID_CREDENTIALS + e.getMessage());
         } catch (final UniFiException e) {
             logger.debug("Unknown error while configuring the UniFi Controller", e);
             updateStatus(OFFLINE, CONFIGURATION_ERROR, e.getMessage());
@@ -174,9 +174,9 @@ private void run() {
             refresh();
             updateStatus(ONLINE);
         } catch (final UniFiCommunicationException e) {
-            updateStatus(OFFLINE, COMMUNICATION_ERROR, STATUS_DESCRIPTION_COMMUNICATION_ERROR);
+            updateStatus(OFFLINE, COMMUNICATION_ERROR, STATUS_DESCRIPTION_COMMUNICATION_ERROR + e.getMessage());
         } catch (final UniFiInvalidCredentialsException e) {
-            updateStatus(OFFLINE, CONFIGURATION_ERROR, STATUS_DESCRIPTION_INVALID_CREDENTIALS);
+            updateStatus(OFFLINE, CONFIGURATION_ERROR, STATUS_DESCRIPTION_INVALID_CREDENTIALS + e.getMessage());
         } catch (final RuntimeException | UniFiException e) {
             logger.debug("Unhandled exception while refreshing the UniFi Controller {}", getThing().getUID(), e);
             updateStatus(OFFLINE, COMMUNICATION_ERROR, e.getMessage());

bundles/org.openhab.binding.unifi/src/main/java/org/openhab/binding/unifi/internal/handler/UniFiPoePortThingHandler.java

@@ -25,9 +25,8 @@
 import static org.openhab.binding.unifi.internal.UniFiBindingConstants.CHANNEL_PORT_POE_VOLTAGE;
 import static org.openhab.core.library.unit.MetricPrefix.MILLI;
 
-import javax.measure.quantity.ElectricCurrent;
-import javax.measure.quantity.ElectricPotential;
-import javax.measure.quantity.Power;
+import javax.measure.Quantity;
+import javax.measure.Unit;
 
 import org.eclipse.jdt.annotation.NonNullByDefault;
 import org.eclipse.jdt.annotation.Nullable;
@@ -126,20 +125,29 @@ protected State getChannelState(final UniFiSwitchPorts ports, final String chann
                 state = StringType.valueOf(port.getPoeMode());
                 break;
             case CHANNEL_PORT_POE_POWER:
-                state = new QuantityType<Power>(Double.valueOf(port.getPoePower()), Units.WATT);
+                state = safeDouble(port.getPoePower(), Units.WATT);
                 break;
             case CHANNEL_PORT_POE_VOLTAGE:
-                state = new QuantityType<ElectricPotential>(Double.valueOf(port.getPoeVoltage()), Units.VOLT);
+                state = safeDouble(port.getPoeVoltage(), Units.VOLT);
                 break;
             case CHANNEL_PORT_POE_CURRENT:
-                state = new QuantityType<ElectricCurrent>(Double.valueOf(port.getPoeCurrent()), MILLI(Units.AMPERE));
+                state = safeDouble(port.getPoeCurrent(), MILLI(Units.AMPERE));
                 break;
             default:
                 state = UnDefType.UNDEF;
         }
         return state;
     }
 
+    private <Q extends Quantity<Q>> State safeDouble(final String value, final Unit<Q> unit) {
+        try {
+            return value == null ? UnDefType.UNDEF : QuantityType.valueOf(Double.parseDouble(value), unit);
+        } catch (final NumberFormatException e) {
+            logger.debug("Could not parse value '{}' for unit {}", value, unit);
+            return UnDefType.UNDEF;
+        }
+    }
+
     private State setOfflineOnNoPoEPortData() {
         if (getThing().getStatus() != ThingStatus.OFFLINE) {
             updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.COMMUNICATION_ERROR,

bundles/org.openhab.binding.unifi/src/main/java/org/openhab/binding/unifi/internal/handler/UniFiThingDiscoveryService.java

@@ -127,8 +127,9 @@ private void discoverWlans(final UniFiControllerCache cache, final ThingUID brid
         for (final UniFiWlan wlan : cache.getWlans()) {
             final ThingUID thingUID = new ThingUID(UniFiBindingConstants.THING_TYPE_WLAN, bridgeUID,
                     stripIdShort(wlan.getId()));
-            final Map<String, Object> properties = Map.of(PARAMETER_WID, wlan.getId(), PARAMETER_SITE,
-                    wlan.getSite().getName(), PARAMETER_WIFI_NAME, wlan.getName());
+            final String siteName = wlan.getSite() == null ? "" : wlan.getSite().getName();
+            final Map<String, Object> properties = Map.of(PARAMETER_WID, wlan.getId(), PARAMETER_SITE, siteName,
+                    PARAMETER_WIFI_NAME, wlan.getName());
 
             thingDiscovered(DiscoveryResultBuilder.create(thingUID).withThingType(UniFiBindingConstants.THING_TYPE_WLAN)
                     .withBridge(bridgeUID).withRepresentationProperty(PARAMETER_WID).withTTL(TTL_SECONDS)
@@ -157,7 +158,7 @@ private void discoverClients(final UniFiControllerCache cache, final ThingUID br
      * @return shortened id or if to short the original id
      */
     private static String stripIdShort(final String id) {
-        return id.length() > THING_ID_LENGTH ? id.substring(id.length() - THING_ID_LENGTH) : id;
+        return id != null && id.length() > THING_ID_LENGTH ? id.substring(id.length() - THING_ID_LENGTH) : id;
     }
 
     private void discoverPoePorts(final UniFiControllerCache cache, final ThingUID bridgeUID) {

bundles/org.openhab.binding.unifi/src/main/java/org/openhab/binding/unifi/internal/handler/UniFiWlanThingHandler.java

@@ -24,7 +24,7 @@
 import static org.openhab.binding.unifi.internal.UniFiBindingConstants.CHANNEL_WPAENC;
 import static org.openhab.binding.unifi.internal.UniFiBindingConstants.CHANNEL_WPAMODE;
 
-import java.util.function.Function;
+import java.util.function.Predicate;
 
 import org.eclipse.jdt.annotation.NonNullByDefault;
 import org.eclipse.jdt.annotation.Nullable;
@@ -48,6 +48,7 @@
 import org.openhab.core.types.UnDefType;
 
 /**
+ * The {@link UniFiWlanThingHandler} is responsible for handling commands and status updates for a wireless network.
  *
  * @author Hilbrand Bouwkamp - Initial contribution
  */
@@ -127,10 +128,17 @@ protected State getChannelState(final UniFiWlan wlan, final String channelId) {
         return state;
     }
 
-    private static State countClients(final UniFiWlan wlan, final Function<UniFiClient, Boolean> filter) {
+    private static State countClients(final UniFiWlan wlan, final Predicate<UniFiClient> filter) {
         final UniFiSite site = wlan.getSite();
-        return new DecimalType(site.getCache().countClients(site, c -> c instanceof UniFiWirelessClient
-                && wlan.getName().equals(((UniFiWirelessClient) c).getEssid()) && filter.apply(c)));
+
+        if (site == null) {
+            return UnDefType.UNDEF;
+        } else {
+            return new DecimalType(site.getCache().countClients(site,
+                    c -> c instanceof UniFiWirelessClient
+                            && (wlan.getName() != null && wlan.getName().equals(((UniFiWirelessClient) c).getEssid()))
+                            && filter.test(c)));
+        }
     }
 
     /**
@@ -161,7 +169,7 @@ protected boolean handleCommand(final UniFiController controller, final UniFiWla
             final ChannelUID channelUID, final Command command) throws UniFiException {
         final String channelID = channelUID.getId();
 
-        if (CHANNEL_ENABLE.equals(channelID) && command instanceof OnOffType) {
+        if (CHANNEL_ENABLE.equals(channelID) && command instanceof OnOffType && entity.getSite() != null) {
             controller.enableWifi(entity, OnOffType.ON == command);
             return true;
         }

bundles/org.openhab.binding.unifi/src/main/resources/OH-INF/i18n/unifi.properties

@@ -132,10 +132,11 @@ channel-type.config.unifi.poeEnable.mode.option.passthrough = Passthrough
 
 # status messages
 
-error.bridge.offline.communication_error = Error communicating with the UniFi controller.
-error.bridge.offline.invalid_credentials = Invalid username and/or password - please double-check your configuration.
-error.bridge.offline.invalid_hostname = Invalid hostname - please double-check your configuration.
-error.bridge.offline.ssl_error = Error establishing an SSL connection with the UniFi controller.
+error.bridge.offline.communication_error = Error communicating with the UniFi controller: {0}
+error.bridge.offline.invalid_credentials = Invalid username and/or password - please double-check your configuration: {0}
+error.bridge.offline.invalid_hostname = Invalid hostname - please double-check your configuration: {0}
+error.bridge.offline.ssl_error = Error establishing an SSL connection with the UniFi controller: {0}
+error.controller.parse_error = Could not parse json from the UniFi Controller. Is the bridge configured with the correct host and/or port?
 error.thing.client.offline.configuration_error = You must define a MAC address, IP address, hostname or alias for this thing.
 error.thing.offline.bridge_offline = The UniFi Controller is currently offline.
 error.thing.offline.configuration_error = You must choose a UniFi Controller for this thing.