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

Update TeslaFi importer to accept newer export formats #3431

Merged
merged 2 commits into from May 24, 2024

Conversation

ithinuel
Copy link
Contributor

@ithinuel ithinuel commented Nov 14, 2023

I am not sure if using "1" as the default id but that's what's suggested in #3158 (comment) and has worked so far.

@ithinuel
Copy link
Contributor Author

Is the ci failure related to the changes in this PR?

@JakobLichterfeld
Copy link
Collaborator

JakobLichterfeld commented Nov 16, 2023

Is the ci failure related to the changes in this PR?

As far as I can see: no. see #3428

@JakobLichterfeld JakobLichterfeld added the area:teslamate Related to TeslaMate core label Nov 16, 2023
Copy link

This pull request has been automatically marked as stale because it has not had recent activity. It could be closed if no further activity occurs. Thank you for your contributions.

Copy link

netlify bot commented Jan 31, 2024

Deploy Preview for teslamate ready!

Name Link
🔨 Latest commit 12560ed
🔍 Latest deploy log https://app.netlify.com/sites/teslamate/deploys/664fa02a836cae000872bfde
😎 Deploy Preview https://deploy-preview-3431--teslamate.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link

This pull request has been automatically marked as stale because it has not had recent activity. It could be closed if no further activity occurs. Thank you for your contributions.

@github-actions github-actions bot added the Stale label Mar 23, 2024
@ithinuel
Copy link
Contributor Author

It is not stale.

To summarize the pending thread:

  • @JakobLichterfeld does not want to see a hard codded value for the vehicle_id
  • This vehicle_id is not exported in TeslaFi’s data exports.
  • In the context of the TeslaFi importer, the input data come from TeslaFi so whether the vehicle_id is present or not in Tesla’s own api is irrelevant
  • As far as I understand, Teslamate relies on the JSON API’s id, not the vehicle_id.

So, is this field essential for the purpose of TeslaMate? Could it be made optionnal and left empty by the importer?

@github-actions github-actions bot removed the Stale label Mar 24, 2024
Copy link

This pull request has been automatically marked as stale because it has not had recent activity. It could be closed if no further activity occurs. Thank you for your contributions.

@github-actions github-actions bot added the Stale label Apr 24, 2024
@ithinuel
Copy link
Contributor Author

Still waiting far input from reviewer.

Copy link
Contributor

@jlestel jlestel left a comment

Choose a reason for hiding this comment

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

LGTM, it's much faster than adding the value in x large csv files by hand...

It might be useful to mention the TESLAFI_IMPORT_VEHICLE_ID in the documentation to show that we can use this env variable to choose the vehicle id on which to import the data (may be in the Requirements section of this file website/docs/import/teslafi.md)

Copy link
Collaborator

@JakobLichterfeld JakobLichterfeld left a comment

Choose a reason for hiding this comment

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

Thanks. Lgtm. Please also add a description about the env file in the doc, under Environment Variables and additional mention it on the doc page for tesla fi import

lib/teslamate/import/line_parser.ex Outdated Show resolved Hide resolved
lib/teslamate/import/line_parser.ex Outdated Show resolved Hide resolved
lib/teslamate/import/line_parser.ex Outdated Show resolved Hide resolved
@JakobLichterfeld
Copy link
Collaborator

Thank you @ithinuel!

@JakobLichterfeld JakobLichterfeld merged commit 0db778e into teslamate-org:master May 24, 2024
12 checks passed
@ithinuel ithinuel deleted the fix-teslafi-import branch May 24, 2024 07:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:teslamate Related to TeslaMate core
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants