-
-
Notifications
You must be signed in to change notification settings - Fork 196
Separate color brightness control pr #1363
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Separate color brightness control pr #1363
Conversation
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
….com/nekajcasa/AL-indepent-brightnes-and-color into separate-color-brightness-control-pr
for more information, see https://pre-commit.ci
protyposis
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a nice feature 👍 Thanks for your work, looking forward to use it!
I am a bit concerned about the UX impact of the current implementation though. I think AL already has a configuration surface that can be overwhelming for the average user, and this adds several new (kind of duplicate) options. Would it be possible to hide them in the UI while the independent_color_adapting checkbox is off?
Generally, I think the approach is right since it allows great flexibility and covers all use cases described in #1333. Code also looks fine, I only skimmed it though.
For the sake of completeness, I also considered a few alternative approaches while thinking about this:
A "Keep color synced to sun" checkbox as mentioned by #1333 (comment) would be very lightweight and only add one config flag, but it would prevent more advanced scenarios, e.g., "If it gets dark at 2 pm, I want my lights to keep daylight temperature until 5 pm (so my brain doesn’t enter sleep mode too early), and only start warming/dimming at 8 pm."
A "Custom sun config" dropdown with options "color", "brightness", "both" would be conceptually similar to the checkbox, but slightly more flexible, as it allows keeping either brightness or color synced to the sun. I’m not sure whether syncing brightness but not color to the sun is a meaningful use case.
An option I explored myself in the past is an "Adaptation mode" dropdown, also with options "color", "brightness", "both", which configures the properties that an AL instance actually handles and adapts. This would allow users to create one AL instance that only adapts brightness and another AL instance (with a different config) that only adapts color, as attempted in #1333 (comment). It's the most flexible, but also by far the most complex option. And it might violate the unique-config-entry quality scale requirement, potentially making it harder to become an official integration.
Changed "independent_color_adapting" to "independent_color", changes to webapp
for more information, see https://pre-commit.ci
I agree with this concern — the Adaptive Lighting options menu is already quite extensive, and adding more always has a UX impact. Unfortunately, Home Assistant’s config / options flows don’t support dynamically hiding or showing fields within the same form based on a checkbox toggle (as far as I know). That said, I completely agree that we should avoid overwhelming users, and I think it would be important to find a better user interface solution that keeps the default experience simple while still allowing full customization for advanced use cases. |
protyposis
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A check is failing because it can't push to forked repos. Please run these commands and push the changes manually:
adaptive-lighting/.github/workflows/update-readme.yml
Lines 25 to 36 in f84ee44
| - name: Install markdown-code-runner and README code dependencies | |
| run: | | |
| uv pip install markdown-code-runner==2.1.0 pandas tabulate | |
| - name: Run markdown-code-runner | |
| run: uv run markdown-code-runner --verbose README.md | |
| - name: Run update services.yaml | |
| run: uv run python .github/update-services.py | |
| - name: Run update strings.json | |
| run: uv run python .github/update-strings.py |
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
protyposis
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, thanks 👍 Handing over to @basnijholt
Summary
This PR adds support for independent control of color temperature and brightness timing.
It allows users to configure separate sunrise and sunset behavior for color temperature, while brightness can continue to follow a different schedule (e.g. time-of-day based). This addresses the use case where color temperature should follow the sun, but brightness should not.
Related issue: #1333
Motivation
Several users have requested the ability to decouple color temperature changes from brightness changes.
In particular:
Currently, Adaptive Lighting couples these behaviors, which makes certain setups (especially in northern regions with short daylight) difficult to achieve.
What this PR does
Notes about testing
For local Home Assistant testing, I temporarily renamed the integration domain and folder so I could run this version side-by-side with the official Adaptive Lighting integration.
This PR restores the original integration domain and folder structure so it remains fully compatible with the upstream project and does not introduce a new integration.
Status