Skip to content
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

Add color_mode support to MQTT JSON light #47993

Merged
merged 4 commits into from
Mar 31, 2021

Conversation

emontnemery
Copy link
Contributor

@emontnemery emontnemery commented Mar 16, 2021

Breaking change

MQTT JSON light now supports color_mode which should be used together with supported_color_modes to signal the light's features.
Feature flags color_temp, hs, rgb, white_value, xy are all deprecated and support will be removed in 2021.10.

Proposed change

Add color_mode support to MQTT JSON light.

color_mode will not be sent to the light on the command topic, the light can unambiguously deduce the desired color made based on which values are present in the color dict or if color_temp is present.

The light must report a supported color_mode in it's state update if the state update includes a color dict or if color_temp is present.

Type of change

  • Dependency upgrade
  • Bugfix (non-breaking change which fixes an issue)
  • New integration (thank you!)
  • New feature (which adds functionality to an existing integration)
  • Breaking change (fix/feature causing existing functionality to break)
  • Code quality improvements to existing code or addition of tests

Example entry for configuration.yaml:

# Example configuration.yaml
light:
  - platform: "mqtt"
    schema: "json"
    "brightness": True,
    "color_mode": True,
    "command_topic": "test_light/set",
    "name": "test",
    "platform": "mqtt",
    "supported_color_modes": ["color_temp", "hs"]

Additional information

Checklist

  • The code change is tested and works locally.
  • Local tests pass. Your PR cannot be merged unless tests pass
  • There is no commented out code in this PR.
  • I have followed the development checklist
  • The code has been formatted using Black (black --fast homeassistant tests)
  • Tests have been added to verify that the new code works.

If user exposed functionality or configuration variables are added/changed:

If the code communicates with devices, web services, or third-party tools:

  • The manifest file has all fields filled out correctly.
    Updated and included derived files by running: python3 -m script.hassfest.
  • New or updated dependencies have been added to requirements_all.txt.
    Updated by running python3 -m script.gen_requirements_all.
  • Untested files have been added to .coveragerc.

The integration reached or maintains the following Integration Quality Scale:

  • No score or internal
  • 🥈 Silver
  • 🥇 Gold
  • 🏆 Platinum

To help with the load of incoming pull requests:

@emontnemery
Copy link
Contributor Author

emontnemery commented Mar 16, 2021

@Koenkk Can you please help to review this change?

@Koenkk Koenkk mentioned this pull request Mar 16, 2021
21 tasks
@Koenkk
Copy link
Contributor

Koenkk commented Mar 16, 2021

Looks good, no comments from my side.

@emontnemery
Copy link
Contributor Author

emontnemery commented Mar 17, 2021

@Koenkk Are you OK with the non-symmetric way color_mode will be used in commands and state updates?

For example, the light will get this command:

{"state": "ON", "color": {"h": 359.0, "s": 78.0}, "brightness": 75}

To which the expected state update from the light is:

{"state": "ON", "color_mode": "hs", "color": {"h": 359.0, "s": 78.0}, "brightness": 75}

This means the light may include some additional data, such as color_temp when it's in hs state, which will be ignored by Home Assistant.

@Koenkk
Copy link
Contributor

Koenkk commented Mar 17, 2021

@emontnemery yes!

Copy link
Member

@frenck frenck left a comment

Choose a reason for hiding this comment

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

This looks really great!

Only docs are missing

@frenck frenck merged commit c7584a1 into home-assistant:dev Mar 31, 2021
@github-actions github-actions bot locked and limited conversation to collaborators Apr 1, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants