FPv2 improvements - Githubissues

sshane commented 1 year ago

[x] Consider extra ecus for present ecu brand matching - https://github.com/commaai/openpilot/pull/27688 For CAN FD for example, only the two ECUs in the DB are being matched against the hyundai brand, when we could match more (like cornerRadar, adas, etc)
[x] Put FW versions inside the config for lots of nice helpers in one place - https://github.com/commaai/openpilot/pull/27691 - started with https://github.com/commaai/openpilot/pull/31227
[x] Consider bus when matching brands - https://github.com/commaai/openpilot/pull/27657
[ ] Sort requests by OBD multiplexing mode, so brands with auto-generated auxiliary requests don't switch unnecessarily because of bad sorting - not really needed at this moment - https://github.com/commaai/openpilot/pull/31263
[ ] Group first subaddress query into parallel query group (reduces total queries by 1) - https://github.com/commaai/openpilot/pull/28675
[x] Only update OBD multiplexing mode when on bus % 1 == 1 - https://github.com/commaai/openpilot/pull/27669
[x] Test that measures the time to query FW versions in the worse case. For each brand and overall. Fail when there is a large number of queries or OBD multiplexing mode switches (soon to be optimized) - mostly this: https://github.com/commaai/openpilot/pull/27753
[ ] Batch tester present for finding present ecus (if required). We now send tester present to 79 addresses at once
[x] ~Filter out addresses that are already on the bus before we start to query, they are likely broadcast addrs and we shouldn't extend our timeouts for them~
[x] Break out of IsoTpParallelQuery loop once either all addresses are timed out or done. Right now, We break if all addrs are timed out or done. For example, we wouldn't break if addr A is timed out and not done, and addr B is done and not timed out - https://github.com/commaai/openpilot/pull/27764
[x] Only extend addr timeout in IsoTpParallelQuery if we got a valid consecutive frame - https://github.com/commaai/openpilot/pull/27762
[x] commaai/openpilot#28615
[x] IsoTpParallelQuery: extend timeout for every frame type (re-do of https://github.com/commaai/openpilot/pull/29654)

More related to fingerprint matching:

[ ] https://github.com/commaai/opendbc/issues/1095

MoreTore commented 1 year ago

Currently trying to debug a vehicle and I'm wondering if you could take a look. Here is a drive 539cfef88f5a6208|2023-04-05--13-49-04. It's a Mazda CX50 and I can't get it to completely fingerprint. I am seeing 4 unique ecu responses (see below) but I assume since 3 of them are exclude_types, it's not matching. In values.py I have engine and transmission FW_VERSIONS defined(see below). Apparently, their Comma 3 was not shipped with a comma power, however, bus 1 should be connected to the OBD port at the BCM (see this wiring diagram below). I am going to try a few more things before I throw in the towel and just make them force the fingerprint.

CAN 0/2 is used for communication between the following modules: ― BCM ― EPS control module ― Adaptive front lighting system (AFS) control module/auto leveling control module ― SAS control module ― DSC HU/CM ― TCM ― PCM

'carFw': [{'address': 1840, 'brand': 'ford', 'bus': 0, 'ecu': 'eps', 'fwVersion': b'VA40-3216Y-B\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00', 'logging': False, 'obdMultiplexing': False, 'request': [b'>\x00', b'"\xf1\x88'], 'responseAddress': 1848, 'subAddress': 0}, {'address': 1798, 'brand': 'ford', 'bus': 0, 'ecu': 'fwdCamera', 'fwVersion': b'VA40-67WK2-\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00', 'logging': False, 'obdMultiplexing': False, 'request': [b'>\x00', b'"\xf1\x88'], 'responseAddress': 1806, 'subAddress': 0}, {'address': 1892, 'brand': 'ford', 'bus': 0, 'ecu': 'fwdRadar', 'fwVersion': b'VA45-67XK2-\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00', 'logging': False, 'obdMultiplexing': False, 'request': [b'>\x00', b'"\xf1\x88'], 'responseAddress': 1900, 'subAddress': 0}, {'address': 1888, 'brand': 'ford', 'bus': 0, 'ecu': 'abs', 'fwVersion': b'VA40-4300F-D\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00', 'logging': False, 'obdMultiplexing': False, 'request': [b'>\x00', b'"\xf1\x88'], 'responseAddress': 1896, 'subAddress': 0}],

This is the values.py

CAR.CX_50 : { (Ecu.eps, 0x730, None): [ b'VA40-3216Y-B\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00', ], (Ecu.engine, 0x7e0, None): [ b'PX06-188K2-N\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00', b'PX08-188K2-L\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00', b'PX4W-188K2-C\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00', ], (Ecu.fwdRadar, 0x764, None): [ b'VA45-67XK2-\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00', ], (Ecu.abs, 0x760, None): [ b'VA40-4300F-D\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00', ], (Ecu.fwdCamera, 0x706, None): [ b'VA40-67WK2-\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00', ], (Ecu.transmission, 0x7e1, None): [ b'PX01-21PS1-E\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00', b'PX03-21PS1-E\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00', b'PX4K-21PS1-B\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00', ], },

incognitojam commented 1 year ago

@MoreTore The carFw you posted shows responses for brand: 'ford', so it won't fingerprint as a Mazda since there are no responses for Mazda FW queries. You should try adding those queries to Mazda's FP config.

MoreTore commented 1 year ago

try adding those queries to Mazda's FP config.

Thank you. I didn't understand what was going on there. I will try that.

MoreTore commented 1 year ago

@incognitojam

Using this as my FW_QUERY_CONFIG:

FW_QUERY_CONFIG = FwQueryConfig(
  requests=[
    Request(
      [StdQueries.MANUFACTURER_SOFTWARE_VERSION_REQUEST],
      [StdQueries.MANUFACTURER_SOFTWARE_VERSION_RESPONSE],
    ),
    Request( # from ford config
      [StdQueries.TESTER_PRESENT_REQUEST, StdQueries.MANUFACTURER_SOFTWARE_VERSION_REQUEST], #  [b'>\x00', b'"\xf1\x88']
      [StdQueries.TESTER_PRESENT_RESPONSE, StdQueries.MANUFACTURER_SOFTWARE_VERSION_RESPONSE],
      bus=0,
      whitelist_ecus=[Ecu.engine, Ecu.transmission],
    ),
  ],
)

And this as the FW_VERSIONS (engine and transmission are sometimes missing):

CAR.CX_50 : {
    (Ecu.eps, 0x730, None): [
      b'VA40-3216Y-B\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00',
    ],
    (Ecu.engine, 0x7e0, None): [
      b'PX06-188K2-N\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00',
      b'PX08-188K2-L\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00',
      b'PX4W-188K2-C\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00',
    ],
    (Ecu.fwdRadar, 0x764, None): [
      b'VA45-67XK2-\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00',
    ],
    (Ecu.abs, 0x760, None): [
      b'VA40-4300F-D\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00',
    ],
    (Ecu.fwdCamera, 0x706, None): [
      b'VA40-67WK2-\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00',
    ],
    (Ecu.transmission, 0x7e1, None): [
      b'PX01-21PS1-E\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00',
      b'PX03-21PS1-E\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00',
      b'PX4K-21PS1-B\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00',
    ],
  },

It doesn't fingerprint.

Old style: MAZDA CX-50 Vin 00000000000000000
New style (exact): set()
New style (fuzzy): set()
  Brand: ford, bus: 0 - (Ecu.eps, 0x730, None): [b'VA40-3216Y-B\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00'],
  Brand: ford, bus: 0 - (Ecu.fwdCamera, 0x706, None): [b'VA40-67WK2-\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00'],
  Brand: ford, bus: 0 - (Ecu.fwdRadar, 0x764, None): [b'VA45-67XK2-\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00'],
  Brand: ford, bus: 0 - (Ecu.abs, 0x760, None): [b'VA40-4300F-D\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00'],
Mismatches

Number of dongle ids checked: 1
Fingerprinted:                0
Not fingerprinted:            1
  of which had a fuzzy match: 0

Correct fuzzy matches:        0
Wrong fuzzy matches:          0

This leads me to believe that the config is not working the way I think it is. However, if I change the FW_VERSIONS to this:

CAR.CX_50 : {
    (Ecu.eps, 0x730, None): [
      b'VA40-3216Y-B\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00',
    ],
    (Ecu.fwdRadar, 0x764, None): [
      b'VA45-67XK2-\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00',
    ],
    (Ecu.abs, 0x760, None): [
      b'VA40-4300F-D\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00',
    ],
    (Ecu.fwdCamera, 0x706, None): [
      b'VA40-67WK2-\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00',
    ],
  },

I get an exact match which makes sense:

Number of dongle ids checked: 1
Fingerprinted:                1
Not fingerprinted:            0
  of which had a fuzzy match: 0

Correct fuzzy matches:        0
Wrong fuzzy matches:          0

My question is now, how do i format the request so that I get an exact match but exclude the engine and transmission since they do not respond on some CX-30/50? For now, I will remove the engine and trans.

sshane commented 1 year ago

When ECUs don't respond and you expect them to, you can check errorLogMessage in useradmin and see what error is being printed for that ECU. If the rx address (usually + 8) exists in ecu responses, you usually just need a different data id for that car and ECU.

MoreTore commented 1 year ago

When ECUs don't respond and you expect them to, you can check errorLogMessage in useradmin and see what error is being printed for that ECU. If the rx address (usually + 8) exists in ecu responses, you usually just need a different data id for that car and ECU.

Well yes I see errors but I have very little understanding of the whole of what to do.

{'errorLogMessage': '{"msg": "vin query retry (1) ...", "ctx": {"daemon": "controlsd", "dongle_id": "539cfef88f5a6208", "version": "0.9.2", "dirty": '
                    'true, "device": "tici"}, "level": "ERROR", "levelnum": 40, "name": "swaglog", "filename": "car_helpers.py", "lineno": 101, '
                    '"pathname": "/data/openpilot/selfdrive/car/car_helpers.py", "module": "car_helpers", "funcName": "fingerprint", "host": "tici", '
                    '"process": 46510, "thread": 547969865552, "threadName": "MainThread", "created": 1680716945.1326694}',
 'logMonoTime': 56865860445,
 'valid': True}

....

{'errorLogMessage': '{"msg": "Error processing UDS response: (1882, None)", "ctx": {"daemon": "controlsd", "dongle_id": "539cfef88f5a6208", '
                    '"version": "0.9.2", "dirty": true, "device": "tici"}, "exc_info": "Traceback (most recent call last):\\n  File '
                    '\\"/data/openpilot/selfdrive/car/isotp_parallel_query.py\\", line 108, in get_data\\n    dat, updated = msg.recv()\\n  File '
                    '\\"/data/openpilot/panda/python/uds.py\\", line 445, in recv\\n    self._isotp_rx_next(msg)\\n  File '
                    '\\"/data/openpilot/panda/python/uds.py\\", line 502, in _isotp_rx_next\\n    assert not self.tx_done, \\"isotp - rx: flow '
                    'control with no active frame\\"\\nAssertionError: isotp - rx: flow control with no active frame", "level": "ERROR", "levelnum": '
                    '40, "name": "swaglog", "filename": "isotp_parallel_query.py", "lineno": 110, "pathname": '
                    '"/data/openpilot/selfdrive/car/isotp_parallel_query.py", "module": "isotp_parallel_query", "funcName": "get_data", "host": '
                    '"tici", "process": 46510, "thread": 547969865552, "threadName": "MainThread", "created": 1680716946.9520495}',
 'logMonoTime': 58696144038,
 'valid': True}
{'errorLogMessage': '{"msg": "Error processing UDS response: (1882, None)", "ctx": {"daemon": "controlsd", "dongle_id": "539cfef88f5a6208", '
                    '"version": "0.9.2", "dirty": true, "device": "tici"}, "exc_info": "Traceback (most recent call last):\\n  File '
                    '\\"/data/openpilot/selfdrive/car/isotp_parallel_query.py\\", line 108, in get_data\\n    dat, updated = msg.recv()\\n  File '
                    '\\"/data/openpilot/panda/python/uds.py\\", line 445, in recv\\n    self._isotp_rx_next(msg)\\n  File '
                    '\\"/data/openpilot/panda/python/uds.py\\", line 502, in _isotp_rx_next\\n    assert not self.tx_done, \\"isotp - rx: flow '
                    'control with no active frame\\"\\nAssertionError: isotp - rx: flow control with no active frame", "level": "ERROR", "levelnum": '
                    '40, "name": "swaglog", "filename": "isotp_parallel_query.py", "lineno": 110, "pathname": '
                    '"/data/openpilot/selfdrive/car/isotp_parallel_query.py", "module": "isotp_parallel_query", "funcName": "get_data", "host": '
                    '"tici", "process": 46510, "thread": 547969865552, "threadName": "MainThread", "created": 1680716947.0009077}',
 'logMonoTime': 58727973257,
 'valid': True}
{'errorLogMessage': '{"msg": "iso-tp query bad response: (2016, None) - 0x7f2231", "ctx": {"daemon": "controlsd", "dongle_id": "539cfef88f5a6208", '
                    '"version": "0.9.2", "dirty": true, "device": "tici"}, "level": "ERROR", "levelnum": 40, "name": "swaglog", "filename": '
                    '"fw_versions.py", "lineno": 294, "pathname": "/data/openpilot/selfdrive/car/fw_versions.py", "module": "fw_versions", '
                    '"funcName": "get_fw_versions", "host": "tici", "process": 46510, "thread": 547969865552, "threadName": "MainThread", "created": '
                    '1680716947.0209897}',
 'logMonoTime': 58748993205,
 'valid': True}
{'errorLogMessage': '{"msg": "iso-tp query bad response: (2017, None) - 0x7f2231", "ctx": {"daemon": "controlsd", "dongle_id": "539cfef88f5a6208", '
                    '"version": "0.9.2", "dirty": true, "device": "tici"}, "level": "ERROR", "levelnum": 40, "name": "swaglog", "filename": '
                    '"fw_versions.py", "lineno": 294, "pathname": "/data/openpilot/selfdrive/car/fw_versions.py", "module": "fw_versions", '
                    '"funcName": "get_fw_versions", "host": "tici", "process": 46510, "thread": 547969865552, "threadName": "MainThread", "created": '
                    '1680716947.0216691}',
 'logMonoTime': 58749541226,
 'valid': True}
{'errorLogMessage': '{"msg": "iso-tp query bad response: (2016, None) - 0x7f2231", "ctx": {"daemon": "controlsd", "dongle_id": "539cfef88f5a6208", '
                    '"version": "0.9.2", "dirty": true, "device": "tici"}, "level": "ERROR", "levelnum": 40, "name": "swaglog", "filename": '
                    '"fw_versions.py", "lineno": 294, "pathname": "/data/openpilot/selfdrive/car/fw_versions.py", "module": "fw_versions", '
                    '"funcName": "get_fw_versions", "host": "tici", "process": 46510, "thread": 547969865552, "threadName": "MainThread", "created": '
                    '1680716947.1305494}',
 'logMonoTime': 58859337997,
 'valid': True}
{'errorLogMessage': '{"msg": "iso-tp query bad response: (2017, None) - 0x7f2231", "ctx": {"daemon": "controlsd", "dongle_id": "539cfef88f5a6208", '
                    '"version": "0.9.2", "dirty": true, "device": "tici"}, "level": "ERROR", "levelnum": 40, "name": "swaglog", "filename": '
                    '"fw_versions.py", "lineno": 294, "pathname": "/data/openpilot/selfdrive/car/fw_versions.py", "module": "fw_versions", '
                    '"funcName": "get_fw_versions", "host": "tici", "process": 46510, "thread": 547969865552, "threadName": "MainThread", "created": '
                    '1680716947.131053}',
 'logMonoTime': 58859879976,
 'valid': True}
{'errorLogMessage': '{"msg": "iso-tp query bad response: (1840, None) - 0x7f2231", "ctx": {"daemon": "controlsd", "dongle_id": "539cfef88f5a6208", '
                    '"version": "0.9.2", "dirty": true, "device": "tici"}, "level": "ERROR", "levelnum": 40, "name": "swaglog", "filename": '
                    '"fw_versions.py", "lineno": 294, "pathname": "/data/openpilot/selfdrive/car/fw_versions.py", "module": "fw_versions", '
                    '"funcName": "get_fw_versions", "host": "tici", "process": 46510, "thread": 547969865552, "threadName": "MainThread", "created": '
                    '1680716948.0305827}',
 'logMonoTime': 59757094246,
 'valid': True}
{'errorLogMessage': '{"msg": "iso-tp query bad response: (2016, None) - 0x7f3e13", "ctx": {"daemon": "controlsd", "dongle_id": "539cfef88f5a6208", '
                    '"version": "0.9.2", "dirty": true, "device": "tici"}, "level": "ERROR", "levelnum": 40, "name": "swaglog", "filename": '
                    '"fw_versions.py", "lineno": 294, "pathname": "/data/openpilot/selfdrive/car/fw_versions.py", "module": "fw_versions", '
                    '"funcName": "get_fw_versions", "host": "tici", "process": 46510, "thread": 547969865552, "threadName": "MainThread", "created": '
                    '1680716948.2595744}',
 'logMonoTime': 59985483673,
 'valid': True}
{'errorLogMessage': '{"msg": "iso-tp query bad response: (2016, None) - 0x7f2231", "ctx": {"daemon": "controlsd", "dongle_id": "539cfef88f5a6208", '
                    '"version": "0.9.2", "dirty": true, "device": "tici"}, "level": "ERROR", "levelnum": 40, "name": "swaglog", "filename": '
                    '"fw_versions.py", "lineno": 294, "pathname": "/data/openpilot/selfdrive/car/fw_versions.py", "module": "fw_versions", '
                    '"funcName": "get_fw_versions", "host": "tici", "process": 46510, "thread": 547969865552, "threadName": "MainThread", "created": '
                    '1680716948.4003496}',
 'logMonoTime': 60127118204,
 'valid': True}
{'errorLogMessage': '{"msg": {"event": "fingerprinted", "car_fingerprint": null, "source": 0, "fuzzy": false, "cached": false, "fw_count": 4, '
                    '"ecu_responses": [[2025, null, 0], [2024, null, 0], [1896, null, 0], [1900, null, 0], [1806, null, 0], [1848, null, 0]], '
                    '"vin_rx_addr": 0, "error": true}, "ctx": {"daemon": "controlsd", "dongle_id": "539cfef88f5a6208", "version": "0.9.2", "dirty": '
                    'true, "device": "tici"}, "level": "ERROR", "levelnum": 40, "name": "swaglog", "filename": "car_helpers.py", "lineno": 174, '
                    '"pathname": "/data/openpilot/selfdrive/car/car_helpers.py", "module": "car_helpers", "funcName": "fingerprint", "host": "tici", '
                    '"process": 46510, "thread": 547969865552, "threadName": "MainThread", "created": 1680716951.6599796}',
 'logMonoTime': 63388235078,
 'valid': True}

Unfortunately it's basically gibberish to me. You have spent lots of time on this stuff so you know how it works. I have looked at the fw querying code for about 2 hours and I thought it was the fuzzy fingerprint issue so clearly I have no idea what I'm doing

adeebshihadeh commented 5 months ago

@sshane any of this still relevant after all the recent FP work?

sshane commented 5 months ago

The remaining here are more for my reference for future improvements (not needed now).

commaai / openpilot

FPv2 improvements #27665