Skip to content

Commit 222fccd

Browse files
dag81dag81
authored
[linktap] Bugfix Issue 18076 (openhab#18090)
[linktap] Bugfix Issue 18076 Signed-off-by: David Goodyear <[email protected]>
1 parent f2e4492 commit 222fccd

File tree

9 files changed

+173
-37
lines changed

9 files changed

+173
-37
lines changed

‎bundles/org.openhab.binding.linktap/src/main/java/org/openhab/binding/linktap/internal/LinkTapBridgeHandler.java

+116-27
Original file line numberDiff line numberDiff line change
@@ -13,9 +13,13 @@
1313
package org.openhab.binding.linktap.internal;
1414

1515
import static org.openhab.binding.linktap.internal.LinkTapBindingConstants.*;
16+
import static org.openhab.binding.linktap.internal.TransactionProcessor.MAX_COMMAND_RETRIES;
17+
import static org.openhab.binding.linktap.protocol.frames.GatewayDeviceResponse.ResultStatus.RET_GATEWAY_BUSY;
18+
import static org.openhab.binding.linktap.protocol.frames.GatewayDeviceResponse.ResultStatus.RET_GW_INTERNAL_ERR;
1619
import static org.openhab.binding.linktap.protocol.frames.TLGatewayFrame.*;
1720
import static org.openhab.binding.linktap.protocol.frames.ValidationError.Cause.BUG;
1821
import static org.openhab.binding.linktap.protocol.frames.ValidationError.Cause.USER;
22+
import static org.openhab.binding.linktap.protocol.http.TransientCommunicationIssueException.TransientExecptionDefinitions.GATEWAY_BUSY;
1923

2024
import java.io.IOException;
2125
import java.net.InetAddress;
@@ -105,6 +109,7 @@ public class LinkTapBridgeHandler extends BaseBridgeHandler {
105109
private volatile LinkTapBridgeConfiguration config = new LinkTapBridgeConfiguration();
106110
private volatile long lastGwCommandRecvTs = 0L;
107111
private volatile long lastMdnsScanMillis = -1L;
112+
private volatile boolean readZeroDevices = false;
108113

109114
private String bridgeKey = "";
110115
private IHttpClientProvider httpClientProvider;
@@ -143,7 +148,7 @@ private void startGwPolling() {
143148
cancelGwPolling();
144149
backgroundGwPollingScheduler = scheduler.scheduleWithFixedDelay(() -> {
145150
if (lastGwCommandRecvTs + 120000 < System.currentTimeMillis()) {
146-
getGatewayConfiguration();
151+
getGatewayConfiguration(false);
147152
}
148153
}, 5000, 120000, TimeUnit.MILLISECONDS);
149154
}
@@ -227,7 +232,15 @@ private boolean registerBridge(final LinkTapBridgeHandler ref) {
227232
return true;
228233
}
229234

230-
public void getGatewayConfiguration() {
235+
public boolean getGatewayConfigurationFreshCheck() {
236+
readZeroDevices = false;
237+
return getGatewayConfiguration(true);
238+
}
239+
240+
public boolean getGatewayConfiguration(final boolean forceFreshRead) {
241+
if (forceFreshRead) {
242+
lastGetConfigCache.invalidateValue();
243+
}
231244
String resp = "";
232245
synchronized (getConfigLock) {
233246
resp = lastGetConfigCache.getValue();
@@ -251,13 +264,17 @@ public void getGatewayConfiguration() {
251264
}
252265
lastGetConfigCache.putValue(resp);
253266
}
254-
255267
}
256268

257269
final GatewayConfigResp gwConfig = LinkTapBindingConstants.GSON.fromJson(resp, GatewayConfigResp.class);
258270
if (gwConfig == null) {
259-
return;
271+
return false;
260272
}
273+
274+
if (gwConfig.isRetryableError()) {
275+
return false;
276+
}
277+
261278
currentGwId = gwConfig.gatewayId;
262279

263280
final String version = gwConfig.version;
@@ -269,7 +286,6 @@ public void getGatewayConfiguration() {
269286
final Map<String, String> props = editProperties();
270287
props.put(BRIDGE_PROP_GW_VER, version);
271288
updateProperties(props);
272-
return;
273289
}
274290
if (!volUnit.equals(editProperties().get(BRIDGE_PROP_VOL_UNIT))) {
275291
final Map<String, String> props = editProperties();
@@ -285,6 +301,20 @@ public void getGatewayConfiguration() {
285301
}
286302
}
287303

304+
// Filter out the processing where we receive just a single response containing no device definitions, ensure
305+
// this is confirmed by a second poll.
306+
if (devIds.length == 0 || devNames.length == 0) {
307+
if (!readZeroDevices) {
308+
logger.trace("Detected ZERO devices in Gateway from CMD 16");
309+
readZeroDevices = true;
310+
lastGetConfigCache.invalidateValue();
311+
return false; // Don't process the potentially incorrect data
312+
}
313+
logger.debug("Confirmed ZERO devices in Gateway from CMD 16");
314+
} else {
315+
readZeroDevices = false;
316+
}
317+
288318
boolean updatedDeviceInfo = devIds.length != discoveredDevices.size();
289319

290320
for (int i = 0; i < devIds.length; ++i) {
@@ -298,19 +328,41 @@ public void getGatewayConfiguration() {
298328

299329
handlers.forEach(x -> x.handleMetadataRetrieved(this));
300330

301-
if (updatedDeviceInfo) {
302-
this.scheduler.execute(() -> {
303-
for (Thing el : getThing().getThings()) {
304-
final ThingHandler th = el.getHandler();
305-
if (th instanceof IBridgeData bridgeData) {
331+
final boolean forceDeviceInit = updatedDeviceInfo;
332+
this.scheduler.execute(() -> {
333+
for (Thing el : getThing().getThings()) {
334+
final ThingHandler th = el.getHandler();
335+
if (th instanceof IBridgeData bridgeData) {
336+
if (forceDeviceInit || ThingStatus.OFFLINE.equals(th.getThing().getStatus())) {
306337
bridgeData.handleBridgeDataUpdated();
307338
}
308339
}
309-
});
340+
}
341+
});
342+
343+
return true;
344+
}
345+
346+
public String sendApiRequest(final TLGatewayFrame request) {
347+
int triesLeft = MAX_COMMAND_RETRIES;
348+
int retry = 0;
349+
while (triesLeft > 0) {
350+
try {
351+
return sendSingleApiRequest(request);
352+
} catch (TransientCommunicationIssueException tcie) {
353+
--triesLeft;
354+
try {
355+
Thread.sleep(1000L * retry);
356+
} catch (InterruptedException ie) {
357+
return "";
358+
}
359+
++retry;
360+
}
310361
}
362+
return "";
311363
}
312364

313-
public String sendApiRequest(final TLGatewayFrame req) {
365+
public String sendSingleApiRequest(final TLGatewayFrame req) throws TransientCommunicationIssueException {
314366
final UUID uid = UUID.randomUUID();
315367

316368
final WebServerApi api = WebServerApi.getInstance();
@@ -329,17 +381,26 @@ public String sendApiRequest(final TLGatewayFrame req) {
329381
logger.debug("{} = APP BRIDGE -> GW -> Request {}", uid, reqData);
330382
final String respData = api.sendRequest(host, reqData);
331383
logger.debug("{} = APP BRIDGE -> GW -> Response {}", uid, respData);
332-
final TLGatewayFrame gwResponseFrame = LinkTapBindingConstants.GSON.fromJson(respData,
333-
TLGatewayFrame.class);
334-
if (confirmGateway && gwResponseFrame != null && !gwResponseFrame.gatewayId.equals(req.gatewayId)) {
335-
logger.warn("{}", getLocalizedText("warning.response-from-wrong-gw-id", uid, req.gatewayId,
336-
gwResponseFrame.gatewayId));
337-
return "";
338-
}
339-
if (gwResponseFrame != null && req.command != gwResponseFrame.command) {
340-
logger.warn("{}",
341-
getLocalizedText("warning.incorrect-cmd-resp", uid, req.command, gwResponseFrame.command));
342-
return "";
384+
final GatewayDeviceResponse gwResponseFrame = LinkTapBindingConstants.GSON.fromJson(respData,
385+
GatewayDeviceResponse.class);
386+
387+
if (gwResponseFrame != null) {
388+
if (confirmGateway && !gwResponseFrame.gatewayId.equals(req.gatewayId)) {
389+
logger.warn("{}", getLocalizedText("warning.response-from-wrong-gw-id", uid, req.gatewayId,
390+
gwResponseFrame.gatewayId));
391+
return "";
392+
}
393+
394+
if (RET_GW_INTERNAL_ERR.equals(gwResponseFrame.getRes())
395+
|| RET_GATEWAY_BUSY.equals(gwResponseFrame.getRes())) {
396+
throw new TransientCommunicationIssueException(GATEWAY_BUSY);
397+
}
398+
399+
if (req.command != gwResponseFrame.command) {
400+
logger.warn("{}",
401+
getLocalizedText("warning.incorrect-cmd-resp", uid, req.command, gwResponseFrame.command));
402+
return "";
403+
}
343404
}
344405
return respData;
345406
} catch (NotTapLinkGatewayException e) {
@@ -385,7 +446,11 @@ private void connect() {
385446
}
386447
}
387448

388-
getGatewayConfiguration();
449+
if (!getGatewayConfiguration(true)) {
450+
logger.debug("{}", getLocalizedText("bridge.info.awaiting-init"));
451+
scheduleReconnect();
452+
return;
453+
}
389454

390455
// Update the GW ID -> this bridge lookup
391456
GW_ID_LOOKUP.registerItem(currentGwId, this, () -> {
@@ -418,13 +483,33 @@ private void connect() {
418483
final Optional<String> servletEpOpt = (!servletEp.isEmpty()) ? Optional.of(servletEp) : Optional.empty();
419484
api.configureBridge(hostname, Optional.of(config.enableMDNS), Optional.of(config.enableJSONComms),
420485
servletEpOpt);
421-
updateStatus(ThingStatus.ONLINE);
422486
if (Thread.currentThread().isInterrupted()) {
423487
return;
424488
}
489+
490+
// Ensure we have a response with data in if not schedule a reconnect in 15 seconds, theres no reason
491+
// for a gateway with no devices.
492+
if (!getGatewayConfigurationFreshCheck()) {
493+
logger.debug("{}", getLocalizedText("bridge.info.awaiting-init"));
494+
scheduleReconnect();
495+
return;
496+
}
497+
498+
updateStatus(ThingStatus.ONLINE);
425499
startGwPolling();
426500
connectRepair = null;
427501

502+
// Force all child things run their init sequences to ensure they are registered by the
503+
// device ID.
504+
this.scheduler.execute(() -> {
505+
for (Thing el : getThing().getThings()) {
506+
final ThingHandler th = el.getHandler();
507+
if (th instanceof IBridgeData bridgeData) {
508+
bridgeData.handleBridgeDataUpdated();
509+
}
510+
}
511+
});
512+
428513
final Firmware firmware = new Firmware(getThing().getProperties().get(BRIDGE_PROP_GW_VER));
429514
if (!firmware.supportsLocalConfig()) {
430515
logger.warn("{}", getLocalizedText("warning.fw-update-local-config", getThing().getLabel(),
@@ -622,13 +707,17 @@ private void processCommand0(final String request) {
622707
}
623708
if (fullScanRequired) {
624709
logger.trace("The configured devices have changed a full scan should be run");
625-
scheduler.execute(this::getGatewayConfiguration);
710+
scheduler.execute(() -> {
711+
getGatewayConfiguration(true);
712+
});
626713
}
627714
}
628715

629716
@Override
630717
public void childHandlerDisposed(ThingHandler childHandler, Thing childThing) {
631-
scheduler.execute(this::getGatewayConfiguration);
718+
scheduler.execute(() -> {
719+
getGatewayConfiguration(false);
720+
});
632721
super.childHandlerDisposed(childHandler, childThing);
633722
}
634723
}

‎bundles/org.openhab.binding.linktap/src/main/java/org/openhab/binding/linktap/internal/LinkTapHandler.java

+6
Original file line numberDiff line numberDiff line change
@@ -445,6 +445,12 @@ private void updateOnOffValue(final String channelName, final @Nullable Boolean
445445
@Override
446446
public void handleBridgeDataUpdated() {
447447
switch (getThing().getStatus()) {
448+
case ONLINE:
449+
if (!initPending) {
450+
logger.trace("Handling new bridge data for {} not required already online and processed",
451+
getThing().getLabel());
452+
return;
453+
}
448454
case OFFLINE:
449455
case UNKNOWN:
450456
logger.trace("Handling new bridge data for {}", getThing().getLabel());

‎bundles/org.openhab.binding.linktap/src/main/java/org/openhab/binding/linktap/internal/PollingDeviceHandler.java

+23-1
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,8 @@
5656
@NonNullByDefault
5757
public abstract class PollingDeviceHandler extends BaseThingHandler implements IBridgeData {
5858

59+
protected boolean initPending = true;
60+
5961
protected static final String MARKER_INVALID_DEVICE_KEY = "---INVALID---";
6062

6163
private final Logger logger = LoggerFactory.getLogger(PollingDeviceHandler.class);
@@ -154,6 +156,7 @@ public void initialize() {
154156
}
155157

156158
protected void initAfterBridge(final LinkTapBridgeHandler bridge) {
159+
initPending = true;
157160
String deviceId = getValidatedIdString();
158161
if (MARKER_INVALID_DEVICE_KEY.equals(deviceId)) {
159162
updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.CONFIGURATION_ERROR,
@@ -167,19 +170,36 @@ protected void initAfterBridge(final LinkTapBridgeHandler bridge) {
167170
registeredDeviceId = deviceId;
168171
}
169172

173+
// This can be called, and then the bridge data gets updated
170174
boolean knownToBridge = bridge.getDiscoveredDevices().anyMatch(x -> deviceId.equals(x.deviceId));
171175
if (knownToBridge) {
172-
updateStatus(ThingStatus.ONLINE);
176+
if (!ThingStatus.ONLINE.equals(getThing().getStatus())) {
177+
updateStatus(ThingStatus.ONLINE);
178+
}
173179
registerDevice();
174180
scheduleInitialPoll();
175181
scheduler.execute(this::runStartInit);
176182
startStatusPolling();
183+
initPending = false;
177184
} else {
178185
updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.CONFIGURATION_ERROR,
179186
getLocalizedText("polling-device.error.unknown-device-id"));
180187
}
181188
}
182189

190+
@Override
191+
protected void updateStatus(ThingStatus status, ThingStatusDetail statusDetail, @Nullable String description) {
192+
super.updateStatus(status, statusDetail, description);
193+
scheduler.execute(() -> {
194+
if (initPending && ThingStatus.ONLINE.equals(status)) {
195+
final BridgeHandler handler = getBridgeHandler();
196+
if (handler instanceof LinkTapBridgeHandler linkTapBridge) {
197+
initAfterBridge(linkTapBridge);
198+
}
199+
}
200+
});
201+
}
202+
183203
protected abstract void runStartInit();
184204

185205
protected abstract void registerDevice();
@@ -247,6 +267,7 @@ public String getValidatedIdString() {
247267
final LinkTapDeviceMetadata metadata = vesyncBridgeHandler.getDeviceLookup().get(devId);
248268

249269
if (metadata != null) {
270+
logger.trace("Using matched Address ID (dev_id) {}", metadata.deviceId);
250271
return metadata.deviceId;
251272
}
252273
}
@@ -266,6 +287,7 @@ public String getValidatedIdString() {
266287
return MARKER_INVALID_DEVICE_KEY;
267288
}
268289

290+
logger.trace("Using matched Address ID (dev_name) {}", matchedAddressIds[0]);
269291
return matchedAddressIds[0];
270292
}
271293
}

‎bundles/org.openhab.binding.linktap/src/main/java/org/openhab/binding/linktap/internal/TransactionProcessor.java

+10-2
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ public final class TransactionProcessor {
6262
// As the Gateway is an embedded device,
6363

6464
private static final WebServerApi API = WebServerApi.getInstance();
65-
private static final int MAX_COMMAND_RETRIES = 3;
65+
static final int MAX_COMMAND_RETRIES = 3;
6666
private static final TransactionProcessor INSTANCE = new TransactionProcessor();
6767

6868
private final Logger logger = LoggerFactory.getLogger(TransactionProcessor.class);
@@ -207,6 +207,14 @@ public String sendRequest(final LinkTapBridgeHandler handler, final TLGatewayFra
207207
try {
208208
return sendSingleRequest(handler, request);
209209
} catch (TransientCommunicationIssueException tcie) {
210+
// Only retry a water timer status read once, with a 3 second delay
211+
if (request.command == CMD_UPDATE_WATER_TIMER_STATUS) {
212+
if (retry > 1) {
213+
return "";
214+
} else {
215+
retry = 3;
216+
}
217+
}
210218
--triesLeft;
211219
try {
212220
Thread.sleep(1000L * retry);
@@ -286,7 +294,7 @@ public String sendSingleRequest(final LinkTapBridgeHandler handler, final TLGate
286294
case RET_GATEWAY_BUSY:
287295
case RET_GW_INTERNAL_ERR:
288296
logger.trace("The request can be re-tried");
289-
break;
297+
throw new TransientCommunicationIssueException(GATEWAY_BUSY);
290298
case RET_CONFLICT_WATER_PLAN:
291299
logger.trace("Gateway rejected command due to water plan conflict");
292300
break;

‎bundles/org.openhab.binding.linktap/src/main/java/org/openhab/binding/linktap/protocol/frames/GatewayDeviceResponse.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ public boolean isSuccess() {
5757

5858
public boolean isRetryableError() {
5959
switch (getRes()) {
60-
case RET_CONFLICT_WATER_PLAN: // Conflict with watering plan
60+
case RET_GATEWAY_BUSY: // Gateway is busy (likely booting up)
6161
case RET_GW_INTERNAL_ERR: // Gateway internal error
6262
return true;
6363
default:

‎bundles/org.openhab.binding.linktap/src/main/java/org/openhab/binding/linktap/protocol/frames/GatewayEndDevListReq.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@
2929
* @author David Goodyear - Initial contribution
3030
*/
3131
@NonNullByDefault
32-
public class GatewayEndDevListReq extends TLGatewayFrame {
32+
public class GatewayEndDevListReq extends GatewayDeviceResponse {
3333

3434
protected static final Pattern FULL_DEVICE_ID_PATTERN = Pattern.compile("[a-zA-Z0-9]{20}");
3535

0 commit comments

Comments
 (0)