Skip to content

Commit

Permalink
[insteon] refactor downloading of modem database (openhab#7794)
Browse files Browse the repository at this point in the history 
Signed-off-by: Rob Nielsen <[email protected]>
  • Loading branch information
@robnielsen @andrewfg
robnielsen authored and andrewfg committed Aug 31, 2020
1 parent 2a467f1 commit 7ad05db
Show file tree
Hide file tree
Showing 8 changed files with 56 additions and 77 deletions.
8 changes: 2 additions & 6 deletions bundles/org.openhab.binding.insteon/README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,6 @@ The Insteon PLM or hub is configured with the following parameters:
|----------|---------:|--------:|-------------|
| port | | Yes | **Examples:**<br>- PLM on Linux: `/dev/ttyS0` or `/dev/ttyUSB0`<br>- Smartenit ZBPLM on Linux: `/dev/ttyUSB0,baudRate=115200`<br>- PLM on Windows: `COM1`<br>- Current hub (2245-222) at 192.168.1.100 on port 25105, with a poll interval of 1000 ms (1 second): `/hub2/my_user_name:[email protected]:25105,poll_time=1000`<br>- Legacy hub (2242-222) at 192.168.1.100 on port 9761:`/hub/192.168.1.100:9761`<br>- Networked PLM using ser2net at 192.168.1.100 on port 9761:`/tcp/192.168.1.100:9761` |
| devicePollIntervalSeconds | 300 | No | Poll interval of devices in seconds. Poll too often and you will overload the insteon network, leading to sluggish or no response when trying to send messages to devices. The default poll interval of 300 seconds has been tested and found to be a good compromise in a configuration of about 110 switches/dimmers. |
|modemDbRetryTimeoutSeconds|120|No|The number of seconds to wait for the modem database to completely download before retrying again. Under normal circumstances this will not need to be modified.|
| additionalDevices | | No | Optional file with additional device types. The syntax of the file is identical to the `device_types.xml` file in the source tree. Please remember to post successfully added device types to the openhab group so the developers can include them into the `device_types.xml` file! |
| additionalFeatures | | No | Optional file with additional feature templates, like in the `device_features.xml` file in the source tree. |

Expand Down Expand Up @@ -827,9 +826,6 @@ If new devices are linked, the binding must be restarted.
Use the [Insteon Terminal](https://github.com/pfrommerd/insteon-terminal) for that.
If using Insteon Terminal (especially as root), ensure any stale lock files (For example, /var/lock/LCK..ttyUSB0) are removed before starting openHAB runtime.
Failure to do so may result in "found no ports".
* Very rarely during binding startup, a message arrives at the modem while the initial read of the modem database happens.
Somehow the modem then stops sending the remaining link records and the binding no longer is able to address the missing devices.
The fix is to simply restart the binding.
* The Insteon PLM device is know to break after about 2-3 years due to poorly sized capacitors of the power supply.
With a bit of soldering skill you can repair it yourself, see [here](http://pfrommer.us/home-automation) or [the original thread](http://forum.universal-devices.com/topic/13866-repair-of-2413s-plm-when-the-power-supply-fails/).
* The Insteon PLM or hub is know to break in about 2-3 years due to poorly sized capacitors.
You can repair it yourself using basic soldering skills, search for "Insteon PLM repair" or "Insteon hub repair".
* Using the Insteon Hub 2014 in conjunction with other applications (such as the InsteonApp) is not supported. Concretely, openHAB will not learn when a switch is flipped via the Insteon App until the next poll, which could take minutes.
11 changes: 3 additions & 8 deletions ...ab.binding.insteon/src/main/java/org/openhab/binding/insteon/internal/InsteonBinding.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import java.util.Map.Entry;
import java.util.Set;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.ScheduledExecutorService;

import javax.xml.parsers.ParserConfigurationException;

Expand Down Expand Up @@ -122,13 +123,13 @@ public class InsteonBinding {
private InsteonNetworkHandler handler;

public InsteonBinding(InsteonNetworkHandler handler, @Nullable InsteonNetworkConfiguration config,
@Nullable SerialPortManager serialPortManager) {
@Nullable SerialPortManager serialPortManager, ScheduledExecutorService scheduler) {
this.handler = handler;

String port = config.getPort();
logger.debug("port = '{}'", Utils.redactPassword(port));

driver = new Driver(port, portListener, serialPortManager);
driver = new Driver(port, portListener, serialPortManager, scheduler);
driver.addMsgListener(portListener);

Integer devicePollIntervalSeconds = config.getDevicePollIntervalSeconds();
Expand All @@ -137,12 +138,6 @@ public InsteonBinding(InsteonNetworkHandler handler, @Nullable InsteonNetworkCon
}
logger.debug("device poll interval set to {} seconds", devicePollIntervalMilliseconds / 1000);

Integer modemDbRetryTimeoutSeconds = config.getModemDbRetryTimeoutSeconds();
if (modemDbRetryTimeoutSeconds != null) {
logger.debug("setting modem db retry timeout to {} seconds", modemDbRetryTimeoutSeconds);
driver.setModemDBRetryTimeout(modemDbRetryTimeoutSeconds * 1000);
}

String additionalDevices = config.getAdditionalDevices();
if (additionalDevices != null) {
try {
Expand Down
6 changes: 0 additions & 6 deletions ...rc/main/java/org/openhab/binding/insteon/internal/config/InsteonNetworkConfiguration.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,6 @@ public class InsteonNetworkConfiguration {

private @Nullable Integer devicePollIntervalSeconds;

private @Nullable Integer modemDbRetryTimeoutSeconds;

private @Nullable String additionalDevices;

private @Nullable String additionalFeatures;
Expand All @@ -42,10 +40,6 @@ public String getPort() {
return devicePollIntervalSeconds;
}

public @Nullable Integer getModemDbRetryTimeoutSeconds() {
return modemDbRetryTimeoutSeconds;
}

public @Nullable String getAdditionalDevices() {
return additionalDevices;
}
Expand Down
77 changes: 42 additions & 35 deletions ...ing.insteon/src/main/java/org/openhab/binding/insteon/internal/device/ModemDBBuilder.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,9 @@
import java.util.List;
import java.util.Map;
import java.util.Map.Entry;
import java.util.concurrent.ScheduledExecutorService;
import java.util.concurrent.ScheduledFuture;
import java.util.concurrent.TimeUnit;

import org.eclipse.jdt.annotation.NonNullByDefault;
import org.eclipse.jdt.annotation.Nullable;
Expand All @@ -37,56 +40,57 @@
*/
@NonNullByDefault
@SuppressWarnings("null")
public class ModemDBBuilder implements MsgListener, Runnable {
public class ModemDBBuilder implements MsgListener {
private static final int MESSAGE_TIMEOUT = 30000;

private final Logger logger = LoggerFactory.getLogger(ModemDBBuilder.class);
private boolean isComplete = false;

private volatile boolean isComplete = false;
private Port port;
private @Nullable Thread writeThread = null;
private int timeoutMillis = 120000;
private ScheduledExecutorService scheduler;
private @Nullable ScheduledFuture<?> job = null;
private volatile long lastMessageTimestamp;
private volatile int messageCount = 0;

public ModemDBBuilder(Port port) {
public ModemDBBuilder(Port port, ScheduledExecutorService scheduler) {
this.port = port;
}

public void setRetryTimeout(int timeout) {
this.timeoutMillis = timeout;
this.scheduler = scheduler;
}

public void start() {
port.addListener(this);
writeThread = new Thread(this);
writeThread.setName("Insteon DBBuilder");
writeThread.setDaemon(true);
writeThread.start();
logger.debug("querying port for first link record");

logger.trace("starting modem db builder");
startDownload();
job = scheduler.scheduleWithFixedDelay(() -> {
if (isComplete()) {
logger.trace("modem db builder finished");
job.cancel(false);
job = null;
} else {
if (System.currentTimeMillis() - lastMessageTimestamp > MESSAGE_TIMEOUT) {
String s = "";
if (messageCount == 0) {
s = " No messages were received, the PLM or hub might be broken. If this continues see "
+ "'Known Limitations and Issues' in the Insteon binding documentation.";
}
logger.warn("Modem database download was unsuccessful, restarting!{}", s);
startDownload();
}
}
}, 0, 1, TimeUnit.SECONDS);
}

public void startDownload() {
private void startDownload() {
logger.trace("starting modem database download");
port.clearModemDB();
lastMessageTimestamp = System.currentTimeMillis();
messageCount = 0;
getFirstLinkRecord();
}

public synchronized boolean isComplete() {
return (isComplete);
}

@Override
public void run() {
logger.trace("starting modem db builder thread");
while (!isComplete()) {
startDownload();
try {
Thread.sleep(timeoutMillis); // wait for download to complete
} catch (InterruptedException e) {
logger.warn("modem db builder thread interrupted");
break;
}
if (!isComplete()) {
logger.warn("modem database download unsuccessful, restarting!");
}
}
logger.trace("exiting modem db builder thread");
public boolean isComplete() {
return isComplete;
}

private void getFirstLinkRecord() {
Expand All @@ -106,6 +110,9 @@ private void getFirstLinkRecord() {
*/
@Override
public void msg(Msg msg) {
lastMessageTimestamp = System.currentTimeMillis();
messageCount++;

if (msg.isPureNack()) {
return;
}
Expand Down
13 changes: 4 additions & 9 deletions ...hab.binding.insteon/src/main/java/org/openhab/binding/insteon/internal/driver/Driver.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
import java.io.IOException;
import java.util.HashMap;
import java.util.Map;
import java.util.concurrent.ScheduledExecutorService;
import java.util.concurrent.locks.ReentrantLock;

import org.eclipse.jdt.annotation.NonNullByDefault;
Expand All @@ -37,19 +38,13 @@ public class Driver {
private DriverListener listener;
private Map<InsteonAddress, @Nullable ModemDBEntry> modemDBEntries = new HashMap<>();
private ReentrantLock modemDBEntriesLock = new ReentrantLock();
private int modemDBRetryTimeout = 120000; // in milliseconds

public Driver(String portName, DriverListener listener, @Nullable SerialPortManager serialPortManager) {
public Driver(String portName, DriverListener listener, @Nullable SerialPortManager serialPortManager,
ScheduledExecutorService scheduler) {
this.listener = listener;
this.portName = portName;

port = new Port(portName, this, serialPortManager);
port.setModemDBRetryTimeout(modemDBRetryTimeout);
}

public void setModemDBRetryTimeout(int timeout) {
modemDBRetryTimeout = timeout;
port.setModemDBRetryTimeout(modemDBRetryTimeout);
port = new Port(portName, this, serialPortManager, scheduler);
}

public boolean isReady() {
Expand Down
10 changes: 4 additions & 6 deletions ...enhab.binding.insteon/src/main/java/org/openhab/binding/insteon/internal/driver/Port.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
import java.util.Map;
import java.util.Random;
import java.util.concurrent.LinkedBlockingQueue;
import java.util.concurrent.ScheduledExecutorService;
import java.util.concurrent.atomic.AtomicBoolean;

import org.eclipse.jdt.annotation.NonNullByDefault;
Expand Down Expand Up @@ -92,7 +93,8 @@ enum ReplyType {
* @param devName the name of the port, i.e. '/dev/insteon'
* @param d The Driver object that manages this port
*/
public Port(String devName, Driver d, @Nullable SerialPortManager serialPortManager) {
public Port(String devName, Driver d, @Nullable SerialPortManager serialPortManager,
ScheduledExecutorService scheduler) {
this.devName = devName;
this.driver = d;
this.logName = Utils.redactPassword(devName);
Expand All @@ -101,7 +103,7 @@ public Port(String devName, Driver d, @Nullable SerialPortManager serialPortMana
this.ioStream = IOStream.create(serialPortManager, devName);
this.reader = new IOStreamReader();
this.writer = new IOStreamWriter();
this.mdbb = new ModemDBBuilder(this);
this.mdbb = new ModemDBBuilder(this, scheduler);
}

public boolean isModem(InsteonAddress a) {
Expand All @@ -128,10 +130,6 @@ public Driver getDriver() {
return driver;
}

public void setModemDBRetryTimeout(int timeout) {
mdbb.setRetryTimeout(timeout);
}

public void addListener(MsgListener l) {
synchronized (listeners) {
if (!listeners.contains(l)) {
Expand Down
2 changes: 1 addition & 1 deletion ...eon/src/main/java/org/openhab/binding/insteon/internal/handler/InsteonNetworkHandler.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ public void initialize() {
updateStatus(ThingStatus.UNKNOWN);

scheduler.execute(() -> {
insteonBinding = new InsteonBinding(this, config, serialPortManager);
insteonBinding = new InsteonBinding(this, config, serialPortManager, scheduler);

// hold off on starting to poll until devices that already are defined as things are added.
// wait SETTLE_TIME_IN_SECONDS to start then check every second afterwards until it has been at
Expand Down
6 changes: 0 additions & 6 deletions bundles/org.openhab.binding.insteon/src/main/resources/ESH-INF/thing/thing-types.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,6 @@
<description>Device poll interval in seconds.</description>
</parameter>

<parameter name="modemDbRetryTimeoutSeconds" type="integer" min="30" max="600">
<label>Modem DB Retry Timeout</label>
<description>Modem DB retry timeout in seconds.</description>
</parameter>


<parameter name="additionalDevices" type="text">
<label>Additional Devices</label>
<description>Optional file with additional device types.</description>
Expand Down

0 comments on commit 7ad05db

Please sign in to comment.