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

VW PQ: add fingerprint for new car model #32843

Closed
wants to merge 12 commits into from
Closed

Conversation

dkiiv
Copy link
Contributor

@dkiiv dkiiv commented Jun 26, 2024

FingerPrint

Car
Volkswagen Jetta Sportwagen TDI, 2013 (2011-2014)

  • behind dashcam until hurdles for VW PQ are resolved

Route
578742b26807f756/00000010--41ee3e5bec

Car Model Port:

Checklist

  • [X ] added entry to CAR in selfdrive/car/*/values.py and ran selfdrive/car/docs.py to generate new docs
    • omitted from PR, car is behind dashcam mode and therefor not included in generated doc
  • [X ] test route added to routes.py
  • [X ] route with openpilot: 578742b26807f756/00000010--41ee3e5bec
  • [N/A ] route with stock system: n/a
  • [X ] car harness used (if comma doesn't sell it, put N/A): VW J533 harness

Copy link
Contributor

github-actions bot commented Jun 26, 2024

Thanks for contributing to openpilot! In order for us to review your PR as quickly as possible, check the following:

  • Convert your PR to a draft unless it's ready to review
  • Read the contributing docs
  • Before marking as "ready for review", ensure:
    • the goal is clearly stated in the description
    • all the tests are passing
    • the change is something we merge
    • include a route or your device' dongle ID if relevant

@dkiiv
Copy link
Contributor Author

dkiiv commented Jun 26, 2024

the route:
578742b26807f756/00000010--41ee3e5bec

fails on the check/tests because it isnt in the CI bucket yet. i have it preserved + public in useradmin for the time being.

@sshane
Copy link
Contributor

sshane commented Jun 27, 2024

@jyoung8607 can you review? I pushed the route

@sshane sshane added car port car vehicle-specific volkswagen labels Jun 27, 2024
Copy link
Collaborator

@jyoung8607 jyoung8607 left a comment

Choose a reason for hiding this comment

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

Looks like Mk5/6 Golf/Jetta are nearly indistinguishable, which is good given the situation with your particular car and with the weird marketing of the NAR "Jetta" Sportwagen. There are enough ACC-equipped Jettas to make your vehicle supportable.

selfdrive/car/volkswagen/values.py Outdated Show resolved Hide resolved
selfdrive/car/volkswagen/values.py Outdated Show resolved Hide resolved
selfdrive/car/volkswagen/values.py Outdated Show resolved Hide resolved
selfdrive/test/process_replay/test_processes.py Outdated Show resolved Hide resolved
dkiiv and others added 3 commits June 30, 2024 18:50
keeping CarTestRoute

Co-authored-by: Jason Young <46612682+jyoung8607@users.noreply.github.com>
Co-authored-by: Jason Young <46612682+jyoung8607@users.noreply.github.com>
Co-authored-by: Jason Young <46612682+jyoung8607@users.noreply.github.com>
Copy link
Contributor Author

@dkiiv dkiiv left a comment

Choose a reason for hiding this comment

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

comments on jyoungs comments

selfdrive/car/volkswagen/values.py Outdated Show resolved Hide resolved
selfdrive/car/volkswagen/values.py Outdated Show resolved Hide resolved
Copy link
Collaborator

@jyoung8607 jyoung8607 left a comment

Choose a reason for hiding this comment

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

This is okay to merge. Someday if the port emerges from dashcam, we'll be documenting support for 2015-2018 Jetta only. The radar firmware in this PR is a valid match for those vehicles. We've decided it's not correct to include the earlier NAR "Jetta" Sportwagen in VWCarDocs, so that's been removed.

@sshane
Copy link
Contributor

sshane commented Aug 20, 2024

We've moved the car interfacing code to our opendbc repository, which is now the new home for car ports and fingerprints. Please re-open your pull request against opendbc at your convenience by using this command below. This will transform all changes under selfdrive/car/ to opendbc_repo/opendbc/car/. Make sure you have initialized submodules beforehand and have checked out the latest opendbc commit.

PR_NUMBER=33045
curl -L https://github.com/commaai/openpilot/pull/$PR_NUMBER.patch | sed -e 's/selfdrive\/car/opendbc_repo\/opendbc\/car/g' | git apply -v --reject

Simply replace the PR number with your own. Once done, add the files, fix any conflicts, and open a new PR. Alternatively, you may start a new PR from scratch if that is easier for you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants