Skip to content

Commit

Permalink
[network] Add retries to speedtest timeouts (openhab#6851)
Browse files Browse the repository at this point in the history 
Fixes openhab#6495 

Signed-off-by: Gaël L'hopital <gael@lhopital.org>
  • Loading branch information
@clinique @andrewfg
clinique authored and andrewfg committed Aug 31, 2020
1 parent b54f1f6 commit 132bc29
Show file tree
Hide file tree
Showing 5 changed files with 86 additions and 46 deletions.
1 change: 1 addition & 0 deletions bundles/org.openhab.binding.network/README.md
View file
Expand Up @@ -64,6 +64,7 @@ Use the following options for a **network:speedtest**:
- **url:** Url of the speed test server.
- **fileName:** Name of the file to download from test server.
- **initialDelay:** Delay (in minutes) before starting the first speed test (can help avoid flooding your server at startup). Default: `5`.
- **maxTimeout:** Number of timeout events that can happend (resetted at success) before setting the thing offline. Default: `3`.

## Presence detection - Configure target device

Expand Down
75 changes: 42 additions & 33 deletions ...binding.network/src/main/java/org/openhab/binding/network/internal/PresenceDetection.java
View file
Expand Up @@ -12,6 +12,20 @@
*/
package org.openhab.binding.network.internal;

import java.io.IOException;
import java.net.Inet4Address;
import java.net.InetAddress;
import java.net.SocketException;
import java.net.UnknownHostException;
import java.util.HashSet;
import java.util.Set;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors;
import java.util.concurrent.ScheduledExecutorService;
import java.util.concurrent.ScheduledFuture;
import java.util.concurrent.TimeUnit;
import java.util.function.Consumer;

import org.eclipse.jdt.annotation.NonNullByDefault;
import org.eclipse.jdt.annotation.Nullable;
import org.eclipse.smarthome.core.cache.ExpiringCache;
Expand All @@ -25,16 +39,6 @@
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import java.io.IOException;
import java.net.Inet4Address;
import java.net.InetAddress;
import java.net.SocketException;
import java.net.UnknownHostException;
import java.util.HashSet;
import java.util.Set;
import java.util.concurrent.*;
import java.util.function.Consumer;

/**
* The {@link PresenceDetection} handles the connection to the Device
*
Expand Down Expand Up @@ -334,22 +338,22 @@ public boolean performPresenceDetection(boolean waitForDetectionToFinish) {
});
}

// ARP ping for IPv4 addresses. Use single executor for Windows tool and
// ARP ping for IPv4 addresses. Use single executor for Windows tool and
// each own executor for each network interface for other tools
if (arpPingMethod == ArpPingUtilEnum.ELI_FULKERSON_ARP_PING_FOR_WINDOWS) {
executorService.execute(() -> {
Thread.currentThread().setName("presenceDetectionARP_" + hostname + " ");
// arp-ping.exe tool capable of handling multiple interfaces by itself
performARPping("");
checkIfFinished();
Thread.currentThread().setName("presenceDetectionARP_" + hostname + " ");
// arp-ping.exe tool capable of handling multiple interfaces by itself
performARPping("");
checkIfFinished();
});
} else if (interfaceNames != null) {
for (final String interfaceName : interfaceNames) {
executorService.execute(() -> {
Thread.currentThread().setName("presenceDetectionARP_" + hostname + " " + interfaceName);
performARPping(interfaceName);
checkIfFinished();
});
Thread.currentThread().setName("presenceDetectionARP_" + hostname + " " + interfaceName);
performARPping(interfaceName);
checkIfFinished();
});
}
}

Expand Down Expand Up @@ -473,14 +477,16 @@ protected void performServicePing(int tcpPort) {
logger.trace("Perform TCP presence detection for {} on port: {}", hostname, tcpPort);
try {
InetAddress destinationAddress = destination.getValue();

networkUtils.servicePing(destinationAddress.getHostAddress(), tcpPort, timeoutInMS).ifPresent(o -> {
if(o.isSuccess()) {
PresenceDetectionValue v = updateReachableValue(PresenceDetectionType.TCP_CONNECTION, getLatency(o, preferResponseTimeAsLatency));
v.addReachableTcpService(tcpPort);
updateListener.partialDetectionResult(v);
}
});
if (destinationAddress != null) {
networkUtils.servicePing(destinationAddress.getHostAddress(), tcpPort, timeoutInMS).ifPresent(o -> {
if (o.isSuccess()) {
PresenceDetectionValue v = updateReachableValue(PresenceDetectionType.TCP_CONNECTION,
getLatency(o, preferResponseTimeAsLatency));
v.addReachableTcpService(tcpPort);
updateListener.partialDetectionResult(v);
}
});
}
} catch (IOException e) {
// This should not happen and might be a user configuration issue, we log a warning message therefore.
logger.warn("Could not create a socket connection", e);
Expand Down Expand Up @@ -510,10 +516,11 @@ protected void performARPping(String interfaceName) {
networkUtils.nativeARPPing(arpPingMethod, arpPingUtilPath, interfaceName,
destinationAddress.getHostAddress(), timeoutInMS).ifPresent(o -> {
if (o.isSuccess()) {
PresenceDetectionValue v = updateReachableValue(PresenceDetectionType.ARP_PING, getLatency(o, preferResponseTimeAsLatency));
PresenceDetectionValue v = updateReachableValue(PresenceDetectionType.ARP_PING,
getLatency(o, preferResponseTimeAsLatency));
updateListener.partialDetectionResult(v);
}
});
});
} catch (IOException e) {
logger.trace("Failed to execute an arp ping for ip {}", hostname, e);
} catch (InterruptedException ignored) {
Expand All @@ -537,7 +544,8 @@ protected void performJavaPing() {

networkUtils.javaPing(timeoutInMS, destinationAddress).ifPresent(o -> {
if (o.isSuccess()) {
PresenceDetectionValue v = updateReachableValue(PresenceDetectionType.ICMP_PING, getLatency(o, preferResponseTimeAsLatency));
PresenceDetectionValue v = updateReachableValue(PresenceDetectionType.ICMP_PING,
getLatency(o, preferResponseTimeAsLatency));
updateListener.partialDetectionResult(v);
}
});
Expand All @@ -553,12 +561,12 @@ protected void performSystemPing() {

networkUtils.nativePing(pingMethod, destinationAddress.getHostAddress(), timeoutInMS).ifPresent(o -> {
if (o.isSuccess()) {
PresenceDetectionValue v = updateReachableValue(PresenceDetectionType.ICMP_PING, getLatency(o, preferResponseTimeAsLatency));
PresenceDetectionValue v = updateReachableValue(PresenceDetectionType.ICMP_PING,
getLatency(o, preferResponseTimeAsLatency));
updateListener.partialDetectionResult(v);
}
});


} catch (IOException e) {
logger.trace("Failed to execute a native ping for ip {}", hostname, e);
} catch (InterruptedException e) {
Expand All @@ -567,7 +575,8 @@ protected void performSystemPing() {
}

private double getLatency(PingResult pingResult, boolean preferResponseTimeAsLatency) {
logger.debug("Getting latency from ping result {} using latency mode {}", pingResult, preferResponseTimeAsLatency);
logger.debug("Getting latency from ping result {} using latency mode {}", pingResult,
preferResponseTimeAsLatency);
// Execution time is always set and this value is also the default. So lets use it first.
double latency = pingResult.getExecutionTimeInMS();

Expand Down
1 change: 1 addition & 0 deletions ...ng.network/src/main/java/org/openhab/binding/network/internal/SpeedTestConfiguration.java
View file
Expand Up @@ -22,6 +22,7 @@ public class SpeedTestConfiguration {
public Integer refreshInterval = 20;
public Integer initialDelay = 5;
public Integer uploadSize = 1000000;
public Integer maxTimeout = 3;
private String url;
private String fileName;

Expand Down
49 changes: 36 additions & 13 deletions ....network/src/main/java/org/openhab/binding/network/internal/handler/SpeedTestHandler.java
View file
Expand Up @@ -56,6 +56,7 @@ public class SpeedTestHandler extends BaseThingHandler implements ISpeedTestList
private @NonNullByDefault({}) ScheduledFuture<?> refreshTask;
private @NonNullByDefault({}) SpeedTestConfiguration configuration;
private State bufferedProgress = UnDefType.UNDEF;
private int timeouts;

public SpeedTestHandler(Thing thing) {
super(thing);
Expand All @@ -64,11 +65,7 @@ public SpeedTestHandler(Thing thing) {
@Override
public void initialize() {
configuration = getConfigAs(SpeedTestConfiguration.class);
logger.info("Speedtests starts in {} minutes, then refreshes every {} minutes", configuration.initialDelay,
configuration.refreshInterval);
refreshTask = scheduler.scheduleWithFixedDelay(this::startSpeedTest, configuration.initialDelay,
configuration.refreshInterval, TimeUnit.MINUTES);
updateStatus(ThingStatus.ONLINE);
startRefreshTask();
}

synchronized private void startSpeedTest() {
Expand Down Expand Up @@ -103,6 +100,7 @@ synchronized private void stopSpeedTest() {

@Override
public void onCompletion(final @Nullable SpeedTestReport testReport) {
timeouts = configuration.maxTimeout;
if (testReport != null) {
BigDecimal rate = testReport.getTransferRateBit();
QuantityType<DataTransferRate> quantity = new QuantityType<DataTransferRate>(rate, BIT_PER_SECOND)
Expand Down Expand Up @@ -132,7 +130,17 @@ public void onError(final @Nullable SpeedTestError testError, final @Nullable St
updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.CONFIGURATION_ERROR, errorMessage);
freeRefreshTask();
return;
} else if (SpeedTestError.SOCKET_TIMEOUT.equals(testError) || SpeedTestError.SOCKET_ERROR.equals(testError)
} else if (SpeedTestError.SOCKET_TIMEOUT.equals(testError)) {
timeouts--;
if (timeouts <= 0) {
updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.COMMUNICATION_ERROR, "Max timeout count reached");
freeRefreshTask();
} else {
logger.warn("Speedtest timed out, {} attempts left. Message '{}'", timeouts, errorMessage);
stopSpeedTest();
}
return;
} else if (SpeedTestError.SOCKET_ERROR.equals(testError)
|| SpeedTestError.INVALID_HTTP_RESPONSE.equals(testError)) {
updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.COMMUNICATION_ERROR, errorMessage);
freeRefreshTask();
Expand All @@ -157,14 +165,20 @@ private void updateProgress(State state) {

@Override
public void handleCommand(ChannelUID channelUID, Command command) {
if (CHANNEL_TEST_ISRUNNING.equals(channelUID.getId())) {
if (command == OnOffType.ON) {
startSpeedTest();
} else if (command == OnOffType.OFF) {
stopSpeedTest();
}
if (command == OnOffType.ON
&& ThingStatusDetail.OFFLINE.COMMUNICATION_ERROR == getThing().getStatusInfo().getStatusDetail()) {
logger.debug("Speedtest was offline, restarting it upon command to do so");
startRefreshTask();
} else {
logger.debug("Command {} is not supported for channel: {}.", command, channelUID.getId());
if (CHANNEL_TEST_ISRUNNING.equals(channelUID.getId())) {
if (command == OnOffType.ON) {
startSpeedTest();
} else if (command == OnOffType.OFF) {
stopSpeedTest();
}
} else {
logger.debug("Command {} is not supported for channel: {}.", command, channelUID.getId());
}
}
}

Expand All @@ -180,4 +194,13 @@ private void freeRefreshTask() {
refreshTask = null;
}
}

private void startRefreshTask() {
logger.info("Speedtests starts in {} minutes, then refreshes every {} minutes", configuration.initialDelay,
configuration.refreshInterval);
refreshTask = scheduler.scheduleWithFixedDelay(this::startSpeedTest, configuration.initialDelay,
configuration.refreshInterval, TimeUnit.MINUTES);
timeouts = configuration.maxTimeout;
updateStatus(ThingStatus.ONLINE);
}
}
6 changes: 6 additions & 0 deletions bundles/org.openhab.binding.network/src/main/resources/ESH-INF/thing/thing-types.xml
View file
Expand Up @@ -143,6 +143,12 @@
<label>File Name</label>
<description>Name of the file to download from test server</description>
</parameter>
<parameter name="maxTimeout" type="integer">
<label>Timeouts</label>
<description>Number of timeout that can happend before the device is stated as offline</description>
<default>3</default>
<advanced>true</advanced>
</parameter>
</config-description>
</thing-type>

Expand Down

0 comments on commit 132bc29

Please sign in to comment.