Volkswagen: fingerprint on VIN chassis code #32148
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
sshane
changed the title
This reverts commit ae77d5c.
sshane
commented
Apr 11, 2024
selfdrive/car/volkswagen/values.py
Outdated
| # as we lose too many ECUs to reliably identify the vehicle | ||
| CHASSIS_CODE_PATTERN = re.compile('[A-Z0-9]{2}') | ||
| # TODO: determine the unknown groups | ||
| FW_PATTERN = re.compile(b'\xf1\x87(?P<gateway>[0-9][0-9A-Z]{2})(?P<unknown>[0-9][0-9A-Z][0-9])(?P<unknown2>[0-9A-Z]{2}[0-9])([A-Z0-9]| )') |
There was a problem hiding this comment.
@jyoung8607 not a blocker, but would be good to add if you know what the unknown part/parts are
|
Actually, as long as we don't set Overriding the matches isn't actually needed. |
match_fw_to_car takes vin
sshane
commented
Apr 12, 2024
adeebshihadeh
approved these changes
Apr 12, 2024
Comment on lines
+433
to
+434
| wmi = vin[:3] | ||
| chassis_code = vin[6:8] |
There was a problem hiding this comment.
we should probably pass around a nice VIN struct instead of a raw string
|
|
||
| # These ECUs are required to match to gain a VIN match | ||
| # TODO: do we want to check camera when we add its FW? | ||
| CHECK_FUZZY_ECUS = {Ecu.fwdRadar} |
There was a problem hiding this comment.
ACC commands come from this ECU?
This was referenced Apr 14, 2024
cydia2020
pushed a commit
to cydia2020/dodgypilot
that referenced
this pull request
May 14, 2024
* add function signature and behavior comment * add test * move chassis codes to platform config! * add a shared chassis code test * function * test matching * this commit isn't complete yet * Revert "this commit isn't complete yet" This reverts commit ae77d5c. * need to check WMI * TODO: test WMI * test wmi * radar FW sanity check * fix test * fixes from merge fixes from merge * whoops * fix static analysis! * do match_fw_to_car match_fw_to_car takes vin * makes sense to keep it one function, and we can return exact or fuzzy! * clean up * kinda pointless * fix more tests * back to function being only fuzzy * revert test_fw_fingerprint * revert test_fw_fingerprint * simplify * clean up/fixes * rename test * less duplicatey WMI descriptions * fix * convert to enum * I am confident about these WMIs * these are also good * we support 5N AUS/NZ and NAR (North American) AX Tiguans fixes * Tiguan also Mexico * only one user for caddy * got from the test route * check that the gateway type matches the platform (each platform has 1 or 2 types) * ~gateway~ -> exact FW match * remove re * ensure WMIs are set * actually no reason to delete * move comment up to the platform config * proper wmis typing * spacing * flip
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Related: #32147
Just a custom fuzzy fingerprinting function that takes VIN.
References for WMI and chassis codes:
Currently we match on the union of the match results from exact, fuzzy and custom fuzzy. Currently, I think if we get multiple results from that (which we will for camera harness users since we only get radar), we throw that out and use the custom function as an alternative.We can refine this behavior as we go, but it seems straightforward enough to me as a start. I don't think we want to get rid of FW matching for VW just yet, we use the ECU addrs to sort the fingerprinting and matching.Cars returning VIN from camera:
VOLKSWAGEN_GOLF_MK7-ed13f591d2a47f7f/00000047--7a38b65908VOLKSWAGEN_ATLAS_MK1-d7251ede344ff957/0000000d--9ada8ed097VOLKSWAGEN_GOLF_MK7-cda8fa70cc0c0fed/00000001--258ebaa655SKODA_OCTAVIA_MK3-c5fc8ee4e8be2d97/0000005a--47a8ef422fVOLKSWAGEN_GOLF_MK7-c45851925d1d1d67/0000004e--2f908d3606SKODA_SUPERB_MK3-a7adec1fdbb46a1c/00000029--17cbb1129cVOLKSWAGEN_TIGUAN_MK2-96597f285021abd0/00000000--0fa17d8c0aVOLKSWAGEN_ATLAS_MK1-8d09dff0af485a2c/00000075--8d6113c49dVOLKSWAGEN_ATLAS_MK1-6439fb08910d24cb/00000095--cd86ea9c80SKODA_OCTAVIA_MK3-44379c05b64fd7e3/00000000--77bf0b2cd6check expected gateway, not any gatewaymatch_fw_to_carinput args