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

feat: add LFP Battery setting for car settings #4007

Conversation

cwanja
Copy link
Collaborator

@cwanja cwanja commented Jun 27, 2024

Disclosure this is not complete in anyway. This adds the settings and updates the webpage. I created this PR based upon a413e8c from @adriankumpf and my work on #2727.

What is missing is:

  1. Dashboards. @swiffer mentioned he would work on those
  2. All the updates to "LC_Messages" - no clue what these are 🤷🏼

Do your worst!

Edit: moving to "completed" from my original post:
Completed after original push

  1. Migration scripts - Can someone validate this? Based it upon a457d17 where the streaming api setting was created.
  2. Test scripts - attempted in 7604d17 (Edit: and then many more 😄 but got it working! 🥳 ).

cwanja added 4 commits June 26, 2024 21:00
Add LFP battery toggle to settings page
Update car settings to store LFP battery setting
Add migration scripts to create column in car settings
@cwanja cwanja added WIP Work in progress area:dashboard Related to a Grafana dashboard area:teslamate Related to TeslaMate core labels Jun 27, 2024
Copy link

netlify bot commented Jun 27, 2024

Deploy Preview for teslamate ready!

Name Link
🔨 Latest commit c1bf616
🔍 Latest deploy log https://app.netlify.com/sites/teslamate/deploys/6685dda67bb3ea0008d507eb
😎 Deploy Preview https://deploy-preview-4007--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.

@cwanja cwanja changed the title [WIP] feat: add LFP Battery for car settings [WIP] feat: add LFP Battery setting for car settings Jun 27, 2024
@cwanja
Copy link
Collaborator Author

cwanja commented Jun 27, 2024

Since we are just creating a toggle, does Tesla have any documentation we can link to for a user to check if their car is a LFP battery or not?

cwanja added 4 commits June 26, 2024 21:32
Added test settings for LFP Battery
Update migrations script to proper name defmodule.
Updated test scripts
Fix get default settings test
@cwanja
Copy link
Collaborator Author

cwanja commented Jun 27, 2024

@mark3-dev - really dumb question. But what updates do I need to do to the default.pot files? It looks like random numbers are added to the end of lines. Mimicking your PR #3993

@mark3-dev
Copy link
Contributor

@mark3-dev - really dumb question. But what updates do I need to do to the default.pot files? It looks like random numbers are added to the end of lines. Mimicking your PR #3993

@cwanja Those numbers are the line numbers of where the gettext translations are used. You can update the pot files by following this https://docs.teslamate.org/docs/development#update-pot-files-extract-messages-for-translation

@cwanja
Copy link
Collaborator Author

cwanja commented Jun 28, 2024

@mark3-dev - really dumb question. But what updates do I need to do to the default.pot files? It looks like random numbers are added to the end of lines. Mimicking your PR #3993

@cwanja Those numbers are the line numbers of where the gettext translations are used. You can update the pot files by following this https://docs.teslamate.org/docs/development#update-pot-files-extract-messages-for-translation

Thanks @mark3-dev. Feel like I know the answer here, but will ask anyways. Is there any way execute this from the GUI in the branch? Might be crazy to say, but I have modified the code in GitHub UI. I do not have a local repo or anything.

@mark3-dev
Copy link
Contributor

@mark3-dev - really dumb question. But what updates do I need to do to the default.pot files? It looks like random numbers are added to the end of lines. Mimicking your PR #3993

@cwanja Those numbers are the line numbers of where the gettext translations are used. You can update the pot files by following this https://docs.teslamate.org/docs/development#update-pot-files-extract-messages-for-translation

Thanks @mark3-dev. Feel like I know the answer here, but will ask anyways. Is there any way execute this from the GUI in the branch? Might be crazy to say, but I have modified the code in GitHub UI. I do not have a local repo or anything.

@cwanja I'm not sure if there is any other way to do it other than pulling down the files and running it.

@07stuntar1
Copy link

07stuntar1 commented Jun 29, 2024

I pulled the PR where is to toggle located? My car isnt LFP but was plotting on Battery Health LFP so now theres no data when I click Battery Health. It's still not plotting on the battery health page.

@swiffer
Copy link
Contributor

swiffer commented Jun 30, 2024

@07stuntar1 - as we've said - in terms of data there is no difference between LFP / non-LFP. The only difference that has been make back then was in terms of how the data is visualized. You seem to are operating on different databases.

@07stuntar1
Copy link

@07stuntar1 - as we've said - in terms of data there is no difference between LFP / non-LFP. The only difference that has been make back then was in terms of how the data is visualized. You seem to are operating on different databases.

How can I fix it? The rest of the pages data is fine. It’s just the battery page

@JakobLichterfeld JakobLichterfeld linked an issue Jul 2, 2024 that may be closed by this pull request
1 task
@cwanja
Copy link
Collaborator Author

cwanja commented Jul 3, 2024

Raising my hand here - I am at a loss for running the pots update. I tried through VS code and GitHub codespaces. Could not get mix working. If there is a way to add a collaborator on this branch, please let me know. Otherwise, someone can open a new branch and use my changes as a baseline to get this added.

Sorry about that.

@cwanja
Copy link
Collaborator Author

cwanja commented Jul 3, 2024

Welp. I have no clue how I did it. But I did 🍾

Can someone review and test? @JakobLichterfeld @swiffer

@cwanja cwanja requested a review from JakobLichterfeld July 3, 2024 23:27
@cwanja cwanja removed the WIP Work in progress label Jul 3, 2024
@cwanja cwanja changed the title [WIP] feat: add LFP Battery setting for car settings feat: add LFP Battery setting for car settings Jul 3, 2024
@cwanja cwanja requested a review from brianmay July 3, 2024 23:29
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.

Awesome, lgtm

@JakobLichterfeld
Copy link
Collaborator

JakobLichterfeld commented Jul 4, 2024

Successfully tested

@swiffer
Copy link
Contributor

swiffer commented Jul 4, 2024

Will take care of changing dashboards after merge!

@JakobLichterfeld
Copy link
Collaborator

Sorry, due to merging #3993, may I ask you to update the pot again? And merge the settings_test? Sorry for the inconvenience.

@JakobLichterfeld JakobLichterfeld changed the base branch from master to feat-lfp-setting July 5, 2024 16:20
@JakobLichterfeld
Copy link
Collaborator

Sorry, due to merging #3993, may I ask you to update the pot again? And merge the settings_test? Sorry for the inconvenience.

I try to solve it via a separate branch as I do not have write access to your repo.

@JakobLichterfeld JakobLichterfeld merged commit 30c0cbb into teslamate-org:feat-lfp-setting Jul 5, 2024
12 checks passed
JakobLichterfeld pushed a commit that referenced this pull request Jul 5, 2024
* Update index.html.heex

Add LFP battery toggle to settings page

* Update car_settings.ex

Update car settings to store LFP battery setting

* Update car_settings.ex

Formatting

* Create 20240627021414_add_lfp_battery_car_setting.exs

Add migration scripts to create column in car settings

* Update settings_test.exs

Added test settings for LFP Battery

* Update 20240627021414_add_lfp_battery_car_setting.exs

Update migrations script to proper name defmodule.

* Update settings_test.exs

Updated test scripts

* Update settings_test.exs

Fix get default settings test

* "Commit pots files"
@cwanja cwanja deleted the cwanja-LFP-battery-setting branch July 6, 2024 00:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:dashboard Related to a Grafana dashboard area:teslamate Related to TeslaMate core
Projects
None yet
5 participants