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

added multiple thermostat support, and 'name' method #5

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

jrpayne
Copy link

@jrpayne jrpayne commented May 1, 2013

I added support for multiple thermostats using an optional thermostat_idx: parameter to initialize, and a "name" method to pull that thermostat's name.

Great work BTW -- cheers.

@ericboehs
Copy link
Owner

Could you add tests for this?

I kind of need to add VCR or something to the test suite as I can't test this with my nest account as I only have one nest.

@unorthodoxgeek
Copy link

👍 on that PR
testing this should be done on the theoretical level, just generate the expected response and use FakeWeb to simulate it. This is definitely an important addition to this gem.

@unorthodoxgeek
Copy link

I would, instead of using thermostat index add a few methods to pick the thermostat, either by the index, name or its ID. @jrpayne - would you mind if I fork your fork and fiddle about with it? you've done an awesome job, I want to make it a tad better (and add tests as per @ericboehs's request)

@jrpayne
Copy link
Author

jrpayne commented Jun 14, 2013

I haven't had a chance to double back and look at this again for a few weeks; it's on my list though.

Thanks.

Jeremy Payne
[email protected]
520-909-4589

On May 23, 2013, at 5:49 AM, Tom Caspy [email protected] wrote:

on that PR
testing this should be done on the theoretical level, just generate the expected response and use FakeWeb to simulate it. This is definitely an important addition to this gem.


Reply to this email directly or view it on GitHub.

@mattsoldo
Copy link

bumping this thread.

I'd love to use this library, but being able to select the thermostat is s requirement for me.

@ericboehs
Copy link
Owner

It would be nice if someone added tests for this. Or at least tested it. I don't have multiple devices so I can't try it out. Does this work for anyone else?

@unorthodoxgeek
Copy link

I would have done it, but it's not really worth the trouble, IMHO, as nest are going to release their official API in a couple of months.

@mattsoldo
Copy link

I just submitted an alternative pull request which handles multiple device support. The syntax is a bit more elegant IMO. I've tested it and it works.

@hersha
Copy link

hersha commented Oct 30, 2013

I think what probably needs to happen ultimately is that the Nest API connection be pulled up into its own class and have it return an array( or hash if you want to use the device id or name as a key) of devices. These could be either Thermostats or SmokeDetectors. This would make it easier to extend as new Nest devices are made available.

@scut77
Copy link

scut77 commented Nov 17, 2013

I have 2 devices. I am new on Ruby but I finally managed to get it work. I would like to help.

Eric I updated my nest.rb anf version.rb with jrpayne relase but I got this output:

C:\RubyProgs>MyNest
C:/RubyProgs/nest_thermostat/nest.rb:24:in >=': comparison of String with 0 failed (ArgumentError) from C:/RubyProgs/nest_thermostat/nest.rb:24:ininitialize'
from C:/RubyProgs/MyNest.rb:4:in new' from C:/RubyProgs/MyNest.rb:4:in

'

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.

6 participants