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

GM: change ignition signal #1662

Merged
merged 2 commits into from
Sep 15, 2023
Merged

GM: change ignition signal #1662

merged 2 commits into from
Sep 15, 2023

Conversation

sshane
Copy link
Contributor

@sshane sshane commented Sep 14, 2023

Background

Previous signal was checking whether the immobilizer ID was being published. New signal is the power mode. The previous signal used before the immobilizer signal was the "backup" power mode which is noisy and doesn't describe cranking or accessory (#845).

This new SystemPowerMode describes the four ignition states: off, accessory, (on, and ignition together), and cranking.

The problem

The problem with the current signal is that the immobilizer signal remains high in accessory mode, where various ECUs can fault or shut off (the EBCM on the Chevy Bolt, the PSCM on the ASCM Volt, as well as the Camera ACC Silverado). Other cars kill power to the camera in this state (Toyota for example).

The fix

This PR only considers on/ignition and crank request valid ignition states so we go offroad when in accessory mode (enterable by pressing power button in neutral).

Checks

Checked that the old-preconditioning issue (shows ignition with remote start) is not present in the new signal manually on the Bolt EUV and Volt, thanks to community member nworby:

nworby ('18 radarless Volt) — Yesterday at 7:27 AM
@Shane 
806907cd33ef2647|2023-09-14--08-18-38--0 - shifting to neutral and pressing power button
806907cd33ef2647|2023-09-14--08-19-55--0 - entering accessory mode while car is off
806907cd33ef2647|2023-09-14--08-21-17--0 - remote start & stop

Data

This signal also switches off a little under 100ms faster on average, preventing a fault from showing when pandaStates is 10Hz:

View

Some examples where the user enters accessory mode and the EBCM fault signal (Bolt) or LKA fault (Volt, Silverado) rises:

View
- -
image image
image

Some cases with more rare cars, all expected.

View
- -
image image
image image
image image
image

The other mismatches are small timing differences or expected accessory mode diffs:

View
- -
image image
image image
image

Interesting notes


Checked 68,867 rlogs for last 1080 days and only 316 segments mismatch with the conditions above.

Checked segments: 68867, dongles: 323 platforms: {'CHEVROLET VOLT PREMIER 2017': 30521, 'CHEVROLET BOLT EUV 2022': 28454, 'CHEVROLET SILVERADO 1500 2020': 5506, 'GMC ACADIA DENALI 2018': 2608, 'CADILLAC ESCALADE ESV 2016': 1455, 'CADILLAC ESCALADE 2017': 319, 'BUICK LACROSSE 2017': 3, 'CADILLAC ATS Premium Performance 2018': 1}

@sshane sshane added car safety vehicle-specific safety code bugfix labels Sep 14, 2023
@sshane sshane marked this pull request as ready for review September 14, 2023 10:34
Copy link
Contributor

@adeebshihadeh adeebshihadeh left a comment

Choose a reason for hiding this comment

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

LGTM, if all the mismatches are legit.

board/drivers/can_common.h Outdated Show resolved Hide resolved
@sshane sshane merged commit 5460871 into master Sep 15, 2023
@sshane sshane deleted the gm-new-ign branch September 15, 2023 09:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugfix car safety vehicle-specific safety code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants