]> git.basschouten.com Git - openhab-addons.git/commitdiff
[nibeuplink] Fix NPEs, minor refactoring (#14508)
authoralexf2015 <alexf2015@users.noreply.github.com>
Mon, 20 Mar 2023 19:09:20 +0000 (20:09 +0100)
committerGitHub <noreply@github.com>
Mon, 20 Mar 2023 19:09:20 +0000 (20:09 +0100)
* improved code quality: added some additional null checks, removed some obsolete null checks. simplified some code sections.

Signed-off-by: Alexander Friese <af944580@googlemail.com>
13 files changed:
bundles/org.openhab.binding.nibeuplink/src/main/java/org/openhab/binding/nibeuplink/internal/NibeUplinkBindingConstants.java
bundles/org.openhab.binding.nibeuplink/src/main/java/org/openhab/binding/nibeuplink/internal/callback/AbstractUplinkCommandCallback.java
bundles/org.openhab.binding.nibeuplink/src/main/java/org/openhab/binding/nibeuplink/internal/command/GenericStatusUpdate.java
bundles/org.openhab.binding.nibeuplink/src/main/java/org/openhab/binding/nibeuplink/internal/command/Login.java
bundles/org.openhab.binding.nibeuplink/src/main/java/org/openhab/binding/nibeuplink/internal/command/NibeUplinkCommand.java
bundles/org.openhab.binding.nibeuplink/src/main/java/org/openhab/binding/nibeuplink/internal/command/UpdateSetting.java
bundles/org.openhab.binding.nibeuplink/src/main/java/org/openhab/binding/nibeuplink/internal/connector/CommunicationStatus.java
bundles/org.openhab.binding.nibeuplink/src/main/java/org/openhab/binding/nibeuplink/internal/connector/UplinkWebInterface.java
bundles/org.openhab.binding.nibeuplink/src/main/java/org/openhab/binding/nibeuplink/internal/handler/NibeUplinkHandler.java
bundles/org.openhab.binding.nibeuplink/src/main/java/org/openhab/binding/nibeuplink/internal/handler/UplinkBaseHandler.java
bundles/org.openhab.binding.nibeuplink/src/main/java/org/openhab/binding/nibeuplink/internal/model/ChannelGroup.java
bundles/org.openhab.binding.nibeuplink/src/main/java/org/openhab/binding/nibeuplink/internal/model/DataResponseTransformer.java
bundles/org.openhab.binding.nibeuplink/src/main/resources/OH-INF/i18n/nibeuplink.properties

index 26280bfa46e2ce11161c73de513fed2e8587e460..2a64f3c42079046532a3b108f30e538942aa094c 100644 (file)
@@ -87,6 +87,10 @@ public final class NibeUplinkBindingConstants {
     public static final String CHANNEL_TYPE_HW_MODE_RW = "rwtype-hw-mode";
     public static final String CHANNEL_TYPE_FAN_SPEED_RW = "rwtype-fan-speed";
 
+    // Status Keys
+    public static final String STATUS_INVALID_NIBE_ID = "@text/status.invalid.nibeId";
+    public static final String STATUS_INVALID_CREDENTIALS = "@text/status.invalid.credentials";
+
     // URLs
     public static final String LOGIN_URL = "https://www.nibeuplink.com/LogIn";
     public static final String DATA_API_URL = "https://www.nibeuplink.com/PrivateAPI/QueueValues";
index 1757d13b9971b04ed9e20329633984f1fac667a8..7e84dc05c36f15e32b2f79c44f26d5b812e00718 100644 (file)
@@ -167,8 +167,11 @@ public abstract class AbstractUplinkCommandCallback extends BufferingResponseLis
     protected abstract String getURL();
 
     @Override
-    public final @Nullable StatusUpdateListener getListener() {
-        return listener;
+    public final void updateListenerStatus() {
+        StatusUpdateListener listener = this.listener;
+        if (listener != null) {
+            listener.update(communicationStatus);
+        }
     }
 
     @Override
index 1d3686737e10007eabd3b671bfdd0206d9e72777..f9cbfbd16f64bcdceee0930b82f65139c3855caa 100644 (file)
@@ -26,7 +26,6 @@ import org.eclipse.jetty.http.HttpMethod;
 import org.eclipse.jetty.http.HttpStatus;
 import org.eclipse.jetty.util.Fields;
 import org.openhab.binding.nibeuplink.internal.callback.AbstractUplinkCommandCallback;
-import org.openhab.binding.nibeuplink.internal.connector.StatusUpdateListener;
 import org.openhab.binding.nibeuplink.internal.handler.NibeUplinkHandler;
 import org.openhab.binding.nibeuplink.internal.model.DataResponse;
 import org.openhab.binding.nibeuplink.internal.model.DataResponseTransformer;
@@ -83,10 +82,7 @@ public class GenericStatusUpdate extends AbstractUplinkCommandCallback implement
         logger.debug("onComplete()");
 
         if (!HttpStatus.Code.OK.equals(getCommunicationStatus().getHttpCode()) && retries++ < MAX_RETRIES) {
-            StatusUpdateListener listener = getListener();
-            if (listener != null) {
-                listener.update(getCommunicationStatus());
-            }
+            updateListenerStatus();
             handler.getWebInterface().enqueueCommand(this);
         } else {
             String json = getContentAsString(StandardCharsets.UTF_8);
index 15366c0cbc3ecb0531d4f4fc04bc382a52e909a7..6aa0696fda0aba7f20e0153587de4d33bad3b9f4 100644 (file)
@@ -59,9 +59,6 @@ public class Login extends AbstractUplinkCommandCallback implements NibeUplinkCo
 
     @Override
     public void onComplete(@Nullable Result result) {
-        StatusUpdateListener listener = getListener();
-        if (listener != null) {
-            listener.update(getCommunicationStatus());
-        }
+        updateListenerStatus();
     }
 }
index c8d25038beb512b94ff0a71997bdfcbde359a1b5..86d28bbb0af124b28a59a7885c531daca625e5a2 100644 (file)
@@ -13,7 +13,6 @@
 package org.openhab.binding.nibeuplink.internal.command;
 
 import org.eclipse.jdt.annotation.NonNullByDefault;
-import org.eclipse.jdt.annotation.Nullable;
 import org.eclipse.jetty.client.HttpClient;
 import org.eclipse.jetty.client.api.Response.CompleteListener;
 import org.eclipse.jetty.client.api.Response.ContentListener;
@@ -39,12 +38,10 @@ public interface NibeUplinkCommand extends SuccessListener, FailureListener, Con
     void performAction(HttpClient asyncclient);
 
     /**
-     * get the current listener
+     * update the status of the registered listener instance
      *
-     * @return instance of the listener, might be null.
      */
-    @Nullable
-    StatusUpdateListener getListener();
+    void updateListenerStatus();
 
     /**
      * register a listener
index 48ecbfbd0f287140b0df283539c71ca8c8d1872f..dfed831effd01d0bd541c5fb833e918c6f3e0646 100644 (file)
@@ -70,8 +70,7 @@ public class UpdateSetting extends AbstractUplinkCommandCallback implements Nibe
         ChannelTypeUID typeUID = channel.getChannelTypeUID();
         String channelId = channel.getUID().getIdWithoutGroup();
 
-        if (typeUID == null || typeUID.getId() == null
-                || !typeUID.getId().startsWith(NibeUplinkBindingConstants.RW_CHANNEL_PREFIX)) {
+        if (typeUID == null || !typeUID.getId().startsWith(NibeUplinkBindingConstants.RW_CHANNEL_PREFIX)) {
             logger.info("channel '{}' does not support write access - value to set '{}'", channelId, value);
             throw new UnsupportedOperationException("channel (" + channelId + ") does not support write access");
         }
index d09c5fae7352bb23b46725c9aafae6d391f775e5..029f3985c9145317ee59a626ce7026016074130a 100644 (file)
@@ -47,7 +47,7 @@ public class CommunicationStatus {
 
     public final String getMessage() {
         Exception err = error;
-        String errMsg = err == null ? err.getMessage() : null;
+        String errMsg = err == null ? null : err.getMessage();
         String msg = getHttpCode().getMessage();
         if (errMsg != null && !errMsg.isEmpty()) {
             return errMsg;
index 04b8b5698bdcd47451e2214cf612a695cdacb302..df310974e067aa7f023b2430820b141d4269ce7a 100644 (file)
@@ -27,7 +27,6 @@ import org.eclipse.jetty.util.BlockingArrayQueue;
 import org.openhab.binding.nibeuplink.internal.AtomicReferenceTrait;
 import org.openhab.binding.nibeuplink.internal.command.Login;
 import org.openhab.binding.nibeuplink.internal.command.NibeUplinkCommand;
-import org.openhab.binding.nibeuplink.internal.config.NibeUplinkConfiguration;
 import org.openhab.binding.nibeuplink.internal.handler.NibeUplinkHandler;
 import org.openhab.core.thing.ThingStatus;
 import org.openhab.core.thing.ThingStatusDetail;
@@ -42,15 +41,8 @@ import org.slf4j.LoggerFactory;
 @NonNullByDefault
 public class UplinkWebInterface implements AtomicReferenceTrait {
 
-    private static final int NIBE_ID_THRESHOLD = 14;
-
     private final Logger logger = LoggerFactory.getLogger(UplinkWebInterface.class);
 
-    /**
-     * Configuration
-     */
-    private NibeUplinkConfiguration config;
-
     /**
      * handler for updating thing status
      */
@@ -88,7 +80,6 @@ public class UplinkWebInterface implements AtomicReferenceTrait {
      *
      * @author afriese - initial contribution
      */
-    @NonNullByDefault
     private class WebRequestExecutor implements Runnable {
 
         /**
@@ -126,41 +117,91 @@ public class UplinkWebInterface implements AtomicReferenceTrait {
          */
         @Override
         public void run() {
+            logger.debug("run queued commands, queue size is {}", commandQueue.size());
             if (!isAuthenticated()) {
                 authenticate();
+            } else if (isAuthenticated() && !commandQueue.isEmpty()) {
+                try {
+                    executeCommand();
+                } catch (Exception ex) {
+                    logger.warn("command execution ended with exception:", ex);
+                }
             }
+        }
 
-            else if (isAuthenticated() && !commandQueue.isEmpty()) {
-                StatusUpdateListener statusUpdater = new StatusUpdateListener() {
-                    @Override
-                    public void update(CommunicationStatus status) {
-                        switch (status.getHttpCode()) {
-                            case SERVICE_UNAVAILABLE:
-                                uplinkHandler.setStatusInfo(ThingStatus.OFFLINE, ThingStatusDetail.BRIDGE_OFFLINE,
-                                        status.getMessage());
-                                setAuthenticated(false);
-                                break;
-                            case FOUND:
-                                uplinkHandler.setStatusInfo(ThingStatus.OFFLINE, ThingStatusDetail.CONFIGURATION_ERROR,
-                                        "most likely your NibeId is wrong. please check your NibeId.");
-                                setAuthenticated(false);
-                                break;
-                            case OK:
-                                // no action needed as the thing is already online.
-                                break;
-                            default:
-                                uplinkHandler.setStatusInfo(ThingStatus.OFFLINE, ThingStatusDetail.COMMUNICATION_ERROR,
-                                        status.getMessage());
-                                setAuthenticated(false);
-                        }
-                    }
-                };
+        /**
+         * executes the next command in the queue. requires authenticated session.
+         *
+         * @throws ValidationException
+         */
+        private void executeCommand() {
+            NibeUplinkCommand command = commandQueue.poll();
+            if (command != null) {
+                command.setListener(this::processExecutionResult);
+                command.performAction(httpClient);
+            }
+        }
 
-                NibeUplinkCommand command = commandQueue.poll();
-                if (command != null) {
-                    command.setListener(statusUpdater);
-                    command.performAction(httpClient);
-                }
+        /**
+         * callback that handles result from command execution.
+         *
+         * @param status status information to be evaluated
+         */
+        private void processExecutionResult(CommunicationStatus status) {
+            switch (status.getHttpCode()) {
+                case SERVICE_UNAVAILABLE:
+                    uplinkHandler.setStatusInfo(ThingStatus.OFFLINE, ThingStatusDetail.BRIDGE_OFFLINE,
+                            status.getMessage());
+                    setAuthenticated(false);
+                    break;
+                case FOUND:
+                    uplinkHandler.setStatusInfo(ThingStatus.OFFLINE, ThingStatusDetail.CONFIGURATION_ERROR,
+                            STATUS_INVALID_NIBE_ID);
+                    setAuthenticated(false);
+                    break;
+                case OK:
+                    // no action needed as the thing is already online.
+                    break;
+                default:
+                    uplinkHandler.setStatusInfo(ThingStatus.OFFLINE, ThingStatusDetail.COMMUNICATION_ERROR,
+                            status.getMessage());
+                    setAuthenticated(false);
+            }
+        }
+
+        /**
+         * authenticates with the Nibe Uplink WEB interface
+         */
+        private synchronized void authenticate() {
+            setAuthenticated(false);
+            new Login(uplinkHandler, this::processAuthenticationResult).performAction(httpClient);
+        }
+
+        /**
+         * callback that handles result from authentication.
+         *
+         * @param status status information to be evaluated
+         */
+        private void processAuthenticationResult(CommunicationStatus status) {
+            switch (status.getHttpCode()) {
+                case FOUND:
+                    uplinkHandler.setStatusInfo(ThingStatus.ONLINE, ThingStatusDetail.NONE, null);
+                    setAuthenticated(true);
+                    break;
+                case OK:
+                    uplinkHandler.setStatusInfo(ThingStatus.OFFLINE, ThingStatusDetail.CONFIGURATION_ERROR,
+                            STATUS_INVALID_CREDENTIALS);
+                    setAuthenticated(false);
+                    break;
+                case SERVICE_UNAVAILABLE:
+                    uplinkHandler.setStatusInfo(ThingStatus.OFFLINE, ThingStatusDetail.BRIDGE_OFFLINE,
+                            status.getMessage());
+                    setAuthenticated(false);
+                    break;
+                default:
+                    uplinkHandler.setStatusInfo(ThingStatus.OFFLINE, ThingStatusDetail.COMMUNICATION_ERROR,
+                            status.getMessage());
+                    setAuthenticated(false);
             }
         }
     }
@@ -171,7 +212,6 @@ public class UplinkWebInterface implements AtomicReferenceTrait {
      * @param config the Bridge configuration
      */
     public UplinkWebInterface(ScheduledExecutorService scheduler, NibeUplinkHandler handler, HttpClient httpClient) {
-        this.config = handler.getConfiguration();
         this.uplinkHandler = handler;
         this.scheduler = scheduler;
         this.requestExecutor = new WebRequestExecutor();
@@ -182,7 +222,6 @@ public class UplinkWebInterface implements AtomicReferenceTrait {
      * starts the periodic request executor job which handles all web requests
      */
     public void start() {
-        this.config = uplinkHandler.getConfiguration();
         setAuthenticated(false);
         updateJobReference(requestExecutorJobReference, scheduler.scheduleWithFixedDelay(requestExecutor,
                 WEB_REQUEST_INITIAL_DELAY, WEB_REQUEST_INTERVAL, TimeUnit.MILLISECONDS));
@@ -197,72 +236,6 @@ public class UplinkWebInterface implements AtomicReferenceTrait {
         requestExecutor.enqueue(command);
     }
 
-    /**
-     * authenticates with the Nibe Uplink WEB interface
-     */
-    private synchronized void authenticate() {
-        setAuthenticated(false);
-
-        if (preCheck()) {
-            StatusUpdateListener statusUpdater = new StatusUpdateListener() {
-
-                @Override
-                public void update(CommunicationStatus status) {
-                    switch (status.getHttpCode()) {
-                        case FOUND:
-                            uplinkHandler.setStatusInfo(ThingStatus.ONLINE, ThingStatusDetail.NONE, "logged in");
-                            setAuthenticated(true);
-                            break;
-                        case OK:
-                            uplinkHandler.setStatusInfo(ThingStatus.UNKNOWN, ThingStatusDetail.CONFIGURATION_ERROR,
-                                    "invalid username or password");
-                            setAuthenticated(false);
-                            break;
-                        case SERVICE_UNAVAILABLE:
-                            uplinkHandler.setStatusInfo(ThingStatus.OFFLINE, ThingStatusDetail.BRIDGE_OFFLINE,
-                                    status.getMessage());
-                            setAuthenticated(false);
-                            break;
-                        default:
-                            uplinkHandler.setStatusInfo(ThingStatus.OFFLINE, ThingStatusDetail.COMMUNICATION_ERROR,
-                                    status.getMessage());
-                            setAuthenticated(false);
-                    }
-                }
-            };
-
-            new Login(uplinkHandler, statusUpdater).performAction(httpClient);
-        }
-    }
-
-    /**
-     * performs some pre cheks on configuration before attempting to login
-     *
-     * @return true on success, false otherwise
-     */
-    private boolean preCheck() {
-        String preCheckStatusMessage = "";
-        String localPassword = config.getPassword();
-        String localUser = config.getUser();
-        String localNibeId = config.getNibeId();
-
-        if (localPassword == null || localPassword.isEmpty()) {
-            preCheckStatusMessage = "please configure password first";
-        } else if (localUser == null || localUser.isEmpty()) {
-            preCheckStatusMessage = "please configure user first";
-        } else if (localNibeId == null || localNibeId.isEmpty()) {
-            preCheckStatusMessage = "please configure nibeId first";
-        } else if (localNibeId.length() > NIBE_ID_THRESHOLD) {
-            preCheckStatusMessage = "your NibeId is too long. Please refer to the documentation on how to set this value.";
-        } else {
-            return true;
-        }
-
-        this.uplinkHandler.setStatusInfo(ThingStatus.OFFLINE, ThingStatusDetail.CONFIGURATION_ERROR,
-                preCheckStatusMessage);
-        return false;
-    }
-
     /**
      * will be called by the ThingHandler to abort periodic jobs.
      */
index 3cd88e9bc155298e9329769d918f372b7539db7d..75c83f68ca0ab3af14fe32223a85df8e3a0057c5 100644 (file)
@@ -15,6 +15,7 @@ package org.openhab.binding.nibeuplink.internal.handler;
 import java.util.Map;
 
 import org.eclipse.jdt.annotation.NonNullByDefault;
+import org.eclipse.jdt.annotation.Nullable;
 import org.openhab.binding.nibeuplink.internal.config.NibeUplinkConfiguration;
 import org.openhab.binding.nibeuplink.internal.connector.UplinkWebInterface;
 import org.openhab.core.thing.Channel;
@@ -38,7 +39,7 @@ public interface NibeUplinkHandler extends ThingHandler, ChannelProvider {
      * @param statusDetail Bridge status detail
      * @param description Bridge status description
      */
-    void setStatusInfo(ThingStatus status, ThingStatusDetail statusDetail, String description);
+    void setStatusInfo(ThingStatus status, ThingStatusDetail statusDetail, @Nullable String description);
 
     /**
      * Provides the web interface object.
index c4db1111c44fc1a9ea34c12a9d27dcdca0a1de70..7645f81d2bcec39271f4f086ab253612a58405ea 100644 (file)
@@ -87,8 +87,7 @@ public abstract class UplinkBaseHandler extends BaseThingHandler implements Nibe
             Channel channel = getSpecificChannel(channelUID.getIdWithoutGroup());
             if (channel != null) {
                 ChannelTypeUID typeUID = channel.getChannelTypeUID();
-                if (typeUID != null && typeUID.getId() != null
-                        && typeUID.getId().startsWith(NibeUplinkBindingConstants.RW_CHANNEL_PREFIX)) {
+                if (typeUID != null && typeUID.getId().startsWith(NibeUplinkBindingConstants.RW_CHANNEL_PREFIX)) {
                     webInterface.enqueueCommand(new UpdateSetting(this, channel, command));
                 }
             }
@@ -192,7 +191,7 @@ public abstract class UplinkBaseHandler extends BaseThingHandler implements Nibe
     }
 
     @Override
-    public void setStatusInfo(ThingStatus status, ThingStatusDetail statusDetail, String description) {
+    public void setStatusInfo(ThingStatus status, ThingStatusDetail statusDetail, @Nullable String description) {
         super.updateStatus(status, statusDetail, description);
     }
 
index fbeb119fce296a8fb674342fffcb7289dfc55234..4cf0765eb8b1e9356eff1882e8bb1441c93bc23d 100644 (file)
  */
 package org.openhab.binding.nibeuplink.internal.model;
 
+import org.eclipse.jdt.annotation.NonNullByDefault;
+
 /**
  * used to determine the group a channel belongs to
  *
  * @author Alexander Friese - initial contribution
  */
+@NonNullByDefault
 public enum ChannelGroup {
     BASE,
     GENERAL,
index 8e197292c634522405006abf29be4605e66173cb..050f242493f492994c71a38643847f6258b4d6de 100644 (file)
@@ -30,6 +30,7 @@ import org.openhab.core.library.unit.Units;
 import org.openhab.core.thing.Channel;
 import org.openhab.core.thing.type.ChannelTypeUID;
 import org.openhab.core.types.State;
+import org.openhab.core.types.UnDefType;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -63,6 +64,9 @@ public class DataResponseTransformer {
             if (channel == null) {
                 // This should not happen but we want to get informed about it
                 logger.warn("Channel not found: {}", channelId);
+            } else if (value == null) {
+                logger.debug("null value for channel: {}", channelId);
+                result.put(channel, UnDefType.UNDEF);
             } else {
                 ChannelTypeUID typeUID = channel.getChannelTypeUID();
                 String type = typeUID == null ? "null" : typeUID.getId();
index 292649bd2bc5dd953d5750b3e7024802b1dcce2d..395d07f08f08f3a03904cd31f5ab18d5eaaef68b 100644 (file)
@@ -467,3 +467,8 @@ channel-type.nibeuplink.type-switch.label = Unnamed Switch
 channel-type.nibeuplink.type-temperature.label = Unnamed Temperature
 channel-type.nibeuplink.type-time-scale10.label = Unnamed Time
 channel-type.nibeuplink.type-time-unscaled.label = Unnamed Time
+
+# status translations
+
+status.invalid.nibeId = "Most likely your NibeId is wrong. Please check your NibeId"
+status.invalid.credentials = "Invalid username or password"