search

philip1986 / pimatic-led-light

A template for creating plugins
http://pimatic.org/
GNU General Public License v2.0
7 stars 13 forks source link

Add nightmode #32

Open Xento opened 8 years ago

Xento commented 8 years ago

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.

Xento commented 8 years ago

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

philip1986 commented 8 years ago

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 commented 8 years ago

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 commented 8 years ago

Ok, do it like this :+1:

Xento commented 8 years ago

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.