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

safety: RX hook should not consider unknown message valid #1656

Closed
sshane opened this issue Sep 7, 2023 · 3 comments · Fixed by #1658
Closed

safety: RX hook should not consider unknown message valid #1656

sshane opened this issue Sep 7, 2023 · 3 comments · Fixed by #1658
Labels
bug car safety vehicle-specific safety code

Comments

@sshane
Copy link
Contributor

sshane commented Sep 7, 2023

Related to #798, but not exact same issue.

We are not checking all gas pedal signal types when using hyundai_canfd_radar_scc_addr_checks. We're completely ignoring the ICE pedal signal. Not caught because addr_safety_check returns true if it's a message it is not checking/not expected.

It's actually more confusing than I thought:

Despite the fact that ICE cars can be radar-SCC cars, we only ever use the ICE address checks in panda, which didn't have the SCC_CONTROL check (from #1031) which would have caught this.

@sshane sshane added bug car safety vehicle-specific safety code labels Sep 7, 2023
@sshane
Copy link
Contributor Author

sshane commented Sep 7, 2023

There's another missing address check:

On the Genesis GV60 Electric, we use the normal buttons in openpilot, but the alt buttons address exists on the bus, so we select that on occasion, not checking the normal buttons any more.

image

@sshane
Copy link
Contributor Author

sshane commented Sep 7, 2023

Checking other brands, there's also a lot of other addresses we're missing:

FAILED test_models.py::TestCarModel_12_CHEVROLET_BOLT_EUV_2022::test_panda_safety_carstate - AssertionError: 1 is not false : panda safety doesn't agree with openpilot: {'regenBraking': 312}
FAILED test_models.py::TestCarModel_40_HONDA_ACCORD_2018::test_panda_safety_carstate - AssertionError: 1 is not false : panda safety doesn't agree with openpilot: {'brakePressed': 2431}
FAILED test_models.py::TestCarModel_0_ACURA_ILX_2016::test_panda_safety_carstate - AssertionError: 1 is not false : panda safety doesn't agree with openpilot: {'gasPressed': 5462}
FAILED test_models.py::TestCarModel_45_HONDA_CIVIC_2016::test_panda_safety_carstate - AssertionError: 2 is not false : panda safety doesn't agree with openpilot: {'gasPressed': 1978, 'controlsAllowed': 4}
FAILED test_models.py::TestCarModel_2_ACURA_RDX_2020::test_panda_safety_carstate - AssertionError: 1 is not false : panda safety doesn't agree with openpilot: {'brakePressed': 520}
FAILED test_models.py::TestCarModel_16_CHEVROLET_VOLT_PREMIER_2017::test_panda_safety_carstate - AssertionError: 1 is not false : panda safety doesn't agree with openpilot: {'regenBraking': 30}
FAILED test_models.py::TestCarModel_39_HONDA_ACCORD_2018::test_panda_safety_carstate - AssertionError: 1 is not false : panda safety doesn't agree with openpilot: {'brakePressed': 823}
FAILED test_models.py::TestCarModel_54_HONDA_FIT_2018::test_panda_safety_carstate - AssertionError: 2 is not false : panda safety doesn't agree with openpilot: {'gasPressed': 2573, 'controlsAllowed': 2}
FAILED test_models.py::TestCarModel_63_HONDA_RIDGELINE_2017::test_panda_safety_carstate - AssertionError: 1 is not false : panda safety doesn't agree with openpilot: {'gasPressed': 12}
FAILED test_models.py::TestCarModel_49_HONDA_CR_V_2017::test_panda_safety_carstate - AssertionError: 1 is not false : panda safety doesn't agree with openpilot: {'brakePressed': 3661}
FAILED test_models.py::TestCarModel_33_GENESIS_GV60_ELECTRIC_1ST_GEN::test_panda_safety_carstate - AssertionError: 1 is not false : panda safety doesn't agree with openpilot: {'controlsAllowed': 9}
FAILED test_models.py::TestCarModel_38_HONDA_ACCORD_2018::test_panda_safety_carstate - AssertionError: 1 is not false : panda safety doesn't agree with openpilot: {'brakePressed': 1084}
FAILED test_models.py::TestCarModel_56_HONDA_HR_V_2023::test_panda_safety_carstate - AssertionError: 1 is not false : panda safety doesn't agree with openpilot: {'brakePressed': 844}
FAILED test_models.py::TestCarModel_197_TOYOTA_COROLLA_2017::test_panda_safety_carstate - AssertionError: 1 is not false : panda safety doesn't agree with openpilot: {'gasPressed': 1749}
FAILED test_models.py::TestCarModel_208_TOYOTA_RAV4_2017::test_panda_safety_carstate - AssertionError: 1 is not false : panda safety doesn't agree with openpilot: {'gasPressed': 137}
FAILED test_models.py::TestCarModel_216_TOYOTA_SIENNA_2018::test_panda_safety_carstate - AssertionError: 1 is not false : panda safety doesn't agree with openpilot: {'brakePressed': 184}

@sshane
Copy link
Contributor Author

sshane commented Sep 7, 2023

Duplicate of #727

@sshane sshane marked this as a duplicate of #727 Sep 7, 2023
@sshane sshane closed this as not planned Won't fix, can't repro, duplicate, stale Sep 7, 2023
@sshane sshane linked a pull request Sep 7, 2023 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug car safety vehicle-specific safety code
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant