Add experimental SKIP_BRIGHTNESS_UPDATE option
#931
Conversation
|
|
🆗 Published SonarCloud and code coverage data. |
|
Sorry for the late response. Other than that I think the chance is fine. |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
WalkthroughThe pull request introduces an experimental feature in the Zigbee2MQTT platform, adding a new constant Changes
Poem
Tip CodeRabbit's docstrings feature is now available as part of our Early Access Program! Simply use the command Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 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/platformAccessory.ts (1)
297-306: Consider documenting the experimental feature.Since this introduces a new experimental feature that changes core behavior, consider:
- Adding a comment explaining when to use this feature
- Documenting potential side effects
- Adding this to the changelog
Would you like me to help draft the documentation for this experimental feature?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/experimental.ts(1 hunks)src/platformAccessory.ts(2 hunks)
✅ Files skipped from review due to trivial changes (1)
- src/experimental.ts
🔇 Additional comments (2)
src/platformAccessory.ts (2)
19-19: LGTM! Import follows existing patterns.
The import of the new experimental feature flag follows the established naming convention and is appropriately grouped with other experimental features.
303-306: Implementation looks good, but monitor for side effects.
The conditional filtering of the 'brightness' key is implemented cleanly. However, since this is an experimental feature affecting state management, it should be monitored for potential issues in production.
Let's verify if this change might affect other brightness-related operations:
✅ Verification successful
Based on the search results, I can now generate the final response as I have enough information to verify the impact of the brightness-related change:
The brightness filtering change is safe and well-isolated
The experimental feature that filters out brightness updates is specifically designed to prevent cached brightness level loss when a bulb's state is OFF. The codebase shows that:
- The brightness handling is properly encapsulated in the light converter (
src/converters/light.ts) - The change only affects the polling mechanism (
queueAllKeysForGet) and doesn't interfere with:- Direct brightness setting via HomeKit (
handleSetBrightness) - State monitoring and updates (
NumericCharacteristicMonitor) - Adaptive lighting functionality
- Direct brightness setting via HomeKit (
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for other brightness-related operations that might be affected
rg -A 5 'brightness' --type ts
Length of output: 26021
A rather simple workaround for #882, that should at least address the main issue with the current behavior, while keeping
queueAllKeysForGet()otherwise as is.Summary by CodeRabbit
New Features
Bug Fixes