Adjustments based on code review.

Signed-off-by: Patrick Koenemann <git@paphko.de>
openhab · Nov 9, 2021 · ff1ec51 · ff1ec51
1 parent 958a67f
commit ff1ec51
Show file tree

Hide file tree

Showing 15 changed files with 252 additions and 543 deletions.
diff --git a/bundles/org.openhab.binding.anel/README.md b/bundles/org.openhab.binding.anel/README.md
@@ -10,13 +10,15 @@ Some NET-PwrCtrl devices also have 8 I/O ports which can either be used to direc
 
 There are three kinds of devices ([overview on manufacturer's homepage](https://en.anel.eu/?src=/produkte/produkte.htm)):
 
-| [Anel NET-PwrCtrl HUT](https://en.anel.eu/?src=/produkte/hut_2/hut_2.htm) | [Anel NET-PwrCtrl IO](https://en.anel.eu/?src=/produkte/io/io.htm) | [Anel NET-PwrCtrl HOME](https://de.anel.eu/?src=produkte/home/home.htm) (only German version) |
+| [Anel NET-PwrCtrl HUT](https://en.anel.eu/?src=/produkte/hut_2/hut_2.htm) <br/> <sub>( _advanced-firmware_ )</sub>  | [Anel NET-PwrCtrl IO](https://en.anel.eu/?src=/produkte/io/io.htm) <br/> <sub>( _advanced-firmware_ )</sub> | [Anel NET-PwrCtrl HOME](https://de.anel.eu/?src=produkte/home/home.htm) <br/> <sub>( _home_ )</sub> <br/> (only German version) |
 | --- | --- | --- |
-| [![Anel NET-PwrCtrl HUT 2](https://de.anel.eu/image/leisten/HUT2LV-P_500.jpg)](https://de.anel.eu/?src=produkte/hut_2/hut_2.htm) | [![Anel NET-PwrCtrl IO](https://de.anel.eu/image/leisten/IO-Stecker.png)](https://de.anel.eu/?src=produkte/io/io.htm) | [![Anel NET-PwrCtrl HOME](https://de.anel.eu/image/leisten/HOME-DE-500.gif)](https://de.anel.eu/?src=produkte/home/home.htm)
+| [![Anel NET-PwrCtrl HUT 2](https://de.anel.eu/image/leisten/HUT2LV-P_500.jpg)](https://de.anel.eu/?src=produkte/hut_2/hut_2.htm) | [![Anel NET-PwrCtrl IO](https://de.anel.eu/image/leisten/IO-Stecker.png)](https://de.anel.eu/?src=produkte/io/io.htm) | [![Anel NET-PwrCtrl HOME](https://de.anel.eu/image/leisten/HOME-DE-500.gif)](https://de.anel.eu/?src=produkte/home/home.htm) |
 
-* The smallest device, the _HOME_, is the only one with only three power sockets and only available in Germany.
-* The _PRO_ and _REDUNDANT_ have eight power sockets and a similar (simplified) firmware as the _HOME_.
-* All others (_ADV_, _IO_, and the different _HUT_ variants) have eight power sockets / relays, eight IO ports, and an advanced firmware.
+Thing type IDs:
+
+* *home*: The smallest device, the _HOME_, is the only one with only three power sockets and only available in Germany.
+* *simple-firmware*: The _PRO_ and _REDUNDANT_ have eight power sockets and a similar (simplified) firmware as the _HOME_.
+* *advanced-firmware*: All others (_ADV_, _IO_, and the different _HUT_ variants) have eight power sockets / relays, eight IO ports, and an advanced firmware.
 
 An [additional sensor](https://en.anel.eu/?src=/produkte/sensor_1/sensor_1.htm) may be used for monitoring temperature, humidity, and brightness.
 The sensor can be attached to a _HUT_ device via an Ethernet cable (max length is 50m).

diff --git a/...enhab.binding.anel/src/main/java/org/openhab/binding/anel/internal/AnelConfiguration.java b/...enhab.binding.anel/src/main/java/org/openhab/binding/anel/internal/AnelConfiguration.java
@@ -23,6 +23,14 @@
 @NonNullByDefault
 public class AnelConfiguration {
 
+    public @Nullable String hostname;
+    public @Nullable String user;
+    public @Nullable String password;
+    /** Port to send data from openhab to device. */
+    public int udpSendPort = IAnelConstants.DEFAULT_SEND_PORT;
+    /** Openhab receives messages via this port from device. */
+    public int udpReceivePort = IAnelConstants.DEFAULT_RECEIVE_PORT;
+
     public AnelConfiguration() {
     }
 
@@ -35,17 +43,6 @@ public AnelConfiguration(@Nullable String hostname, @Nullable String user, @Null
         this.udpReceivePort = receivePort;
     }
 
-    @Nullable
-    public String hostname;
-    @Nullable
-    public String user;
-    @Nullable
-    public String password;
-    /** Port to send data from openhab to device. */
-    public int udpSendPort = IAnelConstants.DEFAULT_SEND_PORT;
-    /** Openhab receives messages via this port from device. */
-    public int udpReceivePort = IAnelConstants.DEFAULT_RECEIVE_PORT;
-
     @Override
     public String toString() {
         final StringBuilder builder = new StringBuilder();

diff --git a/...org.openhab.binding.anel/src/main/java/org/openhab/binding/anel/internal/AnelHandler.java b/...org.openhab.binding.anel/src/main/java/org/openhab/binding/anel/internal/AnelHandler.java
@@ -123,6 +123,11 @@ private void initializeConnection() {
             periodicRefreshTask = scheduler.scheduleWithFixedDelay(this::periodicRefresh, //
                     0, IAnelConstants.REFRESH_INTERVAL_SEC, TimeUnit.SECONDS);
 
+        } catch (InterruptedException e) {
+
+            // OH shutdown - don't log anything, just clean up
+            dispose();
+
         } catch (Exception e) {
 
             logger.debug("Connection to '{}' failed", config, e);
@@ -173,18 +178,17 @@ public void handleCommand(ChannelUID channelUID, Command command) {
         if (udpConnector2 == null || !udpConnector2.isConnected() || getThing().getStatus() != ThingStatus.ONLINE) {
 
             // don't log initial refresh commands because they may occur before thing is online
-            if (!(command instanceof RefreshType) && getThing().getStatus() != ThingStatus.ONLINE) {
-                logger.info("Cannot handle command '{}' for channel '{}' because thing ({}) is not (yet) connected: {}", //
+            if (!(command instanceof RefreshType)) {
+                logger.debug("Cannot handle command '{}' for channel '{}' because thing ({}) is not connected: {}", //
                         command, channelUID.getId(), getThing().getStatus(), config);
             }
             return;
         }
 
-        @Nullable
         String anelCommand = null;
         if (command instanceof RefreshType) {
 
-            final @Nullable State update = stateUpdater.getChannelUpdate(channelUID.getId(), state);
+            final State update = stateUpdater.getChannelUpdate(channelUID.getId(), state);
             if (update != null) {
 
                 updateState(channelUID, update);
@@ -203,7 +207,7 @@ public void handleCommand(ChannelUID channelUID, Command command) {
             }
         } else if (command instanceof OnOffType) {
 
-            final @Nullable State lockedState;
+            final State lockedState;
             synchronized (this) { // lock needed to update the state if needed
 
                 lockedState = commandHandler.getLockedState(state, channelUID.getId());

diff --git a/...nhab.binding.anel/src/main/java/org/openhab/binding/anel/internal/AnelHandlerFactory.java b/...nhab.binding.anel/src/main/java/org/openhab/binding/anel/internal/AnelHandlerFactory.java
@@ -22,8 +22,6 @@
 import org.openhab.core.thing.binding.ThingHandler;
 import org.openhab.core.thing.binding.ThingHandlerFactory;
 import org.osgi.service.component.annotations.Component;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
 
 /**
  * The {@link AnelHandlerFactory} is responsible for creating things and thing
@@ -35,11 +33,8 @@
 @Component(configurationPid = "binding.anel", service = ThingHandlerFactory.class)
 public class AnelHandlerFactory extends BaseThingHandlerFactory {
 
-    private final Logger logger = LoggerFactory.getLogger(AnelHandlerFactory.class);
-
     @Override
     public boolean supportsThingType(ThingTypeUID thingTypeUID) {
-        logger.debug("asking to support '{}': {}", thingTypeUID, SUPPORTED_THING_TYPES_UIDS.contains(thingTypeUID));
         return SUPPORTED_THING_TYPES_UIDS.contains(thingTypeUID);
     }
 

diff --git a/...penhab.binding.anel/src/main/java/org/openhab/binding/anel/internal/AnelUdpConnector.java b/...penhab.binding.anel/src/main/java/org/openhab/binding/anel/internal/AnelUdpConnector.java
@@ -23,6 +23,7 @@
 
 import org.eclipse.jdt.annotation.NonNullByDefault;
 import org.eclipse.jdt.annotation.Nullable;
+import org.openhab.core.common.NamedThreadFactory;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -51,16 +52,16 @@ public class AnelUdpConnector {
     /** Service to spawn new threads for handling status updates. */
     private final ExecutorService executorService;
 
+    /** Thread factory for UDP listening thread. */
+    private final NamedThreadFactory listeningThreadFactory = new NamedThreadFactory(IAnelConstants.BINDING_ID, true);
+
     /** Socket for receiving UDP packages. */
-    @Nullable
-    private DatagramSocket receivingSocket = null;
+    private @Nullable DatagramSocket receivingSocket = null;
     /** Socket for sending UDP packages. */
-    @Nullable
-    private DatagramSocket sendingSocket = null;
+    private @Nullable DatagramSocket sendingSocket = null;
 
     /** The listener that gets notified upon newly received messages. */
-    @Nullable
-    private Consumer<String> listener;
+    private @Nullable Consumer<String> listener;
 
     private int receiveFailures = 0;
     private boolean listenerActive = false;
@@ -95,9 +96,11 @@ public AnelUdpConnector(String host, int udpReceivePort, int udpSendPort, Execut
     /**
      * Initialize socket connection to the UDP receive port for the given listener.
      *
-     * @throws SocketException
+     * @throws SocketException Is only thrown if <code>logNotTHrowException = false</code>.
+     * @throws InterruptedException Typically happens during shutdown.
      */
-    public void connect(Consumer<String> listener, boolean logNotThrowExcpetion) throws SocketException {
+    public void connect(Consumer<String> listener, boolean logNotThrowExcpetion)
+            throws SocketException, InterruptedException {
         if (receivingSocket == null) {
             try {
                 receivingSocket = new DatagramSocket(receivePort);
@@ -106,19 +109,16 @@ public void connect(Consumer<String> listener, boolean logNotThrowExcpetion) thr
 
                 /*-
                  * Due to the issue with 4 concurrently listening threads [1], we should follow Kais suggestion [2]
-                 * to create our own listening thread.
+                 * to create our own listening daemonized thread.
                  *
                  * [1] https://community.openhab.org/t/anel-net-pwrctrl-binding-for-oh3/123378
                  * [2] https://www.eclipse.org/forums/index.php/m/1775932/?#msg_1775429
                  */
-                new Thread(this::listen).start();
+                listeningThreadFactory.newThread(this::listen).start();
 
                 // wait for the listening thread to be active
-                try {
-                    for (int i = 0; i < 20 && !listenerActive; i++) {
-                        Thread.sleep(100); // wait at most 20 * 100ms = 2sec for the listener to be active
-                    }
-                } catch (InterruptedException e) {
+                for (int i = 0; i < 20 && !listenerActive; i++) {
+                    Thread.sleep(100); // wait at most 20 * 100ms = 2sec for the listener to be active
                 }
                 if (!listenerActive) {
                     logger.warn(
@@ -144,6 +144,14 @@ public void connect(Consumer<String> listener, boolean logNotThrowExcpetion) thr
     }
 
     private void listen() {
+        try {
+            listenUnhandledInterruption();
+        } catch (InterruptedException e) {
+            // OH shutdown - don't log anything, just quit
+        }
+    }
+
+    private void listenUnhandledInterruption() throws InterruptedException {
         logger.info("Anel NET-PwrCtrl listener started for: '{}:{}'", host, receivePort);
 
         final Consumer<String> listener2 = listener;
@@ -194,12 +202,8 @@ private void listen() {
                         logger.debug(
                                 "Unexpected error while listening on port {}; waiting 10sec before the next attempt to listen on that port.",
                                 receivePort, e);
-                        try {
-                            for (int i = 0; i < 50 && receivingSocket != null; i++) {
-                                Thread.sleep(200); // 50 * 200ms = 10sec
-                            }
-                        } catch (InterruptedException ie) {
-                            // abort waiting on interrupted exception
+                        for (int i = 0; i < 50 && receivingSocket != null; i++) {
+                            Thread.sleep(200); // 50 * 200ms = 10sec
                         }
                     } else {
 
@@ -212,7 +216,7 @@ private void listen() {
 
     /** Close the socket connection. */
     public void disconnect() {
-        logger.info("Anel NET-PwrCtrl listener stopped for: '{}:{}'", host, receivePort);
+        logger.debug("Anel NET-PwrCtrl listener stopped for: '{}:{}'", host, receivePort);
         listener = null;
         final DatagramSocket receivingSocket2 = receivingSocket;
         if (receivingSocket2 != null) {

diff --git a/....openhab.binding.anel/src/main/java/org/openhab/binding/anel/internal/IAnelConstants.java b/....openhab.binding.anel/src/main/java/org/openhab/binding/anel/internal/IAnelConstants.java
@@ -15,8 +15,6 @@
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
-import java.util.stream.Collectors;
-import java.util.stream.Stream;
 
 import org.eclipse.jdt.annotation.NonNullByDefault;
 import org.openhab.core.thing.ThingTypeUID;
@@ -84,37 +82,29 @@ public interface IAnelConstants {
     String CHANNEL_NAME = "prop#name";
     String CHANNEL_TEMPERATURE = "prop#temperature";
 
-    @SuppressWarnings("null")
-    List<String> CHANNEL_RELAY_NAME = Stream
-            .of("r1#name", "r2#name", "r3#name", "r4#name", "r5#name", "r6#name", "r7#name", "r8#name")
-            .collect(Collectors.toUnmodifiableList());
-
-    @SuppressWarnings("null")
-    List<String> CHANNEL_RELAY_STATE = Stream
-            // second character must be the index b/c it is parsed in AnelCommandHandler!
-            .of("r1#state", "r2#state", "r3#state", "r4#state", "r5#state", "r6#state", "r7#state", "r8#state")
-            .collect(Collectors.toUnmodifiableList());
-
-    @SuppressWarnings("null")
-    List<String> CHANNEL_RELAY_LOCKED = Stream
-            .of("r1#locked", "r2#locked", "r3#locked", "r4#locked", "r5#locked", "r6#locked", "r7#locked", "r8#locked")
-            .collect(Collectors.toUnmodifiableList());
-
-    @SuppressWarnings("null")
-    List<String> CHANNEL_IO_NAME = Stream
-            .of("io1#name", "io2#name", "io3#name", "io4#name", "io5#name", "io6#name", "io7#name", "io8#name")
-            .collect(Collectors.toUnmodifiableList());
-
-    @SuppressWarnings("null")
-    List<String> CHANNEL_IO_MODE = Stream
-            .of("io1#mode", "io2#mode", "io3#mode", "io4#mode", "io5#mode", "io6#mode", "io7#mode", "io8#mode")
-            .collect(Collectors.toUnmodifiableList());
-
-    @SuppressWarnings("null")
-    List<String> CHANNEL_IO_STATE = Stream
-            // third character must be the index b/c it is parsed in AnelCommandHandler!
-            .of("io1#state", "io2#state", "io3#state", "io4#state", "io5#state", "io6#state", "io7#state", "io8#state")
-            .collect(Collectors.toUnmodifiableList());
+    List<String> CHANNEL_RELAY_NAME = List.of("r1#name", "r2#name", "r3#name", "r4#name", "r5#name", "r6#name",
+            "r7#name", "r8#name");
+
+    // second character must be the index b/c it is parsed in AnelCommandHandler!
+    List<String> CHANNEL_RELAY_STATE = List.of("r1#state", "r2#state", "r3#state", "r4#state", "r5#state", "r6#state",
+            "r7#state", "r8#state");
+
+    List<String> CHANNEL_RELAY_LOCKED = List.of("r1#locked", "r2#locked", "r3#locked", "r4#locked", "r5#locked",
+            "r6#locked", "r7#locked", "r8#locked");
+
+    List<String> CHANNEL_IO_NAME = List.of("io1#name", "io2#name", "io3#name", "io4#name", "io5#name", "io6#name",
+            "io7#name", "io8#name");
+
+    List<String> CHANNEL_IO_MODE = List.of("io1#mode", "io2#mode", "io3#mode", "io4#mode", "io5#mode", "io6#mode",
+            "io7#mode", "io8#mode");
+
+    // third character must be the index b/c it is parsed in AnelCommandHandler!
+    List<String> CHANNEL_IO_STATE = List.of("io1#state", "io2#state", "io3#state", "io4#state", "io5#state",
+            "io6#state", "io7#state", "io8#state");
+
+    String CHANNEL_SENSOR_TEMPERATURE = "sensor#temperature";
+    String CHANNEL_SENSOR_HUMIDITY = "sensor#humidity";
+    String CHANNEL_SENSOR_BRIGHTNESS = "sensor#brightness";
 
     /**
      * @param channelId A channel ID.
@@ -130,16 +120,4 @@ static int getIndexFromChannel(String channelId) {
         }
         return -1; // not a relay or io channel
     }
-
-    String CHANNEL_SENSOR_TEMPERATURE = "sensor#temperature";
-    String CHANNEL_SENSOR_HUMIDITY = "sensor#humidity";
-    String CHANNEL_SENSOR_BRIGHTNESS = "sensor#brightness";
-
-    String CHANNEL_POWER_VOLTAGE_RMS = "power#voltageRMS";
-    String CHANNEL_POWER_CURRENT_RMS = "power#currentRMS";
-    String CHANNEL_POWER_LINE_FREQUENCY = "power#lineFrequency";
-    String CHANNEL_POWER_ACTIVE_POWER = "power#activePower";
-    String CHANNEL_POWER_APPARENT_POWER = "power#apparentPower";
-    String CHANNEL_POWER_REACTIVE_POWER = "power#reactivePower";
-    String CHANNEL_POWER_POWER_FACTOR = "power#powerFactor";
 }
diff --git a/...binding.anel/src/main/java/org/openhab/binding/anel/internal/auth/AnelAuthentication.java b/...binding.anel/src/main/java/org/openhab/binding/anel/internal/auth/AnelAuthentication.java
@@ -28,9 +28,9 @@
 public class AnelAuthentication {
 
     public enum AuthMethod {
-        plain,
-        base64,
-        xorBase64;
+        PLAIN,
+        BASE64,
+        XORBASE64;
 
         private static final Pattern NAME_AND_FIRMWARE_PATTERN = Pattern.compile(":NET-PWRCTRL_0?(\\d+\\.\\d)");
         private static final Pattern LAST_SEGMENT_FIRMWARE_PATTERN = Pattern.compile(":(\\d+\\.\\d)$");
@@ -40,22 +40,22 @@ public enum AuthMethod {
 
         public static AuthMethod of(String status) {
             if (status.isEmpty()) {
-                return plain; // fallback
+                return PLAIN; // fallback
             }
             if (status.trim().endsWith(":xor") || status.contains(":xor:")) {
-                return xorBase64;
+                return XORBASE64;
             }
             final String firmwareVersion = getFirmwareVersion(status);
             if (firmwareVersion == null) {
-                return plain;
+                return PLAIN;
             }
             if (firmwareVersion.compareTo(MIN_FIRMWARE_XOR_BASE64) >= 0) {
-                return xorBase64; // >= 6.1
+                return XORBASE64; // >= 6.1
             }
             if (firmwareVersion.compareTo(MIN_FIRMWARE_BASE64) >= 0) {
-                return base64; // exactly 6.0
+                return BASE64; // exactly 6.0
             }
-            return plain; // fallback
+            return PLAIN; // fallback
         }
 
         private static @Nullable String getFirmwareVersion(String fullStatusStringOrFirmwareVersion) {
@@ -74,15 +74,15 @@ public static AuthMethod of(String status) {
     public static String getUserPasswordString(@Nullable String user, @Nullable String password,
             @Nullable AuthMethod authMethod) {
         final String userPassword = (user == null ? "" : user) + (password == null ? "" : password);
-        if (authMethod == null || authMethod == AuthMethod.plain) {
+        if (authMethod == null || authMethod == AuthMethod.PLAIN) {
             return userPassword;
         }
 
-        if (authMethod == AuthMethod.base64 || password == null || password.isEmpty()) {
+        if (authMethod == AuthMethod.BASE64 || password == null || password.isEmpty()) {
             return Base64.getEncoder().encodeToString(userPassword.getBytes());
         }
 
-        if (authMethod == AuthMethod.xorBase64) {
+        if (authMethod == AuthMethod.XORBASE64) {
             final StringBuilder result = new StringBuilder();
 
             // XOR