[homekit] bugfix 7491 / add support for merging several updates to one command #7825

yfre · 2020-05-31T14:29:34Z

Issue:

complex openHAB items like Dimmer and Color have several characteristic. in case of Color all 3 characteristics are updated with one command/state - HSBType.
in the same time, HomeKit home app sends updates for the characteristics independently of each other, e.g. update for hue, update for saturation and update for brightness.
as results, characteristics can be get overwritten.
see #7491 and https://community.openhab.org/t/coloritem-hue-and-saturation-updates/99468/4
for more details.

Solution:
as propose in https://community.openhab.org/t/coloritem-hue-and-saturation-updates/99468/4 this PR introduce a proxy for openHAB item, that collects commands/update first and then sends merged update - either wenn all relevant updates received or after 50ms after first update.

in addition to HUE/Saturation issue, this update also solved Dimmer issue with "ON" and "Bightness" updates at the same time. see https://community.openhab.org/t/homekit-homekit-sending-on-and-100-command/99485

Signed-off-by: Eugen Freiter [email protected]

Signed-off-by: Eugen Freiter <[email protected]>

TravisBuddy · 2020-05-31T14:32:30Z

Travis tests were successful

Hey @yfre,
we found no major flaws with your code. Still you might want to look at this logfile, as we usually suggest some optional improvements.

...org.openhab.io.homekit/src/main/java/org/openhab/io/homekit/internal/HomekitOHItemProxy.java

.../org.openhab.io.homekit/src/main/java/org/openhab/io/homekit/internal/HomekitTaggedItem.java

.../src/main/java/org/openhab/io/homekit/internal/accessories/HomekitCharacteristicFactory.java

....homekit/src/main/java/org/openhab/io/homekit/internal/accessories/HomekitLightbulbImpl.java

...org.openhab.io.homekit/src/main/java/org/openhab/io/homekit/internal/HomekitOHItemProxy.java

J-N-K · 2020-06-01T16:43:06Z

Also, please add yourself as a CODEOWNER. I would like to hear your thoughts on PR to this bundle.

Signed-off-by: Eugen Freiter <[email protected]>

TravisBuddy · 2020-06-01T17:53:44Z

Travis tests were successful

Hey @yfre,
we found no major flaws with your code. Still you might want to look at this logfile, as we usually suggest some optional improvements.

TravisBuddy · 2020-06-01T18:11:59Z

Travis tests were successful

Hey @yfre,
we found no major flaws with your code. Still you might want to look at this logfile, as we usually suggest some optional improvements.

...org.openhab.io.homekit/src/main/java/org/openhab/io/homekit/internal/HomekitOHItemProxy.java

.../org.openhab.io.homekit/src/main/java/org/openhab/io/homekit/internal/HomekitTaggedItem.java

J-N-K · 2020-06-01T18:31:42Z

I didn't think about the names of the config options. I'll comment on that tomorrow

...org.openhab.io.homekit/src/main/java/org/openhab/io/homekit/internal/HomekitOHItemProxy.java

.../org.openhab.io.homekit/src/main/java/org/openhab/io/homekit/internal/HomekitTaggedItem.java

...org.openhab.io.homekit/src/main/java/org/openhab/io/homekit/internal/HomekitOHItemProxy.java

Signed-off-by: Eugen Freiter <[email protected]>

TravisBuddy · 2020-06-01T21:44:34Z

Travis tests were successful

Hey @yfre,
we found no major flaws with your code. Still you might want to look at this logfile, as we usually suggest some optional improvements.

...org.openhab.io.homekit/src/main/java/org/openhab/io/homekit/internal/HomekitCommandType.java

.../org.openhab.io.homekit/src/main/java/org/openhab/io/homekit/internal/HomekitDimmerMode.java

...org.openhab.io.homekit/src/main/java/org/openhab/io/homekit/internal/HomekitOHItemProxy.java

Signed-off-by: Eugen Freiter <[email protected]>

TravisBuddy · 2020-06-02T15:20:09Z

Travis tests were successful

Hey @yfre,
we found no major flaws with your code. Still you might want to look at this logfile, as we usually suggest some optional improvements.

.../org.openhab.io.homekit/src/main/java/org/openhab/io/homekit/internal/HomekitDimmerMode.java

...org.openhab.io.homekit/src/main/java/org/openhab/io/homekit/internal/HomekitOHItemProxy.java

J-N-K · 2020-06-02T16:17:26Z

Otherwise LGTM

Signed-off-by: Eugen Freiter <[email protected]>

TravisBuddy · 2020-06-02T18:54:01Z

Travis tests were successful

Hey @yfre,
we found no major flaws with your code. Still you might want to look at this logfile, as we usually suggest some optional improvements.

yfre · 2020-06-05T15:11:05Z

@cpmeister thank for the review. i have committed all changes. is anything missing ?

...org.openhab.io.homekit/src/main/java/org/openhab/io/homekit/internal/HomekitOHItemProxy.java

Signed-off-by: Eugen Freiter <[email protected]>

TravisBuddy · 2020-06-06T10:05:59Z

Travis tests were successful

Hey @yfre,
we found no major flaws with your code. Still you might want to look at this logfile, as we usually suggest some optional improvements.

Signed-off-by: Eugen Freiter <[email protected]>

TravisBuddy · 2020-06-07T19:24:15Z

Travis tests were successful

Hey @yfre,
we found no major flaws with your code. Still you might want to look at this logfile, as we usually suggest some optional improvements.

@cpmeister

…e command (openhab#7825) * add support for merging several updates to one command * incorporate J-N-K feedback, adapt the logic for dimmer * add yfre to CODEOWNERS * incorporate feedback from @cpmeister * remove some blank lines Signed-off-by: Eugen Freiter <[email protected]> Signed-off-by: CSchlipp <[email protected]>

@cpmeister

…e command (openhab#7825) * add support for merging several updates to one command * incorporate J-N-K feedback, adapt the logic for dimmer * add yfre to CODEOWNERS * incorporate feedback from @cpmeister * remove some blank lines Signed-off-by: Eugen Freiter <[email protected]>

@cpmeister

…e command (openhab#7825) * add support for merging several updates to one command * incorporate J-N-K feedback, adapt the logic for dimmer * add yfre to CODEOWNERS * incorporate feedback from @cpmeister * remove some blank lines Signed-off-by: Eugen Freiter <[email protected]>

@cpmeister

…e command (openhab#7825) * add support for merging several updates to one command * incorporate J-N-K feedback, adapt the logic for dimmer * add yfre to CODEOWNERS * incorporate feedback from @cpmeister * remove some blank lines Signed-off-by: Eugen Freiter <[email protected]>

@cpmeister

…e command (openhab#7825) * add support for merging several updates to one command * incorporate J-N-K feedback, adapt the logic for dimmer * add yfre to CODEOWNERS * incorporate feedback from @cpmeister * remove some blank lines Signed-off-by: Eugen Freiter <[email protected]>

@cpmeister

…e command (openhab#7825) * add support for merging several updates to one command * incorporate J-N-K feedback, adapt the logic for dimmer * add yfre to CODEOWNERS * incorporate feedback from @cpmeister * remove some blank lines Signed-off-by: Eugen Freiter <[email protected]> Signed-off-by: Daan Meijer <[email protected]>

@cpmeister

…e command (openhab#7825) * add support for merging several updates to one command * incorporate J-N-K feedback, adapt the logic for dimmer * add yfre to CODEOWNERS * incorporate feedback from @cpmeister * remove some blank lines Signed-off-by: Eugen Freiter <[email protected]>

add support for merging several updates to one command

c3ad86b

Signed-off-by: Eugen Freiter <[email protected]>

yfre added the bug An unexpected problem or unintended behavior of an add-on label May 31, 2020

yfre linked an issue Jun 1, 2020 that may be closed by this pull request

[homekit] Lightning Dimmer sends 2 commands #2782

Closed

yfre mentioned this pull request Jun 1, 2020

[homekit] Lightning Dimmer sends 2 commands #2782

Closed

J-N-K requested changes Jun 1, 2020

View reviewed changes

jeremystucki mentioned this pull request Jun 1, 2020

[homekit] Add workaround for color picker #7491

Closed

J-N-K self-assigned this Jun 1, 2020

Eugen Freiter added 2 commits June 1, 2020 19:50

incorporate J-N-K feedback, adapt the logic for dimmer

a0c2779

Signed-off-by: Eugen Freiter <[email protected]>

add yfre to CODEOWNERS

b75818f

Signed-off-by: Eugen Freiter <[email protected]>

yfre requested a review from a team as a code owner June 1, 2020 17:53

J-N-K requested changes Jun 1, 2020

View reviewed changes

...org.openhab.io.homekit/src/main/java/org/openhab/io/homekit/internal/HomekitOHItemProxy.java Outdated Show resolved Hide resolved

.../org.openhab.io.homekit/src/main/java/org/openhab/io/homekit/internal/HomekitTaggedItem.java Show resolved Hide resolved

cpmeister requested changes Jun 1, 2020

View reviewed changes

incorporate feedback

8fb23d3

Signed-off-by: Eugen Freiter <[email protected]>

cpmeister requested changes Jun 1, 2020

View reviewed changes

add NonNullByDefault

6601d62

Signed-off-by: Eugen Freiter <[email protected]>

J-N-K requested changes Jun 2, 2020

View reviewed changes

update README and incorporate further review feedback

0c0b4c6

Signed-off-by: Eugen Freiter <[email protected]>

cpmeister requested changes Jun 5, 2020

View reviewed changes

incorporate feedback from @cpmeister

0be4999

Signed-off-by: Eugen Freiter <[email protected]>

remove some blank lines

e653a66

Signed-off-by: Eugen Freiter <[email protected]>

cpmeister approved these changes Jun 7, 2020

View reviewed changes

cpmeister merged commit e271705 into openhab:2.5.x Jun 7, 2020

cpmeister added this to the 2.5.6 milestone Jun 7, 2020

yfre mentioned this pull request Jun 17, 2020

[homekit] fix 100% brightness for dimmer #7932

Merged

yfre deleted the fix_hue branch June 20, 2020 17:30

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[homekit] bugfix 7491 / add support for merging several updates to one command #7825

[homekit] bugfix 7491 / add support for merging several updates to one command #7825

yfre commented May 31, 2020

TravisBuddy commented May 31, 2020

J-N-K commented Jun 1, 2020

TravisBuddy commented Jun 1, 2020

TravisBuddy commented Jun 1, 2020

J-N-K commented Jun 1, 2020

TravisBuddy commented Jun 1, 2020

TravisBuddy commented Jun 2, 2020

J-N-K commented Jun 2, 2020

TravisBuddy commented Jun 2, 2020

yfre commented Jun 5, 2020

TravisBuddy commented Jun 6, 2020

TravisBuddy commented Jun 7, 2020

[homekit] bugfix 7491 / add support for merging several updates to one command #7825

[homekit] bugfix 7491 / add support for merging several updates to one command #7825

Conversation

yfre commented May 31, 2020

TravisBuddy commented May 31, 2020

Travis tests were successful

J-N-K commented Jun 1, 2020

TravisBuddy commented Jun 1, 2020

Travis tests were successful

TravisBuddy commented Jun 1, 2020

Travis tests were successful

J-N-K commented Jun 1, 2020

TravisBuddy commented Jun 1, 2020

Travis tests were successful

TravisBuddy commented Jun 2, 2020

Travis tests were successful

J-N-K commented Jun 2, 2020

TravisBuddy commented Jun 2, 2020

Travis tests were successful

yfre commented Jun 5, 2020

TravisBuddy commented Jun 6, 2020

Travis tests were successful

TravisBuddy commented Jun 7, 2020

Travis tests were successful