-
Notifications
You must be signed in to change notification settings - Fork 1
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
Basic support for multiple cars. #39
Conversation
I will take a look today or tomorrow |
In the end I've just checked it out now :) So, originally I wanted to submit a third PR to refactor a little bit the UI part and properly separate the frontend / backend parts. My goal would be to avoid generating the jinja2 template with hardcoded values from the backend, and instead serve a raw HTML page or a small Angular app which will then query the API with what it needs (car config, periodic map update fetch, etc) Regarding this specific PR, I would suggest to remove the "MULTICAR_SUPPORT" and instead allow all links to be linked to a car id, and if there is only one, use it as default. The backend would fetch the number of cars and then the frontend would allow to choose one for each link. What do you think ? I will try to draft a PR tomorrow with my thoughts. |
I had a look on the endpoints in TeslaLogger, but it doesn't have any easily accessible endpoint that lists all available carid's, short of just testing every single ID sequentially (which has it's downsides too). There may also be people who don't want to allow generating links for all available cars and instead just limit it to a specific one. |
Allow me to rephrase it. I think it's better to have a selector with which you can select the desired car when creating a link. It would also be a nicer touch in the UI |
Just FYI, this PR broke everything on my install, I had to revert it on my repo. |
new_car_id = result.carid | ||
else: | ||
new_car_id = BACKEND_PROVIDER_CAR_ID | ||
BackendProviderFactory.provider.car_id = new_car_id |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will not work because the backend does not provide any "car_id" entity at the runtime, it is only used for the construction of the GET request URL.
Initial revision to adding support for multiple cars as per issue #38 .
@Zegorax, as you refactored quite a bit of the backend, I was wondering if you could have a look and check whether this approach makes sense to you.