Add PSA SoC module

[MartinRinas]

• liest SoC und Range
• Kombination mit manueller Berechnung fehlt noch, diese ist notwendig um den SoC während des Ladevorgangs bestimmen zu können da PSA keinen aktualisierten SoC bereitstellt
• Konfiguration der client_id und client_secret ist nun optional
• unterstützt Accounts mit mehreren Fahrzeugen über optionalen VIN-Filter

[MartinRinas]

offen ist nur mehr die manuelle Berechnung des SoC während des Ladevorgangs, wir bekommen von PSA ja keine Aktualisierung während des Ladens sondern erst mit Ende eines Ladevorgangs.

@LKuemmel hierfür suche ich im Datenmodell nach einem Zähler welcher die ans Auto gelieferte Energiemenge für den aktuellen Ladevorgang zählt. Wichtig wäre dass auch mögliche Lademoduswechsel berücksichtigt sind, konkretes Beispiel: Ich lade mit 2kWh mit PV, schalte dann auf Sofortladen um und lade weitere 10kWh. Ladevorgang wurde nie gestoppt. Der Zähler muss in diesem Fall 12kWh liefern.

Es gibt imported und imported_since_modeswitch, _since_plugged & co - bin mir ad hoc aber nicht sicher ob einer dieser Zähler sich so verhält wie ich das bräuchte.
Kannst Du das aus dem Kopf beantworten? Sonst werde ich das mal in meinem Setup nachstellen und die Zähler beobachten um etwas passendes zu finden.

[yankee42]

Es könnte einfacher sein hier ein CarState zu erzeugen und zurückzugeben.

[MartinRinas]

das ist in den anderen Modulen ähnlich implementiert. Bin aber bei dir, das wäre schöner. Was mir leider nicht klar ist, wie kann ich den CarState in den anderen Funktionen, z.b. fetch_soc und in der soc.py direkt weiterverwenden ohne mir stets ein neues Objekt über carstate=CarState(soc, range, timestamp) zu erzeugen - das CarState Objekt muss hier ja im Grunde nur 1-1 durchgereicht werden. von PSARequest zu fetch_soc zur update in soc.py.

[LKuemmel]

Es gibt einen Zähler für die Energie seit Anstecken und einen für die Energie seit dem letzten Moduswechsel. Einen Zähler seit dem letzten Ladestopp gibt es nicht.

[yankee42]

Dann am besten noch soc_calc.

[MartinRinas]

ist überall zu calculate_soc umbenannt, das trifft es m.E. noch besser.

[yankee42]

Bei vehicle denke ich nicht an einen int. Ich sehe in anderen Modulen heißt das auch so... Mhh... Das könnte man als Anhaltspunkt sehen es so zu lassen zur Standardisierung. Ich hätte es sonst eher vehicle_id oder so genannt. Wie siehst du das @LKuemmel ?

[yankee42]

Die Aufteilung ist so viel besser, aber die schiere Anzahl an Parametern und Rückgabewerten finde ich unschön.

Hier ein Gegenvorschlag:

def create_session(user_id: str, password: str, client_id: Optional[str], client_secret: Optional[str],
                   manufacturer: str) -> Session:
    manufacturer_config = manufacturer_configurations[manufacturer]
    brand = manufacturer_config.brand
    realm = manufacturer_config.realm
    if not client_id or not client_secret:  # use defaults if no client_id and client_secret is specified
        client_id = manufacturer_config.client_id
        client_secret = manufacturer_config.client_secret
        if not client_id or not client_secret:
            raise Exception(
                "No OAuth credentials configured and no default available for manufacturer %s" % manufacturer)

    session = req.get_http_session()
    data = {
        'realm': realm, 'grant_type': 'password', 'password': password, 'username': user_id, 'scope': 'openid profile'
    }

    access_token = session.post(
        "https://idpcvs." + str(brand) + "/am/oauth2/access_token", data=data, auth=(client_id, client_secret)
    ).json()['access_token']

    session.params = {"client_id": client_id}
    session.headers = {
        'Accept': 'application/hal+json', 'Authorization': 'Bearer %s' % access_token, 'x-introspect-realm': realm
    }
    return session

Ungetestet, müsste aber meine ich funktionieren und du hast damit alle Parameter für die ganze Session gesetzt und kannst diese auch in den anderen Funktionen einsparen.

[MartinRinas]

ja, das ist in der Tat schöner, danke! Ist gestestet und funktioniert. PR ist aktualisiert.

[yankee42]

modules/psa/api.py:113: error: Incompatible types in assignment (expression has type "Dict[Any, Any]", variable has type "int")  [assignment]

Hat es recht... Die Variable vehicle weiter oben, die als Parameter rein kommt ist ein int.

Das ist auch insofern nicht optimal, weil 5 Zeilen später in Zeile 118 wo die Exception geworfen wird sowohl der int stehen kann, als auch ein vehicle-dict, je nachdem wann der Fehler im try fliegt.

Ich wäre dafür den Parameter in vehicle_id oder sowas umzubenennen.

[MartinRinas]

ist auch angepasst.

[yankee42]

Aber "falschrum". Die id passt eher zu dem int welches der Parameter ist.

[yankee42]

modules/psa/soc.py:45: error: Too many arguments for "PSAConfiguration"  [call-arg]

Hat es recht, das kann nicht funktionieren. Es fällt auf, dass charge_point doppelt vorkommt.

Da der Konstruktor von PSAConfiguration sehr viele (>4) Parameter hat, könnte es zur Übersicht beitragen die Namen der Parameter mit anzugeben. Damit würden Parameterfehler (Reihenfolge und Anzahl) sichtbarer werden:

Soc(
        PSA(
            configuration=PSAConfiguration(
                user_id=user_id,
                password=password,
                client_id=client_id,
                client_secret=client_secret,
                manufacturer=manufacturer,
                calculate_soc=calculate_soc,
                vin=vin)
        ),
        charge_point
    ).update(False)

[MartinRinas]

danke, ist korrigiert.

[yankee42]

Eine Logausgabe sieht dann zum Beispiel so aus:

2022-11-21 13:06:31,761 - {soc.modules.psa.soc:10} - {WARNING:MainThread} - Hi there

Fokus auf den Loggername: soc.modules.psa.soc. Das soc ist so doppelt. Es kommt einmal von __name__, den die Datei heißt soc.py.

Ich finde es konsequenter nur bei __name__ zu bleiben, denn da steckt alles drin. Wenn die Initialisierung aus einer anderen Datei kommt, zum Beispiel aus der api.py, dann hieße der Logger modules.psa.api, was soc nicht enthält, die Quelle aber trotzdem eindeutig identifiziert.

Ich sehe allerdings, dass auch einige weitere Module das Konstrukt so haben. Falls das eine Designentscheidung so ist müsste das natürlich so bleiben...

[MartinRinas]

Ja, die anderen Module machen das auch genau so, ggf. kann man das zukünftig in allen Modulen ändern wenn sich das Design ändert.

[yankee42]

Der Code sieht für mich gut aus. Ich würde emphelen noch die Git-Historie aufzuräumen, also auf den aktuellen master rebasen und alle commits zusammenfassen. Die kleinen Zwischenschritt vor dem erstmaligen Merge sind nicht so interessant und die Historie und die Verwendung in Werkzeugen wie git blame wird dann leichter zu lesen.

[yankee42]

Da du jetzt wo du den 6afe762 gemergt hast sowas machst:

git reset --soft 6afe7625f80a0b7624693bba28309e544f337ee9
git commit -m "add PSA SoC-Module"
git push -f origin

(unter der Annahme dass das remote für deinen fork "origin" heißt), dann hast du wieder einen übersichtlichen PR mit nurnoch einem commit.

Der soft-Reset setzt den branch pointer auf den angegebenen commit, lässt aber dein Arbeitsverzeichnis unangetastet. Dann kannst du alle Änderung wieder mit einem einzelnen commit committen.

[MartinRinas]

hm, ich weiß nicht was ich hier jetzt angestellt habe 331 geänderte Dateien sind wohl etwas viel. Ist wohl am einfachsten wenn ich einen neuen PR mit sauberer Basis einstelle.

[yankee42]

Du kannst auf den gleichen Branch nochmal neu pushen. Muss kein neues Issue sein.