Skip to content
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

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

Open
Tracked by #27665
sshane opened this issue Jun 10, 2023 · 8 comments
Open
Tracked by #27665
Labels
bug car related to opendbc/car/ fingerprint

Comments

@sshane
Copy link
Contributor

sshane commented Jun 10, 2023

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: commaai/openpilot#27126

@sshane sshane added bug fingerprint car related to opendbc/car/ labels Jun 10, 2023
@sshane sshane changed the title fuzzy fingerprinting: can match to a car while missing crucial ECUs fuzzy fingerprinting: false positive matches with missing critical ECUs Jun 10, 2023
@sshane
Copy link
Contributor Author

sshane commented Jun 10, 2023

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
Copy link
Contributor

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

@jayvin100
Copy link

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
Copy link
Contributor

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

@jayvin100
Copy link

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
Copy link
Contributor Author

sshane commented Jun 16, 2023

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
Copy link
Contributor

@sshane isn't this fixed?

@sshane
Copy link
Contributor Author

sshane commented Apr 15, 2024

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 commaai/openpilot#28481

@sshane sshane transferred this issue from commaai/openpilot Aug 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug car related to opendbc/car/ fingerprint
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants