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: only run rx hooks on whitelisted msgs #1903

Merged
merged 57 commits into from
Mar 12, 2025
Merged

Conversation

sshane
Copy link
Contributor

@sshane sshane commented Mar 3, 2025

#1851 but for rx

catches a few missing rx checks:

FAILED test_models.py::TestCarModel_14_CHEVROLET_BOLT_EUV::test_panda_safety_carstate@CarTestRoute(route='f08912a233c1584f|2022-08-11--18-02-41', car_model=<CAR.CHEVROLET_BOLT_EUV>, segment=1) - AssertionError: 1 is not false : panda safety doesn't agree with openpilot: {'regenBraking': 312}
FAILED test_models.py::TestCarModel_20_CHEVROLET_VOLT::test_panda_safety_carstate@CarTestRoute(route='c950e28c26b5b168|2018-05-30--22-03-41', car_model=<CAR.CHEVROLET_VOLT>, segment=None) - AssertionError: 1 is not false : panda safety doesn't agree with openpilot: {'regenBraking': 30}
FAILED test_models.py::TestCarModel_43_GENESIS_GV60_EV_1ST_GEN::test_panda_safety_carstate@CarTestRoute(route='b5d6dc830ad63071|2022-12-12--21-28-25', car_model=<CAR.GENESIS_GV60_EV_1ST_GEN>, segment=12) - AssertionError: 1 is not false : panda safety doesn't agree with openpilot: {'controlsAllowed': 9}
FAILED test_models.py::TestCarModel_183_RIVIAN_R1_GEN1::test_panda_safety_carstate_fuzzy@CarTestRoute(route='bc095dc92e101734/000000db--ee9fe46e57', car_model=<CAR.RIVIAN_R1_GEN1>, segment=None) - IndexError: list index out of range
FAILED test_models.py::TestCarModel_241_TOYOTA_RAV4_PRIME::test_panda_safety_carstate@CarTestRoute(route='20ba9ade056a8c7b|2021-02-08--21-57-35', car_model=<CAR.TOYOTA_RAV4_PRIME>, segment=None) - AssertionError: 1 is not false : panda safety doesn't agree with openpilot: {'gasPressed': 349}
FAILED test_models.py::TestCarModel_247_TOYOTA_SIENNA::test_panda_safety_carstate@CarTestRoute(route='eb6acd681135480d|2019-06-20--20-00-00', car_model=<CAR.TOYOTA_SIENNA>, segment=None) - AssertionError: 1 is not false : panda safety doesn't agree with openpilot: {'brakePressed': 184}

@sshane sshane changed the title Only run rx hooks on msgs in safety_config.rx_msgs safety: only run rx hooks on whitelisted msgs Mar 3, 2025
@github-actions github-actions bot added the car safety vehicle-specific safety code label Mar 3, 2025
@sshane sshane added this to the openpilot 0.9.8 milestone Mar 3, 2025
@sshane sshane requested a review from adeebshihadeh March 7, 2025 08:04
@sshane
Copy link
Contributor Author

sshane commented Mar 7, 2025

Just did the structure so far, will fix the bugs in another PR first. @adeebshihadeh Did you have any thoughts about how this is done? Maybe it's cleaner if this hook returns a bool like the others, so true if it detects a relay malfunction and false if not, instead of calling this generic_rx_checks everywhere.

@@ -296,6 +301,7 @@ static safety_config chrysler_init(uint16_t param) {
const safety_hooks chrysler_hooks = {
.init = chrysler_init,
.rx = chrysler_rx_hook,
.rx_relay_malfunction = chrysler_rx_relay_malfunction_hook,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
.rx_relay_malfunction = chrysler_rx_relay_malfunction_hook,
.stock_ecu_hook = chrysler_stock_ecu_hook,

?

@sshane
Copy link
Contributor Author

sshane commented Mar 8, 2025

There's actually failures in 7 safety modes, and the Hyundai ones seem the most complex to fix. We might need to clean up the safety first. Going to fix the latest one only for the release (Rivian): #1949

@jyoung8607 Toyota SEC OC doesn't define different RX checks, so it's missing some of the new addresses, can you fix?

@github-actions github-actions bot added car related to opendbc/car/ gm labels Mar 11, 2025
@github-actions github-actions bot removed car related to opendbc/car/ gm labels Mar 12, 2025
@sshane sshane marked this pull request as ready for review March 12, 2025 06:29
@sshane sshane merged commit febbf0f into master Mar 12, 2025
8 checks passed
@sshane sshane deleted the rx-hook-whitelist branch March 12, 2025 06:29
chrispypatt pushed a commit to chrispypatt/opendbc that referenced this pull request Mar 12, 2025
chrispypatt pushed a commit to chrispypatt/opendbc that referenced this pull request Mar 12, 2025
* don't run rx hook on non-allowed messages

* better name

* fix toyota (bug w/ secoc)

* looks like honda is broken

* rivian is also bad! (missing EPAS_SystemStatus)

* misra so far

rivian is also bad!

* nissan is fine

* tesla is also borked

* mazda's good

* subaru's fine

* gm broke

* ford's good

* chrysler's good

* vw is good

* hyundai is broky, canfd is good

* Fix Rivian

* revert these

* do relay malfunction check on all addresses

* Found a Tesla bug

* fix subaru pg

* body

* rm

* Fix Honda

* stash

* fix Hyundai

* fix

* Hyundai: buttons are used always (for interaction)

* revert tesla

* body: we don't rx _torque_cmd_msg

* Revert "body: we don't rx _torque_cmd_msg"

This reverts commit 2f973f6.

* simpler

* GM EV param for correct rxchecks

* no need

* might read better

* rm extras

* fix hyundai

* we weren't testing lfa (non-hda2), alt buttons, long

* fix

* tested

* rm

* not needed

* clean up

* that too

* .
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
car safety vehicle-specific safety code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants