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

Renault: Provide a meaningful error when not paired #3668

Merged
merged 13 commits into from
Jul 3, 2022

Conversation

ickeundso
Copy link
Contributor

@ickeundso ickeundso commented Jun 19, 2022

Fix #3535

ickeundso added 9 commits May 30, 2022 07:37
- log.ERROR.Println added to kamereonVehicles
- no logic changes only a log messages is added

It would also be possible to move this to the ==ACTIVE branch. But I don't know how the kamereon API will behave with other versions
- do not only log the not paired vehicle but create an error response the missconfigured / non-paired vehicle
- do not only log the not paired vehicle but create an error response the missconfigured / non-paired vehicle
- do not check for role "MAIN_DRIVER" string, just check of role existence. The "ConnectedDriver.Role" is not documented at https://renault-api.readthedocs.io/en/latest/reference/kamereon.html#renault_api.kamereon.models.KamereonVehiclesLink but I can reproduce the behaviour. For the case the vehicle is paired the ConnectedDriver.Role attribute is there an has the "MAIN_DRIVER" value and if the vihicle is not paired the ConnectedDriver.Role attribute is not there, the API can be tested of empty string.
- add test for vehicle/renault.go
- use vehicle from account if no one is cofiggured on evcc or use a matching vehicle
@ickeundso ickeundso changed the title #3535 #3535 Renault: Provide a meaningful error when not paired Jun 19, 2022
@andig
Copy link
Member

andig commented Jun 24, 2022

@ickeundso Du vermischst wieder die Funktionen. kamereonVehicles soll die Fahrzeuge zurück geben, nicht mehr und nicht weniger. Dafür braucht es keine VIN. lass es doch wie's ist (ggf. ein []Vehicle zurück geben statt []string) und gib dem Vehicle dann eine IsActive und IsPaired methode. Aufruf und Auswertung dann aus NewRenault.

@andig
Copy link
Member

andig commented Jun 24, 2022

Ich machs mal zu, gerne neuer Anlauf wenn Du Lust hast.

@andig andig closed this Jun 24, 2022
- add test for vehicle/renault.go
- use vehicle from account if no one is configured on evcc or use a matching vehicle
- fix linter warn
@andig
Copy link
Member

andig commented Jun 25, 2022

Ich glaub wir müssen uns nur einigen wie es werden soll: entweder filtern wir alle unzugänglichen Fahrzeuge stillschweigend raus (so ist es heute). Oder wir geben alle Fahrzeuge zurück (es ist ja die vehicles Funktion) und lassen übergeordnet prüfen was davon zu verwenden ist. Auf die Art bleibt der Code sauber, Beides wäre möglich.

@andig andig reopened this Jun 25, 2022
@andig
Copy link
Member

andig commented Jun 25, 2022

Mein Vorschlag wäre #3706 (ich bekomms grad nicht in Deinen PR gepusht, irgendein Git-Problem). Wär das ok für Dich?

@andig
Copy link
Member

andig commented Jun 25, 2022

So, jetzt ist es hier drin. Please check.

@andig
Copy link
Member

andig commented Jun 26, 2022

@ickeundso passt die Funktion? Dann müssten nur noch die Tests entfernt werden oder auf den Typ umgebaut.

@andig andig changed the title #3535 Renault: Provide a meaningful error when not paired Renault: Provide a meaningful error when not paired Jun 26, 2022
@ickeundso
Copy link
Contributor Author

HI @andig , haha, da kann ich sehr gut mit leben. Ist ja wesentlich weiter gedacht Deine Lösung. Ja die test könnte ich umbauen, entweder hier oder ich mach dafür nach Deinem master merge einen separaten PR auf und wir schieben die Test nach, das wäre dann zwar nicht ganz so wie man es macht, aber ....

@andig
Copy link
Member

andig commented Jun 28, 2022

Genau. Dann schmeiss gerne den Test erstmal raus, wäre aber schön wenn Du probieren könntest ob es funktioniert- dafür fehlt mir das Fahrzeug...

@andig andig merged commit 85ef96c into evcc-io:master Jul 3, 2022
dontbyte pushed a commit to dontbyte/evcc that referenced this pull request Aug 2, 2022
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.

Renault: Provide a meaningful error when not paired
2 participants