gboudreau / nest-api

Unofficial Nest Learning Thermostat API
GNU Lesser General Public License v3.0
300 stars 93 forks source link

getUserLocations only returning one location #122

Closed seesoe closed 4 years ago

seesoe commented 4 years ago

I have 3 locations on my account and the that call is only returning one of the locations, i guess a default location of the 3? Is it possible to control devices from the other locations? I am using google login method, the account on web and mobile are able to see all the locations.

bauzer714 commented 4 years ago

Do each of the structures have a thermostat or protect?

seesoe commented 4 years ago

one structure has 1 thermostat (the structure returned) second structure has 0 thermostats third structure has 4 thermostats (the one i'm trying to target)

Edit: actually [structure] shows the first 2 but missing the 3rd one

bauzer714 commented 4 years ago

Structure 2 will not be returned. However, one and 3 should be if they have the mentioned devices.

If you do a var_dump($nest->getUserLocations()) you only see an array of one?

seesoe commented 4 years ago

Yeah getUserLocations only returns structure 1. last_status shows only structure 1 and 2.

seesoe commented 4 years ago

ah seems the issue is with being only a member of structure 3. structure 1 and 2 I am the owner.

user->structures returns structure 1 and 2. user->structure_memberships returns an array of 3 structures, with the 3rd one having a role of member

bauzer714 commented 4 years ago

Great research. We can probably change the getUserLocations to use the memberships (unless it's not always there) or at least accept a param to include memberships.

Is there anything you can't do you can do in the nest apps if you're only a member, that this library might try to do?

seesoe commented 4 years ago

im updating get device list now and testing. we could also merge both objects if theres a diff.

seesoe commented 4 years ago

never mind that won't work since last_status->structure comes back with only owner structures. like you said maybe only a few endpoints can be updated to accept a location param to bypass communicating with member only locations.

bauzer714 commented 4 years ago

I'll look at this next week to see if I can create a solution if this is still an unresolved issue.

seesoe commented 4 years ago

I was still able to control the 3rd structure devices but I had to change getDeviceTemperatureScale since member only devices don't get listed in $this->last_status->device.

from

return $this->last_status->device->{$serial_number}->temperature_scale;

to

$temperature_scale = $this->last_status->device->{$serial_number}->temperature_scale;
return $temperature_scale ? $temperature_scale : "F";

I guess any methods using $this->last_status->device->{$serial_number} or $this->last_status->structure->{$structure_id} will need to be checked for functionality if they are expected to be used with member only structure located devices.

bauzer714 commented 4 years ago

I don't think I'll have a sufficient data set to fully test this. Can you json_encode last status, replace personal data, and provide?

bauzer714 commented 4 years ago

Your code change for getDeviceTemperatureScale isn't sustainable, as it's just falling back to Fahrenheit if it can't find the device settings. This won't make our metric friends happy to make this assumption.

I looked into this a bit today, and I'm curious if structure 3 owner is still using the nest login? Only "Nest" had a concept of "Owner" / "Member" and you've converted the devices you own to Google. Either that or your accounts are in some weird mixed state where it didn't actually complete the conversion correctly.

Assuming I'm correct, I don't think it's worthwhile to support this "mixed" account as it's probably a very edge case scenario and Nest/Google continue to be on a path to remove the Nest only login.

bauzer714 commented 4 years ago

Finally was able to replicate this - was a bit of a challenge, but I got there.

For my non owner user, the /v3/mobile endpoint doesn't return data for "member" thermostats. I'm intrigued you're able to see any data at all for the device. It does look like you can get the data through the app_launch endpoint, but the data isn't matching and requires further manipulation.

@gboudreau is typically better at reverse engineering the Nest APIs, if we're lucky he'll chime in.

seesoe commented 4 years ago

The small tweak was good enough to get my application running, but indeed isn't a good solution.

The owner account hasn't migrated yet. However I don't think it is a matter of migration. I actually posted a while ago having similar issues. The solution I went with then was to put the devices on a seperate master account instead of shared on mine.

https://github.com/gboudreau/nest-api/issues/70

bauzer714 commented 4 years ago

Issue 70 has been committed for years, but I'm not convinced it's the entire solution. I invited a different email that I have and last_status comes back with no user locations and no devices.

It could be related to the age of the additional user and an enforcement on Google/Nest side, a bit of a goose chase. Without more data points and/or payloads I'll try to get my new approach to work in next day or so and see if it works for you.

bauzer714 commented 4 years ago

I think I was able to resolve it. The changes below should resolve the membership issue as well as desired temperature scale. I've had this running for a bit without any issues on my account that was previously receiving no Nest data. Rather than replacing the existing logic, I created a named constant that dictates to use this implementation.

So if you would, please follow the below and let me know if you still have issues.

Use this file (based on latest gboudreau nest). https://github.com/bauzer714/nest-api/blob/master/nest.class.php

Then if you're including file and want to have a clean nest.class.php file add the below in your php file.

define('USE_STATUS_BUCKETS', TRUE)

Alternatively update the class on line 5 to change the default to true.

defined('USE_STATUS_BUCKETS') OR define('USE_STATUS_BUCKETS', TRUE);

seesoe commented 4 years ago

@bauzer714 very well done! Everything seems to be working. The logic looks sound. Several year old bug fixed 😅

~~You linked to your master branch, the bucket code is in the bucket branch (for anyone following) https://github.com/bauzer714/nest-api/blob/bucketSupport/nest.class.php~~

Also you have php 7.x syntax ?? instead of isset() just in case anyone is running 5.6 like me. https://github.com/bauzer714/nest-api/blob/bucketSupport/nest.class.php#L1114

bauzer714 commented 4 years ago

Sorry about that, I've been moving stuff around to break it into reasonable PRs so it's not an "All or nothing" and improves some of the logic. I'll submit the PRs in the next few hours.

I think it's fair to start moving on from 5.x logic, it was EOL in 2018.

seesoe commented 4 years ago

ya i saw the get device branch as well. looks good.

agreed, unfortunately large scale production apps aren't the easiest to upgrade.

thanks!

bauzer714 commented 4 years ago

Final PR will implement php 5.x isset style.

bauzer714 commented 4 years ago

PRs submitted and my master is now updated with the same prs completed. @seesoe - you may want to remove/edit your links above. I typically don't keep my fork open and the deep linked branches have already been deleted.