deconz-rest-plugin icon indicating copy to clipboard operation
deconz-rest-plugin copied to clipboard

Allow more flexible lists of lights in scenes

Open hanskroner opened this issue 3 years ago • 4 comments

The list of lights in a scene may now be a subset of the lights in its parent group. Calling 'Modify scene' with a light state for a light not in the scene, but part of the scene's parent group, will add the light and its state to the scene. Calling it with an empty light state for a light in the scene will remove that light from the scene.

hanskroner avatar Nov 14 '22 20:11 hanskroner

@manup, I could see from comments in the code that something like this had been considered in the past, but not implemented. I'm unsure if this is a welcome addition, but I had need for it recently and thought I would contribute it just in case. I'm not particularly happy with passing an empty object being used as the means to remove a light from a scene - any suggestions around this or comments in general would be very welcome.

hanskroner avatar Nov 14 '22 20:11 hanskroner

I've been testing these changes out for the last few days to try and check that I didn't unintentionally break something - I think I didn't. I understand that this might be a bit of a 'niche addition' since the project has managed without this functionality for a while, but I've already found quite a few scenarios where it has been very helpful to be able to set Scenes that affect only a subset of a Group's Lights.

If this functionality does get merged, the documentation for 'Modify Scenes' will need to be updated to reflect that Lights being modified must now be members of the Group (as opposed to the Scene) and that passing an empty JSON object as the body of the request will remove the Light from the Scene.

hanskroner avatar Nov 21 '22 11:11 hanskroner

Hi sorry for the late reply. Scenes are a very tricky part of the code currently, since the code paths related to are very scattered and full of quirks for various devices. It feels like touching this code always breaks something, usually not in the developer setup but on user setups.

The goal is to clean it up entirely, and not depend on manufacturer specific groups/scenes support in various devices. Once we get more functionality in the DDF space. The recent PR https://github.com/dresden-elektronik/deconz-rest-plugin/pull/6316 is another step in that direction to specify if a device supports "proper" scenes.

Later on this will result in a more robust scene handling as roughly described in this older post https://github.com/dresden-elektronik/deconz-rest-plugin-v2/issues/5

manup avatar Jan 07 '23 14:01 manup

@manup, No worries at all. I of course understand if this is something that you feel is better off left untouched for now. However, I don't think I expressed properly what this PR does - it isn't dependent on manufacturer specific handling of groups or scenes, nor is it (at least I believe...) something that could ever be handled in DDFs.

What the PR allows is to manipulate scene members through the API in a slightly more flexible way that can currently be done. Right now, the API requires the members of a Scene to be exactly the members of its Group. This PR makes it so the Scene's members may be a subset of the members of the Group - something that is entirely doable today, manually making the scene assignments via the UI (though its quite cumbersome).

For instance, I have a Group for a bunch of spots that light up a bathroom with various scenes for preset brightness levels. An additional scene sets only the two spots inside the shower niche to full brightness - without affecting the rest of the spots. Without this PR, I've had to handle this by either manipulating the scene members manually through the UI or creating an additional Group with only those two spots and creating the Scene there- the latter isn't ideal as a general solution as some of "less pricey" bulbs have tighter limits in the number of groups they can hold.

Regardless of your decision, I appreciate the time in providing the above explanations!

hanskroner avatar Jan 07 '23 19:01 hanskroner