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

Standardize powertrain type classification #1478

Closed
wants to merge 9 commits into from

Conversation

ugtthis
Copy link

@ugtthis ugtthis commented Nov 13, 2024

Wanted to add this so I could query powertrain type easier for opendbc-site

Problem

  • Each make has its own way of indicating powertrain type
  • Harder to query well, a certain car's powertrain type.
    Currently I could parse the name which includes the make, model, and model years to see if hybrid, PHEV, or electric is in it or I could check if the certain car make, has a hybrid/EV type flag

Proposed Solution

I am breaking it up into 3 PRs

Current PR:

  • By default, all vehicles are PowertrainType = PowertrainType.ICE for backward compatibility
  • Included changes for Tesla as an example

2nd PR: Added PowertrainType to individual cars #1484

  • Use PowertrainType for all vehicle definitions in values.py
  • I originally thought I would migrate everything to the new powertrain type but then I noticed that Hyundai, Toyota, and Subaru use the older car make specific, powertrain flags on different files that affect hardware. I think it would be better for that migration to be a separate future PR for each make mentioned, which is why you will still see the old powertrain flags in this specific PR

3rd PR: Powertrain name formatter for all car makes #1502

  • Remove older methods of indicating powertrain types from vehicle definitions in values.py
  • Adds a common function into opendbc/car/__init__.py where if a car is a certain PowertrainType then add specified string(EV, Hyrbid, PHEV) to end of name. This is similar to what /car/ford/values.py line 83 does, which I think would be beneficial for all car makes to use versus every car make with their own slightly different solution of accomplishing this

@github-actions github-actions bot added car related to opendbc/car/ tesla labels Nov 13, 2024
@ugtthis
Copy link
Author

ugtthis commented Nov 14, 2024

Ran into a problem where I didn't consider how certain car platforms also have the ability to be available in multiple powertrain types(ex. Ford escape available in ICE, hybrid, and plug-in hybrid ). Converting to draft for now

-UPDATE:-
I ended up explicitly defining the different car models to show separately instead of using the Ford method, which automates creating the separate car model depending on the flag it sees. Refer to commit bb2331c

@ugtthis ugtthis marked this pull request as draft November 14, 2024 18:05
@ugtthis ugtthis marked this pull request as ready for review November 19, 2024 22:24
@adeebshihadeh
Copy link
Contributor

So this is a lot of complexity for something that isn't be used for openpilot (or to control the car). We need opendbc to continue to get easier to port cars, and this adds something that's just for docs.

At the same time, we want good docs. Ideally, anything that's just for docs can be pulled the NHTSA car database: https://www.nhtsa.gov/vin-decoder. Then, we can setup something to dump relevant info from there and store it outside the code.

(Also thanks for splitting up the PRs, but you can just merge those other ones back into this one. I suspect it'll take some time to get a framework in place for docs-only info, and it's easier to do it all in one place.)

@adeebshihadeh adeebshihadeh marked this pull request as draft November 28, 2024 21:17
@ugtthis
Copy link
Author

ugtthis commented Dec 3, 2024

I was originally going to do the NHTSA vin decoder route but then when I was searching through discord it seemed like the NHTSA vin decoder info can be a hit and miss for cars(refer to discord screenshots), and also doesn't include international cars. Though I think the docs/data will be able to make up for the missing/inaccurate NHTSA data over time. Below is what I'm thinking I will add.

docs/data: Acts as the grocery store and will house data from different sources, including:
- NHTSA Vin Decoder API
- Data sources for international makes/models
- Data pulled from values.py
- opendbc-data longitudinal reports
- Other relevant sources as needed

docs/scripts: Contains scripts for:
- Fetching and organizing data from different sources.
- Parsing information from values.py to extract powertrain type and other metrics that are specific to certain car brands or makes.

/workflows/car_docs.yml: A GitHub Actions file that:
- Automates and triggers updates to the data pipeline.
- Ensures scripts are re-run regularly or on-demand to keep generated files up-to-date.

docs/generated: Stores outputs like JSON files that:
- Can be used directly in documentation.
- Provide data for other web experiences, such as a potential opendbc.com


Had some things pop up recently which prevented me from continuing to work on this but will have more time this week. After thinking about your comment this is the general structure I'm thinking about going with so web experiences can get specific data in a better way


discord-comment-nhtsa

@ugtthis ugtthis closed this Dec 4, 2024
@ugtthis ugtthis deleted the added-powertrain-type branch December 4, 2024 17:53
@ugtthis ugtthis restored the added-powertrain-type branch December 4, 2024 17:55
@ugtthis ugtthis reopened this Dec 4, 2024
@ugtthis
Copy link
Author

ugtthis commented Dec 4, 2024

Closing this to start a new PR focused on the docs only framework. I will repost my comments above over in PR #1556

@ugtthis ugtthis closed this Dec 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
car related to opendbc/car/ tesla
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants