Skip to content

Commit

Permalink
Fix CommunicationManager command handling (openhab#3714)
Browse files Browse the repository at this point in the history 
* Fix CommunicationManager

Signed-off-by: Jan N. Klug <[email protected]>
  • Loading branch information
@J-N-K
J-N-K authored Sep 13, 2023
1 parent d87ef1f commit af4fce1
Show file tree
Hide file tree
Showing 3 changed files with 67 additions and 61 deletions.
56 changes: 36 additions & 20 deletions ...penhab.core.thing/src/main/java/org/openhab/core/thing/internal/CommunicationManager.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -220,7 +220,7 @@ private Profile getProfile(ItemChannelLink link, Item item, @Nullable Thing thin

private ProfileCallback createCallback(ItemChannelLink link) {
return new ProfileCallbackImpl(eventPublisher, safeCaller, itemStateConverter, link, thingRegistry::get,
this::getItem);
this::getItem, this::toAcceptedCommand);
}

private @Nullable ProfileTypeUID determineProfileTypeUID(ItemChannelLink link, Item item, @Nullable Thing thing) {
Expand Down Expand Up @@ -281,9 +281,13 @@ private ProfileCallback createCallback(ItemChannelLink link) {
if (item != null && thing != null) {
Channel channel = thing.getChannel(link.getLinkedUID());
if (channel != null) {
String acceptedItemType = Objects.requireNonNullElse(channel.getAcceptedItemType(), "");
if (acceptedItemType.startsWith("Number")) {
acceptedItemType = "Number";
}
context = new ProfileContextImpl(link.getConfiguration(), item.getAcceptedDataTypes(),
item.getAcceptedCommandTypes(), acceptedCommandTypeMap.getOrDefault(
Objects.requireNonNullElse(channel.getAcceptedItemType(), ""), List.of()));
item.getAcceptedCommandTypes(),
acceptedCommandTypeMap.getOrDefault(acceptedItemType, List.of()));
}
}

Expand Down Expand Up @@ -394,17 +398,12 @@ private <T extends Type> void handleEvent(String itemName, T type, @Nullable Str
if (thing != null) {
Channel channel = thing.getChannel(channelUID.getId());
if (channel != null) {
@Nullable
T convertedType = toAcceptedType(type, channel, acceptedTypesFunction, item);
if (convertedType != null) {
if (thing.getHandler() != null) {
Profile profile = getProfile(link, item, thing);
action.applyProfile(profile, thing, convertedType);
}
} else {
logger.debug(
"Received event '{}' ({}) could not be converted to any type accepted by item '{}' ({})",
type, type.getClass().getSimpleName(), itemName, item.getType());
if (thing.getHandler() != null) {
// fix QuantityType/DecimalType, leave others as-is
@Nullable
T uomType = fixUoM(type, channel, item);
Profile profile = getProfile(link, item, thing);
action.applyProfile(profile, thing, uomType != null ? uomType : type);
}
} else {
logger.debug("Received event '{}' for non-existing channel '{}', not forwarding it to the handler",
Expand All @@ -418,8 +417,7 @@ private <T extends Type> void handleEvent(String itemName, T type, @Nullable Str
}

@SuppressWarnings("unchecked")
private <T extends Type> @Nullable T toAcceptedType(T originalType, Channel channel,
Function<@Nullable String, @Nullable List<Class<? extends T>>> acceptedTypesFunction, Item item) {
private <T extends Type> @Nullable T fixUoM(@Nullable T originalType, Channel channel, Item item) {
String channelAcceptedItemType = channel.getAcceptedItemType();

if (channelAcceptedItemType == null) {
Expand All @@ -442,22 +440,40 @@ private <T extends Type> void handleEvent(String itemName, T type, @Nullable Str
Unit<?> unit = Objects.requireNonNull(((NumberItem) item).getUnit());
return (T) new QuantityType<>(decimalType.toBigDecimal(), unit);
}
return null;
}

public @Nullable Command toAcceptedCommand(Command originalType, @Nullable Channel channel, @Nullable Item item) {
if (item == null || channel == null) {
logger.warn("Trying to convert types for non-existing channel or item, discarding command.");
return null;
}
String channelAcceptedItemType = channel.getAcceptedItemType();

if (channelAcceptedItemType == null) {
return originalType;
}

Command uomCommand = fixUoM(originalType, channel, item);
if (uomCommand != null) {
return uomCommand;
}

// handle HSBType/PercentType
if (CoreItemFactory.DIMMER.equals(channelAcceptedItemType) && originalType instanceof HSBType hsb) {
return (T) (hsb.as(PercentType.class));
return hsb.as(PercentType.class);
}

// check for other cases if the type is acceptable
List<Class<? extends T>> acceptedTypes = acceptedTypesFunction.apply(channelAcceptedItemType);
List<Class<? extends Command>> acceptedTypes = acceptedCommandTypeMap.get(channelAcceptedItemType);
if (acceptedTypes == null || acceptedTypes.contains(originalType.getClass())) {
return originalType;
} else if (acceptedTypes.contains(PercentType.class) && originalType instanceof State state
&& PercentType.class.isAssignableFrom(originalType.getClass())) {
return (@Nullable T) state.as(PercentType.class);
return state.as(PercentType.class);
} else if (acceptedTypes.contains(OnOffType.class) && originalType instanceof State state
&& PercentType.class.isAssignableFrom(originalType.getClass())) {
return (@Nullable T) state.as(OnOffType.class);
return state.as(OnOffType.class);
} else {
logger.debug("Received not accepted type '{}' for channel '{}'", originalType.getClass().getSimpleName(),
channel.getUID());
Expand Down
33 changes: 27 additions & 6 deletions ...ore.thing/src/main/java/org/openhab/core/thing/internal/profiles/ProfileCallbackImpl.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import org.openhab.core.items.events.ItemEventFactory;
import org.openhab.core.library.items.StringItem;
import org.openhab.core.library.types.StringType;
import org.openhab.core.thing.Channel;
import org.openhab.core.thing.Thing;
import org.openhab.core.thing.ThingUID;
import org.openhab.core.thing.binding.ThingHandler;
Expand Down Expand Up @@ -52,16 +53,19 @@ public class ProfileCallbackImpl implements ProfileCallback {
private final Function<String, @Nullable Item> itemProvider;
private final SafeCaller safeCaller;
private final ItemStateConverter itemStateConverter;
private final AcceptedTypeConverter acceptedTypeConverter;

public ProfileCallbackImpl(EventPublisher eventPublisher, SafeCaller safeCaller,
ItemStateConverter itemStateConverter, ItemChannelLink link,
Function<ThingUID, @Nullable Thing> thingProvider, Function<String, @Nullable Item> itemProvider) {
Function<ThingUID, @Nullable Thing> thingProvider, Function<String, @Nullable Item> itemProvider,
AcceptedTypeConverter acceptedTypeConverter) {
this.eventPublisher = eventPublisher;
this.safeCaller = safeCaller;
this.itemStateConverter = itemStateConverter;
this.link = link;
this.thingProvider = thingProvider;
this.itemProvider = itemProvider;
this.acceptedTypeConverter = acceptedTypeConverter;
}

@Override
Expand All @@ -78,11 +82,22 @@ public void handleCommand(Command command) {
if (ThingHandlerHelper.isHandlerInitialized(thing)) {
logger.debug("Delegating command '{}' for item '{}' to handler for channel '{}'", command,
link.getItemName(), link.getLinkedUID());
safeCaller.create(handler, ThingHandler.class)
.withTimeout(CommunicationManager.THINGHANDLER_EVENT_TIMEOUT).onTimeout(() -> {
logger.warn("Handler for thing '{}' takes more than {}ms for handling a command",
handler.getThing().getUID(), CommunicationManager.THINGHANDLER_EVENT_TIMEOUT);
}).build().handleCommand(link.getLinkedUID(), command);
Channel channel = thing.getChannel(link.getLinkedUID());
Command convertedCommand = acceptedTypeConverter.toAcceptedCommand(command, channel,
itemProvider.apply(link.getItemName()));
if (convertedCommand != null) {
safeCaller.create(handler, ThingHandler.class)
.withTimeout(CommunicationManager.THINGHANDLER_EVENT_TIMEOUT).onTimeout(() -> {
logger.warn("Handler for thing '{}' takes more than {}ms for handling a command",
handler.getThing().getUID(),
CommunicationManager.THINGHANDLER_EVENT_TIMEOUT);
}).build().handleCommand(link.getLinkedUID(), command);
} else {
logger.debug(
"Not delegating command '{}' for item '{}' to handler for channel '{}', "
+ "because it was not possible to convcert it to an accepted type).",
command, link.getItemName(), link.getLinkedUID());
}
} else {
logger.debug("Not delegating command '{}' for item '{}' to handler for channel '{}', "
+ "because handler is not initialized (thing must be in status UNKNOWN, ONLINE or OFFLINE but was {}).",
Expand Down Expand Up @@ -129,4 +144,10 @@ public void sendUpdate(State state) {
eventPublisher.post(
ItemEventFactory.createStateEvent(link.getItemName(), acceptedState, link.getLinkedUID().toString()));
}

@FunctionalInterface
public interface AcceptedTypeConverter {
@Nullable
Command toAcceptedCommand(Command originalType, @Nullable Channel channel, @Nullable Item item);
}
}
39 changes: 4 additions & 35 deletions ...ing.tests/src/main/java/org/openhab/core/thing/internal/CommunicationManagerOSGiTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
import org.mockito.ArgumentCaptor;
import org.mockito.Mock;
import org.mockito.junit.jupiter.MockitoExtension;
import org.mockito.junit.jupiter.MockitoSettings;
Expand Down Expand Up @@ -81,7 +80,6 @@
import org.openhab.core.thing.type.ChannelType;
import org.openhab.core.thing.type.ChannelTypeUID;
import org.openhab.core.types.Command;
import org.openhab.core.types.State;

/**
*
Expand Down Expand Up @@ -553,44 +551,15 @@ public void testProfileIsNotReusedOnThingChange() {
}

@Test
public void testItemCommandEventTypeDowncast() {
public void testItemCommandTypeDowncast() {
Thing thing = ThingBuilder
.create(THING_TYPE_UID, THING_UID).withChannels(ChannelBuilder
.create(STATE_CHANNEL_UID_2, CoreItemFactory.DIMMER).withKind(ChannelKind.STATE).build())
.build();
thing.setHandler(thingHandlerMock);
when(thingRegistryMock.get(eq(THING_UID))).thenReturn(thing);

manager.receive(ItemEventFactory.createCommandEvent(ITEM_NAME_2, HSBType.fromRGB(128, 128, 128)));
waitForAssert(() -> {
ArgumentCaptor<Command> commandCaptor = ArgumentCaptor.forClass(Command.class);
verify(stateProfileMock).onCommandFromItem(commandCaptor.capture());
Command command = commandCaptor.getValue();
assertNotNull(command);
assertEquals(PercentType.class, command.getClass());
});
verifyNoMoreInteractions(stateProfileMock);
verifyNoMoreInteractions(triggerProfileMock);
}

@Test
public void testItemStateEventTypeDowncast() {
Thing thing = ThingBuilder
.create(THING_TYPE_UID, THING_UID).withChannels(ChannelBuilder
.create(STATE_CHANNEL_UID_2, CoreItemFactory.DIMMER).withKind(ChannelKind.STATE).build())
.build();
thing.setHandler(thingHandlerMock);
when(thingRegistryMock.get(eq(THING_UID))).thenReturn(thing);

manager.receive(ItemEventFactory.createStateUpdatedEvent(ITEM_NAME_2, HSBType.fromRGB(128, 128, 128)));
waitForAssert(() -> {
ArgumentCaptor<State> stateCaptor = ArgumentCaptor.forClass(State.class);
verify(stateProfileMock).onStateUpdateFromItem(stateCaptor.capture());
State state = stateCaptor.getValue();
assertNotNull(state);
assertEquals(PercentType.class, state.getClass());
});
verifyNoMoreInteractions(stateProfileMock);
verifyNoMoreInteractions(triggerProfileMock);
Command command = manager.toAcceptedCommand(HSBType.fromRGB(128, 128, 128),
thing.getChannel(STATE_CHANNEL_UID_2), ITEM_2);
assertEquals(PercentType.class, command.getClass());
}
}

0 comments on commit af4fce1

Please sign in to comment.