[linktap] Bugfix Issue 18076

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

@@ -13,9 +13,13 @@
 package org.openhab.binding.linktap.internal;
 
 import static org.openhab.binding.linktap.internal.LinkTapBindingConstants.*;
+import static org.openhab.binding.linktap.internal.TransactionProcessor.MAX_COMMAND_RETRIES;
+import static org.openhab.binding.linktap.protocol.frames.GatewayDeviceResponse.ResultStatus.RET_GATEWAY_BUSY;
+import static org.openhab.binding.linktap.protocol.frames.GatewayDeviceResponse.ResultStatus.RET_GW_INTERNAL_ERR;
 import static org.openhab.binding.linktap.protocol.frames.TLGatewayFrame.*;
 import static org.openhab.binding.linktap.protocol.frames.ValidationError.Cause.BUG;
 import static org.openhab.binding.linktap.protocol.frames.ValidationError.Cause.USER;
+import static org.openhab.binding.linktap.protocol.http.TransientCommunicationIssueException.TransientExecptionDefinitions.GATEWAY_BUSY;
 
 import java.io.IOException;
 import java.net.InetAddress;
@@ -105,6 +109,7 @@ public class LinkTapBridgeHandler extends BaseBridgeHandler {
     private volatile LinkTapBridgeConfiguration config = new LinkTapBridgeConfiguration();
     private volatile long lastGwCommandRecvTs = 0L;
     private volatile long lastMdnsScanMillis = -1L;
+    private volatile boolean readZeroDevices = false;
 
     private String bridgeKey = "";
     private IHttpClientProvider httpClientProvider;
@@ -143,7 +148,7 @@ private void startGwPolling() {
             cancelGwPolling();
             backgroundGwPollingScheduler = scheduler.scheduleWithFixedDelay(() -> {
                 if (lastGwCommandRecvTs + 120000 < System.currentTimeMillis()) {
-                    getGatewayConfiguration();
+                    getGatewayConfiguration(false);
                 }
             }, 5000, 120000, TimeUnit.MILLISECONDS);
         }
@@ -227,7 +232,15 @@ private boolean registerBridge(final LinkTapBridgeHandler ref) {
         return true;
     }
 
-    public void getGatewayConfiguration() {
+    public boolean getGatewayConfigurationFreshCheck() {
+        readZeroDevices = false;
+        return getGatewayConfiguration(true);
+    }
+
+    public boolean getGatewayConfiguration(final boolean forceFreshRead) {
+        if (forceFreshRead) {
+            lastGetConfigCache.invalidateValue();
+        }
         String resp = "";
         synchronized (getConfigLock) {
             resp = lastGetConfigCache.getValue();
@@ -251,13 +264,17 @@ public void getGatewayConfiguration() {
                 }
                 lastGetConfigCache.putValue(resp);
             }
-
         }
 
         final GatewayConfigResp gwConfig = LinkTapBindingConstants.GSON.fromJson(resp, GatewayConfigResp.class);
         if (gwConfig == null) {
-            return;
+            return false;
         }
+
+        if (gwConfig.isRetryableError()) {
+            return false;
+        }
+
         currentGwId = gwConfig.gatewayId;
 
         final String version = gwConfig.version;
@@ -269,7 +286,6 @@ public void getGatewayConfiguration() {
             final Map<String, String> props = editProperties();
             props.put(BRIDGE_PROP_GW_VER, version);
             updateProperties(props);
-            return;
         }
         if (!volUnit.equals(editProperties().get(BRIDGE_PROP_VOL_UNIT))) {
             final Map<String, String> props = editProperties();
@@ -285,6 +301,20 @@ public void getGatewayConfiguration() {
             }
         }
 
+        // Filter out the processing where we receive just a single response containing no device definitions, ensure
+        // this is confirmed by a second poll.
+        if (devIds.length == 0 || devNames.length == 0) {
+            if (!readZeroDevices) {
+                logger.trace("Detected ZERO devices in Gateway from CMD 16");
+                readZeroDevices = true;
+                lastGetConfigCache.invalidateValue();
+                return false; // Don't process the potentially incorrect data
+            }
+            logger.debug("Confirmed ZERO devices in Gateway from CMD 16");
+        } else {
+            readZeroDevices = false;
+        }
+
         boolean updatedDeviceInfo = devIds.length != discoveredDevices.size();
 
         for (int i = 0; i < devIds.length; ++i) {
@@ -298,19 +328,41 @@ public void getGatewayConfiguration() {
 
         handlers.forEach(x -> x.handleMetadataRetrieved(this));
 
-        if (updatedDeviceInfo) {
-            this.scheduler.execute(() -> {
-                for (Thing el : getThing().getThings()) {
-                    final ThingHandler th = el.getHandler();
-                    if (th instanceof IBridgeData bridgeData) {
+        final boolean forceDeviceInit = updatedDeviceInfo;
+        this.scheduler.execute(() -> {
+            for (Thing el : getThing().getThings()) {
+                final ThingHandler th = el.getHandler();
+                if (th instanceof IBridgeData bridgeData) {
+                    if (forceDeviceInit || ThingStatus.OFFLINE.equals(th.getThing().getStatus())) {
                         bridgeData.handleBridgeDataUpdated();
                     }
                 }
-            });
+            }
+        });
+
+        return true;
+    }
+
+    public String sendApiRequest(final TLGatewayFrame request) {
+        int triesLeft = MAX_COMMAND_RETRIES;
+        int retry = 0;
+        while (triesLeft > 0) {
+            try {
+                return sendSingleApiRequest(request);
+            } catch (TransientCommunicationIssueException tcie) {
+                --triesLeft;
+                try {
+                    Thread.sleep(1000L * retry);
+                } catch (InterruptedException ie) {
+                    return "";
+                }
+                ++retry;
+            }
         }
+        return "";
     }
 
-    public String sendApiRequest(final TLGatewayFrame req) {
+    public String sendSingleApiRequest(final TLGatewayFrame req) throws TransientCommunicationIssueException {
         final UUID uid = UUID.randomUUID();
 
         final WebServerApi api = WebServerApi.getInstance();
@@ -329,17 +381,26 @@ public String sendApiRequest(final TLGatewayFrame req) {
             logger.debug("{} = APP BRIDGE -> GW -> Request {}", uid, reqData);
             final String respData = api.sendRequest(host, reqData);
             logger.debug("{} = APP BRIDGE -> GW -> Response {}", uid, respData);
-            final TLGatewayFrame gwResponseFrame = LinkTapBindingConstants.GSON.fromJson(respData,
-                    TLGatewayFrame.class);
-            if (confirmGateway && gwResponseFrame != null && !gwResponseFrame.gatewayId.equals(req.gatewayId)) {
-                logger.warn("{}", getLocalizedText("warning.response-from-wrong-gw-id", uid, req.gatewayId,
-                        gwResponseFrame.gatewayId));
-                return "";
-            }
-            if (gwResponseFrame != null && req.command != gwResponseFrame.command) {
-                logger.warn("{}",
-                        getLocalizedText("warning.incorrect-cmd-resp", uid, req.command, gwResponseFrame.command));
-                return "";
+            final GatewayDeviceResponse gwResponseFrame = LinkTapBindingConstants.GSON.fromJson(respData,
+                    GatewayDeviceResponse.class);
+
+            if (gwResponseFrame != null) {
+                if (confirmGateway && !gwResponseFrame.gatewayId.equals(req.gatewayId)) {
+                    logger.warn("{}", getLocalizedText("warning.response-from-wrong-gw-id", uid, req.gatewayId,
+                            gwResponseFrame.gatewayId));
+                    return "";
+                }
+
+                if (RET_GW_INTERNAL_ERR.equals(gwResponseFrame.getRes())
+                        || RET_GATEWAY_BUSY.equals(gwResponseFrame.getRes())) {
+                    throw new TransientCommunicationIssueException(GATEWAY_BUSY);
+                }
+
+                if (req.command != gwResponseFrame.command) {
+                    logger.warn("{}",
+                            getLocalizedText("warning.incorrect-cmd-resp", uid, req.command, gwResponseFrame.command));
+                    return "";
+                }
             }
             return respData;
         } catch (NotTapLinkGatewayException e) {
@@ -385,7 +446,11 @@ private void connect() {
                 }
             }
 
-            getGatewayConfiguration();
+            if (!getGatewayConfiguration(true)) {
+                logger.debug("{}", getLocalizedText("bridge.info.awaiting-init"));
+                scheduleReconnect();
+                return;
+            }
 
             // Update the GW ID -> this bridge lookup
             GW_ID_LOOKUP.registerItem(currentGwId, this, () -> {
@@ -418,13 +483,33 @@ private void connect() {
             final Optional<String> servletEpOpt = (!servletEp.isEmpty()) ? Optional.of(servletEp) : Optional.empty();
             api.configureBridge(hostname, Optional.of(config.enableMDNS), Optional.of(config.enableJSONComms),
                     servletEpOpt);
-            updateStatus(ThingStatus.ONLINE);
             if (Thread.currentThread().isInterrupted()) {
                 return;
             }
+
+            // Ensure we have a response with data in if not schedule a reconnect in 15 seconds, theres no reason
+            // for a gateway with no devices.
+            if (!getGatewayConfigurationFreshCheck()) {
+                logger.debug("{}", getLocalizedText("bridge.info.awaiting-init"));
+                scheduleReconnect();
+                return;
+            }
+
+            updateStatus(ThingStatus.ONLINE);
             startGwPolling();
             connectRepair = null;
 
+            // Force all child things run their init sequences to ensure they are registered by the
+            // device ID.
+            this.scheduler.execute(() -> {
+                for (Thing el : getThing().getThings()) {
+                    final ThingHandler th = el.getHandler();
+                    if (th instanceof IBridgeData bridgeData) {
+                        bridgeData.handleBridgeDataUpdated();
+                    }
+                }
+            });
+
             final Firmware firmware = new Firmware(getThing().getProperties().get(BRIDGE_PROP_GW_VER));
             if (!firmware.supportsLocalConfig()) {
                 logger.warn("{}", getLocalizedText("warning.fw-update-local-config", getThing().getLabel(),
@@ -622,13 +707,17 @@ private void processCommand0(final String request) {
         }
         if (fullScanRequired) {
             logger.trace("The configured devices have changed a full scan should be run");
-            scheduler.execute(this::getGatewayConfiguration);
+            scheduler.execute(() -> {
+                getGatewayConfiguration(true);
+            });
         }
     }
 
     @Override
     public void childHandlerDisposed(ThingHandler childHandler, Thing childThing) {
-        scheduler.execute(this::getGatewayConfiguration);
+        scheduler.execute(() -> {
+            getGatewayConfiguration(false);
+        });
         super.childHandlerDisposed(childHandler, childThing);
     }
 }

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

@@ -445,6 +445,12 @@ private void updateOnOffValue(final String channelName, final @Nullable Boolean
     @Override
     public void handleBridgeDataUpdated() {
         switch (getThing().getStatus()) {
+            case ONLINE:
+                if (!initPending) {
+                    logger.trace("Handling new bridge data for {} not required already online and processed",
+                            getThing().getLabel());
+                    return;
+                }
             case OFFLINE:
             case UNKNOWN:
                 logger.trace("Handling new bridge data for {}", getThing().getLabel());

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

@@ -56,6 +56,8 @@
 @NonNullByDefault
 public abstract class PollingDeviceHandler extends BaseThingHandler implements IBridgeData {
 
+    protected boolean initPending = true;
+
     protected static final String MARKER_INVALID_DEVICE_KEY = "---INVALID---";
 
     private final Logger logger = LoggerFactory.getLogger(PollingDeviceHandler.class);
@@ -154,6 +156,7 @@ public void initialize() {
     }
 
     protected void initAfterBridge(final LinkTapBridgeHandler bridge) {
+        initPending = true;
         String deviceId = getValidatedIdString();
         if (MARKER_INVALID_DEVICE_KEY.equals(deviceId)) {
             updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.CONFIGURATION_ERROR,
@@ -167,19 +170,36 @@ protected void initAfterBridge(final LinkTapBridgeHandler bridge) {
             registeredDeviceId = deviceId;
         }
 
+        // This can be called, and then the bridge data gets updated
         boolean knownToBridge = bridge.getDiscoveredDevices().anyMatch(x -> deviceId.equals(x.deviceId));
         if (knownToBridge) {
-            updateStatus(ThingStatus.ONLINE);
+            if (!ThingStatus.ONLINE.equals(getThing().getStatus())) {
+                updateStatus(ThingStatus.ONLINE);
+            }
             registerDevice();
             scheduleInitialPoll();
             scheduler.execute(this::runStartInit);
             startStatusPolling();
+            initPending = false;
         } else {
             updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.CONFIGURATION_ERROR,
                     getLocalizedText("polling-device.error.unknown-device-id"));
         }
     }
 
+    @Override
+    protected void updateStatus(ThingStatus status, ThingStatusDetail statusDetail, @Nullable String description) {
+        super.updateStatus(status, statusDetail, description);
+        scheduler.execute(() -> {
+            if (initPending && ThingStatus.ONLINE.equals(status)) {
+                final BridgeHandler handler = getBridgeHandler();
+                if (handler instanceof LinkTapBridgeHandler linkTapBridge) {
+                    initAfterBridge(linkTapBridge);
+                }
+            }
+        });
+    }
+
     protected abstract void runStartInit();
 
     protected abstract void registerDevice();
@@ -247,6 +267,7 @@ public String getValidatedIdString() {
                 final LinkTapDeviceMetadata metadata = vesyncBridgeHandler.getDeviceLookup().get(devId);
 
                 if (metadata != null) {
+                    logger.trace("Using matched Address ID (dev_id) {}", metadata.deviceId);
                     return metadata.deviceId;
                 }
             }
@@ -266,6 +287,7 @@ public String getValidatedIdString() {
                     return MARKER_INVALID_DEVICE_KEY;
                 }
 
+                logger.trace("Using matched Address ID (dev_name) {}", matchedAddressIds[0]);
                 return matchedAddressIds[0];
             }
         }

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

@@ -62,7 +62,7 @@ public final class TransactionProcessor {
     // As the Gateway is an embedded device,
 
     private static final WebServerApi API = WebServerApi.getInstance();
-    private static final int MAX_COMMAND_RETRIES = 3;
+    static final int MAX_COMMAND_RETRIES = 3;
     private static final TransactionProcessor INSTANCE = new TransactionProcessor();
 
     private final Logger logger = LoggerFactory.getLogger(TransactionProcessor.class);
@@ -207,6 +207,14 @@ public String sendRequest(final LinkTapBridgeHandler handler, final TLGatewayFra
             try {
                 return sendSingleRequest(handler, request);
             } catch (TransientCommunicationIssueException tcie) {
+                // Only retry a water timer status read once, with a 3 second delay
+                if (request.command == CMD_UPDATE_WATER_TIMER_STATUS) {
+                    if (retry > 1) {
+                        return "";
+                    } else {
+                        retry = 3;
+                    }
+                }
                 --triesLeft;
                 try {
                     Thread.sleep(1000L * retry);
@@ -286,7 +294,7 @@ public String sendSingleRequest(final LinkTapBridgeHandler handler, final TLGate
                         case RET_GATEWAY_BUSY:
                         case RET_GW_INTERNAL_ERR:
                             logger.trace("The request can be re-tried");
-                            break;
+                            throw new TransientCommunicationIssueException(GATEWAY_BUSY);
                         case RET_CONFLICT_WATER_PLAN:
                             logger.trace("Gateway rejected command due to water plan conflict");
                             break;

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

@@ -57,7 +57,7 @@ public boolean isSuccess() {
 
     public boolean isRetryableError() {
         switch (getRes()) {
-            case RET_CONFLICT_WATER_PLAN: // Conflict with watering plan
+            case RET_GATEWAY_BUSY: // Gateway is busy (likely booting up)
             case RET_GW_INTERNAL_ERR: // Gateway internal error
                 return true;
             default:

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

@@ -29,7 +29,7 @@
  * @author David Goodyear - Initial contribution
  */
 @NonNullByDefault
-public class GatewayEndDevListReq extends TLGatewayFrame {
+public class GatewayEndDevListReq extends GatewayDeviceResponse {
 
     protected static final Pattern FULL_DEVICE_ID_PATTERN = Pattern.compile("[a-zA-Z0-9]{20}");