Skip to content

Commit

Permalink
[knx] Improve handling of serial gateways (openhab#13897)
Browse files Browse the repository at this point in the history
* [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 openhab#13892.

Signed-off-by: Holger Friedrich <mail@holger-friedrich.de>
  • Loading branch information
holgerfriedrich authored and nemerdaud committed Feb 28, 2023
1 parent 9617a82 commit 0ea7d2f
Show file tree
Hide file tree
Showing 7 changed files with 71 additions and 38 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,6 @@
*/
package org.openhab.binding.knx.internal.config;

import java.math.BigDecimal;

import org.eclipse.jdt.annotation.NonNullByDefault;

/**
Expand All @@ -25,23 +23,23 @@
@NonNullByDefault
public class BridgeConfiguration {
private int autoReconnectPeriod = 0;
private BigDecimal readingPause = BigDecimal.valueOf(0);
private BigDecimal readRetriesLimit = BigDecimal.valueOf(0);
private BigDecimal responseTimeout = BigDecimal.valueOf(0);
private int readingPause = 0;
private int readRetriesLimit = 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;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,6 @@
*/
package org.openhab.binding.knx.internal.config;

import java.math.BigDecimal;

import org.eclipse.jdt.annotation.NonNullByDefault;

/**
Expand All @@ -27,8 +25,8 @@ public class DeviceConfig {

private String address = "";
private boolean fetch = false;
private BigDecimal pingInterval = BigDecimal.valueOf(0);
private BigDecimal readInterval = BigDecimal.valueOf(0);
private int pingInterval = 0;
private int readInterval = 0;

public String getAddress() {
return address;
Expand All @@ -38,11 +36,11 @@ public Boolean getFetch() {
return fetch;
}

public BigDecimal getPingInterval() {
public int getPingInterval() {
return pingInterval;
}

public BigDecimal getReadInterval() {
public int getReadInterval() {
return readInterval;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,6 @@
*/
package org.openhab.binding.knx.internal.config;

import java.math.BigDecimal;

import org.eclipse.jdt.annotation.NonNullByDefault;

/**
Expand All @@ -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 = "";
Expand All @@ -48,7 +46,7 @@ public String getIpAddress() {
return ipAddress;
}

public BigDecimal getPortNumber() {
public int getPortNumber() {
return portNumber;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,7 @@ private void pollDeviceStatus() {
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);
Expand Down Expand Up @@ -181,7 +181,7 @@ protected void attachToClient() {
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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ public DeviceThingHandler(Thing thing) {
public void initialize() {
super.initialize();
DeviceConfig config = getConfigAs(DeviceConfig.class);
readInterval = config.getReadInterval().intValue();
readInterval = config.getReadInterval();
initializeGroupAddresses();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,9 +65,7 @@ public void initialize() {
// initialisation would take too long and show a warning during binding startup
// KNX secure is adding serious delay
updateStatus(ThingStatus.UNKNOWN);
initJob = scheduler.submit(() -> {
initializeLater();
});
initJob = scheduler.submit(this::initializeLater);
}

public void initializeLater() {
Expand Down Expand Up @@ -106,7 +104,7 @@ public void initializeLater() {
}
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;
Expand Down Expand Up @@ -179,8 +177,8 @@ public void initializeLater() {
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(),
config.getReadingPause().intValue(), config.getReadRetriesLimit().intValue(), getScheduler(), this);
secureTunnel.user, secureTunnel.userKey, thing.getUID(), config.getResponseTimeout(),
config.getReadingPause(), config.getReadRetriesLimit(), getScheduler(), this);

final var tmpClient = client;
if (tmpClient != null) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -31,31 +37,66 @@
@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() {
return client;
protected KNXClient getClient() {
KNXClient ret = client;
if (ret == null) {
return new NoOpClient();
}
return ret;
}
}

0 comments on commit 0ea7d2f

Please sign in to comment.