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

Development #23

Closed
wants to merge 5 commits into from
Closed

Development #23

wants to merge 5 commits into from

Conversation

Xento
Copy link
Contributor

@Xento Xento commented Aug 3, 2015

No description provided.

Xento added 3 commits June 27, 2015 13:25
- Fixed bug when receiving Off Button when an other group was selected
before.
- Don't set color and brightness when turning on via zone 0
- reflect all changes made to zone 0 to all other zones
@Xento Xento closed this Aug 3, 2015
@Xento Xento reopened this Aug 3, 2015
@Xento
Copy link
Contributor Author

Xento commented Aug 3, 2015

I hope you can merge it. I tried to merge your changes into my fork, but I couldn't handle it.
I'm to unexperienced with the git commandline.

Update to latest master version
@Xento
Copy link
Contributor Author

Xento commented Aug 3, 2015

I think now I got everything correct ;-)


MilightRF24::__proto__ = events.EventEmitter.prototype
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why you do it that way?

Wouldn't class MilightRF24 extends events.EventEmitter have the same effect?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe
This was how I found it in an example.

I will change it and try it out.

2015-08-04 14:43 GMT+02:00 philip1986 notifications@github.com:

In devices/milightRF24.coffee
#23 (comment)
:

  • MilightRF24::proto = events.EventEmitter.prototype

Why you do it that way?

Wouldn't class MilightRF24 extends events.EventEmitter have the same
effect?


Reply to this email directly or view it on GitHub
https://github.com/philip1986/pimatic-led-light/pull/23/files#r36182778.

@philip1986
Copy link
Owner

Could you please also add some commands to the code, specially how the classes MilightRF24 and MilightRF24Zone are interconnected.

@Xento
Copy link
Contributor Author

Xento commented Aug 4, 2015

I added some comments and improved the code

@Xento
Copy link
Contributor Author

Xento commented Aug 9, 2015

Are this enough comments?

@philip1986
Copy link
Owner

Yes, thanks a lot.
I will take some time to add some tests for it (I do it for all devices), because the complexity of the whole plugin makes it necessary.

@Xento
Copy link
Contributor Author

Xento commented Sep 30, 2015

When you will merge it?
Its running very smooth ;-)

@philip1986
Copy link
Owner

Sorry, its been a while. As I told you I like to have some tests for the plugin. So, I took your branch and add some basic ones. Would be nice if you extend then, e.g. when one device isn't in zone 0.
I also did a change about the configuration and initialization of milightRF24, because I didn't like the approach to set the port via the device config.

I am going to close this PR and open a new one with my modifications. Please have a look at it.

Cheers, Philip

@philip1986 philip1986 closed this Oct 4, 2015
@philip1986 philip1986 mentioned this pull request Oct 4, 2015
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.

2 participants