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.

Sign up for GitHub

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: no speed limit to resume #352

Closed
wants to merge 1 commit into from
Closed

GM: no speed limit to resume #352

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion selfdrive/car/gm/carstate.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
from selfdrive.can.parser import CANParser
from selfdrive.car.gm.values import DBC, CAR, parse_gear_shifter, \
CruiseButtons, is_eps_status_ok, \
STEER_THRESHOLD
STEER_THRESHOLD, AccState

def get_powertrain_can_parser(CP, canbus):
# this function generates lists for signal, messages and initial values
Expand Down
17 changes: 12 additions & 5 deletions selfdrive/car/gm/interface.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@
from selfdrive.controls.lib.drive_helpers import create_event, EventTypes as ET
from selfdrive.controls.lib.vehicle_model import VehicleModel
from selfdrive.car.gm.values import DBC, CAR
from selfdrive.car.gm.carstate import CarState, CruiseButtons, get_powertrain_can_parser
from selfdrive.car.gm.carstate import CarState, CruiseButtons, \
AccState, get_powertrain_can_parser

try:
from selfdrive.car.gm.carcontroller import CarController
Expand Down Expand Up @@ -78,8 +79,12 @@ def get_params(candidate, fingerprint):
std_cargo = 136

if candidate == CAR.VOLT:
# supports stop and go, but initial engage must be above 18mph (which include conservatism)
ret.minEnableSpeed = 18 * CV.MPH_TO_MS
# initial engage must be above ~18mph, and resume
# can happen at any speed, unless brake pedal was pressed,
# in which case resume can only happen above ~7mph.
# TODO: track PCM state to know exactly when set/res are allowed,
# instead of disengaging on a PCM fault.
ret.minEnableSpeed = -1
# kg of standard extra cargo to count for drive, gas, etc...
ret.mass = 1607 + std_cargo
ret.safetyModel = car.CarParams.SafetyModels.gm
Expand Down Expand Up @@ -194,9 +199,9 @@ def update(self, c):

# cruise state
ret.cruiseState.available = bool(self.CS.main_on)
cruiseEnabled = self.CS.pcm_acc_status != 0
cruiseEnabled = self.CS.pcm_acc_status in [AccState.ACTIVE, AccState.STANDSTILL]
ret.cruiseState.enabled = cruiseEnabled
ret.cruiseState.standstill = self.CS.pcm_acc_status == 4
ret.cruiseState.standstill = self.CS.pcm_acc_status == AccState.STANDSTILL

ret.leftBlinker = self.CS.left_blinker_on
ret.rightBlinker = self.CS.right_blinker_on
Expand Down Expand Up @@ -281,6 +286,8 @@ def update(self, c):
events.append(create_event('pedalPressed', [ET.PRE_ENABLE]))
if ret.cruiseState.standstill:
events.append(create_event('resumeRequired', [ET.WARNING]))
if self.CS.pcm_acc_status == AccState.FAULTED:
events.append(create_event('speedTooLow', [ET.NO_ENTRY, ET.IMMEDIATE_DISABLE]))

# handle button presses
for b in ret.buttonEvents:
Expand Down
6 changes: 6 additions & 0 deletions selfdrive/car/gm/values.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,12 @@ class CruiseButtons:
MAIN = 5
CANCEL = 6

class AccState:
OFF = 0
ACTIVE = 1
FAULTED = 3
Copy link
Contributor

Choose a reason for hiding this comment

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

@vntarasov , have you seen the AccState becoming FAULTED for other reasons other than the low speed lockout?

Copy link
Contributor

Choose a reason for hiding this comment

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

Actually, I think I understand what you mean: in this case the AccState will become FAULTED only immediately after the engagement. Does it stay FAULTED or it clears after you stop commanding gas?

Copy link
Author

Choose a reason for hiding this comment

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

Good point, let me find someone willing to test it on 2018 Volt. On my 2017 Volt, speed-related FAULTED clears out as soon as openpilot disengages, but I haven't tested it on 2018 models. It would be rather inconvenient if PCM stays faulted.

Re: other ways to get to FAULTED: There was a bug with "isActive" vs "isEnabled", where PCM would go to FAULTED (for a few seconds on 2017 Volts, and until ACC is reset on 2018), fixed in #352. I don't know of other ways to get it to FAULTED other than stop sending keepalive messages (same GasRegenCmd, with "enabled" = False).

STANDSTILL = 4

def is_eps_status_ok(eps_status, car_fingerprint):
valid_eps_status = []
if car_fingerprint == CAR.VOLT:
Expand Down