-
-
Notifications
You must be signed in to change notification settings - Fork 422
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.
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
Allow ProfileTypeBuilder to pass its three filters list to both implementations of ProfileType #4325
base: main
Are you sure you want to change the base?
Allow ProfileTypeBuilder to pass its three filters list to both implementations of ProfileType #4325
Changes from 7 commits
a3fea88
347fbe7
e092440
7d37835
e3052cc
116046b
86e6852
ef03c14
d0130fd
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -38,10 +38,9 @@ | |
import org.openhab.core.items.ItemUtil; | ||
import org.openhab.core.thing.profiles.ProfileType; | ||
import org.openhab.core.thing.profiles.ProfileTypeRegistry; | ||
import org.openhab.core.thing.profiles.StateProfileType; | ||
import org.openhab.core.thing.profiles.TriggerProfileType; | ||
import org.openhab.core.thing.profiles.dto.ProfileTypeDTO; | ||
import org.openhab.core.thing.profiles.dto.ProfileTypeDTOMapper; | ||
import org.openhab.core.thing.type.ChannelKind; | ||
import org.openhab.core.thing.type.ChannelType; | ||
import org.openhab.core.thing.type.ChannelTypeRegistry; | ||
import org.openhab.core.thing.type.ChannelTypeUID; | ||
|
@@ -128,13 +127,7 @@ private Predicate<ProfileType> matchesChannelUID(@Nullable String channelTypeUID | |
// requested to filter against an unknown channel type -> do not return a ProfileType | ||
return t -> false; | ||
} | ||
switch (channelType.getKind()) { | ||
case STATE: | ||
return t -> stateProfileMatchesProfileType(t, channelType); | ||
case TRIGGER: | ||
return t -> triggerProfileMatchesProfileType(t, channelType); | ||
} | ||
return t -> false; | ||
return t -> profileTypeMatchesChannelType(t, channelType); | ||
J-N-K marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
|
||
private Predicate<ProfileType> matchesItemType(@Nullable String itemType) { | ||
|
@@ -151,31 +144,26 @@ private boolean profileTypeMatchesItemType(ProfileType pt, String itemType) { | |
|| supportedItemTypesOnProfileType.contains(itemType); | ||
} | ||
|
||
private boolean triggerProfileMatchesProfileType(ProfileType profileType, ChannelType channelType) { | ||
if (profileType instanceof TriggerProfileType triggerProfileType) { | ||
if (triggerProfileType.getSupportedChannelTypeUIDs().isEmpty()) { | ||
return true; | ||
} | ||
private boolean profileTypeMatchesChannelType(ProfileType profileType, ChannelType channelType) { | ||
@Nullable | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You don't need There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. My IDE (VSCode) disagrees and outputs a warning saying that the value cannot be null. That's what I've been doing in my plugin bundles, but I can remove it if you prefer. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. IMO it's wrong. The default locations for There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fair enough, I have removed them |
||
ChannelKind supportedChannelKind = profileType.getSupportedChannelKind(); | ||
if (supportedChannelKind != null && supportedChannelKind != channelType.getKind()) | ||
return false; | ||
|
||
if (triggerProfileType.getSupportedChannelTypeUIDs().contains(channelType.getUID())) { | ||
return true; | ||
} | ||
Collection<ChannelTypeUID> supportedChannelTypeUIDsOnProfileType = profileType.getSupportedChannelTypeUIDs(); | ||
if (!supportedChannelTypeUIDsOnProfileType.isEmpty() | ||
&& !supportedChannelTypeUIDsOnProfileType.contains(channelType.getUID())) { | ||
return false; | ||
} | ||
return false; | ||
} | ||
|
||
private boolean stateProfileMatchesProfileType(ProfileType profileType, ChannelType channelType) { | ||
if (profileType instanceof StateProfileType stateProfileType) { | ||
if (stateProfileType.getSupportedItemTypesOfChannel().isEmpty()) { | ||
return true; | ||
} | ||
|
||
Collection<String> supportedItemTypesOfChannelOnProfileType = stateProfileType | ||
.getSupportedItemTypesOfChannel(); | ||
Collection<String> supportedItemTypesOfChannelOnProfileType = profileType.getSupportedItemTypesOfChannel(); | ||
if (supportedItemTypesOfChannelOnProfileType.isEmpty()) { | ||
return true; | ||
} else { | ||
@Nullable | ||
String itemType = channelType.getItemType(); | ||
return itemType != null | ||
&& supportedItemTypesOfChannelOnProfileType.contains(ItemUtil.getMainItemType(itemType)); | ||
} | ||
return false; | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That looks really horrible. It would probably be better to
Why do you check
profileType.getSupportedItemTypesOfChannel
? A profile can change the item-type:Number:Dimensionless
) but can only be linked toDateTime
items. So a check would fail (in fact, currently the profile does not provide information about accepted channel types, so this will not cause an issue, but in principle it could).Number
, but the profileString
. In this case you could never link them.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree that it looks horrible, but it was jut to "fit in" with the already existing code.
As to why I do the check, well it's because I did a global search for the interface methods and made sure both methods are called in all the places where one was called following an
instanceof
call.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree, the style was not good before either and if you want to keep it: ok. But the
getSupportedItemTypesOfChannel
is excluding valid combinations.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm definitely in favor of changing the code style here, but as always when I'm submitting a pull request, I try to limit my changes to the code logic, adapting to the local style I encounter.
I'll try to come up with a more sensible way of writing this up.