-
Notifications
You must be signed in to change notification settings - Fork 9.1k
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
Toyota: parse FW versions for more reliable fuzzy fingerprinting #28641
Conversation
Ported the Hyundai test to figure out which cars this won't work on at the moment, and most of the failures are because we don't have the hybrid ECU logged yet (getting matches for hybrid and ICE variants). Once we add that, our number of platforms that won't work with this new system goes from 20 to 13, out of 51. Blacklisted platforms for being too similar with their major platform identifier code (two characters): AssertionError: Items in the first set but not the second:
'TOYOTA C-HR 2018'
'TOYOTA RAV4 HYBRID 2023'
'TOYOTA CAMRY HYBRID 2021'
'TOYOTA RAV4 HYBRID 2022'
'TOYOTA C-HR HYBRID 2018'
'TOYOTA HIGHLANDER 2020'
'LEXUS RX 2020'
'LEXUS ES HYBRID 2019'
'TOYOTA CAMRY 2021'
'LEXUS ES 2019'
'TOYOTA RAV4 2022'
'TOYOTA HIGHLANDER HYBRID 2020'
'TOYOTA RAV4 2023' : (13, 51) However, we also have minor versions that we can also expand on using in the future (I see a pattern we can use, but I've also seen some overlap, so will take some verification and we can do that after). |
How common is this? Why does it happen? |
Not too common. Ran a notebook on rlog data. pandaState error threshold was >10 errors or lost RX/TX messages on bus 0/2 (not 1 since we don't use it for fingerprinting): The two dongles without any pandaStates errors were a TSS2 RAV4 and Corolla. Corolla didn't have engine because it was responding to another query on startup. RAV4 didn't have EPS because it took over our timeout to respond. Checked routes: 6121, dongles: 973 platforms: {'TOYOTA RAV4 HYBRID 2019': 668, 'TOYOTA CAMRY HYBRID 2021': 402, 'TOYOTA COROLLA TSS2 2019': 532, 'LEXUS RX 2016': 96, 'TOYOTA HIGHLANDER HYBRID 2020': 312, 'TOYOTA HIGHLANDER 2020': 325, 'TOYOTA PRIUS 2017': 737, 'TOYOTA CAMRY HYBRID 2018': 88, 'TOYOTA RAV4 2019': 464, 'TOYOTA PRIUS TSS2 2021': 285, 'TOYOTA RAV4 HYBRID 2017': 115, 'TOYOTA COROLLA HYBRID TSS2 2019': 222, 'TOYOTA SIENNA 2018': 153, 'TOYOTA AVALON HYBRID 2022': 33, 'TOYOTA RAV4 2017': 172, 'TOYOTA COROLLA 2017': 174, 'TOYOTA HIGHLANDER HYBRID 2018': 89, 'TOYOTA RAV4 HYBRID 2022': 231, 'LEXUS IS 2018': 27, 'TOYOTA HIGHLANDER 2017': 84, 'TOYOTA C-HR HYBRID 2022': 7, 'TOYOTA MIRAI 2021': 34, 'TOYOTA CAMRY 2018': 236, 'LEXUS RX 2020': 64, 'TOYOTA AVALON HYBRID 2019': 19, 'LEXUS ES 2019': 52, 'TOYOTA RAV4 2022': 118, 'LEXUS NX 2020': 25, 'TOYOTA C-HR 2018': 16, 'TOYOTA CAMRY 2021': 92, 'TOYOTA AVALON 2019': 21, 'LEXUS ES HYBRID 2019': 66, 'TOYOTA RAV4 HYBRID 2023': 6, 'TOYOTA C-HR HYBRID 2018': 1, 'LEXUS RX HYBRID 2020': 27, 'TOYOTA AVALON 2016': 15, 'LEXUS RX HYBRID 2017': 51, 'LEXUS NX HYBRID 2018': 27, 'LEXUS NX 2018': 7, 'TOYOTA AVALON 2022': 18, 'TOYOTA ALPHARD 2020': 2, 'LEXUS RC 2020': 6, 'TOYOTA PRIUS v 2017': 1, 'LEXUS NX HYBRID 2020': 1}
With pandaStates errors on canState0/2:
Missing ECU routes: 40, dongles: 19
With out pandaStates errors:
Missing ECU routes: 2, dongles: 2
All missing ECU platform (routes): {'TOYOTA CAMRY 2018': 13, 'TOYOTA HIGHLANDER 2020': 18, 'TOYOTA RAV4 2022': 1, 'TOYOTA CAMRY 2021': 1, 'TOYOTA COROLLA TSS2 2019': 2, 'TOYOTA RAV4 2019': 1, 'TOYOTA COROLLA 2017': 1, 'TOYOTA HIGHLANDER 2017': 2, 'TOYOTA RAV4 HYBRID 2019': 1, 'TOYOTA CAMRY HYBRID 2018': 1, 'TOYOTA HIGHLANDER HYBRID 2018': 1} |
Last 10 FW version PRs are below. This PR would have matched 3/10 cases. 4/10 failed due to a counterpart hybrid platform that is too similar to match. And the final 3/10 failures were for legitimate reasons—2 unseen model years and 1 API difference. Using the hybrid ECU to detect if it's a hybrid, it eliminates all 4 of those failures, bringing the matches up to 7/10.
The Camry 2019 in there is also a real case of the user getting a FW update for TSB/recall that we now would have caught. See the FW in that PR, but here is the engine platform code change, it's a minor version just as I hypothesized is common from the TSB docs I found online: |
3/10 is a start, but we definitely have to do better. What's this hybrid similarity? Can we fix it? As for new model years, it's just a convenient proxy for API, so we can eventually get rid of that one when we can more exactly define and fingerprint the API. |
That's the ECUs that are shared that I talked about that will be fixed by using the hybrid ECU. |
There are still a few minor API differences with hybrid/ICE. For example, only hybrid variant of RAV4 and Lexus ES are in I believe that these differences likely aren't important and can still be merged, but I do not want to do too many things + verification of each at once, so I'll remove the gas pedal logging and only merge platforms that I can from the fuzzy exclude list (which should be a lot, but not all) to start. |
I would like to look into that first before merging any of the hybrids. Otherwise, these differences will be difficult to find on new ports. |
The differences are only TSS-P as far as I can tell, so there shouldn't be any confusion with new TSS2 models:
So rule of thumb going forward for new TSS-P car ports: if it can do stop and go and the default doesn't match, make a new platform for ICE/hybrid. If the DSU messages don't fully match or don't work, make a new platform. |
…latively different)
This reverts commit 0f87423.
feaff7d
to
2411967
Compare
Re-ran the notebook and still no false positives with excluded self FW, and no mismatches with online or stock fuzzy fingerprinting function. Should be GTG. |
v2 of #27029 so its state is preserved
Todos: