changes to support guest accounts #42

AWare · 2022-05-15T15:08:44Z

This adds support for the homeId parameter as suggested by @martintoreilly in #32

This also moves homeId out of urls where used and into the param dict.

Caveats:

Devices and products are not filtered on getAll with a homeId.
I wasn't able to use the async api directly.
I only have access to a guest hive account, so can't test parity.

With some assistance testing this against async I think this should work in ha as afaict session.config comes from the integrations config, so by not overwriting homeID in the session, this should work?

getParams would have worked nicely if getAll filtered on homeId, but as it doesn't- could probably do with a little rethink on my part

Tested with:

import pyhiveapi as Hive

tokens = {}
hive_auth = Hive.Auth(🙊,🙊)
authData = hive_auth.login()

session = authData["AuthenticationResult"]
tokens.update({"token": session["IdToken"]})
tokens.update({"refreshToken": session["RefreshToken"]})
tokens.update({"accessToken": session["AccessToken"]})

api = Hive.API(token=session["IdToken"])
api.getProducts()
api.getDevices()
api.setHome(🙊)
api.getProducts()
api.getDevices()

Tested in async mode with

import apyhiveapi as Hive

hiveSession = Hive.session.HiveSession(username="🙈", password="🙈")
await hiveSession.login()
api = hiveSession.api
await api.getHomes()
await api.getProducts()

api.setHome("🙈")
await api.getHomes()
await api.getProducts()

AWare · 2022-05-18T17:31:18Z

@KJonline I'm going to be unavailable for a few weeks from the end of this week as I'm having elbow surgery- and would really like to get this in before I go. can I get a bit of a hand getting this over the line please?

KJonline · 2022-05-18T18:38:53Z

@AWare unfortunately I’m not in a position to at the minute. Im moving house next week so I need to focus on getting that complete. I can help by giving support over chat. Sorry I can’t be more help.

AWare · 2022-05-18T19:07:21Z

@KJonline good luck with the move!

if you're happy to run the workflows, I can fix anything they suggest and then we can pick this up when it's mutually convenient

KJonline

Just had a scan through and spotted the async version of the API it looks like you haven't updated the all url to remove the hard coded params in favour of using the new getParams function.

AWare · 2022-05-18T23:08:04Z

Cheers, have handled that and the linter comments.

Trying to test it now, using python3 -m asyncio and the following:

import apyhiveapi as Hive

hiveSession = Hive.session.HiveSession(username="", password="")
await hiveSession.login()
api = hiveSession.api
await api.getProducts()
api.setHome("")
await api.getProducts()

and getting some weird list index out of range errors on login()- which I'll revisit tomorrow.

AWare · 2022-07-25T22:09:52Z

@KJonline would you be able to give me a hand testing this sometime please?

KJonline · 2022-07-25T22:49:32Z

@AWare yep can do. I think there is still some outstanding changes on this?

AWare · 2022-07-26T17:13:18Z

I'm not 100% happy with the split between getParams and getHomeIdParam but was having problems building a harness to test it with.

KJonline · 2022-07-26T19:19:33Z

@AWare im going to take a look a bit later on tonight

pyhiveapi/apyhiveapi/api/hive_api.py

KJonline · 2022-07-27T15:04:29Z

pyhiveapi/apyhiveapi/api/hive_api.py

+        }
+        if sendhomeID and self.homeID is not None:
+            params.update({"homeId": self.homeID})
+        return params


You need and else if here if someone requests to the send the homeId but its actually set to None. to do this you will also need to create a new exception in the Hive exceptions file and import it into this module

Suggested change

return params

elif sendhomeID and self.homeID is None:

raise HiveInvalidHomeId

return params

This would change the behaviour of the sync api to require a homeId in the case of one home in hive. I've tried to make sure my change only effects cases where a homeID is actually set.

Thinking about it, there's nowhere we don't want to send homeID if it isn't set; except getHomes, which doesn't mind it being set. So we don't need an arg for it.

What about if your the account owner with a single home and you don’t need to send a homeID.

is your thinking to send one anyway?

AWare · 2022-07-27T21:00:20Z

@KJonline thanks for the feedback, I've managed to get it working with the async api now, it turns out that where requests turned key: True into key=true, aiohttp turned it into an exception instead. I've updated the description with my test code.

KJonline · 2022-07-27T22:16:16Z

I think the best way for me to help with testing is if I create a new temp branch to merge it into. As the code lives on your repository I don’t have access to pull the code

AWare · 2022-07-31T16:25:59Z

@KJonline I think you can just set my fork as a remote, and you should be able to push to this branch too because of my PR. Or, there is a way to check out this pr with refs https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/checking-out-pull-requests-locally#modifying-an-inactive-pull-request-locally if you didn't want to push

AWare · 2022-08-20T19:18:31Z

@KJonline hi, have you had a chance to look at this yet?

AWare · 2022-09-01T07:30:57Z

@KJonline sorry to bother you, but would you be able to have another look at this please?

set home if set

martintoreilly · 2022-10-11T15:48:05Z

@AWare @KJonline I can try and find some time this weekend to look at this / test if useful? I've an owner and guest account for my home.

KJonline

Please see feedback in sections of code

KJonline · 2022-10-15T09:07:11Z

pyhiveapi/apyhiveapi/api/hive_api.py

@@ -210,11 +266,13 @@ def getProducts(self):

    def getActions(self):
        """Call the get actions endpoint."""
-        url = self.urls["base"] + self.urls["actions"]
+        url = self.urls["all"]


How come this has been updated to point to all? getDevices & getProducts still points to the individual endpoints. if we can get all data from the all endpoint maybe we should remove the individual endpoints. What do you think?

I have no idea, I did this in July. Sorry.

KJonline · 2022-10-15T09:08:35Z

pyhiveapi/apyhiveapi/api/hive_async_api.py

@@ -222,11 +264,13 @@ async def getProducts(self):
    async def getActions(self):
        """Call the get actions endpoint."""
        json_return = {}
-        url = self.urls["actions"]
+        url = self.urls["all"]


Same comment here as the sync one

KJonline · 2022-10-15T09:15:34Z

pyhiveapi/apyhiveapi/session.py

@@ -101,6 +100,8 @@ def __init__(
        )
        self.devices = {}
        self.deviceList = {}
+        self.api = API(hiveSession=self, websession=websession)


Not sure why this has moved? doesn't really matter I don't think, the linter may complain

iirc there's a race condition here. this makes sure that the api is not defined until after session has everything it needs

KJonline · 2022-10-15T14:01:26Z

pyhiveapi/apyhiveapi/api/hive_api.py

+        if self.session is not None:
+            self.homeID = self.session.config.homeID
+
+    def getParams(self, products=False, devices=False, actions=False):


I did some testing of the the sync code and the api didn't seem to work with the params when they where booleans. When I changed them to be lower case f and in quotes the api then worked as expected.

Suggested change

def getParams(self, products=False, devices=False, actions=False):

def getParams(self, products='false', devices='false', actions='false\):

The other option would be to the leave the params as booleans and update the dictionary to be as per below converting them to string and lowercasing them.

params = { "products": str(products).lower(), "devices": str(devices).lower(), "actions": str(actions).lower(), }

KJonline · 2022-10-15T14:10:23Z

pyhiveapi/apyhiveapi/api/hive_async_api.py

+            "devices": 'true' if devices else 'false',
+            "actions": 'true' if actions else 'false',
+        }
+        if self.homeID:


You seemed to have catered for it here but not in the sync api

AWare · 2022-11-01T17:37:22Z

Sorry, but I won't be continuing to work on this PR.

AWare added 2 commits May 15, 2022 15:57

changes to support guest accounts

6072eec

remove errant import

91ab20c

KJonline requested changes May 18, 2022

View reviewed changes

AWare added 4 commits May 18, 2022 22:58

remove hard coded params

c39b5d0

consistently use params

5204a86

run pre-commit hooks

ea5b05d

docstring

f6cbd82

Merge branch 'master' into master

40c618b

Pyhass deleted a comment from AWare Jul 27, 2022

KJonline requested changes Jul 27, 2022

View reviewed changes

AWare added 2 commits July 27, 2022 21:44

fix async

2d4b6cc

remove sendHomeID parameter from getParams

4e1a239

errant comma

10a8642

AWare added 2 commits September 16, 2022 00:16

init error in hass

ed484dc

Update session.py

a3b705a

set home if set

KJonline requested changes Oct 15, 2022

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

changes to support guest accounts #42

changes to support guest accounts #42

AWare commented May 15, 2022 •

edited

Loading

AWare commented May 18, 2022

KJonline commented May 18, 2022

AWare commented May 18, 2022

KJonline left a comment •

edited

Loading

AWare commented May 18, 2022

AWare commented Jul 25, 2022

KJonline commented Jul 25, 2022

AWare commented Jul 26, 2022

KJonline commented Jul 26, 2022

KJonline Jul 27, 2022

AWare Jul 27, 2022

AWare Jul 27, 2022

KJonline Jul 27, 2022 •

edited

Loading

AWare commented Jul 27, 2022

KJonline commented Jul 27, 2022

AWare commented Jul 31, 2022

AWare commented Aug 20, 2022

AWare commented Sep 1, 2022

martintoreilly commented Oct 11, 2022

KJonline left a comment

KJonline Oct 15, 2022

AWare Nov 1, 2022

KJonline Oct 15, 2022 •

edited

Loading

KJonline Oct 15, 2022 •

edited

Loading

AWare Nov 1, 2022

KJonline Oct 15, 2022

KJonline Oct 15, 2022 •

edited

Loading

AWare commented Nov 1, 2022

	def getParams(self, products=False, devices=False, actions=False):
	def getParams(self, products='false', devices='false', actions='false\):

changes to support guest accounts #42

Are you sure you want to change the base?

changes to support guest accounts #42

Conversation

AWare commented May 15, 2022 • edited Loading

AWare commented May 18, 2022

KJonline commented May 18, 2022

AWare commented May 18, 2022

KJonline left a comment • edited Loading

Choose a reason for hiding this comment

AWare commented May 18, 2022

AWare commented Jul 25, 2022

KJonline commented Jul 25, 2022

AWare commented Jul 26, 2022

KJonline commented Jul 26, 2022

KJonline Jul 27, 2022

Choose a reason for hiding this comment

AWare Jul 27, 2022

Choose a reason for hiding this comment

AWare Jul 27, 2022

Choose a reason for hiding this comment

KJonline Jul 27, 2022 • edited Loading

Choose a reason for hiding this comment

AWare commented Jul 27, 2022

KJonline commented Jul 27, 2022

AWare commented Jul 31, 2022

AWare commented Aug 20, 2022

AWare commented Sep 1, 2022

martintoreilly commented Oct 11, 2022

KJonline left a comment

Choose a reason for hiding this comment

KJonline Oct 15, 2022

Choose a reason for hiding this comment

AWare Nov 1, 2022

Choose a reason for hiding this comment

KJonline Oct 15, 2022 • edited Loading

Choose a reason for hiding this comment

KJonline Oct 15, 2022 • edited Loading

Choose a reason for hiding this comment

AWare Nov 1, 2022

Choose a reason for hiding this comment

KJonline Oct 15, 2022

Choose a reason for hiding this comment

KJonline Oct 15, 2022 • edited Loading

Choose a reason for hiding this comment

AWare commented Nov 1, 2022

AWare commented May 15, 2022 •

edited

Loading

KJonline left a comment •

edited

Loading

KJonline Jul 27, 2022 •

edited

Loading

KJonline Oct 15, 2022 •

edited

Loading

KJonline Oct 15, 2022 •

edited

Loading

KJonline Oct 15, 2022 •

edited

Loading