[yeelight] Add support for yeelight 650 with ambient light (Closes #6… #6749

vkoop · 2020-01-02T20:45:08Z

Fixes #6227 - Support for Yeelight 650 (ceiling4)

added needed enums, constants etc.
added channel to control ambient light color / brightness
added channel to switch to nightlight mode
started implementing unit tests

) Signed-off-by: Viktor Koop <[email protected]>

TravisBuddy · 2020-01-02T20:49:25Z

Travis tests were successful

Hey @vkoop,
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-01-03T23:30:30Z

Travis tests were successful

Hey @vkoop,
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: Viktor Koop <[email protected]>

… create a cached thread pool once. Signed-off-by: Viktor Koop <[email protected]>

Hilbrand · 2020-01-06T14:41:49Z

bundles/org.openhab.binding.yeelight/README.md

@@ -39,6 +39,10 @@ All devices support some of the following channels:
 |`color` | `Color` | This channel supports color control, it is available on `wonder` and `stripe`.|
 |`colorTemperature` | `Dimmer` | This channel supports adjusting the color temperature, it is available on `wonder` and `stripe` and `ceiling`.|
 |`command` | `String` | This channel sends a command directly to the device, it is available on all Yeelight Things.|
+|`backgroundColor` | `Color` | This channel supports color control for the ambient light, it is available on `ceiling4`.|
+|`backgroundBrightness` | `Dimmer` | This channel supports adjusting the ambient light brightness value, it is available on `ceiling4`.|


backgroundBrightness should probably not be a separate channel. The convention is that a user can create a Dimmer item on the color channel and that should be handled as setting the brightness.

I wasn't aware of this convention. I will adapt the code.

I can imagine, there is a lot to know. For this see https://www.openhab.org/docs/concepts/items.html#a-note-on-items-which-accept-multiple-state-data-types

@Hilbrand I've implemented the changes as suggested and combined ambient light color and brightness into one channel.

TravisBuddy · 2020-01-08T18:31:29Z

Travis tests were successful

Hey @vkoop,
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: Viktor Koop <[email protected]>

TravisBuddy · 2020-01-11T17:25:09Z

Travis tests were successful

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

J-N-K

I have left two comments. Thanks for adding tests.

J-N-K · 2020-01-31T18:33:45Z

...in/java/org/openhab/binding/yeelight/internal/lib/device/CeilingDeviceWithAmbientDevice.java

+                    mDeviceStatus.setActiveMode(ActiveMode.values()[activeMode]);
+                }
+            }
+        } catch (Exception e) {


Is it really necessary to catch Exception here? Whatr type of exceptions do you expect? The problem with catching Exception is that it also catches NPE and therefore might hide programming errors.

I don't think the try/catch block is needed at all and will remove it.

J-N-K · 2020-01-31T18:37:11Z

...ding.yeelight/src/main/java/org/openhab/binding/yeelight/internal/lib/device/DeviceBase.java

-                            updateProp += " power";
+                    for (Entry<String, JsonElement> prop : propsObject.entrySet()) {
+                        final YeelightDeviceProperty property = YeelightDeviceProperty.fromString(prop.getKey());
+                        if (YeelightDeviceProperty.POWER.equals(property)) {


Wouldn't it be possible to use switch (property) here? Since Java 7 we can switch on strings.

Initially I didn't want to refactor to much of the code to simplify the review process.

I've changed the if/else block to a switch statement.

Signed-off-by: Viktor Koop <[email protected]>

TravisBuddy · 2020-01-31T20:26:40Z

Travis tests were successful

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

J-N-K · 2020-02-02T09:03:14Z

@Hilbrand I guess your request was fulfilled. Can you approve?

…model, e.g. Teufel 3sixty Signed-off-by: Markus Michels <[email protected]>

seems to be fulfilled

…enhab#6… (openhab#6749) * [yeelight] Add support for yeelight 650 with ambient light (Closes openhab#6227) Signed-off-by: Viktor Koop <[email protected]> Signed-off-by: leluna <[email protected]>

…enhab#6… (openhab#6749) * [yeelight] Add support for yeelight 650 with ambient light (Closes openhab#6227) Signed-off-by: Viktor Koop <[email protected]> Signed-off-by: Hans-Reiner Hoffmann <[email protected]>

openhab#6749) * [yeelight] Add support for yeelight 650 with ambient light (Closes openhab#6227) Signed-off-by: Viktor Koop <[email protected]>

…enhab#6… (openhab#6749) * [yeelight] Add support for yeelight 650 with ambient light (Closes openhab#6227) Signed-off-by: Viktor Koop <[email protected]>

openhab#6749) * [yeelight] Add support for yeelight 650 with ambient light (Closes openhab#6227) Signed-off-by: Viktor Koop <[email protected]>

…enhab#6… (openhab#6749) * [yeelight] Add support for yeelight 650 with ambient light (Closes openhab#6227) Signed-off-by: Viktor Koop <[email protected]>

…enhab#6… (openhab#6749) * [yeelight] Add support for yeelight 650 with ambient light (Closes openhab#6227) Signed-off-by: Viktor Koop <[email protected]> Signed-off-by: Daan Meijer <[email protected]>

openhab#6749) * [yeelight] Add support for yeelight 650 with ambient light (Closes openhab#6227) Signed-off-by: Viktor Koop <[email protected]>

[yeelight] Add support for yeelight 650 with ambient light (Closes #6227

633d32e

) Signed-off-by: Viktor Koop <[email protected]>

vkoop requested a review from claell as a code owner January 2, 2020 20:45

vkoop mentioned this pull request Jan 2, 2020

[Yeelight] Added support for YEELIGHT JIAOYUE 650 lamps #6301

Closed

vkoop added 3 commits January 4, 2020 01:18

[yeelight] Combine switch cases with similar logic

8e258d4

Signed-off-by: Viktor Koop <[email protected]>

[yeelight] don't initialize variable with useless value

27d03d3

Signed-off-by: Viktor Koop <[email protected]>

[yeelight] Instead of creating an executor service on every search we…

d875836

… create a cached thread pool once. Signed-off-by: Viktor Koop <[email protected]>

Hilbrand added the enhancement An enhancement or new feature for an existing add-on label Jan 6, 2020

Hilbrand previously requested changes Jan 6, 2020

View reviewed changes

[yeelight] Fixes code review remarks

f0e0c2f

Signed-off-by: Viktor Koop <[email protected]>

martinvw added the cre Coordinated Review Effort label Jan 13, 2020

J-N-K requested changes Jan 31, 2020

View reviewed changes

[yeelight] Fixed code review remarks.

dbcbb99

Signed-off-by: Viktor Koop <[email protected]>

J-N-K approved these changes Feb 2, 2020

View reviewed changes

markus7017 added a commit to markus7017/openhab-addons that referenced this pull request Feb 8, 2020

openhab#6749: Support for auto discovery with empty manufacturer and …

cf60e01

…model, e.g. Teufel 3sixty Signed-off-by: Markus Michels <[email protected]>

J-N-K merged commit 98e3746 into openhab:2.5.x Feb 29, 2020

cpmeister added this to the 2.5.3 milestone Mar 19, 2020

vkoop deleted the feature/6227-yeelight-ceiling-v4-support branch March 28, 2020 09:00

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[yeelight] Add support for yeelight 650 with ambient light (Closes #6… #6749

[yeelight] Add support for yeelight 650 with ambient light (Closes #6… #6749

vkoop commented Jan 2, 2020 •

edited

Loading

TravisBuddy commented Jan 2, 2020

TravisBuddy commented Jan 3, 2020

Hilbrand Jan 6, 2020

vkoop Jan 8, 2020

Hilbrand Jan 8, 2020

vkoop Jan 11, 2020

TravisBuddy commented Jan 8, 2020

TravisBuddy commented Jan 11, 2020

J-N-K left a comment

J-N-K Jan 31, 2020

vkoop Jan 31, 2020

J-N-K Jan 31, 2020

vkoop Jan 31, 2020 •

edited

Loading

TravisBuddy commented Jan 31, 2020

J-N-K commented Feb 2, 2020

[yeelight] Add support for yeelight 650 with ambient light (Closes #6… #6749

[yeelight] Add support for yeelight 650 with ambient light (Closes #6… #6749

Conversation

vkoop commented Jan 2, 2020 • edited Loading

TravisBuddy commented Jan 2, 2020

Travis tests were successful

TravisBuddy commented Jan 3, 2020

Travis tests were successful

Hilbrand Jan 6, 2020

Choose a reason for hiding this comment

vkoop Jan 8, 2020

Choose a reason for hiding this comment

Hilbrand Jan 8, 2020

Choose a reason for hiding this comment

vkoop Jan 11, 2020

Choose a reason for hiding this comment

TravisBuddy commented Jan 8, 2020

Travis tests were successful

TravisBuddy commented Jan 11, 2020

Travis tests were successful

J-N-K left a comment

Choose a reason for hiding this comment

J-N-K Jan 31, 2020

Choose a reason for hiding this comment

vkoop Jan 31, 2020

Choose a reason for hiding this comment

J-N-K Jan 31, 2020

Choose a reason for hiding this comment

vkoop Jan 31, 2020 • edited Loading

Choose a reason for hiding this comment

TravisBuddy commented Jan 31, 2020

Travis tests were successful

J-N-K commented Feb 2, 2020

vkoop commented Jan 2, 2020 •

edited

Loading

vkoop Jan 31, 2020 •

edited

Loading