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

HS220 dimmer switch feature - set_dimmer_transition duration #143

Closed
barrar opened this issue Nov 28, 2018 · 5 comments
Closed

HS220 dimmer switch feature - set_dimmer_transition duration #143

barrar opened this issue Nov 28, 2018 · 5 comments

Comments

@barrar
Copy link

barrar commented Nov 28, 2018

The duration of the brightness transition can be modified using the following code:
self._query_helper("smartlife.iot.dimmer", "set_dimmer_transition",{"brightness": value, "duration":1})
A value of 1 is the minimum. Setting duration to 1000 results in a 1 second transition. Should I create a pull request implementing a new flag called dimmer_transition that accepts both a brightness and a duration?

This could also be implemented as a separate flag, if this duration flag is present then the set_dimmer_transition key can be used instead of set_brightness and the two values would be passed to the dimmer switch.

@rytilahti
Copy link
Collaborator

Is this behaviour different (Adjusting global transition? Only for brightness?) from #98?

@barrar
Copy link
Author

barrar commented Nov 28, 2018

This is for the smartplug class (dimmer switch), not smartbulb. Also, this is a independent function and the duration only applies to the change in brightness. It's not yet implemented anywhere in this codebase. It allows you to change the brightness setting from 0% to 100% and set a time period for this change, instant or gradual. I found it very useful and I can make a pull request soon.

@rytilahti
Copy link
Collaborator

I think the best way to add this would be similar to described in that PR. So instead of overloading brightness setter, a new set_brightness(self, value, *, duration=None) changing the brightness should be added. Its behaviour shall depend on whether the duration is given or not. The current setter should be marked as deprecated, and changed to call the new method. Do you think this sounds reasonable?

@barrar
Copy link
Author

barrar commented Nov 30, 2018

I'm not sure exactly how you want this implemented, here is my attempt
#145

@rytilahti
Copy link
Collaborator

rytilahti commented Nov 30, 2018

So here's how I imagine it should work, the existing code in the brightness setter should be moved into set_brightness as below:

def set_brightness(self, value, *, duration=None):
    # here the regular checks (is_dimmable, value in range, ..)
    # if duration is not None, and the bulb supports transitions, call `set_dimmer_transition`
    # otherwise execute the regular brightness setting

and the existing def brightness(self, value) setter should call the above method simply without duration:

@brightness.setter
def brightness(self, value: int):
    # the comment
    return self.set_brightness(value)

This way the only API change is adding a new method, which will allow specifying the transition. The existing brightness setter keeps functioning as it used to for those who want to use it. I think this keeps the code cleaner and avoids breaking the API (for the time being).

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

No branches or pull requests

2 participants