Config flow for the Flux integration
Proposed change
I've made some key improvements to the Flux integration:
Improvements:
- Simplified Installation: Flux can now be installed and set up directly from the UI, removing the need for yaml configuration.
- UI Parameter Consolidation: I've consolidated
brightnessanddisable_brightness_adjustinto one UI option, "Adjust Brightness".
Type of change
- [ ] Dependency upgrade
- [ ] Bugfix (non-breaking change which fixes an issue)
- [ ] New integration (thank you!)
- [x] New feature (which adds functionality to an existing integration)
- [x] Deprecation (breaking change to happen in the future)
- [ ] 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: https://github.com/home-assistant/home-assistant.io/pull/27742
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] I have followed the perfect PR recommendations
- [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:
- [x] 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. - [x] 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.
To help with the load of incoming pull requests:
- [x] I have reviewed two other open pull requests in this repository.
At https://github.com/home-assistant/core/pull/95447 you can see how a migration to the configuration flow works.
@jpbede I've added the migration to the configuration flow. Thanks again for the review!
@jpbede Can you take another look at the code?
Please take a look at the requested changes, and use the Ready for review button when you are done, thanks :+1:
There hasn't been any activity on this pull request recently. This pull request has been automatically marked as stale because of that and will be closed if no further activity occurs within 7 days. If you are the author of this PR, please leave a comment if you want to keep it open. Also, please rebase your PR onto the latest dev branch to ensure that it's up to date with the latest changes. Thank you for your contribution!
Bumping this one to be on my list again
I discussed this PR with @balloob , @frenck and some other core developers. The concern is that the flux integration has known issues and limitations and we don't want to promote it by adding a config flow to it at this point.
There are also a couple of well maintained and popular custom integration with more options:
- https://github.com/claytonjn/hass-circadian_lighting/,
- https://github.com/basnijholt/adaptive-lighting
@balloob thinks the functionality implemented by flux is very useful, which lines up with the popularity of the custom integrations, but he suggested a deeper integration with Home Assistant than an integration. Maybe a built-in option for lights with adjustable color-temperature to make them automatically adjust the color temperature during the day? Going in this direction would require an architecture discussion.
Another view (which I personally do not share) is that the functionality provided by the flux integration is too simple and could just be replaced with a blueprint, such as this one https://community.home-assistant.io/t/advanced-circadian-lighting/383543
Until the way forward is clear, we do not want to merge this PR.
@emontnemery Thank you for the comment.
In an attempt to turn this reply into a ~~concise and~~ coherent story, I've chosen to structure it as follows: I will start by providing some context; Explain what in my view the goal should be, and what the most feasible approach is to reach it. Then I will reply/address/discuss the concern and alternatives that were mentioned.
Some context + chosen approach My opinion is that light control (including the functionality provided by flux) should be easy to use by users of all knowledge levels. (If installation is required, it should also be easy) It currently is not easy to configure/use this in home assistant.
I know that it can be very difficult to make something "easy to use". I do have (big) ideas on how things can be improved, but do not have the knowledge and experience required to effectively implement them. (and getting it merged might also be a challenge 😉). Contributing to this project becomes more realistic if I make an existing thing "easier to use" with multiple smaller changes.
That is why I decided to improve this integration. In its current state it probably isn't good, but:
- it has potential to be easily installable (because it is in the core),
- by iteratively adding multiple small improvements, it might even become decent.
The first small improvement I had in mind was this one: https://github.com/home-assistant/core/pull/93181 But I found out I was the third person to propose exactly this change.
Apparently making yaml changes is not allowed, and a config flow is needed. (See https://github.com/home-assistant/core/pull/86201#discussion_r1080679662) That is the reason why this merge request exists.
Suggested alternatives.
I will address/discuss alternatives that were mentioned, and relate them to the context provided above.
There are also a couple of well maintained and popular custom integration with more options:
The integrations seem really good, but sadly they need to be installed using HACS. HACS itself is not easy to install by users of all knowledge levels. ("clear your browser caches", having/creating a Github account)
@balloob ... suggested a deeper integration with Home Assistant than an integration. ... Going in this direction would require an architecture discussion.
As stated earlier I don't feel I have the knowledge to implement architecture changes. But I do like the idea of deeper integration. Not having to install/add an integration is definitely very easy. Maybe the deeper integration could be a (very future) continuation of flux (without the issues and limitations).
The Concern
I discussed this PR with @balloob , @frenck and some other core developers. The concern is that the
fluxintegration has known issues and limitations and we don't want to promote it by adding a config flow to it at this point.
I don't know what the "known issues and limitations" of flux are.
But I can understand that you don't want to promote an integration if it has issues.
Proposed way forward:
- Someone makes a list of the "known issues and limitations" (knowing them is a prerequisite).
- Identify which issues and limitations need to be addressed before
fluxbecomes config-flow-worthy. - Someone addresses the issues and limitations that need to be addressed before
fluxbecomes config-flow-worthy. fluxbecomes config-flow-worthy
There hasn't been any activity on this pull request recently. This pull request has been automatically marked as stale because of that and will be closed if no further activity occurs within 7 days. If you are the author of this PR, please leave a comment if you want to keep it open. Also, please rebase your PR onto the latest dev branch to ensure that it's up to date with the latest changes. Thank you for your contribution!
Hi there @schelv
As pointed out correctly above, we indeed don't want to move forward with this at this point.
You ask about the issues with this integration; there are quite a few (especially when working with multiple light sources), but also feature completeness.
From as user perspective point of view, eventually, we don't want this to be an integration that provides a "device automation"-as-integration, but a native, built-in part of just lights in Home Assistant.
In the end, wrapping a light in an integration to automate stuff like this is odd. For that same reason, we don't want to level up a few more of these integrations (like alert and device_sun_light_trigger).
Nevertheless, thanks for being willing to contribute ❤️
../Frenck