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

Fixes Siri impatience and Colored Lights #29

Merged
merged 3 commits into from
Jan 23, 2016
Merged

Fixes Siri impatience and Colored Lights #29

merged 3 commits into from
Jan 23, 2016

Conversation

pdlove
Copy link
Contributor

@pdlove pdlove commented Jan 21, 2016

Modified to allow sProperty and sTarget to be an array for setting several things at the same time on a device.

Moved call back to allow the plugin to verify the command is possible and the light is connected and then report back to Siri. (#14, #21, #23, #26)
Fixed color change lights in the case of Phillips Hue. It is likely that this fixed other color light too, but I was only able to test with that one brand. (#18)

…veral things at the same time on a device.

Moved call back to allow the plugin to verify the command is possible and the light is connected and then report back to Siri. (#14, #21, #23, #26)
Fixed color change lights in the case of Phillips Hue. It is likely that this fixed other color light too, but I was only able to test with that one brand. (#18)
…d of Doors.

Added Shades time for Window Blinds (#25)
Added ability to enable Low Security mode for Garage Doors. This will only apply to Garage Doors and not to Door Locks. (#15)
@pdlove
Copy link
Contributor Author

pdlove commented Jan 21, 2016

I added a few other options. Most notably the Window Shades.
I also added the ability to set the Garage Door to "Low Security" so Siri can Open and Close the garage door while the phone is locked if you acknowledge that's what you want and set the flag to true in the config file.

@blaineam
Copy link

@KraigM & @pdlove I am ready to install this release as soon as it gets pushed to nam

@KraigM
Copy link
Owner

KraigM commented Jan 22, 2016

I've said it before and I will say it again, I strongly disagree with the "low security" option. Has any other homebridge plugin done that?

@pdlove
Copy link
Contributor Author

pdlove commented Jan 22, 2016

@KraigM I don't mind removing that. The biggest thing on it is that it is very manual to enable and has some warnings in the code and probably shouldn't be on the help file. If we do yank it from here, let's also close the issue request on that so I'll quit thinking "I need to get that in there to remove the issue" :)

@msroest
Copy link
Contributor

msroest commented Jan 22, 2016

I'm with @KraigM on this the seems like allowing that is a pretty big hole and will open people up to liability

@KraigM
Copy link
Owner

KraigM commented Jan 22, 2016

Yeah go ahead and remove that and I will close the other issue

@blaineam
Copy link

oh I didn't even notice the low security option. I was mainly wanting the color light support.

@pdlove
Copy link
Contributor Author

pdlove commented Jan 22, 2016

Low Security has been removed and will not be approached again on this or any plugin I approach. If anyone really wants to have it work like that, feel free to run the code I did prior to the most recent commit, but don't blame me if you lose valuables!

KraigM added a commit that referenced this pull request Jan 23, 2016
@KraigM
Copy link
Owner

KraigM commented Jan 23, 2016

@pdlove I made a slight modification to your fix of the "not responding problem" ^. Which is funny because I'm pretty sure we are back to almost exactly the same code we found that we thought was weird and "fixed". Im actually thinking either I or someone else put that in there as a hack to be fixed later. Anywho, I changed it to at least wait until the request had officially been made to wink and capture any errors that may have happened. Does that look good to you?

@pdlove
Copy link
Contributor Author

pdlove commented Jan 23, 2016

That's great. So if it can't communicate with Wink then Siri will still error out. I'm good with it.

@KraigM KraigM merged commit d278b4d into KraigM:master Jan 23, 2016
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.

None yet

4 participants