[tuya] Improve handling of battery devices (openhab#20760)

* [tuya] Improve handling of battery devices

Signed-off-by: Mike Jagdis <mjagdis@eris-associates.co.uk>
Signed-off-by: olemr <olemr@olemr.com>

Author: mjagdis

bundles/org.openhab.binding.tuya/src/main/java/org/openhab/binding/tuya/internal/TuyaBindingConstants.java

@@ -52,6 +52,10 @@ public class TuyaBindingConstants {
     public static final List<String> DIMMER_CHANNEL_CODES = List.of("bright_value", "bright_value_1", "bright_value_2",
             "temp_value");
 
+    // The maximum length of time a connection to the device is maintained. After this we close
+    // and reconnect in an attempt to limit possible connection related, device-side memory leaks.
+    public static final int TCP_CONNECTION_MAX_LIFETIME = 86400; // Seconds
+
     // The heartbeat interval specifies the maximum amount of time that can pass without us
     // sending anything to a device. Once the heartbeat interval is reached we send a heartbeat
     // message to let the device know we are still present. Devices that do not receive traffic
@@ -62,13 +66,13 @@ public class TuyaBindingConstants {
     // The amount of time a device has to respond to a message. If we don't see anything from
     // the device for this long after sending a message we consider the connection dead, close
     // it, and start trying to reconnect.
-    public static final int TCP_CONNECTION_MESSAGE_RESPONSE = 1; // Seconds
+    public static final int TCP_CONNECTION_MESSAGE_RESPONSE = 200; // Milliseconds
 
     // How long to wait for a TCP session to connect before closing it and starting again. We do
     // not rely on TCP's own retry strategy because that varies between implementations so we
     // cannot know when the retry interval has become so great that it exceeds the amount of
     // time a battery device may be awake.
-    public static final int TCP_CONNECT_TIMEOUT = 1000; // Milliseconds
+    public static final int TCP_CONNECT_TIMEOUT = 500; // Milliseconds
 
     // How long to wait before attempting another connection after the previous closed or failed.
     // Note that if the previous attempt failed because the device was not reachable the interval
@@ -78,5 +82,19 @@ public class TuyaBindingConstants {
     // of TCP_CONNECT_TIMEOUT and TCP_CONNECT_RETRY_INTERVAL must therefore be small enough that at
     // least one, preferably two or three, connection attempts will be made during the time the
     // device is awake.
-    public static final int TCP_CONNECT_RETRY_INTERVAL = 1000; // Milliseconds
+    public static final int TCP_CONNECT_RETRY_INTERVAL = 50; // Milliseconds
+
+    // How long to wait before sending the initial query after connecting.
+    // We need to delay the initial query because some battery devices seem to ignore requests that
+    // come too soon and sometimes they claim DP_QUERY isn't supported when it really is. Perhaps the
+    // TCP stack is initialized before the API?
+    public static final int TCP_CONNECT_INITIAL_DELAY = 750; // Milliseconds
+
+    // How long to wait before attempting another connection after the first "Connection refused".
+    // When battery devices wake up their TCP can come online before the API server is started
+    // and even before the API backend is plugged in to the API server. Hammering battery devices
+    // (which tend to be especially slow) with connection attempts will just waste their CPU cycles
+    // and can mean they don't even come online until a second event has overwritten the first
+    // (especially a problem for contact sensors).
+    public static final int TCP_CONNECT_INITIAL_INTERVAL = 1000; // Milliseconds
 }

bundles/org.openhab.binding.tuya/src/main/java/org/openhab/binding/tuya/internal/handler/TuyaDeviceHandler.java

@@ -147,17 +147,6 @@ public TuyaDeviceHandler(Thing thing, Gson gson,
 
     @Override
     public void processDeviceStatus(Map<Integer, Object> deviceStatus) {
-        // Older devices may need to use the control method to request device status.
-        if (deviceStatus.isEmpty()) {
-            TuyaDevice tuyaDevice = this.tuyaDevice;
-            if (tuyaDevice != null) {
-                logger.debug("'{}' switching to control instead of query", thing.getUID());
-                tuyaDevice.setQueryUsesControl();
-                tuyaDevice.requestStatus(List.of());
-            }
-            return;
-        }
-
         // Changes to function DPs might lead to changes in status DPs. For instance, if a
         // power switch is turned on the measured current and power can be expected to change
         // within a few seconds.
@@ -178,6 +167,8 @@ public void processDeviceStatus(Map<Integer, Object> deviceStatus) {
         }
 
         if (needRefresh && configuration.pollingInterval > 0) {
+            // We cannot assume that DPs are current or that the device will automatically
+            // re-measure on a function change.
             TuyaDevice tuyaDevice = this.tuyaDevice;
             if (tuyaDevice != null) {
                 ScheduledFuture<?> pollingJob = this.pollingJob;
@@ -186,7 +177,7 @@ public void processDeviceStatus(Map<Integer, Object> deviceStatus) {
                 }
 
                 pollBurst = 3;
-                this.pollingJob = scheduler.scheduleWithFixedDelay(this::burstPoller, 1, 1, TimeUnit.SECONDS);
+                this.pollingJob = scheduler.scheduleWithFixedDelay(this::burstPoller, 0, 1, TimeUnit.SECONDS);
             }
         } else if (!missingStatus) {
             // If we have updates for everything we can stand down the burst polling.
@@ -200,19 +191,19 @@ private void burstPoller() {
         TuyaDevice tuyaDevice = this.tuyaDevice;
         if (tuyaDevice != null) {
             if (pollBurst > 0) {
-                tuyaDevice.refreshStatus(List.of());
+                tuyaDevice.refreshStatus();
                 pollBurst = pollBurst - 1;
             } else {
                 ScheduledFuture<?> pollingJob = this.pollingJob;
                 if (pollingJob != null) {
                     this.pollingJob = null;
-                    pollingJob.cancel(true);
+                    pollingJob.cancel(false);
                 }
 
                 int pollingInterval = configuration.pollingInterval;
                 if (pollingInterval > 0) {
                     this.pollingJob = scheduler.scheduleWithFixedDelay(() -> {
-                        tuyaDevice.refreshStatus(List.of());
+                        tuyaDevice.refreshStatus();
                     }, pollingInterval, pollingInterval, TimeUnit.SECONDS);
                 }
             }
@@ -324,7 +315,7 @@ private void processChannelStatus(Integer dp, Object value) {
     }
 
     @Override
-    public void connectionStatus(boolean status) {
+    public void connectionStatus(boolean status, int initialDelay) {
         if (status) {
             logger.debug("{}: connected", thing.getUID().getId());
 
@@ -335,17 +326,25 @@ public void connectionStatus(boolean status) {
 
             TuyaDevice tuyaDevice = this.tuyaDevice;
             if (tuyaDevice != null) {
-                // When we first connect the device state is unknown so we want to know everything
-                // it is willing to tell us.
-                tuyaDevice.requestStatus(List.of());
-
                 if (pollingJob == null) {
-                    int pollingInterval = configuration.pollingInterval;
-                    if (pollingInterval > 0) {
-                        pollingJob = scheduler.scheduleWithFixedDelay(() -> {
-                            tuyaDevice.refreshStatus(List.of());
-                        }, pollingInterval, pollingInterval, TimeUnit.SECONDS);
-                    }
+                    // When we first connect the device state is unknown so we want to query for everything.
+                    // Some devices seem to initialize their stacks in the wrong order and
+                    // requests that come too soon can be either ignored completely or responded
+                    // to with a, "not supported". The lower level protocol handler suggests an
+                    // initial delay where it might be advisable.
+                    this.pollingJob = scheduler.schedule(() -> {
+                        tuyaDevice.requestStatus();
+
+                        // After that we poll for the measurable DPs.
+                        int pollingInterval = configuration.pollingInterval;
+                        if (pollingInterval > 0) {
+                            // The first refresh request is immediate because we do not know how old
+                            // the current status might be.
+                            pollingJob = scheduler.scheduleWithFixedDelay(() -> {
+                                tuyaDevice.refreshStatus();
+                            }, 0, pollingInterval, TimeUnit.SECONDS);
+                        }
+                    }, initialDelay, TimeUnit.MILLISECONDS);
                 } else {
                     logger.debug("{}: polling job already exists?!?", thing.getUID().getId());
                 }
@@ -586,7 +585,7 @@ public void initialize() {
 
             this.tuyaDevice = new TuyaDevice(gson, this, eventLoopGroup, configuration.deviceId,
                     configuration.localKey.getBytes(StandardCharsets.UTF_8), configuration.ip, configuration.port,
-                    configuration.protocol);
+                    configuration.protocol, schemaDps.values().stream().map(e -> e.id).toList());
         } else {
             updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.CONFIGURATION_PENDING, "@text/offline.wait-for-ip");
         }
@@ -618,7 +617,7 @@ public void deviceInfoChanged(DeviceInfo deviceInfo) {
 
                 this.tuyaDevice = new TuyaDevice(gson, this, eventLoopGroup, configuration.deviceId,
                         configuration.localKey.getBytes(StandardCharsets.UTF_8), configuration.ip, configuration.port,
-                        configuration.protocol);
+                        configuration.protocol, schemaDps.values().stream().map(e -> e.id).toList());
             } catch (IllegalArgumentException e) {
                 logger.warn("{}", e.getMessage());
                 updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.COMMUNICATION_ERROR, e.getMessage());

bundles/org.openhab.binding.tuya/src/main/java/org/openhab/binding/tuya/internal/local/CommandType.java

@@ -58,8 +58,7 @@ public enum CommandType {
     LAN_REMOVE_GW(249),
     LAN_CHECK_GW_UPDATE(250),
     LAN_GW_UPDATE(251),
-    LAN_SET_GW_CHANNEL(252),
-    DP_QUERY_NOT_SUPPORTED(-1); // this is an internal value
+    LAN_SET_GW_CHANNEL(252);
 
     private final int code;
 

bundles/org.openhab.binding.tuya/src/main/java/org/openhab/binding/tuya/internal/local/DeviceStatusListener.java

@@ -25,5 +25,5 @@
 public interface DeviceStatusListener {
     void processDeviceStatus(Map<Integer, Object> deviceStatus);
 
-    void connectionStatus(boolean status);
+    void connectionStatus(boolean status, int initialDelay);
 }