Skip to content

params: clear the keys pandad reads at the offroad transition too - #38780

Open
SpysyWeeb wants to merge 21 commits into
commaai:masterfrom
SpysyWeeb:params-clear-pandad-keys
Open

SpysyWeeb wants to merge 21 commits into
commaai:masterfrom
SpysyWeeb:params-clear-pandad-keys

Conversation

@SpysyWeeb

Copy link
Copy Markdown

pandad's safety setter runs off deviceState.started and reads FirmwareQueryDone, ControlsReady and CarParams. manager clears them at the onroad transition, but it reacts to the same deviceState edge as pandad with no ordering between the two, so pandad can read the previous drive's values first, find everything ready, and apply the car safety mode from stale CarParams.

Issue #28050 described this race and #28151 fixed it in 2023 by starting the safety setter off an IsOnroad param that manager wrote only after its own clear. #37999 moved pandad's onroad signal to a live deviceState subscription, which dropped that guarantee, and #38205 then removed the unused param. The firmware query hid the regression: 5 to 9 s in ELM327 before the car safety mode meant nothing stale was ever acted on. A car whose VIN is cached skips the query and exposes it.

This adds CLEAR_ON_OFFROAD_TRANSITION to those three keys, so nothing stale is left for the next ignition whichever process reacts first, and a params test for it. The other CLEAR_ON_ONROAD_TRANSITION keys are left alone: the Offroad_* alerts are written onroad to be shown offroad.

Seen on a 2021 Hyundai Palisade (comma 3X) with openpilot longitudinal: the first ignition with a cached fingerprint applied the hyundai safety mode from the first logged frame, before card had disabled the stock radar, so the radar's SCC11/12/14 on bus 0 tripped the relay check and it stayed latched for the drive. The harness relay itself was fine, LKAS11 never left bus 2. With this change, every cached ignition since (two power cycles a day for three days) applied the safety mode 7 to 12 s after the route began, after card had written the drive's own CarParams, with no fault.

Routes: ff9dba54b42b3afa|00000043--b7fb6bc078 (the failure); ff9dba54b42b3afa|00000051--39b47d07c0 and ff9dba54b42b3afa|00000053--cfcb09886f (this change, cached ignitions, safety mode applied after card's own CarParams, no fault). Since it landed on my fork on 2026-09-03 every cached ignition has been clean; the first ignition after each update still runs the full query, as before.

Companion: commaai/opendbc PR "vin: retries query the standard VIN addresses directly", which is what put this car on the cached path.

🤖 Generated with Claude Code

pandad's safety setter runs off deviceState.started and reads
FirmwareQueryDone, ControlsReady and CarParams. manager clears them at
the onroad transition, but it reacts to the same deviceState edge as
pandad with no ordering between the two, so pandad can read the previous
drive's values first, find everything ready, and apply the car safety
mode from stale CarParams.

Issue commaai#28050 described this race and commaai#28151 fixed it in 2023 by starting
the safety setter off an IsOnroad param that manager wrote only after its
own clear. commaai#37999 moved pandad's onroad signal to a live deviceState
subscription, which dropped that guarantee, and commaai#38205 then removed the
unused param. The firmware query hid the regression: 5 to 9 s in ELM327
before the car safety mode meant nothing stale was ever acted on. A car
whose VIN is cached skips the query and exposes it.

Clearing the three keys on the way out leaves nothing stale to read at
the next ignition, whichever process reacts first. The other
CLEAR_ON_ONROAD_TRANSITION keys stay as they are: the Offroad_* alerts
are written onroad to be shown offroad.

Seen on a 2021 Hyundai Palisade with openpilot longitudinal: the first
ignition with a cached fingerprint applied the hyundai safety mode from
the first frame, before card had disabled the stock radar, so the radar's
SCC messages on bus 0 tripped the relay check and it stayed latched for
the drive. With this change, three further cached ignitions in one power
cycle applied the safety mode 7 to 12 s in, with no fault.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@github-actions

github-actions Bot commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

Process replay diff report

Replays driving segments through this PR and compares the behavior to master.
Please review any changes carefully to ensure they are expected.

✅ 0 changed, 66 passed, 0 errors

@zappybiby

zappybiby commented Sep 11, 2026 •

Copy link
Copy Markdown
Contributor

I found this PR while investigating an issue I had in my 2020 Subaru Crosstrek on sunnypilot dev. I had an LKAS fault immediately after pressing the 'Reset Calibration' button. I was on road, in drive, and not moving at the time I pressed in.

I found that Sunnypilot and Openpilot both share the relevant manager/pandad startup logic and OnroadCycleRequested mechanism. And that this fix is identical to what I had locally came up with to fix this.

Timeline of my issue:

# BEFORE RESET - ONROAD; the existing card process is running.
# Reset Calibration pressed

# transition from ONROAD to OFFROAD
22:26:10.984193  Startup blocked: ignition=true, not_onroad_cycle=false
22:26:11.060843  killing card

# after reset, transition from OFFROAD back to ONROAD
# the new card process has not launched yet.
22:26:11.998263  Startup after block: ignition=true, not_onroad_cycle=true
22:26:12.091338  Finished FW query, Waiting for params to set safety model
22:26:12.091700  got 1200 bytes CarParams
22:26:12.091830  setting safety model: 11, param: 0, …

# fault
~22:26:12.642   [decoded CAN] Steering_Torque (0x119, bus 0): Steer_Error_1=1

# only now does the new card process launch
22:26:12.726210  starting python openpilot.selfdrive.car.card

In my logs, pandad had passed its FirmwareQueryDone/ControlsReady checks and loaded the previous run’s CarParams, requesting Subaru safety 634 ms before card launched. That mode blocks the factory ES_LKAS (0x122) steering messages, but card wasn’t running yet to supply replacements.

Simulated test before / after PR:

BEFORE PR

 0.000s  RESET_CONFIRMED       OnroadCycleRequested=true
 0.000s  DEVICE_STATE          onroad=false
 0.018s  MANAGER_OFFROAD_CLEAR CarParams=1200 bytes  FirmwareQueryDone=true  ControlsReady=true
 1.014s  DEVICE_STATE          onroad=true
 1.108s  READINESS_ACCEPTED    CarParams=1200 bytes
 1.108s  SUBARU_SAFETY_REQUEST mode=11 param=0
 1.350s  MANAGER_ONROAD_CLEAR  CarParams=0 bytes     FirmwareQueryDone=false ControlsReady=false
 1.742s  NEW_CARD_LAUNCHED
10.709s  GENERATED_SENDCAN     address=0x122  panda_tx_allowed=true
10.709s  CONTROLS_READY_VISIBLE


Largest steering-message interruption:                    9.051 seconds

AFTER PR

 0.000s  RESET_CONFIRMED       OnroadCycleRequested=true
 0.000s  DEVICE_STATE          onroad=false
 0.018s  MANAGER_OFFROAD_CLEAR CarParams=0 bytes  FirmwareQueryDone=false ControlsReady=false
 1.014s  DEVICE_STATE          onroad=true
 1.108s  ELM327_SAFETY_REQUEST mode=3 param=1
 1.350s  MANAGER_ONROAD_CLEAR  CarParams=0 bytes  FirmwareQueryDone=false ControlsReady=false
 1.742s  NEW_CARD_LAUNCHED
10.709s  GENERATED_SENDCAN     address=0x122  panda_tx_allowed=false
10.709s  CONTROLS_READY_VISIBLE
10.808s  READINESS_ACCEPTED    CarParams=1336 bytes
10.808s  SUBARU_SAFETY_REQUEST mode=11 param=0
10.809s  GENERATED_SENDCAN     address=0x122  panda_tx_allowed=true

Largest steering-message interruption:                    30 milliseconds

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants