[knx] Improve handling of serial gateways (#13897)

* [knx] Improve handling of serial gateways

Take over initialization logic from KNX IP gateway for serial gateway.
Properly re-initialize serial gateway after configuration changes done in UI.
Fixes #13892.

Signed-off-by: Holger Friedrich <mail@holger-friedrich.de>

bundles/org.openhab.binding.knx/src/main/java/org/openhab/binding/knx/internal/config/BridgeConfiguration.java

@@ -12,8 +12,6 @@
  */
 package org.openhab.binding.knx.internal.config;
 
-import java.math.BigDecimal;
-
 import org.eclipse.jdt.annotation.NonNullByDefault;
 
 /**
@@ -25,23 +23,23 @@ import org.eclipse.jdt.annotation.NonNullByDefault;
 @NonNullByDefault
 public class BridgeConfiguration {
     private int autoReconnectPeriod = 0;
-    private BigDecimal readingPause = BigDecimal.valueOf(0);
+    private int readingPause = 0;
-    private BigDecimal readRetriesLimit = BigDecimal.valueOf(0);
+    private int readRetriesLimit = 0;
-    private BigDecimal responseTimeout = BigDecimal.valueOf(0);
+    private int responseTimeout = 0;
 
     public int getAutoReconnectPeriod() {
         return autoReconnectPeriod;
     }
 
-    public BigDecimal getReadingPause() {
+    public int getReadingPause() {
         return readingPause;
     }
 
-    public BigDecimal getReadRetriesLimit() {
+    public int getReadRetriesLimit() {
         return readRetriesLimit;
     }
 
-    public BigDecimal getResponseTimeout() {
+    public int getResponseTimeout() {
         return responseTimeout;
     }
 

bundles/org.openhab.binding.knx/src/main/java/org/openhab/binding/knx/internal/config/DeviceConfig.java

@@ -12,8 +12,6 @@
  */
 package org.openhab.binding.knx.internal.config;
 
-import java.math.BigDecimal;
-
 import org.eclipse.jdt.annotation.NonNullByDefault;
 
 /**
@@ -27,8 +25,8 @@ public class DeviceConfig {
 
     private String address = "";
     private boolean fetch = false;
-    private BigDecimal pingInterval = BigDecimal.valueOf(0);
+    private int pingInterval = 0;
-    private BigDecimal readInterval = BigDecimal.valueOf(0);
+    private int readInterval = 0;
 
     public String getAddress() {
         return address;
@@ -38,11 +36,11 @@ public class DeviceConfig {
         return fetch;
     }
 
-    public BigDecimal getPingInterval() {
+    public int getPingInterval() {
         return pingInterval;
     }
 
-    public BigDecimal getReadInterval() {
+    public int getReadInterval() {
         return readInterval;
     }
 }

bundles/org.openhab.binding.knx/src/main/java/org/openhab/binding/knx/internal/config/IPBridgeConfiguration.java

@@ -12,8 +12,6 @@
  */
 package org.openhab.binding.knx.internal.config;
 
-import java.math.BigDecimal;
-
 import org.eclipse.jdt.annotation.NonNullByDefault;
 
 /**
@@ -28,7 +26,7 @@ public class IPBridgeConfiguration extends BridgeConfiguration {
     private boolean useNAT = false;
     private String type = "";
     private String ipAddress = "";
-    private BigDecimal portNumber = BigDecimal.valueOf(0);
+    private int portNumber = 0;
     private String localIp = "";
     private String localSourceAddr = "";
     private String routerBackboneKey = "";
@@ -48,7 +46,7 @@ public class IPBridgeConfiguration extends BridgeConfiguration {
         return ipAddress;
     }
 
-    public BigDecimal getPortNumber() {
+    public int getPortNumber() {
         return portNumber;
     }
 

bundles/org.openhab.binding.knx/src/main/java/org/openhab/binding/knx/internal/handler/AbstractKNXThingHandler.java

@@ -152,7 +152,7 @@ public abstract class AbstractKNXThingHandler extends BaseThingHandler implement
                     if (!filledDescription && config.getFetch()) {
                         Future<?> descriptionJob = this.descriptionJob;
                         if (descriptionJob == null || descriptionJob.isCancelled()) {
-                            long initialDelay = Math.round(config.getPingInterval().longValue() * random.nextFloat());
+                            long initialDelay = Math.round(config.getPingInterval() * random.nextFloat());
                             this.descriptionJob = getBackgroundScheduler().schedule(() -> {
                                 filledDescription = describeDevice(address);
                             }, initialDelay, TimeUnit.SECONDS);
@@ -181,7 +181,7 @@ public abstract class AbstractKNXThingHandler extends BaseThingHandler implement
                 updateStatus(ThingStatus.UNKNOWN);
                 address = new IndividualAddress(config.getAddress());
 
-                long pingInterval = config.getPingInterval().longValue();
+                long pingInterval = config.getPingInterval();
                 long initialPingDelay = Math.round(INITIAL_PING_DELAY * random.nextFloat());
 
                 ScheduledFuture<?> pollingJob = this.pollingJob;

bundles/org.openhab.binding.knx/src/main/java/org/openhab/binding/knx/internal/handler/DeviceThingHandler.java

@@ -84,7 +84,7 @@ public class DeviceThingHandler extends AbstractKNXThingHandler {
     public void initialize() {
         super.initialize();
         DeviceConfig config = getConfigAs(DeviceConfig.class);
-        readInterval = config.getReadInterval().intValue();
+        readInterval = config.getReadInterval();
         initializeGroupAddresses();
     }
 

bundles/org.openhab.binding.knx/src/main/java/org/openhab/binding/knx/internal/handler/IPBridgeThingHandler.java

@@ -65,9 +65,7 @@ public class IPBridgeThingHandler extends KNXBridgeBaseThingHandler {
         // initialisation would take too long and show a warning during binding startup
         // KNX secure is adding serious delay
         updateStatus(ThingStatus.UNKNOWN);
-        initJob = scheduler.submit(() -> {
+        initJob = scheduler.submit(this::initializeLater);
-            initializeLater();
-        });
     }
 
     public void initializeLater() {
@@ -106,7 +104,7 @@ public class IPBridgeThingHandler extends KNXBridgeBaseThingHandler {
         }
         String localSource = config.getLocalSourceAddr();
         String connectionTypeString = config.getType();
-        int port = config.getPortNumber().intValue();
+        int port = config.getPortNumber();
         String ip = config.getIpAddress();
         InetSocketAddress localEndPoint = null;
         boolean useNAT = false;
@@ -179,8 +177,8 @@ public class IPBridgeThingHandler extends KNXBridgeBaseThingHandler {
         updateStatus(ThingStatus.UNKNOWN);
         client = new IPClient(ipConnectionType, ip, localSource, port, localEndPoint, useNAT, autoReconnectPeriod,
                 secureRouting.backboneGroupKey, secureRouting.latencyToleranceMs, secureTunnel.devKey,
-                secureTunnel.user, secureTunnel.userKey, thing.getUID(), config.getResponseTimeout().intValue(),
+                secureTunnel.user, secureTunnel.userKey, thing.getUID(), config.getResponseTimeout(),
-                config.getReadingPause().intValue(), config.getReadRetriesLimit().intValue(), getScheduler(), this);
+                config.getReadingPause(), config.getReadRetriesLimit(), getScheduler(), this);
 
         final var tmpClient = client;
         if (tmpClient != null) {

bundles/org.openhab.binding.knx/src/main/java/org/openhab/binding/knx/internal/handler/SerialBridgeThingHandler.java

@@ -12,12 +12,18 @@
  */
 package org.openhab.binding.knx.internal.handler;
 
+import java.util.concurrent.Future;
+
 import org.eclipse.jdt.annotation.NonNullByDefault;
-import org.openhab.binding.knx.internal.client.AbstractKNXClient;
+import org.eclipse.jdt.annotation.Nullable;
+import org.openhab.binding.knx.internal.client.KNXClient;
+import org.openhab.binding.knx.internal.client.NoOpClient;
 import org.openhab.binding.knx.internal.client.SerialClient;
 import org.openhab.binding.knx.internal.config.SerialBridgeConfiguration;
 import org.openhab.core.thing.Bridge;
 import org.openhab.core.thing.ThingStatus;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 /**
  * The {@link IPBridgeThingHandler} is responsible for handling commands, which are
@@ -31,31 +37,66 @@ import org.openhab.core.thing.ThingStatus;
 @NonNullByDefault
 public class SerialBridgeThingHandler extends KNXBridgeBaseThingHandler {
 
-    private final SerialClient client;
+    private @Nullable SerialClient client = null;
+    private @Nullable Future<?> initJob = null;
+
+    private final Logger logger = LoggerFactory.getLogger(SerialBridgeThingHandler.class);
 
     public SerialBridgeThingHandler(Bridge bridge) {
         super(bridge);
-        SerialBridgeConfiguration config = getConfigAs(SerialBridgeConfiguration.class);
-        client = new SerialClient(config.getAutoReconnectPeriod(), thing.getUID(),
-                config.getResponseTimeout().intValue(), config.getReadingPause().intValue(),
-                config.getReadRetriesLimit().intValue(), getScheduler(), config.getSerialPort(), config.useCemi(),
-                this);
     }
 
     @Override
     public void initialize() {
+        // create new instance using current configuration settings;
+        // when a parameter change is done from UI, dispose() and initialize() are called
+        SerialBridgeConfiguration config = getConfigAs(SerialBridgeConfiguration.class);
+        client = new SerialClient(config.getAutoReconnectPeriod(), thing.getUID(), config.getResponseTimeout(),
+                config.getReadingPause(), config.getReadRetriesLimit(), getScheduler(), config.getSerialPort(),
+                config.useCemi(), this);
+
         updateStatus(ThingStatus.UNKNOWN);
-        client.initialize();
+        // delay actual initialization, allow for longer runtime of actual initialization
+        initJob = scheduler.submit(this::initializeLater);
+    }
+
+    public void initializeLater() {
+        final var tmpClient = client;
+        if (tmpClient != null) {
+            tmpClient.initialize();
+        }
     }
 
     @Override
     public void dispose() {
-        client.dispose();
+        final var tmpInitJob = initJob;
+        if (tmpInitJob != null) {
+            if (!tmpInitJob.isDone()) {
+                logger.trace("Bridge {}, shutdown during init, trying to cancel", thing.getUID());
+                tmpInitJob.cancel(true);
+                try {
+                    Thread.sleep(1000);
+                } catch (InterruptedException e) {
+                    logger.trace("Bridge {}, cancellation interrupted", thing.getUID());
+                }
+            }
+            initJob = null;
+        }
+
+        final var tmpClient = client;
+        if (tmpClient != null) {
+            tmpClient.dispose();
+            client = null;
+        }
         super.dispose();
     }
 
     @Override
-    protected AbstractKNXClient getClient() {
+    protected KNXClient getClient() {
-        return client;
+        KNXClient ret = client;
+        if (ret == null) {
+            return new NoOpClient();
+        }
+        return ret;
     }
 }