commaai / commaai/opendbc

Tesla: Autopark init variable breaks safety replay

Open
#2,768 2 comments 1 reaction 0 assignees View on GitHub
Dominant language
Python
Stars
3.4k
Forks
2.3k
Avg merge
3d 2h
Merged PRs (30d)
38

Description

Many thanks to @sunnyhaibin for discovering this issue, they asked me to raise it here.

To allow autopark to work correctly, we assume that it is always active when openpilot first boots up, until we hear a message from the canbus saying otherwise, to prevent it blocking stock steering commands: https://github.com/commaai/opendbc/blob/84d1d15b5b22d2268058b43018574654215e3852/opendbc/safety/modes/tesla.h#L340-L343

But it seems that doing this breaks the `./opendbc/safety/tests/safety_replay/replay_drive.py` script as it always faults with the following error message on any Tesla route, including this one from the test routes: https://github.com/commaai/opendbc/blob/84d1d15b5b22d2268058b43018574654215e3852/opendbc/car/tests/routes.py#L323

```
(openpilot) amy@Amy-PC:~/git/openpilot$ ./opendbc/safety/tests/safety_replay/replay_drive.py 7dc058789994da80/00000112--adb970f6a8
replaying 7dc058789994da80/00000112--adb970f6a8 with safety mode 10, param 0, alternative experience 0
Traceback (most recent call last):
File "/home/amy/git/openpilot/./opendbc/safety/tests/safety_replay/replay_drive.py", line 99, in
replay_drive(list(lr), args.mode, args.param, args.alternative_experience)
File "/home/amy/git/openpilot/./opendbc/safety/tests/safety_replay/replay_drive.py", line 20, in replay_drive
init_segment(safety, msgs, safety_mode, param)
File "/home/amy/git/openpilot/opendbc/safety/tests/safety_replay/helpers.py", line 109, in init_segment
assert safety.safety_tx_hook(msg), "failed to initialize panda safety for segment"
^^^^^^^^^^^^^^^^^^^^^^^^^^
AssertionError: failed to initialize panda safety for segment
```

If we instead set it to `tesla_autopark = false;` in the code above, it works as expected.

```
(openpilot) amy@Amy-PC:~/git/openpilot$ ./opendbc/safety/tests/safety_replay/replay_drive.py 7dc058789994da80/00000112--adb970f6a8
replaying 7dc058789994da80/00000112--adb970f6a8 with safety mode 10, param 0, alternative experience 0
100%|█████████████████████████████████████████████████████████████| 135146/135146 [00:13<00:00, 9808.01it/s]

RX
total rx msgs: 4335547
invalid rx msgs: 0
safety tick rx invalid: False
invalid addrs: set()

TX
total openpilot msgs: 40471
total msgs with controls allowed: 28233
blocked msgs: 1
blocked with controls allowed: 0
blocked addrs: Counter({1160: 1})
```

In the safety test helpers, we actually do disable it here already: https://github.com/commaai/opendbc/blob/84d1d15b5b22d2268058b43018574654215e3852/opendbc/safety/tests/libsafety/safety_helpers.h#L190-L191

So I am thinking we might need something similar to this in the safety replay code, or maybe for Tesla specifically assume that the TX hook will fail and don't assert on it here, which is where the error is coming from: https://github.com/commaai/opendbc/blob/84d1d15b5b22d2268058b43018574654215e3852/opendbc/safety/tests/safety_replay/helpers.py#L109

Contributor guide

No contributing guide indexed for this repository

Research direction

Start with opendbc/safety/tests/safety_replay/replay_drive.py and helpers.py, especially init_segment where Tesla replay fails, then compare the initialization in opendbc/safety/tests/libsafety/safety_helpers.h with the Tesla safety code in tesla.h. Reproduce the failure with the listed Tesla route and verify that replay completes without the initialization assertion while the existing safety-test setup remains correct.

Written by the indexing model from the issue text.

Assessment

Tech stack
c, python
Domain
testing-qa
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.