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 nightmode #32

Open
wants to merge 64 commits into
base: develop
Choose a base branch
from
Open

Add nightmode #32

wants to merge 64 commits into from

Conversation

Xento
Copy link
Contributor

@Xento Xento commented Oct 10, 2015

I added a new mode to the base class "NIGHT".
Because Milight LEDs have a nightmode which means they light in white but with very little brightness.
Maybe it would be good to have the ability to add specialmodes to devices because they have a fourth mode "DISCO" with different effects.

Philip Patzer and others added 6 commits October 5, 2015 21:58
…unce_logic

use async queue to only execute one command at the time
Implemented night mode for milight led.
- Save the mode when set to white mode
- Only set color and brightnes if turned on
@Xento Xento changed the title Development Add nightmode Oct 10, 2015
@Xento
Copy link
Contributor Author

Xento commented Oct 10, 2015

I will change the ci tests when you reviewed the code an you would merge it.

@philip1986
Copy link
Owner

I see two options:

  1. we only support the "Night" mode Milight, but then I would argue you should not touch the base class with this PR and only attach the ActionProvider in case of Milight.
  2. We also start to support the Night mode for other devices (if its not supported by the device itself, we can just emulate it). In this case we could also add a night mode ability to the frontend.

I personally would vote for the 2. solution, but @mwittig had once the plan to split the plugin by devices, than the 1. approach would be better.

So bottom line we will do it one way or the other. If you adjust the tests it would be nice if you also add some for the ActionHandler.

@Xento
Copy link
Contributor Author

Xento commented Oct 18, 2015

Yeah I thought about the second solution.
I could add the emulation to the base class and every device which has an own night mode could overwrite it.

@philip1986
Copy link
Owner

Ok, do it like this 👍

@Xento
Copy link
Contributor Author

Xento commented Feb 14, 2016

I removed the nightmode from the base class.
Than I made the _updateState function more generic, so that it works even with an for the baseclass unknown mode.
My intention is that the device can add custom modes such as nightmode or disco without having to modify the baseclass.

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.

5 participants