Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Polestar soc #1430

Merged
merged 12 commits into from
Apr 4, 2024
Merged

Polestar soc #1430

merged 12 commits into from
Apr 4, 2024

Conversation

isomacM
Copy link
Contributor

@isomacM isomacM commented Feb 19, 2024

SOC module for Polestar2 added

  • fix for wrong imports in _simcounter_store.py (please check, if correct)
  • changed some log.error to log.info in internal_chargepoint_handler/clients.py (please check, if correct)

Philipp

Copy link
Contributor

@LKuemmel LKuemmel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bitte grundsätzlich nicht mehrere Themen in einem PR mischen.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Das Loglevel steht auf Error, damit diese Meldung immer unabhängig vom eingestellten Loglevel geloggt werden.
Eine elegantere Lösung für diese Anforderung wäre die Verwendung von ModifyLoglevelContext aus packages/helpermodules/logger.py. Wenn Du das anpassen möchtest, erstelle bitte einen separaten PR dazu.
Bitte in diesem PR die Änderung rückgängig machen.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wenn diese Meldungen für euch so wichtig sind, dass sie immer geloggt werden sollen, ist das ok.

Die Änderungen in den beiden Module _simcounter_store.py und client.py habe ich zurückgenommen.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bitte nur Tests, die von pytest automatisiert getestet werden können. (Nicht zwingend erforderlich) Jedoch keine manuellen Testdateien.
Bitte diese Datei aus dem PR entfernen.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Du meinst soc_test.py, oder? Das habe ich entfernt.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Die Fehlerbehandlung passt noch nicht. So wie es jetzt implementiert ist, werden keine Fehlermeldungen im Frontend angezeigt, sondern nur im Log.
Exceptions werden mit dem Kontextmanager abgefangen

with SingleComponentUpdateContext(self.fault_state):
Dieser loggt diese und sendet eine entsprechende Meldung an den Broker, die vom frontend angezeigt wird.

Damit nicht bei einem kurzen Serverausfall sofort die Ladung gestartet wird, gibt es einen Error-Counter, der den SoC zurücksetzt.

if ev.data.set.soc_error_counter >= 3:

packages/modules/vehicles/polestar/api.py Outdated Show resolved Hide resolved
Comment on lines +33 to +34
log.error("query_params:http error:%s", e)
return None
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Muss die Exception hier abgefangen werden? Wenn keine weitere Fehlerbehandlung erfolgt, macht es Sinn, die Exception vom Kontextmanager behandeln zu lassen.

packages/modules/vehicles/polestar/api.py Outdated Show resolved Hide resolved
packages/modules/vehicles/polestar/api.py Outdated Show resolved Hide resolved
packages/modules/vehicles/polestar/api.py Outdated Show resolved Hide resolved
isomacM and others added 5 commits March 25, 2024 09:20
…es/modules/internal_chargepoint_handler/clients.py, removed soc_test.py
Co-authored-by: LKuemmel <76958050+LKuemmel@users.noreply.github.com>
Co-authored-by: LKuemmel <76958050+LKuemmel@users.noreply.github.com>
Co-authored-by: LKuemmel <76958050+LKuemmel@users.noreply.github.com>
Co-authored-by: LKuemmel <76958050+LKuemmel@users.noreply.github.com>
@isomacM
Copy link
Contributor Author

isomacM commented Mar 26, 2024

Muss ich jetzt noch was machen, oder passt das so?

@LKuemmel
Copy link
Contributor

Es ist nicht möglich, einzelne Softwarekomponenten unter einer eigenen Lizenz zu veröffentlichen. Unsere Software unterliegt der GNU GENERAL PUBLIC LICENSE Version 3: Link zur Lizenz. Bitte überprüfe, ob du deinen Code unter dieser Lizenz veröffentlichen möchtest, und entferne dann die Lizenz aus deinem Pull Request.

@isomacM
Copy link
Contributor Author

isomacM commented Mar 27, 2024

Das war eh die GNU Lizenz, ich habe sie jetzt rausgenommen

@LKuemmel
Copy link
Contributor

Bitte nicht unsere Lizenz löschen, sondern die im Polestar-Ordner...

@isomacM
Copy link
Contributor Author

isomacM commented Mar 27, 2024

Sorry, da war ich wohl etwas zu schnell - sollte jetzt passen 😄

@LKuemmel
Copy link
Contributor

LKuemmel commented Apr 3, 2024

Bitte noch die eine Zeile kürzen, damit die Formatierung passt und der Test durchläuft.

@LKuemmel LKuemmel added this to the 2.4.1 Step 2 milestone Apr 3, 2024
@isomacM
Copy link
Contributor Author

isomacM commented Apr 3, 2024

Hallo,
die Zeile ist jetzt gekürzt - hoffentlich passt es jetzt

@LKuemmel LKuemmel merged commit bba7a8b into openWB:master Apr 4, 2024
1 check passed
@LKuemmel
Copy link
Contributor

LKuemmel commented Apr 4, 2024

So passt es. Danke!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants