commaai / openpilot

openpilot is an open source driver assistance system. openpilot performs the functions of Automated Lane Centering and Adaptive Cruise Control for 250+ supported car makes and models.
https://comma.ai/
MIT License
48.72k stars 8.84k forks source link

fuzzy fingerprinting: false positive matches with missing critical ECUs #28483

Open sshane opened 1 year ago

sshane commented 1 year ago

For example, if the radar or camera failed to respond or does not exist (or any other important ECU), but we get enough matches with other ECUs, we would match when we really shouldn't.

this is already masking a fingerprinting/car/hardware bug with a 2022 Civic where the camera fails to respond, but we match any way: https://github.com/commaai/openpilot/pull/27126

sshane commented 1 year ago

Just looking into that case since we would fail to fingerprint for that user after this change.

@jayvin100 I would recommend trying a different OBD-C cable or ethernet cable since this looks to be a hardware issue. panda drops some messages on both OBD 2 port and powertrain bus right around when we expect responses from the camera ECU.

8fcafab4167b8c6c|2023-06-08--12-24-23: image

Nothing back from ECU:

image

@robbederks does this look like a harness box issue or cable issue?

robbederks commented 1 year ago

Does this work on other 2022 civics? Strange that there would be a drop on two buses

jayvin100 commented 1 year ago

Just looking into that case since we would fail to fingerprint for that user after this change.

@jayvin100 I would recommend trying a different OBD-C cable or ethernet cable since this looks to be a hardware issue. panda drops some messages on both OBD 2 port and powertrain bus right around when we expect responses from the camera ECU.

8fcafab4167b8c6c|2023-06-08--12-24-23: image

Nothing back from ECU:

image

@robbederks does this look like a harness box issue or cable issue?

Battery just died, got a fresh one installed. Not sure if that could cause the issue also am on sunnypilot running nicki model, shall I install back stock openpilot to see if this issue persist?

adeebshihadeh commented 1 year ago

Yes, make sure you can repro on stock openpilot. Do a bunch of these users have a community built harness or something?

jayvin100 commented 1 year ago

Yes, make sure you can repro on stock openpilot. Do a bunch of these users have a community built harness or something?

Have been running master-ci since this issue was brought up last week. It's has been close to a week now, is it giving the expected responses? @sshane

sshane commented 1 year ago

Yes, this is still happening on master-ci for you. I checked and about 8 Civic 2022 user are intermittently missing the radar, so not sure what's going on here. I'm seeing totalRxLostCnt and totalTxLostCnt rise on startup, which could the the radar's first frame and some of our query frames.

We're looking into it! In the mean time, can you try a different cable, or flip the orientation on the harness box side?

adeebshihadeh commented 3 months ago

@sshane isn't this fixed?

sshane commented 3 months ago

this issue? no. the civic blocker? yes. i need to check some data to make sure other cars aren't relying on this and we can merge https://github.com/commaai/openpilot/pull/28481