refactor powerwall: de-duplicate code, use http-session with logging and automatic error detection #2056
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Wegen diesen Forenthread habe ich nochmal in das Powerwall-Script reingeschaut, dessen Batterieteil ich vor kurzem in #1817 überarbeitet habe.
Es werden natürlich alle möglichen HTTP-Responses in dem Modul geloggt, aber leider nicht die Response, um die es in dem Thread im Forum geht.
Allgemein ist der Code ist nicht mehr auf dem Stand der Technik. Dank #1823 gibt es die Möglichkeit automatisch zu loggen und auch die Fehlerekennung (
raise_for_status()
) automatisch zu machen und damit kann jede Menge Code entfallen.Leider hatte der Code mit #1889 ein erhebliches Copy&paste-Erlebnis erlitten. Wenn man jetzt irgendwas irgendwo fixen will muss man das gleich 3x machen.
Ich habe daher den ganzen Code überarbeitet: Der Teil, der zwischen allen Modulen geteilt wird, ist in eine gemeinsame Bibliothek extrahiert und dank der Verwendung von
get_http_session
wird jetzt alles geloggt, was per http zurück kommt. Das wird zwar das im Forum beschriebene Problem nicht lösen, aber das debugging des Problems erleichtern.Der von mir in #1817 erzeugte Unittest ist noch da und funktioniert, das gibt etwas Konfidenz darein, dass die Änderungen nicht alles völlig kaputt gemacht haben, allerdings sind die anderen beiden Module vollends ungetestet und mangels Powerwall habe ich überhaupt nichts im laufenden Betrieb testen können.