core
core copied to clipboard
Create new HALO Home integration
Proposed change
This change adds support for HALO Home smart lights. Original PR https://github.com/home-assistant/core/pull/58262, which was closed due to missing a config_flow which I have since added.
Type of change
- [ ] Dependency upgrade
- [ ] Bugfix (non-breaking change which fixes an issue)
- [x] 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
- This PR fixes or closes issue: fixes #
- This PR is related to issue:
- Link to documentation pull request: https://github.com/home-assistant/home-assistant.io/pull/20812
Checklist
- [x] The code change is tested and works locally.
- [x] Local tests pass. Your PR cannot be merged unless tests pass
- [x] There is no commented out code in this PR.
- [x] I have followed the development checklist
- [x] The code has been formatted using Black (
black --fast homeassistant tests
) - [x] 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:
- [x] The manifest file has all fields filled out correctly.
Updated and included derived files by running:python3 -m script.hassfest
. - [x] New or updated dependencies have been added to
requirements_all.txt
.
Updated by runningpython3 -m script.gen_requirements_all
. - [ ] For the updated dependencies - a link to the changelog, or at minimum a diff between library versions is added to the PR description.
- [x] Untested files have been added to
.coveragerc
.
The integration reached or maintains the following Integration Quality Scale:
- [x] 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.
Please make sure the CI passes
@frenck Is there anything else I need to do before this is ready to merge?
@nayaverdier Is there anything left to finish this integration? Very much looking forward to using it.
@mishakorablin Just resolved merge conflicts so AFAIK just awaiting final review of this PR.
CI has raised an issue with COLOR_MODE_COLOR_TEMP, which is now deprecated.
Warning: homeassistant/components/halohome/light.py:4:0: W7422: COLOR_MODE_COLOR_TEMP is deprecated, replaced by ColorMode enum (hass-deprecated-import)
@epenet Thanks for pushing the CI through, I think I've resolved that issue now. I do see another test failing, test_numpy_errors[pyloop]
, however it seems unrelated to my changes.
Hello @frenck @balloob! Is there anything left to merge this update?
@frenck Please let me know if there's anything remaining to do before this PR can be merged.
Sadly, the introduction of the new Bluetooth stack in 2022.8 completely broke this PR. I don't see an easy way to fix it. Issues I found so far:
- Home Assistant overrides BleakClient and BleakScanner globally since 2022.8. You can not pass a MAC address directly to the BleakClient constructor anymore which means this breaks the halohome python library.
- You are supposed to always pass a BLEDevice to the BleakClient() constructor now. Which is a problem as Halo devices do not broadcast and therefore can not be obtained through BleakScanner.discover() (side note: the actual MAC addresses are obtained through a cloud API). Worse, Home Assistant does not implement BleakScanner.find_device_by_name() so we are really out of luck here.
- There is a bug in the halohome python library where a user specified timeout is not forwarded correctly to the LocationConnection when connecting. In my case the default 5 seconds is not enough, I need 20. So no devices are ever found. Also that timeout should be exposed as a setting in HA.
- Long timeouts seem to break HA badly, I get timeout errors constantly. I think this needs a completely different approach as to not block HA when trying to connect to bluetooth devices when setting properties.
- The product ID for HLBSL6099 lights is 162. The halohome python was apparently never tested with other Halo devices as it is hard coded to a single product ID: 93. There is an API option to pass product IDs in the halohome python library but this PR does not expose that as a setting in HA which makes this integration not very useful unless you have that specific light fixture.
Looking at the overall picture I think a better approach to support these Halo devices might be to run a minimal MQTT client outside of HA, in a container or VM, and with a dedicated bluetooth dongle.
That said I think the current HA bluetooth implementation leaves a lot to be desired. It smells like a bug farm right now.
Closing this PR in light of the above.