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

Added new actions #472

Merged
merged 5 commits into from
Apr 30, 2022
Merged

Added new actions #472

merged 5 commits into from
Apr 30, 2022

Conversation

sabaatworld
Copy link
Contributor

This pull request is for adding the following actions: on_min_max_brightness, on_max_min_brightness, on_min_max_color_temp, on_max_min_color_temp.

These are something that I needed and thought it would be best to implement in controllerx. This should not impact any existing setups. Please check out the docs to see the intended purpose. I was able to test out the actions with E1810 IKEA controller (Z2M) but could not test out generating the docs. There shouldn't be any errors though.

@sabaatworld
Copy link
Contributor Author

don't merge this yet. performing some tests.

@sabaatworld
Copy link
Contributor Author

Tested! Everything seems to work as expected.

@xaviml
Copy link
Owner

xaviml commented Apr 29, 2022

Hi @sabaatworld,

Thank you for the PR. It looks good to me, but I would like to add some integration tests for this feature. I will add them in your branch and merge this away! :)

Regards,
Xavi M.

xaviml added a commit to sabaatworld/controllerx that referenced this pull request Apr 30, 2022
xaviml added a commit that referenced this pull request Apr 30, 2022
@xaviml
Copy link
Owner

xaviml commented Apr 30, 2022

Hi @sabaatworld,

I added some integration tests and refactor the code a bit to be more clear. I have also considered the light state for an edge case it was missed.

Regards,
Xavi M.

@xaviml xaviml merged commit cd8d5b4 into xaviml:dev Apr 30, 2022
@sabaatworld
Copy link
Contributor Author

I like your version more. It's much cleaner! Thanks :)

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