Closed philip1986 closed 9 years ago
Hi, I have reviewed your refactoring work which is great! I also did some refactoring to simplify the Milight device implementation and to make sure Milight can recover its state from DB as the Milight Controller cannot be queried for status information. I also did to some changes to the BaseLedLight device - see getState(). Hopefully this does not break the IwyMaster. See PR #9. Note I have also improved the base package node-milight-promise
To get attributes properly stored to DB the value must comply with the data-type, e.g. the internal mode value 'COLOR' (this is is a string even though it has been declared as label) must be transformed to true (and propagated via event trigger). This works fine now for mode, color, and brightness - but I can't get it to work properly for power as the power event trigger is also used to propagate event changes to the frontend. I think it would be better to use the SwitchActuator device as a basis. This way you "can delete your action handler for this. Because it is handled by the normal action provider from pimatic framework." as @Icesory suggested on http://forum.pimatic.org/topic/332/plugin-for-wifi-controlled-led-light-iwy-light/5
I have also noticed a bug with the color picker: If the color picker is open and you toggle the switch the switch event is not fired or gets lost somehow.
I have filed an issue for the color picker bug - https://github.com/philip1986/pimatic-led-light/issues/10
This PR is based those previous PRs:
https://github.com/philip1986/pimatic-led-light/pull/6 from @mwittig https://github.com/philip1986/pimatic-led-light/pull/4 from @Fjuxx
It aims to restructure the plugin in order to support implementaions of multiple LED light devices in an easy way. And it also adds Milight as a new device.