Split login logic #767

tetienne · 2023-02-14T11:33:37Z

The new main entrypoint is now Overkiz.get_client_for.

OverkizClient does not manage anymore the session, it’s up to the calling code to manage it. See my example below.

A StrEnum Server has been created to list all the existing supported servers.

import asyncio
import time

from aiohttp import ClientSession

from pyoverkiz.clients.overkiz import OverkizClient
from pyoverkiz.const import Server
from pyoverkiz.overkiz import Overkiz

USERNAME = ""
PASSWORD = ""


async def main() -> None:

    async with ClientSession() as session:
        client = Overkiz.get_client_for(
            Server.SOMFY_EUROPE, USERNAME, PASSWORD, session
        )
        try:
            await client.login()
        except Exception as exception:  # pylint: disable=broad-except
            print(exception)
            return

        gateways = await client.get_gateways()
        token = await client.generate_local_token(gateways[0].id)
        print(token)
        await client.activate_local_token(gateways[0].id, token, "pyoverkiz")

        local_client: OverkizClient = Overkiz.get_client_for(
            Server.SOMFY_DEV_MODE, gateways[0].id, token, session
        )

        devices = await local_client.get_devices()

        for device in devices:
            print(f"{device.label} ({device.id}) - {device.controllable_name}")
            print(f"{device.widget} - {device.ui_class}")

        await local_client.register_event_listener()

        while True:
            events = await local_client.fetch_events()
            print(events)

            time.sleep(2)


asyncio.run(main())

iMicknl

Looks great already! Would be good to think how this could work for LocalServers as well.

pyoverkiz/servers/overkiz_server.py

pyoverkiz/clients/somfy_local.py

iMicknl · 2023-02-16T22:01:02Z

pyoverkiz/const.py

+    ),
+    "somfy_europe_local": lambda gateway_id, token, session: SomfyLocalClient(
+        name="Somfy Local(Europe)",
+        endpoint=f"https://gateway-{gateway_id}.local:8443/enduser-mobile-web/1/enduserAPI/",


This is not flexible enough I would say. There will be users (like me) where bonjour is not working (e.g. they have a VPN / different VNET). It would be good if we can specify a full URL, thus an IP or a .local address is both possible.

Hum, that’s good to now. So on HA side, you have to enter manually the IP of your box?

pyoverkiz/const.py

iMicknl

Had a quick look! Thanks, great improvement.

README.md

iMicknl · 2023-02-22T11:45:08Z

README.md

@@ -57,13 +63,72 @@ async def main() -> None:
            print(f"{device.label} ({device.id}) - {device.controllable_name}")
            print(f"{device.widget} - {device.ui_class}")

+        await client.register_event_listener()


Is this a change compared to our previous behavior? Where we did register this by default during login.

I forgot to clean this from the previous example. There is still no need to do it.

iMicknl · 2023-02-22T11:46:16Z

pyoverkiz/clients/somfy_local.py

+
+    async def _login(self, _username: str, _password: str) -> bool:
+        """There is no login for Somfy Local"""
+        return True


Would be good to raise an exception here.

iMicknl · 2023-02-22T11:49:51Z

pyoverkiz/overkiz.py

+        manufacturer="Somfy",
+        configuration_url=None,
+        session=session,
+        username=domain,  # not used


Suggested change

username=domain, # not used

username="", # not used

Shall we just pass an empty string? I am still a bit in doubt if we should not make username and password optional, and add token as an optional string.

You can check my lastest modifications, username and password are no more within the parent class, but are defined within the specific implementation.

iMicknl · 2023-02-22T11:51:17Z

pyoverkiz/overkiz.py

+
+class Overkiz:
+    @staticmethod
+    def get_client_for(


Should we call it get_client_for or create_client. With every call, we create a new client, we don't reuse the existing client in Python like this?

Yes, that’s a static method which create a new client each time. I will rename it if it confused you.

Co-authored-by: Mick Vleeshouwer <[email protected]>

tetienne · 2023-02-23T14:05:04Z

README.md

@@ -57,13 +63,72 @@ async def main() -> None:
            print(f"{device.label} ({device.id}) - {device.controllable_name}")
            print(f"{device.widget} - {device.ui_class}")

+        await client.register_event_listener()


I forgot to clean this from the previous example. There is still no need to do it.

tetienne · 2023-02-23T14:05:18Z

README.md

+asyncio.run(main())
+```
+
+### Cloud API


Suggested change

### Cloud API

### Local API

tetienne · 2023-02-23T14:06:30Z

pyoverkiz/overkiz.py

+
+class Overkiz:
+    @staticmethod
+    def get_client_for(


Yes, that’s a static method which create a new client each time. I will rename it if it confused you.

tetienne · 2023-03-02T08:52:34Z

pyoverkiz/overkiz.py

+        manufacturer="Somfy",
+        configuration_url=None,
+        session=session,
+        username=domain,  # not used


You can check my lastest modifications, username and password are no more within the parent class, but are defined within the specific implementation.

tetienne · 2023-03-02T08:54:17Z

pyoverkiz/overkiz.py

+
+class Overkiz:
+    @staticmethod
+    def get_client_for(


iMicknl · 2023-04-06T18:14:38Z

pyoverkiz/clients/somfy_local.py

+    _ssl: bool = field(default=False, init=False)
+    token: str = field(repr=lambda _: "***")
+
+    async def _login(self) -> bool:


How can we verify if the local login works?

We can call any command to see if it works.
But I don’t think we should put a check here. There is no login endpoint for the local api, and so we should not call it.

Let's see how it looks when we have a basic implementation using this branch in core :)

github-actions bot added the breaking label Feb 14, 2023

tetienne force-pushed the breaking/rework-login branch 2 times, most recently from 304e2a6 to e3fae0b Compare February 14, 2023 11:46

iMicknl reviewed Feb 14, 2023

View reviewed changes

pyoverkiz/servers/overkiz_server.py Outdated Show resolved Hide resolved

tetienne force-pushed the breaking/rework-login branch from e3fae0b to 170393b Compare February 14, 2023 14:42

tetienne added 6 commits February 14, 2023 22:37

Split login logic

62d4e1a

Move constants

a68b325

Fixes

d3f4c6e

Fix missing session object

cc895fc

Rework event listener logic

b632423

Update example

835e6bb

tetienne force-pushed the breaking/rework-login branch from f8ad47e to 835e6bb Compare February 14, 2023 21:38

tetienne added 6 commits February 15, 2023 08:36

Remove unused variables

f5b0128

Move all commands to OverkizServer

fb85fc9

Rename server as client

812ce78

Rollback username and password to constructor

d2a1fee

Add local support for Somfy Europe

5c57e7d

Move ssl to attribute

36b163d

iMicknl reviewed Feb 16, 2023

View reviewed changes

pyoverkiz/clients/somfy_local.py Outdated Show resolved Hide resolved

iMicknl reviewed Feb 16, 2023

View reviewed changes

pyoverkiz/clients/somfy_local.py Outdated Show resolved Hide resolved

iMicknl reviewed Feb 16, 2023

View reviewed changes

pyoverkiz/const.py Outdated Show resolved Hide resolved

tetienne added 3 commits February 17, 2023 10:49

Create a single entrypoint to create client

9bea32d

Update example

21dae94

Accept domain as parameter for local api

bcb5c73

tetienne marked this pull request as ready for review February 22, 2023 08:12

tetienne requested a review from vlebourl as a code owner February 22, 2023 08:12

Update documentation

58959d1

tetienne force-pushed the breaking/rework-login branch from 56be3a5 to 58959d1 Compare February 22, 2023 08:13

iMicknl reviewed Feb 23, 2023

View reviewed changes

Apply suggestions from code review

cdafe27

Co-authored-by: Mick Vleeshouwer <[email protected]>

tetienne force-pushed the breaking/rework-login branch from b4971f3 to cdafe27 Compare March 2, 2023 08:25

tetienne added 4 commits March 2, 2023 09:28

Fix examples

571cdc8

Raise an error if we try to call login on the Somfy Local API

1be84ee

Don’t make username and password mandatories

a039f80

Rename get_client_for to create_client

f4cd69f

tetienne commented Mar 2, 2023

View reviewed changes

iMicknl reviewed Apr 6, 2023

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Split login logic #767

Split login logic #767

tetienne commented Feb 14, 2023 •

edited

Loading

iMicknl left a comment

iMicknl Feb 16, 2023

tetienne Feb 17, 2023

iMicknl Feb 18, 2023

iMicknl left a comment

iMicknl Feb 22, 2023

tetienne Feb 23, 2023

iMicknl Feb 22, 2023

iMicknl Feb 22, 2023

tetienne Mar 2, 2023

iMicknl Feb 22, 2023

tetienne Feb 23, 2023

tetienne Mar 2, 2023

tetienne Feb 23, 2023

tetienne Feb 23, 2023

tetienne Feb 23, 2023

tetienne Mar 2, 2023

tetienne Mar 2, 2023

iMicknl Apr 6, 2023

tetienne Apr 7, 2023

iMicknl Apr 7, 2023

Split login logic #767

Are you sure you want to change the base?

Split login logic #767

Conversation

tetienne commented Feb 14, 2023 • edited Loading

iMicknl left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

iMicknl left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tetienne commented Feb 14, 2023 •

edited

Loading