Add Kuler Sky Bluetooth floor lamp integration #42372

emlove · 2020-10-25T21:17:31Z

Proposed change

This PR introduces a new component for Brightech Kuler Sky bluetooth floor lamps. https://smile.amazon.com/Brightech-Kuler-Sky-Torchiere-Adjustable/dp/B01LDT39AE

Type of change

Dependency upgrade
Bugfix (non-breaking change which fixes an issue)
New integration (thank you!)
New feature (which adds functionality to an existing integration)
Breaking change (fix/feature causing existing functionality to break)
Code quality improvements to existing code or addition of tests

Additional information

Link to documentation pull request: Add docs for the new Kuler Sky integration home-assistant.io#15410

Checklist

The code change is tested and works locally.
Local tests pass. Your PR cannot be merged unless tests pass
There is no commented out code in this PR.
I have followed the development checklist
The code has been formatted using Black (black --fast homeassistant tests)
Tests have been added to verify that the new code works.

If user exposed functionality or configuration variables are added/changed:

Documentation added/updated for www.home-assistant.io

If the code communicates with devices, web services, or third-party tools:

The manifest file has all fields filled out correctly.
Updated and included derived files by running: python3 -m script.hassfest.
New or updated dependencies have been added to requirements_all.txt.
Updated by running python3 -m script.gen_requirements_all.
Untested files have been added to .coveragerc.

The integration reached or maintains the following Integration Quality Scale:

No score or internal
🥈 Silver
🥇 Gold
🏆 Platinum

To help with the load of incoming pull requests:

I have reviewed two other open pull requests in this repository.

balloob · 2020-11-11T11:30:54Z

homeassistant/components/kulersky/config_flow.py

+            _LOGGER.error("Exception scanning for Kuler Sky devices", exc_info=exc)
+            return self.async_abort(reason="scan_error")
+
+        options = [


Why allow the user to pick? We could replace this config flow with https://developers.home-assistant.io/docs/config_entries_config_flow_handler#discoverable-integrations-that-require-no-authentication and then have the light platform set them all up. The user can disable the entities of the lights they don't want.

So I could have sworn I commented this somewhere, but I don't see it. 🤦‍♀️ The problem with these devices is they don't expose any identifying information, at least nothing available to the underlying pygatt library. So the best that pykulersky can do is discover all nearby bluetooth devices. (I need to update that library discover method name to make it more clear) So the options we've got are either have the user pick the correct device, or attempt to connect to each discovered bluetooth device, which could potentially leave the user waiting at the spinner for a while if there are a lot of other bluetooth devices in the vicinity. I'm OK with either option, but those are the tradeoffs.

Thanks for the review!

I pushed up some comments, and renamed the upstream method so that the current behavior is at least more clear when reading the code. Still happy to switch to plan B if you'd prefer.

The discovery config flow only requires a single light to be discovered to set up the config flow. (Just wants to make sure a device can be found).

After that it finishes the flow and lights will be added to Home Assistant as discovery picks up more.

I think that having the user pick a bluetooth address is putting burden on the user. Having a user wait while we figure out if they have any device is a lot better experience compared to having the user set up each individual device ~~and having to figure out their addresses too~~ (noticed we have name too).

Even discovering the first light could be really slow though, since we'll have to attempt a connection with every nearby bluetooth device before we know if it's a valid device. I'll switch it over though. Sounds like simple but slow is the preferred solution.

Maybe we can change the discovery flow to just check if Bluetooth works ?

Oh, I like that idea! Should be a quick test to ensure the bluetooth stack is working, and the devices will be available once they're discovered. 👍

balloob · 2020-11-11T11:32:45Z

homeassistant/components/kulersky/light.py

+        if brightness == 0 and white_value == 0 and not kwargs:
+            # If the light would be off, and no additional parameters were
+            # passed, just turn the light on full brightness.
+            brightness = 255


Is it possible to pass nothing, and it turns on with the last values before it was turned off?

Unfortunately these lights don't have that capability.

Laaaaaaaaaaame

homeassistant/components/kulersky/light.py

balloob · 2020-11-11T11:34:24Z

homeassistant/components/kulersky/strings.json

@@ -0,0 +1,20 @@
+{
+  "title": "Kuler Sky",


No need for this, HA defaults to the manifest title. Since it's a brandname, we don't want it translated.

balloob

Looks good. Few minor comments!

Co-authored-by: Paulus Schoutsen <paulus@home-assistant.io>

emlove · 2020-11-27T19:25:48Z

OK, updated to the new discovery strategy! It's looking a lot cleaner now.

balloob

Sorry for the delay. My notifications are flooded.

balloob · 2020-12-02T21:29:34Z

Tagged it so it will be in the next release :)

emlove · 2020-12-03T02:24:33Z

No worries. Thanks for your help!

MartinHjelmare

Nice! Some small comments.

MartinHjelmare · 2020-12-03T02:48:12Z

homeassistant/components/kulersky/config_flow.py

+
+
+config_entry_flow.register_discovery_flow(
+    DOMAIN, "Kuler Sky", _async_has_devices, config_entries.CONN_CLASS_UNKNOWN


We should update the connection class to be one of the standard classes except unknown, eg CONN_CLASS_LOCAL_POLL.

core/homeassistant/config_entries.py

Lines 85 to 89 in 4c7e17c

CONN_CLASS_CLOUD_PUSH = "cloud_push"

CONN_CLASS_CLOUD_POLL = "cloud_poll"

CONN_CLASS_LOCAL_PUSH = "local_push"

CONN_CLASS_LOCAL_POLL = "local_poll"

CONN_CLASS_ASSUMED = "assumed"

MartinHjelmare · 2020-12-03T02:52:21Z

homeassistant/components/kulersky/light.py

+                    # device. If the vendor's app is connected to the light when
+                    # home assistant tries to connect, this connection will fail.
+                    await hass.async_add_executor_job(light.connect)
+                    await hass.async_add_executor_job(light.get_color)


We could optimize this a bit by wrapping both calls inside of one function that we schedule on the executor.

MartinHjelmare · 2020-12-03T02:53:49Z

homeassistant/components/kulersky/light.py

+                async_add_entities([KulerskyLight(light)], update_before_add=True)
+
+    # Start initial discovery
+    hass.async_add_job(discover)


Please use hass.async_create_task instead. hass.async_add_job is legacy for general use.

emlove · 2020-12-03T15:58:23Z

Thanks for the follow-up @MartinHjelmare ! I've pushed up a cleanup PR here #43901

Co-authored-by: Paulus Schoutsen <paulus@home-assistant.io>

probot-home-assistant bot added the Hacktoberfest label Oct 25, 2020

homeassistant added the cla-signed label Oct 25, 2020

probot-home-assistant bot added has-tests new-integration integration: kulersky labels Oct 25, 2020

emlove force-pushed the kulersky branch from d9c1c7a to 9eb5721 Compare October 25, 2020 21:20

emlove mentioned this pull request Oct 25, 2020

Add docs for the new Kuler Sky integration home-assistant/home-assistant.io#15410

Merged

8 tasks

springstan changed the title ~~Kuler Sky Bluetooth floor lamp component~~ Add Kuler Sky Bluetooth floor lamp integration Oct 25, 2020

emlove mentioned this pull request Oct 25, 2020

Add brand images for Kuler Sky integration home-assistant/brands#1973

Merged

11 tasks

MartinHjelmare added the hacktoberfest-accepted label Oct 31, 2020

emlove force-pushed the kulersky branch 3 times, most recently from 5ecd51f to c6a717f Compare November 10, 2020 17:55

balloob reviewed Nov 11, 2020

View reviewed changes

homeassistant/components/kulersky/light.py Outdated Show resolved Hide resolved

balloob reviewed Nov 11, 2020

View reviewed changes

homeassistant/components/kulersky/light.py Outdated Show resolved Hide resolved

balloob reviewed Nov 11, 2020

View reviewed changes

balloob requested changes Nov 11, 2020

View reviewed changes

emlove and others added 6 commits November 11, 2020 11:00

Initial Kuler Sky component

e7f5c4c

Don't print full stack trace in log

113011a

Co-authored-by: Paulus Schoutsen <paulus@home-assistant.io>

Don't check for equality with False

e3cf925

Co-authored-by: Paulus Schoutsen <paulus@home-assistant.io>

Remove title from translation strings

eba8dcf

Bump to latest pykulersky

a7ee786

Comment to explain current discovery code

d636a37

emlove force-pushed the kulersky branch 3 times, most recently from 6c15a74 to 6dab4c5 Compare November 13, 2020 20:16

emlove force-pushed the kulersky branch 2 times, most recently from fc06d2e to 2d44a3f Compare November 13, 2020 20:21

Switch to config_flow_discovery config flow

3dccf7a

emlove force-pushed the kulersky branch from 2d44a3f to 3dccf7a Compare November 13, 2020 20:37

balloob approved these changes Dec 2, 2020

View reviewed changes

balloob merged commit 7c83092 into home-assistant:dev Dec 2, 2020

balloob added this to the 1.0.0 milestone Dec 2, 2020

emlove deleted the kulersky branch December 3, 2020 02:23

MartinHjelmare reviewed Dec 3, 2020

View reviewed changes

emlove mentioned this pull request Dec 3, 2020

Kulersky cleanups #43901

Merged

21 tasks

frenck added the cherry-picked label Dec 5, 2020

frenck pushed a commit that referenced this pull request Dec 5, 2020

Add Kuler Sky Bluetooth floor lamp integration (#42372)

ec90ebe

Co-authored-by: Paulus Schoutsen <paulus@home-assistant.io>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Kuler Sky Bluetooth floor lamp integration #42372

Add Kuler Sky Bluetooth floor lamp integration #42372

emlove commented Oct 25, 2020 •

edited

Loading

balloob Nov 11, 2020

emlove Nov 11, 2020

emlove Nov 11, 2020

balloob Nov 11, 2020

balloob Nov 11, 2020 •

edited

Loading

emlove Nov 11, 2020

balloob Nov 11, 2020

emlove Nov 12, 2020

balloob Nov 11, 2020

emlove Nov 11, 2020

balloob Nov 11, 2020

balloob Nov 11, 2020

balloob left a comment

emlove commented Nov 27, 2020

balloob left a comment

balloob commented Dec 2, 2020

emlove commented Dec 3, 2020

MartinHjelmare left a comment

MartinHjelmare Dec 3, 2020

MartinHjelmare Dec 3, 2020

MartinHjelmare Dec 3, 2020

emlove commented Dec 3, 2020



		config_entry_flow.register_discovery_flow(
		DOMAIN, "Kuler Sky", _async_has_devices, config_entries.CONN_CLASS_UNKNOWN

	CONN_CLASS_CLOUD_PUSH = "cloud_push"
	CONN_CLASS_CLOUD_POLL = "cloud_poll"
	CONN_CLASS_LOCAL_PUSH = "local_push"
	CONN_CLASS_LOCAL_POLL = "local_poll"
	CONN_CLASS_ASSUMED = "assumed"

Add Kuler Sky Bluetooth floor lamp integration #42372

Add Kuler Sky Bluetooth floor lamp integration #42372

Conversation

emlove commented Oct 25, 2020 • edited Loading

Proposed change

Type of change

Additional information

Checklist

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

balloob Nov 11, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

balloob left a comment

Choose a reason for hiding this comment

emlove commented Nov 27, 2020

balloob left a comment

Choose a reason for hiding this comment

balloob commented Dec 2, 2020

emlove commented Dec 3, 2020

MartinHjelmare left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

emlove commented Dec 3, 2020

emlove commented Oct 25, 2020 •

edited

Loading

balloob Nov 11, 2020 •

edited

Loading