This topic is about the ability to program a gradual lighting of a Hue bulb, as presented in this issue
I haven’t had much time to spend on this topic, but I have a first draft that is functional.
I think it needs discussion before proceeding and finishing everything necessary for the PR to be valid.
Topics for discussion:
There is not yet in the project the notion of « scenario » associated with a Scene. So far, every scene is composed of a sequence of basic actions such as turning on a light or operating a switch. Fading in requires chaining several basic actions, as the Hue API does not offer a direct action to perform a gradual lighting. Here, we need to chain several simple actions: turn on the light and then change the brightness at regular intervals. This « scenario » requires a more advanced set of parameters than for basic actions: we need both the duration of the lighting and the target brightness. It is not possible to integrate these parameters into the current model and I had to cheat by modifying the validation of the format of the actions of a scene to have something that works. So, I think we need to think about something more suitable.
On the front end, I had to cheat again for the Component LightFadeInParams to handle this « action » with multiple parameters. Also to be discussed to make something cleaner.
I played with setTimeout to perform the gradual lighting but I don’t like this solution much: we risk having a lighting that ends late. It would probably be better to use a more suitable library. If there is no suitable package already used in Gladys, I will look for what could do the job. Moreover, when the lighting time is short, the Hue API seems to change the state of the light a bit late compared to the return of the call. This causes « jerks » in the process of gradual lighting. I will look to possibly limit the number of calls to the API.
There are many other things to review on the PR but I prefer to discuss these topics before going further.
That’s not « cheating », that’s how we develop every time! If you need multiple attributes in an action, it’s normal to add these attributes to the validation.
Could you show the code directly to show what « cheating » looks like?
Haha indeed, I only focused on the API on the Philips website and wasn’t curious enough to check what the node module offered directly. My bad, I’ll look into using it.
To clarify my point on where I modified the model validation: link
Here’s where I cheated in the front end, in the implementation of the fade-in action creation form: link
This is related to the validation of a scene model, as I added a « parameters » attribute to manage all my values (see the previous link), I update the entire « parameters » attribute each time rather than just updating a sub-attribute of « parameters ». It’s not very clean, but in its current state I don’t see any other way to do it.
Oh indeed that’s not great, why didn’t you make 2 variables? No need for a parent variable « parameters ».
We do it in several scene actions though.
Example in your case:
handleChangeTargetBrightness = e => {
this.props.updateActionProperty(this.props.columnIndex, this.props.index, 'target_brightness', e.target.value);
};
By the way, I notice that you’re doing a setState in addition to your updateActionProperty, that’s not great because it forces you to synchronize the two states. You could directly use the prop props.action which is injected by the parent!
If I use three variables, I need to modify the actionSchema of the scene model and add three attributes target_brightness, duration_value, and duration_unit. These attributes will only be used for this type of action. Hence the idea of using a single parameters attribute of type object to make the schema generic and reusable, albeit very permissive.
That’s exactly what we want to avoid! In Gladys 4, we do everything as strictly as possible. If the schema is too flexible, we’ll end up with all sorts of things in the DB, users will have mysterious problems, and it will be very hard to debug and fix.
Where I agree with you is that this schema that validates all actions in the same way is strange and not strict enough.
That’s why I wanted to discuss it, I don’t like the idea of having options that will be null most of the time and I also think that parameters is too permissive.
Hello,
After a 4-month hibernation, I’ve spent some more time on the PR.
I think it’s ready for review, I built an image for my Rasp 4 and it runs well.
Thanks for your PR and all the work you’ve put into it, that’s cool
I’ve written a review on your PR.
Your PR makes quite major changes to the API in several places that are not related to Philips Hue, and I think it would be worth discussing this beforehand, and if these changes are really necessary (I’m not sure), I think it would be better to have them in a separate PR.
We generally try to keep PRs rather small, and when there are major API changes (like in your PR) with migrations, this is something we usually discuss beforehand.
I think it’s very important to have a stable and non-breaking API over time, as users depend on it, and potentially code scripts, external integrations that depend on it, so this kind of change should not be taken lightly.
I mentioned earlier in this conversation why I needed to make these changes.
I need three parameters to configure my action: the target intensity, the time unit, and the value for the action duration.
I didn’t think the action object was suitable:
The only possible values were text, value, and unit, providing no information about the actual content of the variable.
These values are at the same level as type and devices; there’s no distinction between the parameters that define the nature of an action and those that allow an action of a certain type to run.
However, I hadn’t considered that the API can be consumed by other consumers besides the Gladys front end. I think I should indeed revert the changes to the API interface, but I propose keeping the data model migration for the reasons stated above.
Introducing the notion of a scenario allows for defining more complex actions than simply activating a feature. I didn’t call it fadeIn because the scenario notion could be used for any type of device, whereas fadeIn is specific to bulbs. After that, I called it scenario, but I don’t mind using different wording.
One of the difficulties I’ve encountered in understanding the concept of scenes and actions is the lack of hierarchy in action objects. I find that a flat object makes it complicated to understand the role of each property.
My understanding of setValue is the transition of a feature from one state to another for a given device. The call triggers the state change and returns directly. This allows responding to an API call in a reasonable time.
Here, the action is part of a scene that can be composed of several groups of actions that must be executed sequentially.
Imagine a scene where I want my light bulb to start turning on at 7:20 for 10 minutes and then have my radio turn on. With a setValue that returns directly, my radio would turn on at 7:20. Therefore, I need to wait for the state of my light bulb to be the desired final state to end my action and trigger the next one.
Additionally, in my « scenario » I call two different features of the light object: binary and brightness. The setValue should remain on a single feature.
I think that if someone ever codes a small tool to make scenes in YAML and import them into Gladys in the future, it will be more readable.
After all, it’s a matter of taste and preference, I think it’s also good to keep something light, without too many objects to instantiate, and not too nested ^^
In the API yes, but in a scene you can make a setValue that waits for the device to respond, everything is possible
I thought you were using the Philips Hue API which does that in one command?
Don’t take my remarks the wrong way, I think it’s great to be challenged on the choices made in different places
But I also defend the chosen data format, it wasn’t chosen lightly and I think it’s important to remain backward compatible as much as possible (unless it’s really blocking and we want to move forward).
I think it’s a good signal to send to the community that the API is « stable » and doesn’t change in minor versions of Gladys every other day
I need to turn on the lamp and reset the intensity to the minimum at the start of the action before using the gradual intensity change command. I therefore change several states on different features of the same device.
