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 ability to enable and disable data collection for cars #3993

Merged
merged 6 commits into from
Jul 5, 2024

Conversation

mark3-dev
Copy link
Contributor

This feature will allow individual cars to be enabled and disabled from the settings page.

There is one new column added to the car_settings table for storing the enabled flag. It is not nullallable and has a default value of true.

When a change to the enabled value of any car it will call the existing restart method in the vehicles file.

NOTE: This will not affect any grafana dashboards other than no new data for a car will be collected.

I found two discussions that relate to this.
#3918
#1049

Copy link

netlify bot commented Jun 23, 2024

Deploy Preview for teslamate ready!

Name Link
🔨 Latest commit 52e727e
🔍 Latest deploy log https://app.netlify.com/sites/teslamate/deploys/6687083526f89e000834e4cc
😎 Deploy Preview https://deploy-preview-3993--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.

@JakobLichterfeld JakobLichterfeld added the enhancement New feature or request label Jun 26, 2024
@JakobLichterfeld
Copy link
Collaborator

JakobLichterfeld commented Jun 26, 2024

Thank you for your suggestion. This is obviously not as permanent as removing the vehicle from the database: https://docs.teslamate.org/docs/maintenance/manually_fixing_data#remove-a-vehicle-from-the-database

@mark3-dev
Copy link
Contributor Author

Thank you for your suggestion. This is obviously not as permanent as removing the vehicle from the database: https://docs.teslamate.org/docs/maintenance/manually_fixing_data#remove-a-vehicle-from-the-database

Thanks JakobLichterfeld, I agree this is not as permanent as removing the vehicle from the database. This is more for stopping data collection on active cars associated with your Tesla account. I have a car on my account that belongs to a family member and I need to prevent data collection on that car without removing driver access.

@JakobLichterfeld
Copy link
Collaborator

@mark3-dev Why the feature is not yet merged:
I'm thinking about how we can document this feature for the user. The most user-friendly way would certainly be an explanation directly in the settings.

@mark3-dev
Copy link
Contributor Author

@mark3-dev Why the feature is not yet merged: I'm thinking about how we can document this feature for the user. The most user-friendly way would certainly be an explanation directly in the settings.

Is there anything else I can do?

My original entry in the settings page was "Enabled" as the section title and "Enable data collection" for the text beside the slider. But after I realized that there were many different languages that the interface was translated to I changed it to Enabled for both since that was already translated in all the languages.

@cwanja
Copy link
Collaborator

cwanja commented Jul 3, 2024

I'd suggest put a 'Learn more' link and add a page under the 'Maintenance' section with more details. Including a link back to the removing a vehicle from the database details as well.

Could also seeing merging that section and this PR under a 'Vehicle Management' page under 'Maintenance'.

@JakobLichterfeld
Copy link
Collaborator

My original entry in the settings page was "Enabled" as the section title and "Enable data collection" for the text beside the slider. But after I realized that there were many different languages that the interface was translated to I changed it to Enabled for both since that was already translated in all the languages.

For now I would go with your initial suggestion. Necessary translation should not prevent us from being precise in our wording for the sake of UX.

@mark3-dev
Copy link
Contributor Author

My original entry in the settings page was "Enabled" as the section title and "Enable data collection" for the text beside the slider. But after I realized that there were many different languages that the interface was translated to I changed it to Enabled for both since that was already translated in all the languages.

For now I would go with your initial suggestion. Necessary translation should not prevent us from being precise in our wording for the sake of UX.

I'll try and get that added later today.

@JakobLichterfeld
Copy link
Collaborator

Not saying you need to translate the message, we start with English

@mark3-dev
Copy link
Contributor Author

Screenshot from 2024-07-04 16-26-44

@mark3-dev
Copy link
Contributor Author

I think I like this one better
Screenshot from 2024-07-04 16-35-45

@cwanja
Copy link
Collaborator

cwanja commented Jul 5, 2024

I think I like this one better

Screenshot from 2024-07-04 16-35-45

Agreed.

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.

perfect, ty!

@JakobLichterfeld JakobLichterfeld changed the title Add ability to enable and disable cars feat: Add ability to enable and disable data collection for cars Jul 5, 2024
@JakobLichterfeld JakobLichterfeld merged commit c90cde8 into teslamate-org:master Jul 5, 2024
12 checks passed
JakobLichterfeld pushed a commit that referenced this pull request Jul 5, 2024
* Add ability to enable and disable cars

* Add default value for enabled in car_settings.

* Fix unit tests.

* Fix formatting.

* Update text to be more descriptive

* Update lables.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants