[homematic] Remove double press events and improve long press events for button trigg...

HM devices provide not only 'long press' events, but also 'long
press continued' (sent in configured long press interval) and 'long
press released' events. So far, those events were swallowed in the
button datapoint handler.
Improve the situation by forwarding those events to the button trigger
channel, making them usable in e.g. rules that react on button long
presses.

A double press timeout of 2 seconds is too long and disturbs single
press processing. Additionally, for double press processing to be
useful, the first press would need to be swallowed until double press
timeout elapses, which is not what happened here: the first PRESS was
sent out as SINGLE_PRESS event, making it impossible to meaningfully
distinguish the 'single press' and 'double press' events within rules.

If needed, double press handling can be implemented equally well within
a rule.

Signed-off-by: Danny Baumann <dannybaumann@web.de>

diff --git a/bundles/org.openhab.binding.homematic/README.md b/bundles/org.openhab.binding.homematic/README.md
index c923afcfc8437d8ee36e5b7c0091fa492513aa59..1dd7842f7fc882f40378f3958bd281f5727abf2c 100644 (file)
--- a/bundles/org.openhab.binding.homematic/README.md
+++ b/bundles/org.openhab.binding.homematic/README.md
@@ -343,16 +343,17 @@ A virtual datapoint (String) to simulate a key press, available on all channels
 
 Available values:
 
-- `SHORT_PRESS`: triggered on a short key press
-- `LONG_PRESS`: triggered on a key press longer than `LONG_PRESS_TIME` (variable configuration per key, default is 0.4 s)
-- `DOUBLE_PRESS`: triggered on a short key press but only if the latest `SHORT_PRESS` or `DOUBLE_PRESS` event is not older than 2.0 s (not related to `DBL_PRESS_TIME` configuration, which is more like a key lock because if it is other than `0.0` single presses are not notified anymore)
+- `SHORT_PRESSED`: triggered on a short key press
+- `LONG_PRESSED`: triggered on a key press longer than `LONG_PRESS_TIME` (variable configuration per key, default is 0.4 s)
+- `LONG_REPEATED`: triggered on long key press repetition, that is, in `LONG_PRESS_TIME` intervals as long as key is held
+- `LONG_RELEASED`: triggered when a key is released after being long pressed
 
 **Example:** to capture a short key press on the 19 button remote control in a rule
 
 ```javascript
 rule "example trigger rule"
 when
-    Channel 'homematic:HM-RC-19-B:ccu:KEQ0012345:1#BUTTON' triggered SHORT_PRESS
+    Channel 'homematic:HM-RC-19-B:ccu:KEQ0012345:1#BUTTON' triggered SHORT_PRESSED
 then
     ...
 end

diff --git a/bundles/org.openhab.binding.homematic/src/main/java/org/openhab/binding/homematic/internal/communicator/virtual/ButtonVirtualDatapointHandler.java b/bundles/org.openhab.binding.homematic/src/main/java/org/openhab/binding/homematic/internal/communicator/virtual/ButtonVirtualDatapointHandler.java
index b4feca685fad37505a2105f9a571a8fda2d1752d..db121ccb13cbfd6bffd992827aae18105e1bf602 100644 (file)
--- a/bundles/org.openhab.binding.homematic/src/main/java/org/openhab/binding/homematic/internal/communicator/virtual/ButtonVirtualDatapointHandler.java
+++ b/bundles/org.openhab.binding.homematic/src/main/java/org/openhab/binding/homematic/internal/communicator/virtual/ButtonVirtualDatapointHandler.java
@@ -33,6 +33,9 @@ import org.slf4j.LoggerFactory;
 public class ButtonVirtualDatapointHandler extends AbstractVirtualDatapointHandler {
     private final Logger logger = LoggerFactory.getLogger(ButtonVirtualDatapointHandler.class);
 
+    private static final String LONG_REPEATED_EVENT = "LONG_REPEATED";
+    private static final String LONG_RELEASED_EVENT = "LONG_RELEASED";
+
     @Override
     public String getName() {
         return VIRTUAL_DATAPOINT_NAME_BUTTON;
@@ -45,7 +48,7 @@ public class ButtonVirtualDatapointHandler extends AbstractVirtualDatapointHandl
                 HmDatapoint dp = addDatapoint(device, channel.getNumber(), getName(), HmValueType.STRING, null, false);
                 dp.setTrigger(true);
                 dp.setOptions(new String[] { CommonTriggerEvents.SHORT_PRESSED, CommonTriggerEvents.LONG_PRESSED,
-                        CommonTriggerEvents.DOUBLE_PRESSED });
+                        LONG_REPEATED_EVENT, LONG_RELEASED_EVENT });
             }
         }
     }
@@ -57,33 +60,59 @@ public class ButtonVirtualDatapointHandler extends AbstractVirtualDatapointHandl
 
     @Override
     public void handleEvent(VirtualGateway gateway, HmDatapoint dp) {
-        HmDatapoint vdp = getVirtualDatapoint(dp.getChannel());
+        HmChannel channel = dp.getChannel();
+        HmDatapoint vdp = getVirtualDatapoint(channel);
+        int usPos = dp.getName().indexOf("_");
+        String pressType = usPos == -1 ? dp.getName() : dp.getName().substring(usPos + 1);
+        boolean isLongPressActive = CommonTriggerEvents.LONG_PRESSED.equals(vdp.getValue())
+                || LONG_REPEATED_EVENT.equals(vdp.getValue());
         if (MiscUtils.isTrueValue(dp.getValue())) {
-            int usPos = dp.getName().indexOf("_");
-            String pressType = usPos == -1 ? dp.getName() : dp.getName().substring(usPos + 1);
             switch (pressType) {
-                case "SHORT":
-                    if (dp.getValue() == null || !dp.getValue().equals(dp.getPreviousValue())) {
-                        vdp.setValue(CommonTriggerEvents.SHORT_PRESSED);
-                    } else {
-                        // two (or more) PRESS_SHORT events were received
-                        // within AbstractHomematicGateway#DEFAULT_DISABLE_DELAY seconds
-                        vdp.setValue(CommonTriggerEvents.DOUBLE_PRESSED);
-                    }
+                case "SHORT": {
+                    vdp.setValue(null); // Force sending new event
+                    vdp.setValue(CommonTriggerEvents.SHORT_PRESSED);
                     break;
+                }
                 case "LONG":
-                    vdp.setValue(CommonTriggerEvents.LONG_PRESSED);
+                    if (LONG_REPEATED_EVENT.equals(vdp.getValue())) {
+                        // Suppress long press events during an ongoing long press
+                        vdp.setValue(LONG_REPEATED_EVENT);
+                    } else {
+                        vdp.setValue(CommonTriggerEvents.LONG_PRESSED);
+                    }
                     break;
                 case "LONG_RELEASE":
+                    // Only send release events if we sent a pressed event before
+                    vdp.setValue(isLongPressActive ? LONG_RELEASED_EVENT : null);
+                    break;
                 case "CONT":
+                    // Clear previous value to force re-triggering of repetition
                     vdp.setValue(null);
+                    // Only send repetitions if there was a pressed event before
+                    // (a CONT might arrive simultaneously with the initial LONG event)
+                    if (isLongPressActive) {
+                        vdp.setValue(LONG_REPEATED_EVENT);
+                    }
                     break;
                 default:
                     vdp.setValue(null);
                     logger.warn("Unexpected vaule '{}' for PRESS virtual datapoint", pressType);
             }
         } else {
-            vdp.setValue(null);
+            if ("LONG".equals(pressType) && LONG_REPEATED_EVENT.equals(vdp.getValue())) {
+                // If we're currently processing a repeated long-press event, don't let the initial LONG
+                // event time out the repetitions, the CONT delay handler will take care of it
+                vdp.setValue(LONG_REPEATED_EVENT);
+            } else if (isLongPressActive) {
+                // We seemingly missed an event (either a CONT or the final LONG_RELEASE),
+                // so end the long press cycle now
+                vdp.setValue(LONG_RELEASED_EVENT);
+            } else {
+                vdp.setValue(null);
+            }
         }
+        logger.debug("Handled virtual button event on {}:{}: press type {}, value {}, button state {} -> {}",
+                channel.getDevice().getAddress(), channel.getNumber(), pressType, dp.getValue(), vdp.getPreviousValue(),
+                vdp.getValue());
     }
 }

diff --git a/bundles/org.openhab.binding.homematic/src/main/java/org/openhab/binding/homematic/internal/handler/HomematicThingHandler.java b/bundles/org.openhab.binding.homematic/src/main/java/org/openhab/binding/homematic/internal/handler/HomematicThingHandler.java
index 39899a5d652cd702d5ee657aee3c2ff17f3df062..204e29dc458fc58f5102ccb26e72be520c3c57b6 100644 (file)
--- a/bundles/org.openhab.binding.homematic/src/main/java/org/openhab/binding/homematic/internal/handler/HomematicThingHandler.java
+++ b/bundles/org.openhab.binding.homematic/src/main/java/org/openhab/binding/homematic/internal/handler/HomematicThingHandler.java
@@ -384,8 +384,9 @@ public class HomematicThingHandler extends BaseThingHandler {
     private void updateChannelState(final HmDatapoint dp, Channel channel)
             throws IOException, GatewayNotAvailableException, ConverterException {
         if (dp.isTrigger()) {
-            if (dp.getValue() != null) {
-                triggerChannel(channel.getUID(), dp.getValue() == null ? "" : dp.getValue().toString());
+            final Object value = dp.getValue();
+            if (value != null && !value.equals(dp.getPreviousValue())) {
+                triggerChannel(channel.getUID(), value.toString());
             }
         } else if (isLinked(channel)) {
             loadHomematicChannelValues(dp.getChannel());

diff --git a/bundles/org.openhab.binding.homematic/src/test/java/org/openhab/binding/homematic/internal/communicator/virtual/ButtonDatapointTest.java b/bundles/org.openhab.binding.homematic/src/test/java/org/openhab/binding/homematic/internal/communicator/virtual/ButtonDatapointTest.java
index ca5498eb97019586b07e0cd7ba53f11628033311..02ac032cdca637af73d3d0fbe53675ec62be8faf 100644 (file)
--- a/bundles/org.openhab.binding.homematic/src/test/java/org/openhab/binding/homematic/internal/communicator/virtual/ButtonDatapointTest.java
+++ b/bundles/org.openhab.binding.homematic/src/test/java/org/openhab/binding/homematic/internal/communicator/virtual/ButtonDatapointTest.java
@@ -18,7 +18,6 @@ import static org.hamcrest.MatcherAssert.assertThat;
 import java.io.IOException;
 
 import org.junit.jupiter.api.BeforeEach;
-import org.junit.jupiter.api.Disabled;
 import org.junit.jupiter.api.Test;
 import org.openhab.binding.homematic.internal.misc.HomematicClientException;
 import org.openhab.binding.homematic.internal.misc.HomematicConstants;
@@ -66,8 +65,15 @@ public class ButtonDatapointTest extends JavaTest {
         HmDatapoint buttonVirtualDatapoint = getButtonVirtualDatapoint(longPressDp);
 
         mockEventReceiver.eventReceived(longPressDp);
-
         assertThat(buttonVirtualDatapoint.getValue(), is(CommonTriggerEvents.LONG_PRESSED));
+
+        HmDatapoint contPressDp = createPressDatapointFrom(longPressDp, "PRESS_CONT", Boolean.TRUE);
+        mockEventReceiver.eventReceived(contPressDp);
+        assertThat(buttonVirtualDatapoint.getValue(), is("LONG_REPEATED"));
+
+        HmDatapoint releaseDp = createPressDatapointFrom(longPressDp, "PRESS_LONG_RELEASE", Boolean.TRUE);
+        mockEventReceiver.eventReceived(releaseDp);
+        assertThat(buttonVirtualDatapoint.getValue(), is("LONG_RELEASED"));
     }
 
     @Test
@@ -83,35 +89,14 @@ public class ButtonDatapointTest extends JavaTest {
         mockEventReceiver.eventReceived(releaseDp);
 
         HmDatapoint crapDp = createPressDatapoint("CRAP", Boolean.TRUE);
-        HmDatapoint crapButtonVirtualDatapoint = getButtonVirtualDatapoint(releaseDp);
+        HmDatapoint crapButtonVirtualDatapoint = getButtonVirtualDatapoint(crapDp);
 
         mockEventReceiver.eventReceived(crapDp);
 
+        // CONT and LONG_RELEASE events without previous LONG event are supposed to yield no trigger
         assertThat(contButtonVirtualDatapoint.getValue(), nullValue());
         assertThat(releaseButtonVirtualDatapoint.getValue(), nullValue());
-        assertThat(crapButtonVirtualDatapoint.getValue(), nullValue());
-    }
-
-    @Test
-    @Disabled(value = "Test is unstable see #10753")
-    public void testDoublePress() throws IOException, HomematicClientException, InterruptedException {
-        HmDatapoint shortPressDp = createPressDatapoint("PRESS_SHORT", Boolean.TRUE);
-        HmDatapoint buttonVirtualDatapoint = getButtonVirtualDatapoint(shortPressDp);
-
-        mockEventReceiver.eventReceived(shortPressDp);
-        assertThat(buttonVirtualDatapoint.getValue(), is(CommonTriggerEvents.SHORT_PRESSED));
-
-        Thread.sleep(DISABLE_DATAPOINT_DELAY / 2);
-
-        shortPressDp.setValue(Boolean.TRUE);
-        mockEventReceiver.eventReceived(shortPressDp);
-        assertThat(buttonVirtualDatapoint.getValue(), is(CommonTriggerEvents.DOUBLE_PRESSED));
-
-        Thread.sleep(DISABLE_DATAPOINT_DELAY * 2);
-
-        shortPressDp.setValue(Boolean.TRUE);
-        mockEventReceiver.eventReceived(shortPressDp);
-        assertThat(buttonVirtualDatapoint.getValue(), is(CommonTriggerEvents.SHORT_PRESSED));
+        assertThat(crapButtonVirtualDatapoint, nullValue());
     }
 
     private HmDatapoint createPressDatapoint(String channelName, Object value) {
@@ -127,6 +112,15 @@ public class ButtonDatapointTest extends JavaTest {
         return pressDp;
     }
 
+    private HmDatapoint createPressDatapointFrom(HmDatapoint originalDatapoint, String channelName, Object value) {
+        HmDatapoint pressDp = new HmDatapoint(channelName, "", HmValueType.ACTION, value, true, HmParamsetType.VALUES);
+        HmChannel hmChannel = originalDatapoint.getChannel();
+        hmChannel.addDatapoint(pressDp);
+        pressDp.setChannel(hmChannel);
+
+        return pressDp;
+    }
+
     private HmDatapoint getButtonVirtualDatapoint(HmDatapoint originalDatapoint) {
         return originalDatapoint.getChannel().getDatapoints().stream()
                 .filter(dp -> HomematicConstants.VIRTUAL_DATAPOINT_NAME_BUTTON.equals(dp.getName())).findFirst()