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.

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

[chain] Initial contribution #97

Merged
merged 9 commits into from
May 16, 2021
Merged

[chain] Initial contribution #97

merged 9 commits into from
May 16, 2021

Conversation

J-N-K
Copy link
Member

@J-N-K J-N-K commented May 12, 2021

This adds a profile that allows chaining transformations.

Signed-off-by: Jan N. Klug jan.n.klug@rub.de

@J-N-K J-N-K added newbinding workinprogress Not yet ready for merge labels May 12, 2021
@J-N-K
Copy link
Member Author

J-N-K commented May 13, 2021

@cweitkamp: WDYT?

@cweitkamp
Copy link
Contributor

This looks cool. 😎 Very useful.

@J-N-K J-N-K removed the workinprogress Not yet ready for merge label May 15, 2021
@J-N-K J-N-K changed the title [WIP][transform.chain] Initial contribution [transform.chain] Initial contribution May 15, 2021
@J-N-K J-N-K requested a review from cweitkamp May 16, 2021 11:19
Copy link
Contributor

@cweitkamp cweitkamp left a comment

Choose a reason for hiding this comment

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

Thanks. Looks straight forward even if the ValueTransformationProvider is a black box for me.

I left a few comments.


## Example: Door lock

A lock that can report its status and accepts commands as JSON and accepts commands.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
A lock that can report its status and accepts commands as JSON and accepts commands.
A lock that can report its status and accepts commands in JSON.

@Override
public @Nullable Profile createProfile(ProfileTypeUID profileTypeUID, ProfileCallback callback,
ProfileContext profileContext) {
return new ChainTransformationProfile(callback, profileContext, valueTransformationProvider);
Copy link
Contributor

Choose a reason for hiding this comment

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

IIRC you should check for matching ProfileTypeUID or return null otherwise. I am not sure if the calling instance checks the factory for getSupportedProfileTypeUIDs() or not.

Suggested change
return new ChainTransformationProfile(callback, profileContext, valueTransformationProvider);
return ChainTransformationProfile.PROFILE_TYPE_UID.equals(profileTypeUID) ? new ChainTransformationProfile(callback, profileContext, valueTransformationProvider) : null;

* @author Jan N. Klug - Initial contribution
*/
@NonNullByDefault
public class TestProfileContext implements ProfileContext {
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you really need this helper class? Or can you mock it?

Copy link
Contributor

@cweitkamp cweitkamp left a comment

Choose a reason for hiding this comment

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

LGTM.

There is a remaining conflict in bundles/pom.xml.

cweitkamp
cweitkamp previously approved these changes May 16, 2021
J-N-K added 9 commits May 16, 2021 17:14
Signed-off-by: Jan N. Klug <jan.n.klug@rub.de>
Signed-off-by: Jan N. Klug <jan.n.klug@rub.de>
Signed-off-by: Jan N. Klug <jan.n.klug@rub.de>
Signed-off-by: Jan N. Klug <jan.n.klug@rub.de>
Signed-off-by: Jan N. Klug <jan.n.klug@rub.de>
Signed-off-by: Jan N. Klug <jan.n.klug@rub.de>
Signed-off-by: Jan N. Klug <jan.n.klug@rub.de>
Signed-off-by: Jan N. Klug <jan.n.klug@rub.de>
Signed-off-by: Jan N. Klug <jan.n.klug@rub.de>
@J-N-K J-N-K merged commit ccef807 into smarthomej:main May 16, 2021
@J-N-K J-N-K added this to the 3.1.2 milestone May 16, 2021
@J-N-K J-N-K deleted the tranform-chain branch May 16, 2021 15:30
J-N-K added a commit that referenced this pull request May 16, 2021
* Initial contribution

Signed-off-by: Jan N. Klug <jan.n.klug@rub.de>
@J-N-K J-N-K changed the title [transform.chain] Initial contribution [chain] Initial contribution Oct 4, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants