bajansen / home-assistant-frank_energie

Custom Component voor Home Assistant Frank Energie prijsinformatie
68 stars 19 forks source link

Implement sign-in flow to get extra account information #71

Closed DCSBL closed 1 year ago

DCSBL commented 1 year ago

This adds a 'do you want to sign in' question during first setup. For now you won't get any extra information but it is easy to implement when this is added!

Things to do (in a follow up or here)

bajansen commented 1 year ago

Mooie ontwikkeling. Het lijkt me voor nu het handigst om de verdere ontwikkeling eerst binnen deze pull request te houden zolang er er nog geen extra functies worden aangeboden.

DCSBL commented 1 year ago

Wat betreft features is dit wat mij betreft ready om als pre-release uit te brengen en te zien wat er gebeurd. Kan je een kijkje nemen of je iets geks ziet?

Voor de gebruiker zou dit de situatie zijn na de update:

Alleen https://github.com/DCSBL/python-frank-energie/issues/19 staat nog open, ik heb een token van 2 weken oud werkend gehad en krijg geen 're-authenticate' verzoekjes in de app. Ik denk dat we in het veld moeten kijken of we hier wat van zien.

Andere datavelden en tests wil ik toevoegen in follow-ups, om deze PR nog enigszins compact te houden.

bajansen commented 1 year ago

Ik heb helaas nog niet de tijd gehad om hier uitgebreid naar te kijken, maar op basis van de commits die ik voorbij heb zien komen ziet het er goed uit.

Ik heb even ruw je branch naar een Home Assistantinstallatie gekopieerd. Hierin was eerder al een vorige versie van de integratie actief, en nu zijn alle oude sensoren niet langer beschikbaar en vervangen door een nieuwe sensor met het _2 achtervoegsel. Dit heeft denk ik met de unique_id spulleboel te maken?

Verder krijg ik onderstaande foutmelding wanneer ik nog een extra instantie van de integratie probeer in te stellen en in te loggen met mijn accountgegevens:

Traceback (most recent call last):
  File "/usr/local/lib/python3.10/site-packages/aiohttp/web_protocol.py", line 433, in _handle_request
    resp = await request_handler(request)
  File "/usr/local/lib/python3.10/site-packages/aiohttp/web_app.py", line 504, in _handle
    resp = await handler(request)
  File "/usr/local/lib/python3.10/site-packages/aiohttp/web_middlewares.py", line 117, in impl
    return await handler(request)
  File "/usr/src/homeassistant/homeassistant/components/http/security_filter.py", line 85, in security_filter_middleware
    return await handler(request)
  File "/usr/src/homeassistant/homeassistant/components/http/forwarded.py", line 227, in forwarded_middleware
    return await handler(request)
  File "/usr/src/homeassistant/homeassistant/components/http/request_context.py", line 28, in request_context_middleware
    return await handler(request)
  File "/usr/src/homeassistant/homeassistant/components/http/ban.py", line 80, in ban_middleware
    return await handler(request)
  File "/usr/src/homeassistant/homeassistant/components/http/auth.py", line 235, in auth_middleware
    return await handler(request)
  File "/usr/src/homeassistant/homeassistant/components/http/view.py", line 146, in handle
    result = await result
  File "/usr/src/homeassistant/homeassistant/components/config/config_entries.py", line 180, in post
    return await super().post(request, flow_id)
  File "/usr/src/homeassistant/homeassistant/components/http/data_validator.py", line 72, in wrapper
    result = await method(view, request, data, *args, **kwargs)
  File "/usr/src/homeassistant/homeassistant/helpers/data_entry_flow.py", line 110, in post
    result = await self._flow_mgr.async_configure(flow_id, data)
  File "/usr/src/homeassistant/homeassistant/data_entry_flow.py", line 271, in async_configure
    result = await self._async_handle_step(
  File "/usr/src/homeassistant/homeassistant/data_entry_flow.py", line 373, in _async_handle_step
    if not isinstance(result["type"], FlowResultType):
TypeError: 'coroutine' object is not subscriptable
DCSBL commented 1 year ago

Thanks voor t testen! Dat _2 is precies wat voorkomen zou moeten worden met dat unique_id gebeuren. Ik had een testsituatie gecreëerd waar dit goed ging… Nog even uitzoeken dus!

Die melding geeft niet aan dat er iets mis ging in de integratie, maar ik gok dat er iets async niet ‘await’ is wat had gemoeten.

Ik gooi ‘m weer even op draft!

DCSBL commented 1 year ago

Ik heb geprobeerd te reproduceren wat je aangaf door te switchen tussen main en sign-in branch, maar in alle gevallen gaat het bij mij goed zonder errors.. Weet je zeker dat je alle files heb gekopieerd?

bajansen commented 1 year ago

Ik heb nu ook een paar keer gewisseld tussen deze branch en v2.0.0 en nu gaat het goed zonder _2 achtervoegsels. Misschien dat ik hiervoor nog een oude dev branch draaide.

Het inloggen in de integratie wil echter nog altijd niet lukken. Ik denk nu echter dat dit aan mijn account ligt, want als ik rechtstreeks met de python library probeer in te loggen krijg ik de error python_frank_energie.exceptions.AuthException: user-error:user-not-active. Dit wordt door de integratie op dit moment dus nog niet afgevangen.

DCSBL commented 1 year ago

Top, was inderdaad wat vreemd rondom inloggen. Moeten we toch maar unit tests maken voor config_flow 🤔.

Ik kan niet vinden hoe we de error message van de API kunnen tonen in de front-end, maar de melding is was duidelijker en de log is versimpeld.

DCSBL commented 1 year ago

Net de changes uitgeprobeerd op mijn eigen installatie, ik krijg ook dubbele sensoren. Even uitzoeken wat er aan de hand is..

DCSBL commented 1 year ago

Fix voor verkeerde migratie opgelost, in een oudere versie was unique_id ingesteld, later niet meer. Door de eerdere versie werd de migratie niet getriggerd.