Skip to content

Conversation

andrewfg
Copy link
Contributor

@andrewfg andrewfg commented Aug 29, 2025

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:

  • Implementation of a complete light state model with flexible capability configuration
  • Support for various light types from simple on/off to full RGB with color temperature
  • Command handling for different openHAB command types (HSB, OnOff, PercentType, etc.)
  • Handles inter- dependencies between on/off and brightness commands and status, and the 'B' part of HSB
  • Handles inter- dependencies between color temperature commands and status, and the 'HS' part of HSB

Signed-off-by: Andrew Fiddian-Green software@whitebear.ch

Signed-off-by: Andrew Fiddian-Green <software@whitebear.ch>
Signed-off-by: Andrew Fiddian-Green <software@whitebear.ch>
@andrewfg andrewfg requested a review from a team as a code owner August 29, 2025 17:45
@andrewfg andrewfg marked this pull request as draft August 29, 2025 17:45
@andrewfg
Copy link
Contributor Author

Ping @lsiepel ..

Signed-off-by: Andrew Fiddian-Green <software@whitebear.ch>
Signed-off-by: Andrew Fiddian-Green <software@whitebear.ch>
Signed-off-by: Andrew Fiddian-Green <software@whitebear.ch>
Signed-off-by: Andrew Fiddian-Green <software@whitebear.ch>
Signed-off-by: Andrew Fiddian-Green <software@whitebear.ch>
Signed-off-by: Andrew Fiddian-Green <software@whitebear.ch>
Signed-off-by: Andrew Fiddian-Green <software@whitebear.ch>
Signed-off-by: Andrew Fiddian-Green <software@whitebear.ch>
@wborn wborn requested a review from Copilot August 31, 2025 10:42
Copilot

This comment was marked as outdated.

Signed-off-by: Andrew Fiddian-Green <software@whitebear.ch>
@andrewfg andrewfg requested a review from Copilot August 31, 2025 14:57
Copilot

This comment was marked as outdated.

Signed-off-by: Andrew Fiddian-Green <software@whitebear.ch>
@andrewfg andrewfg changed the title Additional utility methods for lighting New class for modelling the state of lights in Thing handlers Aug 31, 2025
@andrewfg andrewfg requested a review from Copilot August 31, 2025 15:20
Copilot

This comment was marked as outdated.

Signed-off-by: Andrew Fiddian-Green <software@whitebear.ch>
@andrewfg andrewfg changed the title New class for modelling the state of lights in Thing handlers New class to model the state of lights in Thing handlers Aug 31, 2025
@andrewfg andrewfg changed the title New class to model the state of lights in Thing handlers Utility class to model the state of lights in Thing handlers Aug 31, 2025
@jimtng
Copy link
Contributor

jimtng commented Sep 1, 2025

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 <software@whitebear.ch>
Signed-off-by: Andrew Fiddian-Green <software@whitebear.ch>
@andrewfg andrewfg requested a review from Copilot September 8, 2025 12:22
Copilot

This comment was marked as outdated.

andrewfg and others added 3 commits September 8, 2025 13:24
…ightModel.java

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Signed-off-by: Andrew Fiddian-Green <software@whitebear.ch>
…ightModel.java

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Signed-off-by: Andrew Fiddian-Green <software@whitebear.ch>
Signed-off-by: Andrew Fiddian-Green <software@whitebear.ch>
@andrewfg andrewfg requested a review from Copilot September 8, 2025 12:50
Copy link

@Copilot Copilot AI left a 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 state machine class LightModel for managing light properties in Thing handlers, providing support for various light types from simple on/off to full RGB with color temperature control. It includes mathematical utilities for RGB/RGBCW conversions and handles complex inter-dependencies between brightness, color, and color temperature states.

  • Implementation of a flexible light state model with configurable capabilities
  • Command handling for different openHAB command types (HSB, OnOff, PercentType, etc.)
  • Mathematical conversion utilities between RGB, RGBW, and RGBCW color formats

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.

File Description
LightModel.java Core state machine implementation with configuration methods, runtime state management, and internal RGBCW math utilities
LightModelTest.java Comprehensive test suite covering all light types, command handling, color conversions, and edge cases
Comments suppressed due to low confidence (1)

bundles/org.openhab.core/src/main/java/org/openhab/core/util/LightModel.java:1

  • The magic number 25500 appears twice without explanation. This should be extracted to a named constant with a descriptive comment explaining that it's 255 * 100 for scaling normalized values to [0..255] range.
/*

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

andrewfg and others added 2 commits September 8, 2025 13:54
…ightModel.java

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Signed-off-by: Andrew Fiddian-Green <software@whitebear.ch>
Signed-off-by: Andrew Fiddian-Green <software@whitebear.ch>
@andrewfg andrewfg requested a review from Copilot September 8, 2025 13:14
Signed-off-by: Andrew Fiddian-Green <software@whitebear.ch>
Copilot

This comment was marked as outdated.

@andrewfg andrewfg requested a review from Copilot September 8, 2025 16:11
Copilot

This comment was marked as outdated.

Signed-off-by: Andrew Fiddian-Green <software@whitebear.ch>
Signed-off-by: Andrew Fiddian-Green <software@whitebear.ch>
@andrewfg andrewfg requested a review from Copilot September 8, 2025 22:10
Copy link

@Copilot Copilot AI left a 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 LightModel state machine with flexible capability configuration supporting various light types
  • Support for complex RGB data types including RGBCW with mathematical conversion utilities
  • Addition of abstract BaseLightThingHandler base 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.

andrewfg and others added 2 commits September 8, 2025 23:17
…ightModel.java

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Signed-off-by: Andrew Fiddian-Green <software@whitebear.ch>
Signed-off-by: Andrew Fiddian-Green <software@whitebear.ch>
* Composes an RGBCW from the given RGB. The result depends on the main input RGB values and the RGB sub-
* component contributions of the cold and warm white LEDs. It solves to find the maximum usable C and W scalar
* values such that none of the RGB' channels become negative. It solves for C and W such that:
* <p>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do you have a reference for where this algorithm came from? many of the methods in ColorUtil contain references for their algorithms. while it looks reasonable to me, it's always nice if we can link to something that gives a more in depth reasoning of how it works.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not really. I spent about one day faffing around with variations of code based on suggestions by Copilot. Finally none of its suggestions seemed right to me, so my final approach was to ask Copilot to define the test parameters (either in words or in code) for the JUnit assertions that would confirm that the algorithm achieves the desired result. And then I wrote the conversion algorithm to pass those tests.

Signed-off-by: Andrew Fiddian-Green <software@whitebear.ch>
Signed-off-by: Andrew Fiddian-Green <software@whitebear.ch>
@andrewfg
Copy link
Contributor Author

@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.

@andrewfg
Copy link
Contributor Author

@ccutrer based on learnings while writing openhab/openhab-addons#19340 there are a couple more changes I need to make..

@openhab-bot
Copy link
Collaborator

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Provide some more core utility classes for lighting
5 participants