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

2018 GMC Acadia Denali Support #338

Closed
wants to merge 54 commits into
base: devel
from

Conversation

Projects
None yet
4 participants
@CryptoKylan
Copy link
Contributor

CryptoKylan commented Aug 28, 2018

added 2018 GMC Acadia Denali fingerprint to volt as requested. lat and long in OP work however still a bit buggy

CryptoKylan added some commits Aug 28, 2018

Update values.py
added 2018 GMC Acadia Denali fingerprint to volt profile
added 2018 gmc acadia fingerprint
adding 2018 gmc acadia fingerprint to volt profile as requested. lat and long working in OP but buggy
@vntarasov

This comment has been minimized.

Copy link
Contributor

vntarasov commented Aug 28, 2018

Awesome, glad it just worked!
@CryptoKylan, can you please add ACADIA_DENALI to "class CAR", move the fingerprint to it's own CAR.ACADIA_DENALI, and add corresponding dbcs to "DBC = {...}" on line 56?

CryptoKylan added some commits Aug 28, 2018

@rbiasini

This comment has been minimized.

Copy link
Contributor

rbiasini commented Aug 28, 2018

@CryptoKylan , great!
What do you think it's still buggy?

CryptoKylan added some commits Aug 28, 2018

Update carcontroller.py
added acadia first try
Update carstate.py
added acadia info
Update interface.py
added acadia info
Update radar_interface.py
added acadia info
@rbiasini

This comment has been minimized.

Copy link
Contributor

rbiasini commented on selfdrive/car/gm/radar_interface.py in 8cfb5e3 Aug 28, 2018

@CryptoKylan , why duplicating the code?
if car_fingerprint is in [CAR.VOLT, CAR.ACADIA_DENALI] should be ok, right?

@@ -115,6 +120,9 @@ def update(self, sendcan, enabled, CS, frame, actuators, \
if self.car_fingerprint == CAR.VOLT:
can_sends.append(gmcan.create_steering_control(self.packer_pt,
canbus.powertrain, apply_steer, idx, lkas_enabled))
if self.car_fingerprint == CAR.ACADIA_DENALI:

This comment has been minimized.

@rbiasini

rbiasini Aug 28, 2018

Contributor

same as above

This comment has been minimized.

@CryptoKylan

CryptoKylan Aug 29, 2018

Author Contributor

thank you. im still figuring out coding stuff. i just copied it to see if i could even make it work. i didnt realize my edits were updating here too. i will fix the files as suggested

This comment has been minimized.

@CryptoKylan

CryptoKylan Aug 29, 2018

Author Contributor

i duplicated alot of the settings as a starting point. im really not sure what will be different from the volt but i know some settings will need to be different from the volt.

i am having an issue with steering temp unavailable message after different amounts of time. im wondering if it has to do with the volt being electric? maybe something to do with regen breaking throwing errors? im still very new to openpilot so any help would be greatly appreciated

https://community.comma.ai/cabana/?route=7cc2a8365b4dd8a9%7C2018-08-28--19-07-07&max=6&url=https%3A%2F%2Fchffrprivate.blob.core.windows.net%2Fchffrprivate3%2Fv2%2F7cc2a8365b4dd8a9%2F2d522ddc11f1292d5dd307d63aa0321b_2018-08-28--19-07-07

CryptoKylan added some commits Aug 28, 2018

Update carstate.py
changed to self.regen_pressed = False
Update carstate.py
reverted, no steering
Update interface.py
changed candidate stats to caddy stats
Update carcontroller.py
refactored
Update carstate.py
refactored
Update interface.py
refactored
Update radar_interface.py
refactored
Update values.py
refactored
Update values.py
fixed dbc = correctly i hope
@CryptoKylan

This comment has been minimized.

Copy link
Contributor Author

CryptoKylan commented Aug 29, 2018

i am having an issue with steering temp unavailable message after different amounts of time. im wondering if it has to do with the volt being electric? maybe something to do with regen breaking throwing errors? im still very new to openpilot so any help would be greatly appreciated

https://community.comma.ai/cabana/?route=7cc2a8365b4dd8a9%7C2018-08-28--19-07-07&max=6&url=https%3A%2F%2Fchffrprivate.blob.core.windows.net%2Fchffrprivate3%2Fv2%2F7cc2a8365b4dd8a9%2F2d522ddc11f1292d5dd307d63aa0321b_2018-08-28--19-07-07

CryptoKylan added some commits Aug 29, 2018

Update values.py
trying fix for dbc
Update values.py
another attempt to fix dbc
Update carcontroller.py
changed car controller params to caddy to see if it makes any diff

CryptoKylan added some commits Sep 13, 2018

Merge pull request #10 from CryptoKylan/Acadia
Update carcontroller.py

elif candidate == CAR.CADILLAC_CT6:
# engage speed is decided by pcm
ret.minEnableSpeed = -1
# kg of standard extra cargo to count for drive, gas, etc...
ret.mass = 4016. * CV.LB_TO_KG + std_cargo
ret.mass = 4016 * CV.LB_TO_KG + std_cargo

This comment has been minimized.

@vntarasov

vntarasov Sep 18, 2018

Contributor

CarParams.mass is Float32, why change? Same for CAR.ACADIA_DENALI

CryptoKylan added some commits Sep 18, 2018

corrected steer settings to stock lkas
stock lkas is much closer to the cadillac apparently.  2 up and 5 down. finally figured out  what to look for. will test this tomorrow
@CryptoKylan

This comment has been minimized.

Copy link
Contributor Author

CryptoKylan commented Oct 26, 2018

alright so iv finally narrowed down the problem. im not sure how to proceed from here.

this line specifically in panda gm safety:

// no torque if controls is not allowed
if (!current_controls_allowed && (desired_torque != 0)) {
violation = 1;

it appears that sometimes when i disengage the panda GM safety code is still seeing the last desired torque msg which is above zero and causing a violation which then causes panda to not send a message at all which causes my EPS to fault.

I can either set that line to = 0 or have panda return true for violations in the end and i have zero issues.
Why do you block the whole message with panda and not just reset the torque limits to zero as it does now and let the message through instead of blocking the whole message itself?

@energee

This comment has been minimized.

Copy link
Contributor

energee commented Oct 26, 2018

It should be letting the message through, this code is restricting the panda from sending torque in situations where "violations" occur.

@CryptoKylan

This comment has been minimized.

Copy link
Contributor Author

CryptoKylan commented Oct 26, 2018

right

It should be letting the message through, this code is restricting the panda from sending torque in situations where "violations" occur.

right. thats why im wondering why the code first resets and sends zero torque to just stop the msg entirely in the next step

// reset to 0 if either controls is not allowed or there's a violation
if (violation || !current_controls_allowed) {
gm_desired_torque_last = 0;
gm_rt_torque_last = 0;
gm_ts_last = ts;
}

if (violation) {
  return false;

it sort of seems redundant doesn't it?

wouldn't it make more sense to just send zero/reset torque and let the msg through so it doesnt cause an EPS fault when the car sees a missing message on disengagement?

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