Adaptive Lighting tweaks #868
Conversation
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #868 +/- ##
==========================================
- Coverage 66.17% 63.73% -2.45%
==========================================
Files 40 40
Lines 2903 2509 -394
Branches 792 650 -142
==========================================
- Hits 1921 1599 -322
+ Misses 883 739 -144
- Partials 99 171 +72
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
… is changed or turned on via HomeKit. Still need to fix tests
itavero
force-pushed
the
revisit-adaptive-lighting
branch
from
6d6c526 to
1da7611
Compare
|
|
🆗 Published SonarCloud and code coverage data. |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
WalkthroughThe pull request introduces significant enhancements to the Adaptive Lighting feature across multiple files. The changes primarily focus on transforming the adaptive lighting configuration from a simple boolean toggle to a more flexible object-based approach. The new configuration allows users to enable/disable adaptive lighting, control updates only when the light is on, and set transition times. By default, adaptive lighting is now enabled for compatible lights, with additional logic added to reset the internal color temperature reference when lights are adjusted via HomeKit. Changes
Sequence DiagramsequenceDiagram
participant User
participant HomeKit
participant Light
participant AdaptiveLighting
User->>Light: Turn On/Adjust Brightness
Light->>AdaptiveLighting: Reset Color Temperature
Light->>HomeKit: Update Light State
HomeKit->>User: Confirm Changes
Poem
Tip CodeRabbit's docstrings feature is now available as part of our Early Access Program! Simply use the command 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/converters/light.ts (1)
81-90: Consider simplifying the configuration logicThe configuration initialization could be simplified using object spread operator with conditional properties.
Consider this alternative implementation:
-let adaptiveLightingConfig: AdaptiveLightingConfig = { ...LightCreator.DEFAULT_CONFIG }; -if (isLightConfig(converterConfig)) { - if (isAdaptiveLightingConfig(converterConfig.adaptive_lighting)) { - adaptiveLightingConfig = converterConfig.adaptive_lighting; - if (adaptiveLightingConfig.enabled === undefined) { - adaptiveLightingConfig.enabled = true; - } - } else if (converterConfig.adaptive_lighting === false) { - adaptiveLightingConfig.enabled = false; - } -} +const adaptiveLightingConfig: AdaptiveLightingConfig = { + ...LightCreator.DEFAULT_CONFIG, + ...(isLightConfig(converterConfig) && { + ...(isAdaptiveLightingConfig(converterConfig.adaptive_lighting) + ? { ...converterConfig.adaptive_lighting, enabled: converterConfig.adaptive_lighting.enabled ?? true } + : { enabled: converterConfig.adaptive_lighting !== false }) + }) +};
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
CHANGELOG.md(1 hunks)config.schema.json(1 hunks)docs/light.md(2 hunks)src/converters/light.ts(9 hunks)test/light.spec.ts(5 hunks)test/testHelpers.ts(8 hunks)
✅ Files skipped from review due to trivial changes (1)
- test/testHelpers.ts
🧰 Additional context used
🪛 LanguageTool
docs/light.md
[uncategorized] ~15-~15: Loose punctuation mark.
Context: ...uration (light) - adaptive_lighting: Set to false to disable [Adaptive Lig...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~16-~16: A comma may be missing after the conjunctive/linking adverb ‘Additionally’.
Context: ...a Color Temperature characteristic. Additionally you can also configure the following op...
(SENT_START_CONJUNCTIVE_LINKING_ADVERB_COMMA)
🪛 Markdownlint (0.37.0)
CHANGELOG.md
12-12: Expected: 2; Actual: 3
Unordered list indentation
(MD007, ul-indent)
13-13: Expected: 2; Actual: 3
Unordered list indentation
(MD007, ul-indent)
🔇 Additional comments (13)
docs/light.md (3)
15-15: LGTM: Clear documentation for disabling adaptive lighting
The documentation clearly explains how to disable the Adaptive Lighting feature.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~15-~15: Loose punctuation mark.
Context: ...uration (light) - adaptive_lighting: Set to false to disable [Adaptive Lig...
(UNLIKELY_OPENING_PUNCTUATION)
17-19: LGTM: Well-documented configuration options
The new configuration options are clearly documented with their default values and purposes:
enabled: Controls adaptive lighting (defaults to true)only_when_on: Controls updates timing (defaults to true)transition: Controls transition time (optional)
🧰 Tools
🪛 LanguageTool
[uncategorized] ~19-~19: Loose punctuation mark.
Context: ...on. Defaults to true. - transition: Transition time to send along with the ...
(UNLIKELY_OPENING_PUNCTUATION)
[grammar] ~19-~19: Consider using either the past participle “sent” or the present participle “sending” here.
Context: ...f not defined, transition will not be send. ```json { "converters": { "ligh...
(BEEN_PART_AGREEMENT)
26-26: LGTM: Clear configuration example
The JSON example clearly demonstrates how to configure the new options.
config.schema.json (1)
164-186: LGTM: Well-structured schema for adaptive lighting configuration
The schema properly defines the new configuration structure with appropriate titles, types, and defaults:
enabled: boolean, defaults to trueonly_when_on: boolean, defaults to truetransition: integer, defaults to 0
src/converters/light.ts (6)
33-33: LGTM: Clean interface extension
The AdaptiveLightingConfig interface is properly extended with the new optional enabled property.
44-45: LGTM: Updated type guard
The type guard for AdaptiveLightingConfig correctly handles the new enabled property while maintaining type safety.
57-60: LGTM: Well-defined defaults
The default configuration properly sets the initial values:
enabled: true: Enables adaptive lighting by defaultonly_when_on: true: Prevents unnecessary updates when light is offtransition: undefined: Allows flexible transition timing
386-393: LGTM: Proper state handling with adaptive lighting reset
The handleSetOn method correctly:
- Handles the state change
- Resets adaptive lighting temperature when turning on
416-417: LGTM: Proper brightness handling with adaptive lighting reset
The brightness handler correctly resets the adaptive lighting temperature when brightness changes.
Line range hint 544-560: LGTM: Robust adaptive lighting implementation
The adaptive lighting handler properly implements:
- Skip updates when light is off (if configured)
- Transition timing for smooth changes
- Proper temperature change threshold
- Detailed debug logging
CHANGELOG.md (1)
9-13: LGTM! Clear and well-structured changelog entry.
The changelog entry properly documents the key changes related to Adaptive Lighting, following the Keep a Changelog format.
🧰 Tools
🪛 Markdownlint (0.37.0)
12-12: Expected: 2; Actual: 3
Unordered list indentation
(MD007, ul-indent)
13-13: Expected: 2; Actual: 3
Unordered list indentation
(MD007, ul-indent)
test/light.spec.ts (2)
169-170: Consistent update to adaptive lighting configuration.
The configuration has been updated to use the new object-based structure for adaptive lighting settings, which aligns with the changes in the main codebase. The numberOfExpectedControllers is appropriately set based on the device capabilities.
Also applies to: 496-497
Line range hint 866-959: Well-structured test suite for Innr RB-249-T.
The new test suite thoroughly covers:
- Basic light functionality (on/off, brightness)
- Color temperature bounds handling
- Proper controller initialization
Good job on including edge cases for color temperature bounds testing.
Tweaks in preparation of new release:
Still to do:
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Tests
Chores