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

Store current light state in scenes and recall group scenes #157

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

vanviegen
Copy link

When updating or creating a scene, it should store the current light state for all lights in the scene's group if lightstates is not set or storelightstate is true. This PR implement that behavior. It has been tested with Hue Essentials.

The patch includes a tiny second change: scene recalls were only applied for group "0". Just ignoring the group seems less wrong, and makes scene recall work properly with at least Hue Essentials.

@alexyao2015
Copy link
Member

@vanviegen Would you be able to fix the linting by running pre-commit run --all-files? Also what is __normalize_local_item? It appears that it no longer returns a string like the type hint would suggest. This furthermore appears to me that running this function does not do anything as the values are not returns or stored anywhere. It does not look like that is the intended behavior. Please review and confirm.

@vanviegen
Copy link
Author

__normalize_local_item normalizes the incoming data (for both async_create_localitem and async_update_localitem) into something that can be stored in the configuration.

itemtype == "groups"
and data["type"] in ["LightGroup", "Room", "Zone"]
and "class" not in data
async def __normalize_local_item(self, data: Any, itemtype: str) -> None:
Copy link
Member

Choose a reason for hiding this comment

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

While this may work, can we use this structure instead as it makes the code more readable?

def func(value) -> output:
  # Code
  return value
  
result = func(value)

@@ -319,17 +317,6 @@ async def async_group_action(self, request: web.Request, request_data: dict):
# Create success responses for all received keys
return send_success_response(request.path, request_data, username)

@routes.post("/api/{username}/groups")
@check_request()
async def async_create_group(self, request: web.Request, request_data: dict):
Copy link
Member

Choose a reason for hiding this comment

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

The change of the groups logic is very different from the other component of this pr (allowing scene recalls for group 0). Can you create a separate pr for the other change instead of combining them into one like this?

@github-actions
Copy link
Contributor

This pull request has been marked as stale due to no activity and will be closed in 7 days.

@github-actions github-actions bot added the stale label Aug 20, 2021
@github-actions github-actions bot closed this Aug 27, 2021
@alexyao2015 alexyao2015 added confirmed We know what should be done to resolve the issue and removed stale labels Aug 27, 2021
@alexyao2015 alexyao2015 reopened this Aug 27, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
confirmed We know what should be done to resolve the issue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants