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 device-restricted off to min brightness feature #9

Merged
merged 3 commits into from
Jan 13, 2020
Merged

Add device-restricted off to min brightness feature #9

merged 3 commits into from
Jan 13, 2020

Conversation

EPMatt
Copy link
Contributor

@EPMatt EPMatt commented Jan 11, 2020

In IKEA Zigbee implementation controllers have the ability to turn on the connected bulb (or light group) at minimum brightness if the action to increment brightness occurs (i.e. pressing the dedicated button for a E1810, turning the device to the right for a ICTC-G-1 etc.).

This PR implements the feature, which can be device restricted, since not all controllers which will be integrated in the future could be designed to have this capability.
The supports_from_off_to_min_brightness() method, which can be overridden by every controller, defines whether the device supports this feature or not. By default, devices will not implement this feature.

Also, I had to edit before_action() signatures, passing action arguments to the methods, something that could come in handy if more complex checks will have to be performed in the future.

I succesfully tested and enabled the feature on E1810 and E1743 controllers. I left the ICTC-G-1 unsupported since the integration is quite unstable and there are still issues with handling duplicate and unwanted messages.

Copy link
Owner

@xaviml xaviml left a comment

Choose a reason for hiding this comment

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

Hi @EPMatt,

Thanks for your collaboration, I think this is a cool feature to have. However, here are a couple of points to it. As well as we do it device-specific, we should do it user-specific as well. So we should add a new attribute to the LightController that reads from self.args and leave the default as false, since that is the current behaviour and we don't know how many people will like this feature as default or not.

apps/z2m_ikea_controller/z2m_ikea_controller.py Outdated Show resolved Hide resolved
apps/z2m_ikea_controller/z2m_ikea_controller.py Outdated Show resolved Hide resolved
@EPMatt
Copy link
Contributor Author

EPMatt commented Jan 12, 2020

Hi @xaviml,
I made some tests today with a E1810 and an IKEA bulb paired together and I confirm that, both with holding and clicking the brightness_up button when the light is off, the lamp is turned on at minimum brightness.
I updated the branch with the suggested changes, enabling the user to specify whether to enable this feature or not for the single app instance, and removed the defaults for E1810 and E1743 controllers.
If everything is fine and the PR is merged we should also update docs including the new feature.

@xaviml xaviml merged commit 4b12b74 into xaviml:master Jan 13, 2020
@xaviml
Copy link
Owner

xaviml commented Jan 13, 2020

@EPMatt Thanks you for your contribution! This will be included in the next release.

@EPMatt
Copy link
Contributor Author

EPMatt commented Jan 13, 2020

You're welcome @xaviml! 👍

@EPMatt EPMatt deleted the from-off-to-min-brightness-feature branch January 13, 2020 22:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants