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

[homekit] increase flexibility of ColorTemperature #13538

Merged
merged 2 commits into from
Oct 22, 2022

Conversation

ccutrer
Copy link
Contributor

@ccutrer ccutrer commented Oct 11, 2022

allow Number or Dimmer items, and mired or Kelvin units.

Dependent on openhab/openhab-core#3108 and hap-java/HAP-Java#167

allow Number or Dimmer items, and mired or Kelvin units.

Signed-off-by: Cody Cutrer <cody@cutrer.us>
@wborn wborn added awaiting other PR Depends on another PR enhancement An enhancement or new feature for an existing add-on labels Oct 12, 2022
@ccutrer
Copy link
Contributor Author

ccutrer commented Oct 14, 2022

fixes #12517

@wborn wborn added rebuild Triggers Jenkins PR build and removed awaiting other PR Depends on another PR rebuild Triggers Jenkins PR build labels Oct 16, 2022
@ccutrer
Copy link
Contributor Author

ccutrer commented Oct 18, 2022

@yfre : this is ready for review now; the openhab-core dependencies have been merged.

final int finalMinValue = minValue;
final int range = maxValue - minValue;

// final int defaultValue = minValue;
Copy link
Contributor

Choose a reason for hiding this comment

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

this commented out line can be probably removed

Copy link
Contributor

@yfre yfre left a comment

Choose a reason for hiding this comment

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

thank you @ccutrer
homekit specific and logic looks good to me. good examples in the docu.
i have no experience with openHAB QuantityType, but the unit conversions looks correct.

Signed-off-by: Cody Cutrer <cody@cutrer.us>
@lolodomo
Copy link
Contributor

Code is too much complex for me with a notion I don't understand at all: invertible unit
@fwolter @jlaur : please review

@ccutrer
Copy link
Contributor Author

ccutrer commented Oct 20, 2022

Code is too much complex for me with a notion I don't understand at all: invertible unit
@fwolter @jlaur : please review

An invertible unit is simply a conversion that allows changing dimensions. This is required because mired is 1/K (plus a multiplication), and that's technically a different dimension (of 1/temperature, not temperature). It's a new concept introduced into core for this. I need to do a PR for the MQTT binding that uses it to allow implicit conversion between K<->Mired with that binding as well.

Copy link
Contributor

@lolodomo lolodomo left a comment

Choose a reason for hiding this comment

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

LGTM

@lolodomo lolodomo merged commit 66c7211 into openhab:main Oct 22, 2022
@lolodomo lolodomo added this to the 3.4 milestone Oct 22, 2022
@ccutrer ccutrer deleted the homekit-color-temp branch October 23, 2022 16:01
andan67 pushed a commit to andan67/openhab-addons that referenced this pull request Nov 6, 2022
* [homekit] increase flexibility of ColorTemperature

allow Number or Dimmer items, and mired or Kelvin units.

Signed-off-by: Cody Cutrer <cody@cutrer.us>
andrasU pushed a commit to andrasU/openhab-addons that referenced this pull request Nov 12, 2022
* [homekit] increase flexibility of ColorTemperature

allow Number or Dimmer items, and mired or Kelvin units.

Signed-off-by: Cody Cutrer <cody@cutrer.us>
Signed-off-by: Andras Uhrin <andras.uhrin@gmail.com>
borazslo pushed a commit to borazslo/openhab-mideaac-addon that referenced this pull request Jan 8, 2023
* [homekit] increase flexibility of ColorTemperature

allow Number or Dimmer items, and mired or Kelvin units.

Signed-off-by: Cody Cutrer <cody@cutrer.us>
psmedley pushed a commit to psmedley/openhab-addons that referenced this pull request Feb 23, 2023
* [homekit] increase flexibility of ColorTemperature

allow Number or Dimmer items, and mired or Kelvin units.

Signed-off-by: Cody Cutrer <cody@cutrer.us>
nemerdaud pushed a commit to nemerdaud/openhab-addons that referenced this pull request Feb 28, 2023
* [homekit] increase flexibility of ColorTemperature

allow Number or Dimmer items, and mired or Kelvin units.

Signed-off-by: Cody Cutrer <cody@cutrer.us>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement An enhancement or new feature for an existing add-on
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants