@mwittig said:
@Fjux See also my pending PR. May be you try to merge it into you PR.
I do not know how I can merge into a PR. Where should I look?
@Fjux It is basically the same as merging an upstream repository. See https://help.github.com/articles/merging-an-upstream-repository-into-your-fork for example. Rather than using the original own git you can use https://github.com/mwittig/pimatic-led-light.git as this is identical with my PR.
"It always takes longer than you expect, even when you take into account Hofstadter's Law.", Hofstadter's Law
@Fjux said:
Did you already have a finished UI, or was it just a concept?
As the feature request has not received any votes yet I have not spent more work on this yet.
"It always takes longer than you expect, even when you take into account Hofstadter's Law.", Hofstadter's Law
@mwittig Oh ofcourse,
didn’t think about that thanks
@mwittig Good job. The code looks well and the IwyLight device is behaving like before. I merged your PR, but I will wait for a new release until @Fjux is done.
@Fjux I reopened your PR against master, now you should catch up with the master branch.
I also add one comment about setting white mode in case of color is set to white. For me white mode and white in color mode are different things. And I would prefer to don’t mix it up. But maybe I am wrong with this, any other opinions about this??
@philip1986 I have made some additions to the pimiatic-led-light plugin to support Blinkstick. It have been working great for a couple of days: https://github.com/tester22/pimatic-led-light
Please merge the pull request when possible.
@tester22 I merged you PR, good job!
I merged also the PR from @Xento to support MilightRF24 and published the changes also on npm. @sweetpi can you please update the homepage arcorrding to https://www.npmjs.com/package/pimatic-led-light version 0.1.0.
I think it is great idea to support different types of led light through a single control interface and I highly appreciate the contribution made by various developers. What I don’t like is to provide this as a single plugin as this is contributing to bloat of dependencies. The list of indirect dependencies is breath-taking. These dependencies will be loaded into memory no matter which led light used. If you have multiple plugins written this way this may cause a serious bottleneck and stability issues. For this reason I would like to suggest a split up of the plugin into three plugin plus a common base package. I think this is fairly easy to do and I am happy to undertake this effort.
"It always takes longer than you expect, even when you take into account Hofstadter's Law.", Hofstadter's Law
@mwittig I see your point. How would you provide the shared code to all plugins? I see there only to options, one is to include it to pimatic core and the other one is to have a abstract plugin repo for this purpose. Ok there is also the third options to just duplicate the shared code in each plugin, but this would be fairly unclean.
This all looks very good. I started wondering if its possible to also include Philips hue in this plugin?
@Cryonic90 Yes, should be relatively easy since they have an official API (http://www.developers.meethue.com/documentation/getting-started). I you like fell free to contribute
@philip1986 said:
How would you provide the shared code to all plugins?
This is fairly simple and straightforward. I would wrap-up the BaseLedLight, ColorActionHandler, and ColorActionProvider classes and the “ui” bit into a node package and require this in the plugin implementation. This way, you end up with one plugin per LED-technology with a common base implementation. I think this is better as I suppose most user just use one or two LED light technologies. Note, beyond a package required by multiple plugin will be only loaded once if it is the same version.
"It always takes longer than you expect, even when you take into account Hofstadter's Law.", Hofstadter's Law
I also saw the problems some guys had when we start to support new devices which had additional dependencies. So I am ok with splitting it.
If you like to undertake this effort, just go for it! I currently don’t have much time to spend for this project.
@philip1986 said:
…
If you like to undertake this effort, just go for it! I currently don’t have much time to spend for this project.
I am happy to do that but I’ll will not able to start working on this before mid August. I’ll keep you post about when I am ready to start.
"It always takes longer than you expect, even when you take into account Hofstadter's Law.", Hofstadter's Law
Well… sweetpi mentioned above he would like to include the gui part directly into pimatic.
What happened about that?
I can’t change colors with my wifi370 from pimatic, any one else got that problem?
Follow my domotica project on http://maredana.nl
@Oitzu said:
Well… sweetpi mentioned above he would like to include the gui part directly into pimatic.
What happened about that?
To get the ball rolling someone has to provide a PR on the pimatic-frontend repository. This should be done of the color picker is considered as being stable and everyone is happy with it as is. Once the PR is in place it is on @sweetpi to review it for acceptance.
"It always takes longer than you expect, even when you take into account Hofstadter's Law.", Hofstadter's Law
Hm… should be a simple task of copy & paste task to integrate the base-class?
Is somebody on this? If not i would maybe look into this at the weekend.
@mwittig said:
@Oitzu said:
Well… sweetpi mentioned above he would like to include the gui part directly into pimatic.
What happened about that?To get the ball rolling someone has to provide a PR on the pimatic-frontend repository. This should be done of the color picker is considered as being stable and everyone is happy with it as is. Once the PR is in place it is on @sweetpi to review it for acceptance.