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

Jump to bottom

[tuya] Add dimmer reversed value support #542

Merged
merged 1 commit into from
Dec 26, 2023
Merged

[tuya] Add dimmer reversed value support #542

merged 1 commit into from
Dec 26, 2023

Conversation

jsetton
Copy link
Contributor

@jsetton jsetton commented Dec 21, 2023

Some dimmer type channel should have the ability to support inverted values. This is useful for the temp_value channel which is generally implemented, compared to similar light control addons, from colder (0%) to warmer (100%).

@jsetton jsetton requested a review from J-N-K as a code owner December 21, 2023 06:36
@J-N-K
Copy link
Member

J-N-K commented Dec 23, 2023

Wouldn't this also be possible with the INVERT profile?

@J-N-K J-N-K added the enhancement New feature or request label Dec 23, 2023
@jsetton
Copy link
Contributor Author

jsetton commented Dec 23, 2023

As far as I can see (using the latest OH 4.2-SNAPSHOT), I don't see any INVERT profile available. However, I am aware that a MAP or a SCRIPT profile can achieve this. But I think this is added complexity especially since the channel definitions for the Tuya addon are already generic, I figured it wouldn't hurt to have that option at that level.

I could make the case that the addon should actually invert the temp_value data point by default to keep the consistency across the OH ecosystem. If you rather hardcode that use case, I don't have any issue with that.

J-N-K
J-N-K requested changes Dec 23, 2023
View reviewed changes
Copy link
Member

@J-N-K J-N-K left a comment

Choose a reason for hiding this comment

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

Otherwise LGTM. Since this is also backward compatible we can merge it without any further issues.

...ing.tuya/src/main/java/org/smarthomej/binding/tuya/internal/config/ChannelConfiguration.java Outdated Show resolved Hide resolved
bundles/org.smarthomej.binding.tuya/src/main/resources/OH-INF/thing/thing-types.xml Outdated Show resolved Hide resolved
@jsetton jsetton changed the title [tuya] Add dimmer inverted value support [tuya] Add dimmer reversed value support Dec 23, 2023
@jsetton
Copy link
Contributor Author

jsetton commented Dec 23, 2023

I made the changes based on your comments.

@J-N-K
Copy link
Member

J-N-K commented Dec 23, 2023

Looks good. the only thing missing is the documentation. Can you add the parameter to the channel documentation in README.md?

@jsetton
Copy link
Contributor Author

jsetton commented Dec 23, 2023

I updated the documentation.

J-N-K
J-N-K previously approved these changes Dec 24, 2023
View reviewed changes
Copy link
Member

@J-N-K J-N-K left a comment

Choose a reason for hiding this comment

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

Thanks for the other doc improvements!

@J-N-K J-N-K added this to the 4.0.3 milestone Dec 24, 2023
@J-N-K
Copy link
Member

J-N-K commented Dec 24, 2023

Unfortunately there is now a merge conflict after I merged your other PR. Can you fix that? Thanks.

@jsetton
Copy link
Contributor Author

jsetton commented Dec 24, 2023

Done.

@J-N-K J-N-K merged commit 8ed983b into smarthomej:4.0.x Dec 26, 2023
2 checks passed
@jsetton jsetton deleted the tuya-dimmer-inverted branch December 26, 2023 17:00
J-N-K pushed a commit that referenced this pull request Dec 26, 2023
@jsetton @J-N-K
[tuya] Add dimmer reversed value support (#542)
e45ae52 
Signed-off-by: jsetton <[email protected]>
(cherry picked from commit 8ed983b)
J-N-K pushed a commit that referenced this pull request Dec 26, 2023
@jsetton @J-N-K
[tuya] Add dimmer reversed value support (#542)
61266ac 
Signed-off-by: jsetton <[email protected]>
(cherry picked from commit 8ed983b)
(cherry picked from commit e45ae52)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@J-N-K J-N-K J-N-K approved these changes

Assignees
No one assigned
Labels
enhancement New feature or request
Projects
None yet
Milestone
4.0.3
Development

Successfully merging this pull request may close these issues.
2 participants
@jsetton @J-N-K