Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[homematic] Remove double press events and improve long press events for button trigger #11186

Merged
merged 3 commits into from
Sep 18, 2021
Merged

[homematic] Remove double press events and improve long press events for button trigger #11186

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
f5be5ff
[homematic] Improve long press events for button trigger
maniac103 Sep 1, 2021
3522088
[homematic] Remove double press processing
maniac103 Sep 14, 2021
1ea3fe0
[homematic] Update button trigger channel docs
maniac103 Sep 17, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 5 additions & 4 deletions bundles/org.openhab.binding.homematic/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
57 changes: 43 additions & 14 deletions ...penhab/binding/homematic/internal/communicator/virtual/ButtonVirtualDatapointHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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 });
}
}
}
Expand All @@ -57,33 +60,59 @@ public boolean canHandleEvent(HmDatapoint dp) {

@Override
public void handleEvent(VirtualGateway gateway, HmDatapoint dp) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You have made major changes to the logic here. Is the behavior regarding the triggered events really like before? It would be bad if rules behaved differently than before, because the recognition of a double click or long press works differently.

I don't have any buttons in use right now, otherwise I could do some regression tests.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I hope so ;-) The goal is SINGLE_PRESS being triggered as before (just with shorter spacing due to the changed DOUBLE_PRESS handling), LONG_PRESS being triggered as before and just the new 2 events being triggered in addition to that.

Copy link
Contributor Author

@maniac103 maniac103 Sep 17, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With the removal of DOUBLE_PRESSED events, the goal is now

  • triggering SHORT_PRESSED events on every short button press (before it was either SHORT_PRESSED or DOUBLE_PRESSED, depending on timing)
  • triggering LONG_PRESSED events on every long press start (as before)
  • triggering LONG_REPEATED events on long press repetition (CONT being sent from device; new, didn't exist before)
  • triggering LONG_RELEASED events on long press end (new, didn't exist at all before)

But yes, if somebody with an actual remote could test this, that would be great. I only have my own button panels which run AskSinPP and implement 'remote button channels' (device description XML is identical to a original HM remote in that regard).

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());
}
}
5 changes: 3 additions & 2 deletions ...c/src/main/java/org/openhab/binding/homematic/internal/handler/HomematicThingHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand Down
46 changes: 20 additions & 26 deletions ...java/org/openhab/binding/homematic/internal/communicator/virtual/ButtonDatapointTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -66,8 +65,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
Expand All @@ -83,35 +89,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());
}

@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) {
Expand All @@ -127,6 +112,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()
Expand Down