-
-
Notifications
You must be signed in to change notification settings - Fork 459
State machine to model lights in Thing handlers #4995
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?
Conversation
Signed-off-by: Andrew Fiddian-Green <[email protected]>
Signed-off-by: Andrew Fiddian-Green <[email protected]>
|
Ping @lsiepel .. |
Signed-off-by: Andrew Fiddian-Green <[email protected]>
Signed-off-by: Andrew Fiddian-Green <[email protected]>
Signed-off-by: Andrew Fiddian-Green <[email protected]>
Signed-off-by: Andrew Fiddian-Green <[email protected]>
Signed-off-by: Andrew Fiddian-Green <[email protected]>
Signed-off-by: Andrew Fiddian-Green <[email protected]>
Signed-off-by: Andrew Fiddian-Green <[email protected]>
Signed-off-by: Andrew Fiddian-Green <[email protected]>
Signed-off-by: Andrew Fiddian-Green <[email protected]>
Signed-off-by: Andrew Fiddian-Green <[email protected]>
Signed-off-by: Andrew Fiddian-Green <[email protected]>
|
I don't quite understand the background for this. Why not just add whatever is missing into ColorUtils, or into HSBType, or ColorItem? |
- add support for RGBW - add support for RGB(W) linked to HSB 'B' part - various refactoring - documentation - extended test cases Signed-off-by: Andrew Fiddian-Green <[email protected]>
Signed-off-by: Andrew Fiddian-Green <[email protected]>
Signed-off-by: Andrew Fiddian-Green <[email protected]>
Signed-off-by: Andrew Fiddian-Green <[email protected]>
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.
Pull Request Overview
This PR introduces a comprehensive light state machine model to simplify light control in Thing handlers by providing a unified approach for managing different light capabilities and handling complex inter-dependencies between brightness, color, and color temperature commands.
- Implementation of a complete
LightModelstate machine with flexible capability configuration supporting various light types - Support for complex RGB data types including RGBCW with mathematical conversion utilities
- Addition of abstract
BaseLightThingHandlerbase class to standardize light Thing handler implementations
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| LightModelTest.java | Comprehensive unit tests covering all light capabilities, RGB conversions, and RGBCW mathematical operations |
| LightModel.java | Core state machine implementation with configuration, runtime state management, and color conversion utilities |
| BaseLightThingHandler.java | Abstract base class providing standardized structure for light Thing handlers using the LightModel |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
bundles/org.openhab.core/src/main/java/org/openhab/core/util/LightModel.java
Outdated
Show resolved
Hide resolved
...g.openhab.core.thing/src/main/java/org/openhab/core/thing/binding/BaseLightThingHandler.java
Outdated
Show resolved
Hide resolved
…ightModel.java Co-authored-by: Copilot <[email protected]> Signed-off-by: Andrew Fiddian-Green <[email protected]>
Signed-off-by: Andrew Fiddian-Green <[email protected]>
bundles/org.openhab.core/src/main/java/org/openhab/core/util/LightModel.java
Outdated
Show resolved
Hide resolved
bundles/org.openhab.core/src/main/java/org/openhab/core/util/LightModel.java
Outdated
Show resolved
Hide resolved
bundles/org.openhab.core/src/main/java/org/openhab/core/util/LightModel.java
Outdated
Show resolved
Hide resolved
bundles/org.openhab.core/src/main/java/org/openhab/core/util/LightModel.java
Outdated
Show resolved
Hide resolved
bundles/org.openhab.core/src/main/java/org/openhab/core/util/LightModel.java
Outdated
Show resolved
Hide resolved
bundles/org.openhab.core/src/main/java/org/openhab/core/util/LightModel.java
Outdated
Show resolved
Hide resolved
bundles/org.openhab.core/src/main/java/org/openhab/core/util/LightModel.java
Show resolved
Hide resolved
bundles/org.openhab.core/src/main/java/org/openhab/core/util/LightModel.java
Outdated
Show resolved
Hide resolved
bundles/org.openhab.core/src/main/java/org/openhab/core/util/LightModel.java
Outdated
Show resolved
Hide resolved
Signed-off-by: Andrew Fiddian-Green <[email protected]>
Signed-off-by: Andrew Fiddian-Green <[email protected]>
|
@ccutrer thanks for the LGTM.. .. however having seen the PR for Wiz where he has lights that flip from color mode to color temperature mode (and I think I saw this also in Govee) .. probably I should tweak the code in this PR to address that. |
This comment was marked as resolved.
This comment was marked as resolved.
|
This pull request has been mentioned on openHAB Community. There might be relevant details there: https://community.openhab.org/t/esphome-binding-for-the-native-api-4-0-0-6-0-0/146849/434 |
Signed-off-by: Andrew Fiddian-Green <[email protected]>
- see openhab/openhab-addons#19340 Signed-off-by: Andrew Fiddian-Green <[email protected]>
|
I am using a clone of this code this in my new HomeKit binding here. Note that once this PR will have been merged the clone in that binding can be deleted and instead linked to use this library. |
Signed-off-by: Andrew Fiddian-Green <[email protected]>
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.
Thanks, LGTM
Do we need to extend documentation for developers?
Yes that makes sense. I am not sure where though.. EDIT: probably we need a whole new chapter to describe all the OH core utility functions. (Maybe also the OH standard channel types..) EDIT2: or maybe in FrameWork Tools ? |
Resolves #4994
This PR adds a comprehensive state machine class for modelling the state of lights within Thing handlers by introducing a new LightModel state machine for managing light properties like brightness, color, and color temperature.
Key changes include:
Signed-off-by: Andrew Fiddian-Green [email protected]