Poll dimmer device for dimmer value when "restore last value" is enabled on the channel

[jvolkman]

Following up from a community thread: https://community.openhab.org/t/z-wave-dimmer-restore-last-value-setting/25808

The binding should poll a dimmer device for its final value when the "restore last value" option is enabled and the device is turned on remotely. Currently, the binding does not poll the device and OpenHAB incorrectly assumes the dimmer level is set to 100%.

I know that the HomeSeer dimmers I'm using report their current dim level even when they're in the middle of a ramp up or ramp down action, so the device would need to be polled multiple times until the dimming action has finished.

• OH2 Version: 2.0.0-1
• Device name: HomeSeer HS-WD100+
• Device version: 5.19
• Link to database: http://www.cd-jackson.com/index.php/zwave/zwave-device-database/zwave-device-list/devicesummary/346

[jvolkman]

I finally got around to testing the snapshot build with this change. It seems the binding only polls once after the switch is turned on/off or the level is changed. The effect, for example, is that when turning a light on from 0%, the slider first jumps to 100% and then back to ~1% or 2%, depending on the switch's ramp rate. I can work around this by disabling ramping for remote commands, but it would be better if the binding would poll until the value stopped changing.

[jvolkman]

Here's a quick hack that provides the desired functionality. It polls a multilevel switch device every 200ms until the value stops changing. The effect is that, when turning a switch from off to on, the slider first jumps to 100% and then quickly jumps back to the correct level. Not perfect, but good enough.

Obviously this implementation isn't great. It uses a static map within the converter to store per-node temporary data. I'm sure there's a better place. It also uses a thread to delay sending the next polling packet. Not sure whether there's a better delay mechanism available. Also, maybe this should be a channel option rather than always enabled. Theoretically there could be a device that never returns the same level for two subsequent gets.

diff --git a/src/main/java/org/openhab/binding/zwave/internal/converter/ZWaveMultiLevelSwitchConverter.java b/src/main/java/org/openhab/binding/zwave/internal/converter/ZWaveMultiLevelSwitchConverter.java
index 6a0feb8..9c76a9c 100644
--- a/src/main/java/org/openhab/binding/zwave/internal/converter/ZWaveMultiLevelSwitchConverter.java
+++ b/src/main/java/org/openhab/binding/zwave/internal/converter/ZWaveMultiLevelSwitchConverter.java
@@ -9,7 +9,9 @@
 package org.openhab.binding.zwave.internal.converter;

 import java.util.ArrayList;
+import java.util.HashMap;
 import java.util.List;
+import java.util.Map;

 import org.eclipse.smarthome.core.library.types.OnOffType;
 import org.eclipse.smarthome.core.library.types.PercentType;
@@ -38,6 +40,8 @@
 public class ZWaveMultiLevelSwitchConverter extends ZWaveCommandClassConverter {

     private final static Logger logger = LoggerFactory.getLogger(ZWaveMultiLevelSwitchConverter.class);
+
+    private static final Map<Integer, UpdatedValuePoller> valuePollers = new HashMap<>();

     /**
      * Constructor. Creates a new instance of the {@link ZWaveMultiLevelSwitchConverter} class.
@@ -101,6 +105,7 @@
                 if (((PercentType) state).intValue() >= 99) {
                     state = new PercentType(100);
                 }
+
                 break;
             case OnOffType:
                 if (value == 0) {
@@ -122,6 +127,18 @@
             default:
                 logger.warn("No conversion in {} to {}", getClass().getSimpleName(), channel.getDataType());
                 break;
+        }
+
+        if (valuePollers.containsKey(event.getNodeId())) {
+            if (valuePollers.get(event.getNodeId()).pollForCompletion(state)) {
+                // Finished
+                logger.debug("Polling FINISHED for node {}", event.getNodeId());
+                valuePollers.remove(event.getNodeId());
+            } else {
+                // Still polling
+                logger.debug("Polling CONTINUING for node {}", event.getNodeId());
+                return null;
+            }
         }

         return state;
@@ -211,6 +228,41 @@
         // Don't poll immediately since some devices return the original value, and some the new value.
         // This conflicts with OH that will move the slider immediately.
         messages.add(node.encapsulate(commandClass.getValueMessage(), commandClass, channel.getEndpoint()));
+        valuePollers.put(node.getNodeId(), new UpdatedValuePoller(node, commandClass, channel.getEndpoint()));
+
         return messages;
     }
+
+    private class UpdatedValuePoller {
+        private final ZWaveNode node;
+        private final ZWaveMultiLevelSwitchCommandClass commandClass;
+        private final int endpoint;
+        private State lastState;
+
+        private UpdatedValuePoller(ZWaveNode node, ZWaveMultiLevelSwitchCommandClass commandClass, int endpoint) {
+            this.node = node;
+            this.commandClass = commandClass;
+            this.endpoint = endpoint;
+            this.lastState = null;
+        }
+
+        boolean pollForCompletion(State newState) {
+            if (newState == null || newState.equals(lastState)) {
+                return true;
+            }
+            // Poll after a delay.
+            new Thread() {
+                @Override
+                public void run() {
+                    try {
+                        Thread.sleep(200);
+                    } catch (InterruptedException e) {
+                    }
+                    controller.sendData(node.encapsulate(commandClass.getValueMessage(), commandClass, endpoint));
+                }
+            }.start();
+            lastState = newState;
+            return false;
+        }
+    }
 }

[mhilbush]

I would be a little worried about the network traffic the above proposed change would generate, especially for people with large networks. For example, if I have a rule or a group item that changes the state of several dimmers, this could generate a substantial amount of traffic in addition to the traffic that's already generated through regular polling. Since queues already can grow pretty large, this may make that situation worse.

The change that was implemented causes annoying UI behavior (as you describe above). @chris Maybe in the short run, it would be best to revert this change until a more permanent solution can be found (such as introducing a delay between the SWITCH_MULTILEVEL_SET and the SWITCH_MULTILEVEL_GET.

[jvolkman]

For what it's worth, this hack has been working well for me for the past ~3 weeks. But I only have ~20 devices in my network with a low polling rate since they all report status. A final solution should be optional with configurable delay and polling rates, possibly with exponential backoff. Also, I think the polling can be limited to turning a device on when the "use previous level" option is enabled. No need to poll when turning a device off or setting it to a specific level.