Skip to content

Conversation

Rendman
Copy link

@Rendman Rendman commented Aug 7, 2017

Hello,

Great work on providing a node to control Milights! I added support for full color white temperature (warm white to cool white) and also added a way to activate the built in effect modes.

I also changed the node default name from 'MiLight' to 'milight' to better fit the default naming convention of other Node Red nodes. I can change this back and resubmit the pull request if you find this unacceptable.

Thanks again for a wonderful node.

…ns. Changed some very minor grammer issues in the HTML and README files. Added support for activating the built in effect modes. Added support for changing the white color temperature on full color bulbs.
@mwittig
Copy link

mwittig commented Aug 7, 2017

Note, the plugin already contains an effect mode function which works with all color bulb types. The named effect modes you have added are only applicable for the color bulb.

Renaming the node is not a great idea as it breaks backwards compatibility. All changes made so far had been done with backwards compatibility in mind

@Rendman
Copy link
Author

Rendman commented Aug 8, 2017

The color bulb types that I am aware of are the RGBW and RGBWW/CW types. Both have the ability to directly set an effect mode rather than cycling through each mode individually. Is there another bulb type that I am missing that does not allow for setting the mode effect directly?

Also, by allowing to change effect modes by name, we can handle changes to the mode number behind the scenes which allows for consistent mode changing between different bulb types.

For example I have both the RGBW and RGBWW/CW bulbs. If I have one node that I want to send a message from to change both bulbs to redflash. I can send just the one message and have them both do the same thing as opposed to having to modenext to each effect. (Which can be in different positions.)

Please let me know what you think. After I hear back from you with any other feedback, Ill change the name of the node back, make any other discussed changes and resubmit the pull request.

@mwittig
Copy link

mwittig commented Aug 8, 2017

Both have the ability to directly set an effect mode rather than cycling through each mode individually.

Well that's true for the v6 protocol supported with the new "iBox" bridges. However, the older protocol version v3 and v4 ("legacy" bridges), only support "mode next" for RGBW. Note, besides RGBW and RGBWW/CW types there is also single-channel RGB controller and bulbs. However, RGB is currently not supported as part of the plugin.

As far as v6 is concerned I think the disco modes provided RGBW and RGBWW/CW types differ eventhough you tried to align the different them. Btw, where did you find the named modes? Is this described somewhere?

@Rendman
Copy link
Author

Rendman commented Aug 8, 2017

Okay, I was not aware of the single channel RGB controller. Is that something that this plugin will support in the future?

By disco mode are your referring to the effect modes in the general or a specific effect mode? If you are referring to the modes in general, as I mentioned before, I have both and tested them side by side. Though the RGBW color cycle mode is different for the RGBWW/CW then then RGBW (additional white color cycle), overall they line up well enough that I believe the feature is useful.

@Rendman
Copy link
Author

Rendman commented Aug 8, 2017

Sorry, also I did not find the effect names anywhere. I sort of made them up.

@rspaargaren
Copy link

Hi is there any progress on this pull request?

@mwittig
Copy link

mwittig commented Oct 5, 2017

As the author is not replying to e-mails and pending issues since months, I decided to spawn my fork as a new package: https://www.npmjs.com/package/node-red-contrib-milight-2

I am also planning to take care of the pending pull requests soon.

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.

3 participants