From f5be5ff17d5c8a211e43eca6a0d3991580cdca13 Mon Sep 17 00:00:00 2001 From: Danny Baumann Date: Wed, 1 Sep 2021 14:43:55 +0200 Subject: [PATCH 1/3] [homematic] Improve long press events for button trigger 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. Signed-off-by: Danny Baumann --- .../ButtonVirtualDatapointHandler.java | 45 ++++++++++++++++--- .../handler/HomematicThingHandler.java | 5 ++- .../virtual/ButtonDatapointTest.java | 23 ++++++++-- 3 files changed, 62 insertions(+), 11 deletions(-) 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 b4feca685fad3..9175630673b3c 100644 --- 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 @@ 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 void initialize(HmDevice device) { 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, CommonTriggerEvents.DOUBLE_PRESSED }); } } } @@ -57,10 +60,13 @@ public boolean canHandleEvent(HmDatapoint dp) { @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())) { @@ -72,18 +78,45 @@ public void handleEvent(VirtualGateway gateway, HmDatapoint dp) { } 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 39899a5d652cd..204e29dc458fc 100644 --- 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 @@ protected void updateDatapointState(HmDatapoint dp) { 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 ca5498eb97019..b65ec37bf978e 100644 --- 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 @@ -66,8 +66,15 @@ public void testLongPress() throws IOException, HomematicClientException { 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,13 +90,14 @@ public void testUnsupportedEvents() throws IOException, HomematicClientException 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()); + assertThat(crapButtonVirtualDatapoint, nullValue()); } @Test @@ -127,6 +135,15 @@ private HmDatapoint createPressDatapoint(String channelName, Object value) { 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() From 352208808394707cfbe9c93c34201b743bfa7d69 Mon Sep 17 00:00:00 2001 From: Danny Baumann Date: Tue, 14 Sep 2021 16:25:23 +0200 Subject: [PATCH 2/3] [homematic] Remove double press processing 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 --- .../ButtonVirtualDatapointHandler.java | 14 ++++------- .../virtual/ButtonDatapointTest.java | 23 ------------------- 2 files changed, 5 insertions(+), 32 deletions(-) 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 9175630673b3c..db121ccb13cbf 100644 --- 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 @@ -48,7 +48,7 @@ public void initialize(HmDevice device) { HmDatapoint dp = addDatapoint(device, channel.getNumber(), getName(), HmValueType.STRING, null, false); dp.setTrigger(true); dp.setOptions(new String[] { CommonTriggerEvents.SHORT_PRESSED, CommonTriggerEvents.LONG_PRESSED, - LONG_REPEATED_EVENT, LONG_RELEASED_EVENT, CommonTriggerEvents.DOUBLE_PRESSED }); + LONG_REPEATED_EVENT, LONG_RELEASED_EVENT }); } } } @@ -68,15 +68,11 @@ public void handleEvent(VirtualGateway gateway, HmDatapoint dp) { || LONG_REPEATED_EVENT.equals(vdp.getValue()); if (MiscUtils.isTrueValue(dp.getValue())) { 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": if (LONG_REPEATED_EVENT.equals(vdp.getValue())) { // Suppress long press events during an ongoing long press 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 b65ec37bf978e..02ac032cdca63 100644 --- 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 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; @@ -100,28 +99,6 @@ public void testUnsupportedEvents() throws IOException, HomematicClientException assertThat(crapButtonVirtualDatapoint, 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)); - } - private HmDatapoint createPressDatapoint(String channelName, Object value) { HmDatapoint pressDp = new HmDatapoint(channelName, "", HmValueType.ACTION, value, true, HmParamsetType.VALUES); HmChannel hmChannel = new HmChannel(channelName, 1); From 1ea3fe02503fa2d0e76420f1a35e0dd100aa797d Mon Sep 17 00:00:00 2001 From: Danny Baumann Date: Fri, 17 Sep 2021 12:03:46 +0200 Subject: [PATCH 3/3] [homematic] Update button trigger channel docs Signed-off-by: Danny Baumann --- bundles/org.openhab.binding.homematic/README.md | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/bundles/org.openhab.binding.homematic/README.md b/bundles/org.openhab.binding.homematic/README.md index c923afcfc8437..1dd7842f7fc88 100644 --- 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