martinl / openpilot

Open source driving agent - Subaru fork
https://github.com/martinl/openpilot/wiki/
MIT License
70 stars 72 forks source link

Add missing IMPREZA f/w plus significant FPv2 overhaul #73

Closed ErichMoraga closed 2 years ago

ErichMoraga commented 2 years ago
ErichMoraga commented 2 years ago

prlifestyle93#2241 DongleID/route 8cd51ce762cec95a|2021-12-22--18-30-55 verified working... https://discord.com/channels/469524606043160576/525718620517564446/923374644814315591

ClockeNessMnstr commented 2 years ago

b'\xf1\x00\xac\x03\x00' is not a correct address for the fwdcamera. 0x7C4 (1988) is something else responding on subaru

ErichMoraga commented 2 years ago

b'\xf1\x00\xac\x03\x00' is not a correct address for the fwdcamera. 0x7C4 (1988) is something else responding on subaru

I agree, but... the f/w dumper is very problematic for Subarus, and this got it to fingerprint as the correct car accordingly.

As seen in carParams...

{'carParams': {'carFingerprint': 'SUBARU IMPREZA LIMITED 2019',
               'carFw': [{'address': 2016, 'ecu': 'engine', 'fwVersion': b'\xaa!aw\x07', 'subAddress': 0},
                         {'address': 2017, 'ecu': 'transmission', 'fwVersion': b'\xe3\xf5\x06\x00\x00', 'subAddress': 0},
                         {'address': 2000, 'ecu': 'fwdRadar', 'fwVersion': b'\xf1\x00\x00\x00\x02', 'subAddress': 0},
                         {'address': 1988, 'ecu': 'fwdCamera', 'fwVersion': b'\xf1\x00\xac\x03\x00', 'subAddress': 0},
                         {'address': 1862, 'ecu': 'eps', 'fwVersion': b'z\xc0\x00\x00', 'subAddress': 0},
                         {'address': 1927, 'ecu': 'fwdCamera', 'fwVersion': b'\x00\x00c\xf4\x00\x00\x00\x00', 'subAddress': 0},
                         {'address': 1968, 'ecu': 'esp', 'fwVersion': b'z\x94\x08\x90\x00', 'subAddress': 0}],
               'carName': 'subaru',
ClockeNessMnstr commented 2 years ago

It's not actually added though because the address doesn't match, right?

would need a new tuple for a key. (Ecu.fwdCamera, 0x7C4, None): [ b'\xf1\x00\xac\x03\x00', ]

ErichMoraga commented 2 years ago

It's not actually added though because the address doesn't match, right?

would need a new tuple for a key. (Ecu.fwdCamera, 0x7C4, None): [ b'\xf1\x00\xac\x03\x00', ]

Ugh, damn. You're right (1988/0x7c4 vs. 1987/0x787), so it's interesting that it still fingerprinted as SUBARU IMPREZA LIMITED 2019.

I do see the 2nd fwdCamera entry for b'\x00\x00c\xf4\x00\x00\x00\x00' which I should put in its place.

martinl commented 2 years ago

Tests show one duplicate fw value, I added the prlifestyle93 fw values in separate commit, since reviewing all the changes will take more time. @ErichMoraga can you share the fw dumper script, having hex values instead of mix octal/hex seems useful for adding firmware values in the future

ErichMoraga commented 2 years ago

@ErichMoraga can you share the fw dumper script, having hex values instead of mix octal/hex seems useful for adding firmware values in the future

carParams can be pulled directly from the route via https://useradmin.comma.ai/ or via the FPv2 cmd. I have pinned in #installation-help in the official Comma.ai Discord... PYTHONPATH=/data/openpilot python /data/openpilot/selfdrive/debug/dump.py carParams

ErichMoraga commented 2 years ago

Tests show one duplicate fw value

It's strange it thinks xc5!dr\x07 is a duplicate f/w value, since I didn't add it (note that it's in the whitespace)... image

martinl commented 2 years ago

FPv2 values were upstreamed as is, closing this PR