Skip to content

Commit

Permalink
[neohub] Recover faster if NeoHub produces empty responses (#13889)
Browse files Browse the repository at this point in the history 
* [neohub] resolve issue #13829
* [neohub] harmonise exceptions and logging
* [neohub] improve field name, and log messages

Signed-off-by: Andrew Fiddian-Green <software@whitebear.ch>
  • Loading branch information
@andrewfg
andrewfg authored Jan 4, 2023
1 parent de6ef7e commit 31fd911
Show file tree
Hide file tree
Showing 7 changed files with 166 additions and 86 deletions.
4 changes: 3 additions & 1 deletion ...hab.binding.neohub/src/main/java/org/openhab/binding/neohub/internal/NeoHubException.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,9 @@
import org.eclipse.jdt.annotation.NonNullByDefault;

/**
* The {@link NeoHubException} is a custom exception for NeoHub
* The {@link NeoHubException} is a custom checked exception for NeoHubs. It is thrown when the NeoHub encounters an
* error that is NOT I/O related. i.e. if the binding is able to connect to the NeoHub but the NeoHub returns unexpected
* results.
*
* @author Andrew Fiddian-Green - Initial contribution
*
Expand Down
37 changes: 25 additions & 12 deletions ...enhab.binding.neohub/src/main/java/org/openhab/binding/neohub/internal/NeoHubHandler.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@
public class NeoHubHandler extends BaseBridgeHandler {

private static final String SEE_README = "See documentation chapter \"Connection Refused Errors\"";
private static final int MAX_FAILED_SEND_ATTEMPTS = 2;

private final Logger logger = LoggerFactory.getLogger(NeoHubHandler.class);

Expand Down Expand Up @@ -86,6 +87,7 @@ private ApiVersion(String label) {

private ApiVersion apiVersion = ApiVersion.LEGACY;
private boolean isApiOnline = false;
private int failedSendAttempts = 0;

public NeoHubHandler(Bridge bridge) {
super(bridge);
Expand Down Expand Up @@ -146,11 +148,11 @@ public void initialize() {
NeoHubSocketBase socket;
try {
if (config.useWebSocket) {
socket = new NeoHubWebSocket(config);
socket = new NeoHubWebSocket(config, thing.getUID().getAsString());
} else {
socket = new NeoHubSocket(config);
socket = new NeoHubSocket(config, thing.getUID().getAsString());
}
} catch (NeoHubException e) {
} catch (IOException e) {
logger.debug("\"hub '{}' error creating web/tcp socket: '{}'", getThing().getUID(), e.getMessage());
return;
}
Expand Down Expand Up @@ -230,7 +232,7 @@ public void dispose() {
}
}

/*
/**
* device handlers call this to initiate a burst of fast polling requests (
* improves response time to users when openHAB changes a channel value )
*/
Expand All @@ -255,7 +257,7 @@ public synchronized NeoHubReturnResult toNeoHubSendChannelValue(String commandSt
startFastPollingBurst();

return NeoHubReturnResult.SUCCEEDED;
} catch (Exception e) {
} catch (IOException | NeoHubException e) {
updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.COMMUNICATION_ERROR);
logger.warn(MSG_FMT_SET_VALUE_ERR, getThing().getUID(), commandStr, e.getMessage());
return NeoHubReturnResult.ERR_COMMUNICATION;
Expand Down Expand Up @@ -331,7 +333,7 @@ public synchronized NeoHubReturnResult toNeoHubSendChannelValue(String commandSt
}

return deviceData;
} catch (Exception e) {
} catch (IOException | NeoHubException e) {
logger.warn(MSG_FMT_DEVICE_POLL_ERR, getThing().getUID(), e.getMessage());
updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.COMMUNICATION_ERROR);
return null;
Expand Down Expand Up @@ -376,13 +378,13 @@ public synchronized NeoHubReturnResult toNeoHubSendChannelValue(String commandSt
}

return systemData;
} catch (Exception e) {
} catch (IOException | NeoHubException e) {
logger.warn(MSG_FMT_SYSTEM_POLL_ERR, getThing().getUID(), e.getMessage());
return null;
}
}

/*
/**
* this is the callback used by the lazy polling scheduler.. fetches the info
* for all devices from the NeoHub, and passes the results the respective device
* handlers
Expand All @@ -394,7 +396,18 @@ private synchronized void lazyPollingSchedulerExecute() {
}

NeoHubAbstractDeviceData deviceData = fromNeoHubGetDeviceData();
if (deviceData != null) {
if (deviceData == null) {
if (fastPollingCallsToGo.get() == 0) {
failedSendAttempts++;
if (failedSendAttempts < MAX_FAILED_SEND_ATTEMPTS) {
logger.debug("lazyPollingSchedulerExecute() deviceData:null, running again");
scheduler.submit(() -> lazyPollingSchedulerExecute());
}
}
return;
} else {
failedSendAttempts = 0;

// dispatch deviceData to each of the hub's owned devices ..
List<Thing> children = getThing().getThings();
for (Thing child : children) {
Expand Down Expand Up @@ -448,7 +461,7 @@ private synchronized void lazyPollingSchedulerExecute() {
}
}

/*
/**
* this is the callback used by the fast polling scheduler.. checks if a fast
* polling burst is scheduled, and if so calls lazyPollingSchedulerExecute
*/
Expand All @@ -458,7 +471,7 @@ private void fastPollingSchedulerExecute() {
}
}

/*
/**
* select whether to use the old "deprecated" API or the new API
*/
private void selectApi() {
Expand Down Expand Up @@ -516,7 +529,7 @@ private void selectApi() {
this.isApiOnline = true;
}

/*
/**
* get the Engineers data
*/
public @Nullable NeoHubGetEngineersData fromNeoHubGetEngineersData() {
Expand Down
43 changes: 16 additions & 27 deletions ...penhab.binding.neohub/src/main/java/org/openhab/binding/neohub/internal/NeoHubSocket.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,10 @@
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import com.google.gson.JsonParser;

/**
* Handles the ASCII based communication via TCP socket between openHAB and NeoHub
* Handles the text based communication via TCP socket between openHAB and NeoHub
*
* @author Sebastian Prehn - Initial contribution
* @author Andrew Fiddian-Green - Refactoring for openHAB v2.x
Expand All @@ -36,8 +38,8 @@ public class NeoHubSocket extends NeoHubSocketBase {

private final Logger logger = LoggerFactory.getLogger(NeoHubSocket.class);

public NeoHubSocket(NeoHubConfiguration config) {
super(config);
public NeoHubSocket(NeoHubConfiguration config, String hubId) {
super(config, hubId);
}

@Override
Expand All @@ -52,19 +54,13 @@ public synchronized String sendMessage(final String requestJson) throws IOExcept

try (InputStreamReader reader = new InputStreamReader(socket.getInputStream(), US_ASCII);
OutputStreamWriter writer = new OutputStreamWriter(socket.getOutputStream(), US_ASCII)) {
if (logger.isDebugEnabled()) {
logger.debug("sending {} characters..", requestJson.length());
logger.debug(">> {}", requestJson);
}

//
logger.debug("hub '{}' sending characters:{}", hubId, requestJson.length());
writer.write(requestJson);
writer.write(0); // NULL terminate the command string
writer.flush();
socket.shutdownOutput();

if (logger.isTraceEnabled()) {
logger.trace("sent {} characters..", requestJson.length());
}
logger.trace("hub '{}' sent:{}", hubId, requestJson);

int inChar;
boolean done = false;
Expand All @@ -81,28 +77,21 @@ public synchronized String sendMessage(final String requestJson) throws IOExcept
caughtException = e;
}

String responseJson = builder.toString();

if (logger.isTraceEnabled()) {
logger.trace("received {} characters..", responseJson.length());
logger.trace("<< {}", responseJson);
} else
String responseJson = builder.toString().strip();

if (logger.isDebugEnabled()) {
logger.debug("received {} characters (set log level to TRACE to see full string)..", responseJson.length());
logger.debug("<< {} ...", responseJson.substring(0, Math.min(responseJson.length(), 30)));
}
logger.debug("hub '{}' received characters:{}", hubId, responseJson.length());
logger.trace("hub '{}' received:{}", hubId, responseJson);

// if any type of Exception was caught above, re-throw it again to the caller
// if an IOException was caught above, re-throw it again
if (caughtException != null) {
throw caughtException;
}

if (responseJson.isEmpty()) {
throw new NeoHubException("empty response string");
if (JsonParser.parseString(responseJson).isJsonObject()) {
return responseJson;
}

return responseJson;
logger.debug("hub '{}' Response is not a JSON object; response:{}", hubId, responseJson);
throw new NeoHubException("Invalid response");
}

@Override
Expand Down
10 changes: 6 additions & 4 deletions ...ab.binding.neohub/src/main/java/org/openhab/binding/neohub/internal/NeoHubSocketBase.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
import org.eclipse.jdt.annotation.NonNullByDefault;

/**
* Base abstract class for ASCII based communication between openHAB and NeoHub
* Base abstract class for text based communication between openHAB and NeoHub
*
* @author Andrew Fiddian-Green - Initial contribution
*
Expand All @@ -27,18 +27,20 @@
public abstract class NeoHubSocketBase implements Closeable {

protected final NeoHubConfiguration config;
protected final String hubId;

public NeoHubSocketBase(NeoHubConfiguration config) {
public NeoHubSocketBase(NeoHubConfiguration config, String hubId) {
this.config = config;
this.hubId = hubId;
}

/**
* Sends the message over the network to the NeoHub and returns its response
*
* @param requestJson the message to be sent to the NeoHub
* @return responseJson received from NeoHub
* @throws NeoHubException, IOException
*
* @throws IOException if there was a communication error or the socket state would not permit communication
* @throws NeoHubException if the communication returned a response but the response was not valid JSON
*/
public abstract String sendMessage(final String requestJson) throws IOException, NeoHubException;
}
Loading

0 comments on commit 31fd911

Please sign in to comment.