Skip to content

Commit

Permalink
[innogysmarthome] Reconnect fixes (openhab#8182) (openhab#8318)
Browse files Browse the repository at this point in the history 
* When an error occurs, it is waited at least for 30 seconds to avoid too many requests (on re-occurring errors).
* Workaround for side-effect of "Fix possible resource leak openhab#8080" - Improved by not rescheduling when the close/stop was actively triggered by the binding (in this case there is already a reschedule triggered)
* NPE fixed
* Code-Optimization

Closes openhab#8182 

Signed-off-by: Sven Strohschein <[email protected]>
  • Loading branch information
@Novanic @markus7017
Novanic authored and markus7017 committed Sep 18, 2020
1 parent c1e5d49 commit e80655d
Show file tree
Hide file tree
Showing 3 changed files with 26 additions and 30 deletions.
5 changes: 3 additions & 2 deletions ...smarthome/src/main/java/org/openhab/binding/innogysmarthome/internal/InnogyWebSocket.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ public class InnogyWebSocket {
/**
* Constructs the {@link InnogyWebSocket}.
*
* @param bridgeHandler the responsible {@link InnogyBridgeHandler}
* @param eventListener the responsible {@link InnogyBridgeHandler}
* @param webSocketURI the {@link URI} of the websocket endpoint
* @param maxIdleTimeout
*/
Expand Down Expand Up @@ -128,7 +128,8 @@ public void onConnect(Session session) {
public void onClose(int statusCode, String reason) {
if (statusCode == StatusCode.NORMAL) {
logger.info("Connection to innogy Webservice was closed normally.");
} else {
} else if (!closing) {
//An additional reconnect attempt is only required when the close/stop wasn't executed by the binding.
logger.info("Connection to innogy Webservice was closed abnormally (code: {}). Reason: {}", statusCode,
reason);
eventListener.connectionClosed();
Expand Down
26 changes: 12 additions & 14 deletions ...c/main/java/org/openhab/binding/innogysmarthome/internal/handler/InnogyBridgeHandler.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@
* The {@link InnogyBridgeHandler} is responsible for handling the innogy SmartHome controller including the connection
* to the innogy backend for all communications with the innogy {@link Device}s.
* <p/>
* It implements the {@link CredentialRefreshListener} to handle updates of the oauth2 tokens and the
* It implements the {@link AccessTokenRefreshListener} to handle updates of the oauth2 tokens and the
* {@link EventListener} to handle {@link Event}s, that are received by the {@link InnogyWebSocket}.
* <p/>
* The {@link Device}s are organized by the {@link DeviceStructureManager}, which is also responsible for the connection
Expand All @@ -97,8 +97,6 @@ public class InnogyBridgeHandler extends BaseBridgeHandler

public static final Set<ThingTypeUID> SUPPORTED_THING_TYPES = Collections.singleton(THING_TYPE_BRIDGE);

private static final long WEBSOCKET_TIMEOUT_RETRY_SECONDS = 5;

private final Logger logger = LoggerFactory.getLogger(InnogyBridgeHandler.class);
private final Gson gson = new Gson();
private final Object lock = new Object();
Expand Down Expand Up @@ -242,8 +240,10 @@ private void startClient() {
return;
}
}
setBridgeProperties(deviceStructMan.getBridgeDevice());
bridgeId = deviceStructMan.getBridgeDevice().getId();

Device bridgeDevice = deviceStructMan.getBridgeDevice();
setBridgeProperties(bridgeDevice);
bridgeId = bridgeDevice.getId();
startWebsocket();
cancelReinitJob();
}
Expand Down Expand Up @@ -508,7 +508,7 @@ public void onEvent(final String msg) {

case BaseEvent.TYPE_DISCONNECT:
logger.debug("Websocket disconnected.");
scheduleRestartClient(0);
scheduleRestartClient(REINITIALIZE_DELAY_SECONDS);
break;

case BaseEvent.TYPE_CONFIGURATION_CHANGED:
Expand Down Expand Up @@ -892,18 +892,17 @@ public void commandSetRollerShutterStop(final String deviceId, ShutterAction.Shu
* @return boolean true, if binding should continue.
*/
private boolean handleClientException(final Exception e) {
long reinitialize = REINITIALIZE_DELAY_SECONDS;
boolean isReinitialize = true;
if (e instanceof SessionExistsException) {
logger.debug("Session already exists. Continuing...");
reinitialize = -1;
isReinitialize = false;
} else if (e instanceof InvalidActionTriggeredException) {
logger.debug("Error triggering action: {}", e.getMessage());
reinitialize = -1;
isReinitialize = false;
} else if (e instanceof RemoteAccessNotAllowedException) {
// Remote access not allowed (usually by IP address change)
logger.debug("Remote access not allowed. Dropping access token and reinitializing binding...");
refreshAccessToken();
reinitialize = 0;
} else if (e instanceof ControllerOfflineException) {
logger.debug("innogy SmartHome Controller is offline.");
updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.BRIDGE_OFFLINE, e.getMessage());
Expand All @@ -919,12 +918,11 @@ private boolean handleClientException(final Exception e) {
updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.COMMUNICATION_ERROR, e.getMessage());
} else if (e instanceof TimeoutException) {
logger.debug("WebSocket timeout: {}", e.getMessage());
reinitialize = WEBSOCKET_TIMEOUT_RETRY_SECONDS;
} else if (e instanceof SocketTimeoutException) {
logger.debug("Socket timeout: {}", e.getMessage());
updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.COMMUNICATION_ERROR, e.getMessage());
} else if (e instanceof InterruptedException) {
reinitialize = -1;
isReinitialize = false;
Thread.currentThread().interrupt();
} else if (e instanceof ExecutionException) {
logger.debug("ExecutionException: {}", ExceptionUtils.getRootCauseMessage(e));
Expand All @@ -933,8 +931,8 @@ private boolean handleClientException(final Exception e) {
logger.debug("Unknown exception", e);
updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.NONE, e.getMessage());
}
if (reinitialize >= 0) {
scheduleRestartClient(reinitialize);
if (isReinitialize) {
scheduleRestartClient(REINITIALIZE_DELAY_SECONDS);
return true;
}
return false;
Expand Down
25 changes: 11 additions & 14 deletions ...c/main/java/org/openhab/binding/innogysmarthome/internal/handler/InnogyDeviceHandler.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ public InnogyDeviceHandler(final Thing thing) {
public void handleCommand(final ChannelUID channelUID, final Command command) {
logger.debug("handleCommand called for channel '{}' of type '{}' with command '{}'", channelUID,
getThing().getThingTypeUID().getId(), command);
final InnogyBridgeHandler innogyBridgeHandler = getInnogyBridgeHandler();
@Nullable final InnogyBridgeHandler innogyBridgeHandler = getInnogyBridgeHandler();
if (innogyBridgeHandler == null) {
logger.warn("BridgeHandler not found. Cannot handle command without bridge.");
return;
Expand All @@ -91,7 +91,7 @@ public void handleCommand(final ChannelUID channelUID, final Command command) {
}

if (command instanceof RefreshType) {
final Device device = innogyBridgeHandler.getDeviceById(deviceId);
@Nullable final Device device = innogyBridgeHandler.getDeviceById(deviceId);
if (device != null) {
onDeviceStateChanged(device);
}
Expand All @@ -102,8 +102,8 @@ public void handleCommand(final ChannelUID channelUID, final Command command) {
if (CHANNEL_SWITCH.equals(channelUID.getId())) {
// DEBUGGING HELPER
// ----------------
final Device device = innogyBridgeHandler.getDeviceById(deviceId);
if (DEBUG.equals(device.getConfig().getName())) {
@Nullable final Device device = innogyBridgeHandler.getDeviceById(deviceId);
if (device != null && DEBUG.equals(device.getConfig().getName())) {
logger.debug("DEBUG SWITCH ACTIVATED!");
if (OnOffType.ON.equals(command)) {
innogyBridgeHandler.onEvent(
Expand Down Expand Up @@ -244,7 +244,7 @@ private void initializeThing(@Nullable final ThingStatus bridgeStatus) {
*/
private boolean initializeProperties() {
synchronized (this.lock) {
final Device device = getDevice();
@Nullable final Device device = getDevice();
if (device != null) {
final Map<String, String> properties = editProperties();
properties.put(PROPERTY_ID, device.getId());
Expand Down Expand Up @@ -327,11 +327,11 @@ private boolean initializeProperties() {
private @Nullable InnogyBridgeHandler getInnogyBridgeHandler() {
synchronized (this.lock) {
if (this.bridgeHandler == null) {
final Bridge bridge = getBridge();
@Nullable final Bridge bridge = getBridge();
if (bridge == null) {
return null;
}
final ThingHandler handler = bridge.getHandler();
@Nullable final ThingHandler handler = bridge.getHandler();
if (handler instanceof InnogyBridgeHandler) {
this.bridgeHandler = (InnogyBridgeHandler) handler;
this.bridgeHandler.registerDeviceStatusListener(this);
Expand All @@ -356,7 +356,7 @@ public void onDeviceStateChanged(final Device device) {

// DEVICE STATES
if (device.hasDeviceState()) {
Boolean reachable = null;
@Nullable Boolean reachable = null;
if (device.getDeviceState().hasIsReachableState()) {
reachable = device.getDeviceState().isReachable();
}
Expand Down Expand Up @@ -904,18 +904,17 @@ public void onDeviceStateChanged(final Device changedDevice, final Event event)
} else {
logger.debug("Unsupported capability type {}.", capability.getType());
}
} else { // capability.hasState()
} else {
logger.debug("Capability {} has no state (yet?) - refreshing device.", capability.getName());
final InnogyBridgeHandler innogyBridgeHandler = getInnogyBridgeHandler();

@Nullable final InnogyBridgeHandler innogyBridgeHandler = getInnogyBridgeHandler();
if (innogyBridgeHandler != null) {
device = innogyBridgeHandler.refreshDevice(deviceId);
}
if (device != null) {
capabilityMap = device.getCapabilityMap();
capability = capabilityMap.get(linkedCapabilityId);
if (capability.hasState()) {
capabilityState = capability.getCapabilityState();
deviceChanged = true;
}
}
Expand All @@ -929,9 +928,7 @@ public void onDeviceStateChanged(final Device changedDevice, final Event event)
onDeviceStateChanged(device);
} else {
logger.debug("Device {}/{} has no state.", device.getConfig().getName(), device.getId());
return;
}

}
}
}
Expand All @@ -947,8 +944,8 @@ private int invertValueIfConfigured(final String channelId, final int value) {
logger.debug("Channel {} cannot be inverted.", channelId);
return value;
}
final Channel channel = getThing().getChannel(channelId);

@Nullable final Channel channel = getThing().getChannel(channelId);
if (channel == null) {
logger.debug("Channel {} was null! Value not inverted.", channelId);
return value;
Expand Down

0 comments on commit e80655d

Please sign in to comment.