From a0c1f3b4f44d0b75d28f73502940f516c3a218f2 Mon Sep 17 00:00:00 2001 From: J-N-K Date: Sun, 18 Feb 2024 22:03:24 +0100 Subject: [PATCH] Fix KNX dimmer channels (#16421) Signed-off-by: Jan N. Klug Signed-off-by: Ciprian Pascu --- .../knx/internal/channel/KNXChannel.java | 51 ++++++++++--------- .../knx/internal/channel/TypeColor.java | 2 +- .../knx/internal/channel/TypeDimmer.java | 2 +- .../internal/channel/TypeRollershutter.java | 2 +- .../internal/handler/DeviceThingHandler.java | 2 +- .../knx/internal/channel/KNXChannelTest.java | 36 ++++++++++--- 6 files changed, 61 insertions(+), 34 deletions(-) diff --git a/bundles/org.openhab.binding.knx/src/main/java/org/openhab/binding/knx/internal/channel/KNXChannel.java b/bundles/org.openhab.binding.knx/src/main/java/org/openhab/binding/knx/internal/channel/KNXChannel.java index 923759b0e1d..b08a309203c 100644 --- a/bundles/org.openhab.binding.knx/src/main/java/org/openhab/binding/knx/internal/channel/KNXChannel.java +++ b/bundles/org.openhab.binding.knx/src/main/java/org/openhab/binding/knx/internal/channel/KNXChannel.java @@ -15,8 +15,8 @@ package org.openhab.binding.knx.internal.channel; import static java.util.stream.Collectors.toList; import static org.openhab.binding.knx.internal.KNXBindingConstants.*; -import java.util.HashMap; -import java.util.HashSet; +import java.util.ArrayList; +import java.util.LinkedHashMap; import java.util.List; import java.util.Map; import java.util.Objects; @@ -47,21 +47,21 @@ import tuwien.auto.calimero.GroupAddress; @NonNullByDefault public abstract class KNXChannel { private final Logger logger = LoggerFactory.getLogger(KNXChannel.class); - private final Set gaKeys; + private final List gaKeys; - private final Map groupAddressConfigurations = new HashMap<>(); - private final Set listenAddresses = new HashSet<>(); - private final Set writeAddresses = new HashSet<>(); + private final Map groupAddressConfigurations = new LinkedHashMap<>(); + private final List listenAddresses = new ArrayList<>(); + private final List writeAddresses = new ArrayList<>(); private final String channelType; private final ChannelUID channelUID; private final boolean isControl; private final Class preferredType; KNXChannel(List> acceptedTypes, Channel channel) { - this(Set.of(GA), acceptedTypes, channel); + this(List.of(GA), acceptedTypes, channel); } - KNXChannel(Set gaKeys, List> acceptedTypes, Channel channel) { + KNXChannel(List gaKeys, List> acceptedTypes, Channel channel) { this.gaKeys = gaKeys; this.preferredType = acceptedTypes.get(0); @@ -109,16 +109,17 @@ public abstract class KNXChannel { return preferredType; } - public final Set getAllGroupAddresses() { + public final List getAllGroupAddresses() { return listenAddresses; } - public final Set getWriteAddresses() { + public final List getWriteAddresses() { return writeAddresses; } public final @Nullable OutboundSpec getCommandSpec(Type command) { logger.trace("getCommandSpec checking keys '{}' for command '{}' ({})", gaKeys, command, command.getClass()); + // first check if there is a direct match for the provided command for all GAs for (Map.Entry entry : groupAddressConfigurations.entrySet()) { String dpt = Objects.requireNonNullElse(entry.getValue().getDPT(), getDefaultDPT(entry.getKey())); Set> expectedTypeClasses = DPTUtil.getAllowedTypes(dpt); @@ -128,19 +129,23 @@ public abstract class KNXChannel { "getCommandSpec key '{}' has one of the expectedTypeClasses '{}', matching command '{}' and dpt '{}'", entry.getKey(), expectedTypeClasses, command, dpt); return new WriteSpecImpl(entry.getValue(), dpt, command); - } else { - for (Class expectedTypeClass : expectedTypeClasses) { - if (command instanceof State state && State.class.isAssignableFrom(expectedTypeClass)) { - var subClass = expectedTypeClass.asSubclass(State.class); - if (state.as(subClass) != null) { - logger.trace( - "getCommandSpec command class '{}' is a sub-class of the expectedTypeClass '{}' for key '{}'", - command.getClass(), expectedTypeClass, entry.getKey()); - Class expectedTypeAsStateClass = expectedTypeClass.asSubclass(State.class); - State convertedState = state.as(expectedTypeAsStateClass); - if (convertedState != null) { - return new WriteSpecImpl(entry.getValue(), dpt, convertedState); - } + } + } + // if we didn't find a match, check if we find a sub-type match + for (Map.Entry entry : groupAddressConfigurations.entrySet()) { + String dpt = Objects.requireNonNullElse(entry.getValue().getDPT(), getDefaultDPT(entry.getKey())); + Set> expectedTypeClasses = DPTUtil.getAllowedTypes(dpt); + for (Class expectedTypeClass : expectedTypeClasses) { + if (command instanceof State state && State.class.isAssignableFrom(expectedTypeClass)) { + var subClass = expectedTypeClass.asSubclass(State.class); + if (state.as(subClass) != null) { + logger.trace( + "getCommandSpec command class '{}' is a sub-class of the expectedTypeClass '{}' for key '{}'", + command.getClass(), expectedTypeClass, entry.getKey()); + Class expectedTypeAsStateClass = expectedTypeClass.asSubclass(State.class); + State convertedState = state.as(expectedTypeAsStateClass); + if (convertedState != null) { + return new WriteSpecImpl(entry.getValue(), dpt, convertedState); } } } diff --git a/bundles/org.openhab.binding.knx/src/main/java/org/openhab/binding/knx/internal/channel/TypeColor.java b/bundles/org.openhab.binding.knx/src/main/java/org/openhab/binding/knx/internal/channel/TypeColor.java index a9580a76b19..07f2599f6af 100644 --- a/bundles/org.openhab.binding.knx/src/main/java/org/openhab/binding/knx/internal/channel/TypeColor.java +++ b/bundles/org.openhab.binding.knx/src/main/java/org/openhab/binding/knx/internal/channel/TypeColor.java @@ -40,7 +40,7 @@ class TypeColor extends KNXChannel { public static final Set SUPPORTED_CHANNEL_TYPES = Set.of(CHANNEL_COLOR, CHANNEL_COLOR_CONTROL); TypeColor(Channel channel) { - super(Set.of(SWITCH_GA, POSITION_GA, INCREASE_DECREASE_GA, HSB_GA), + super(List.of(SWITCH_GA, POSITION_GA, INCREASE_DECREASE_GA, HSB_GA), List.of(HSBType.class, PercentType.class, OnOffType.class, IncreaseDecreaseType.class), channel); } diff --git a/bundles/org.openhab.binding.knx/src/main/java/org/openhab/binding/knx/internal/channel/TypeDimmer.java b/bundles/org.openhab.binding.knx/src/main/java/org/openhab/binding/knx/internal/channel/TypeDimmer.java index e75243c5eee..edf20832708 100644 --- a/bundles/org.openhab.binding.knx/src/main/java/org/openhab/binding/knx/internal/channel/TypeDimmer.java +++ b/bundles/org.openhab.binding.knx/src/main/java/org/openhab/binding/knx/internal/channel/TypeDimmer.java @@ -39,7 +39,7 @@ class TypeDimmer extends KNXChannel { public static final Set SUPPORTED_CHANNEL_TYPES = Set.of(CHANNEL_DIMMER, CHANNEL_DIMMER_CONTROL); TypeDimmer(Channel channel) { - super(Set.of(SWITCH_GA, POSITION_GA, INCREASE_DECREASE_GA), + super(List.of(SWITCH_GA, POSITION_GA, INCREASE_DECREASE_GA), List.of(PercentType.class, OnOffType.class, IncreaseDecreaseType.class), channel); } diff --git a/bundles/org.openhab.binding.knx/src/main/java/org/openhab/binding/knx/internal/channel/TypeRollershutter.java b/bundles/org.openhab.binding.knx/src/main/java/org/openhab/binding/knx/internal/channel/TypeRollershutter.java index b708db7ac52..50fdfc9b03b 100644 --- a/bundles/org.openhab.binding.knx/src/main/java/org/openhab/binding/knx/internal/channel/TypeRollershutter.java +++ b/bundles/org.openhab.binding.knx/src/main/java/org/openhab/binding/knx/internal/channel/TypeRollershutter.java @@ -39,7 +39,7 @@ class TypeRollershutter extends KNXChannel { CHANNEL_ROLLERSHUTTER_CONTROL); TypeRollershutter(Channel channel) { - super(Set.of(UP_DOWN_GA, STOP_MOVE_GA, POSITION_GA), + super(List.of(UP_DOWN_GA, STOP_MOVE_GA, POSITION_GA), List.of(PercentType.class, UpDownType.class, StopMoveType.class), channel); } diff --git a/bundles/org.openhab.binding.knx/src/main/java/org/openhab/binding/knx/internal/handler/DeviceThingHandler.java b/bundles/org.openhab.binding.knx/src/main/java/org/openhab/binding/knx/internal/handler/DeviceThingHandler.java index 00554934280..4c69b73e229 100644 --- a/bundles/org.openhab.binding.knx/src/main/java/org/openhab/binding/knx/internal/handler/DeviceThingHandler.java +++ b/bundles/org.openhab.binding.knx/src/main/java/org/openhab/binding/knx/internal/handler/DeviceThingHandler.java @@ -300,7 +300,7 @@ public class DeviceThingHandler extends BaseThingHandler implements GroupAddress if (knxChannel == null) { return; } - Set rsa = knxChannel.getWriteAddresses(); + List rsa = knxChannel.getWriteAddresses(); if (!rsa.isEmpty()) { logger.trace("onGroupRead size '{}'", rsa.size()); OutboundSpec os = groupAddressesRespondingSpec.get(destination); diff --git a/bundles/org.openhab.binding.knx/src/test/java/org/openhab/binding/knx/internal/channel/KNXChannelTest.java b/bundles/org.openhab.binding.knx/src/test/java/org/openhab/binding/knx/internal/channel/KNXChannelTest.java index b3ca61b8584..00f702c3adf 100644 --- a/bundles/org.openhab.binding.knx/src/test/java/org/openhab/binding/knx/internal/channel/KNXChannelTest.java +++ b/bundles/org.openhab.binding.knx/src/test/java/org/openhab/binding/knx/internal/channel/KNXChannelTest.java @@ -21,7 +21,6 @@ import static org.mockito.Mockito.when; import java.util.List; import java.util.Map; import java.util.Objects; -import java.util.Set; import org.eclipse.jdt.annotation.NonNullByDefault; import org.eclipse.jdt.annotation.Nullable; @@ -32,6 +31,7 @@ import org.openhab.binding.knx.internal.dpt.ValueEncoder; import org.openhab.core.config.core.Configuration; import org.openhab.core.library.items.ColorItem; import org.openhab.core.library.types.HSBType; +import org.openhab.core.library.types.OnOffType; import org.openhab.core.library.types.PercentType; import org.openhab.core.thing.Channel; import org.openhab.core.thing.type.ChannelTypeUID; @@ -159,10 +159,10 @@ class KNXChannelTest { MyKNXChannel knxChannel = new MyKNXChannel(channel); - Set listenAddresses = knxChannel.getAllGroupAddresses(); + List listenAddresses = knxChannel.getAllGroupAddresses(); assertEquals(5, listenAddresses.size()); // we don't check the content since parsing has been checked before and the quantity is correct - Set writeAddresses = knxChannel.getWriteAddresses(); + List writeAddresses = knxChannel.getWriteAddresses(); assertEquals(2, writeAddresses.size()); assertTrue(writeAddresses.contains(new GroupAddress("1/2/3"))); assertTrue(writeAddresses.contains(new GroupAddress("7/1/9"))); @@ -194,19 +194,41 @@ class KNXChannelTest { KNXChannel knxChannel = KNXChannelFactory.createKnxChannel(channel); assertThat(knxChannel, instanceOf(TypeDimmer.class)); - Command command = new PercentType("100"); + Command command = new PercentType("90"); @Nullable OutboundSpec outboundSpec = knxChannel.getCommandSpec(command); - assertNotNull(outboundSpec); + assertThat(outboundSpec, is(notNullValue())); + assertThat(outboundSpec.getGroupAddress(), is(new GroupAddress("1/2/2"))); String mappedValue = ValueEncoder.encode(outboundSpec.getValue(), outboundSpec.getDPT()); - assertThat(mappedValue, is("100")); + assertThat(mappedValue, is("90")); assertThat(outboundSpec.getValue(), is(instanceOf(PercentType.class))); } + @Test + void test1001ToDimmerChannel() throws KNXFormatException { + Configuration configuration = new Configuration(Map.of("switch", "1.001:1/2/1", "position", "5.001:1/2/2")); + Channel channel = Objects.requireNonNull(mock(Channel.class)); + when(channel.getChannelTypeUID()) + .thenReturn(new ChannelTypeUID(KNXBindingConstants.BINDING_ID, KNXBindingConstants.CHANNEL_DIMMER)); + when(channel.getConfiguration()).thenReturn(configuration); + + KNXChannel knxChannel = KNXChannelFactory.createKnxChannel(channel); + assertThat(knxChannel, instanceOf(TypeDimmer.class)); + + Command command = OnOffType.ON; + OutboundSpec outboundSpec = knxChannel.getCommandSpec(command); + assertThat(outboundSpec, is(notNullValue())); + assertThat(outboundSpec.getGroupAddress(), is(new GroupAddress("1/2/1"))); + + String mappedValue = ValueEncoder.encode(outboundSpec.getValue(), outboundSpec.getDPT()); + assertThat(mappedValue, is("on")); + assertThat(outboundSpec.getValue(), is(instanceOf(OnOffType.class))); + } + private static class MyKNXChannel extends KNXChannel { public MyKNXChannel(Channel channel) { - super(Set.of("key1", "key2"), List.of(UnDefType.class), channel); + super(List.of("key1", "key2"), List.of(UnDefType.class), channel); } @Override