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

GMC Acadia Denali: cruise fault immediately after releasing the accelerator when engaged #24135

Closed
Verylukyguy opened this issue Apr 5, 2022 · 8 comments · Fixed by #24167
Closed
Assignees
Labels
bug car vehicle-specific
Milestone

Comments

@Verylukyguy
Copy link
Contributor

Verylukyguy commented Apr 5, 2022

Describe the bug

When OpenPilot is already engaged and you begin accelerating with the Disengage on Accelerator toggle turned off, you get a Cruise Fault: Take Control Immediately RSOD immediately upon releasing the accelerator.

This also happens when you engage OpenPilot while accelerating and then the accelerator is released.

I provided a route with several examples.

What hardware does this issue affect?

comma two

Provide a route where the issue occurs

e38a02cae1ef1e0c|2022-04-05--11-10-35--0

openpilot version

Comma Master 0.8.14 @ f0ac808

Additional info

This behavior is the same when the Disengage on Gas toggle is on, see route: e38a02cae1ef1e0c|2022-04-05--11-02-43--0

@Verylukyguy Verylukyguy added the bug label Apr 5, 2022
@sshane sshane changed the title Cruise Fault immediately after releasing the accelerator when engaged. GMC Denali: cruise fault immediately after releasing the accelerator when engaged Apr 5, 2022
@sshane sshane changed the title GMC Denali: cruise fault immediately after releasing the accelerator when engaged GMC Acadia Denali: cruise fault immediately after releasing the accelerator when engaged Apr 5, 2022
@sshane
Copy link
Contributor

sshane commented Apr 5, 2022

Can you try this branch out? https://github.com/commaai/openpilot/tree/gm-cruise-faults

I think it may be related to how we now use longActive instead of enabled for a couple of signals related to ACC (changed in PR #23859).

When you press the gas, we set ASCMGasRegenCmd->GasRegenCmdActive to False which causes AcceleratorPedal2->CruiseState to fall, then when you release the gas we start commanding gas/regen immediately while the CruiseState takes a frame to rise again, so maybe that's the cause of the faults? Let me know how the branch goes! @Verylukyguy

Screenshot from 2022-04-05 14-14-24

@adeebshihadeh adeebshihadeh added this to the 0.8.14 milestone Apr 5, 2022
@pd0wm pd0wm added car bug and removed bug labels Apr 5, 2022
@Verylukyguy
Copy link
Contributor Author

Can you try this branch out? https://github.com/commaai/openpilot/tree/gm-cruise-faults

I think it may be related to how we now use longActive instead of enabled for a couple of signals related to ACC (changed in PR #23859).

When you press the gas, we set ASCMGasRegenCmd->GasRegenCmdActive to False which causes AcceleratorPedal2->CruiseState to fall, then when you release the gas we start commanding gas/regen immediately while the CruiseState takes a frame to rise again, so maybe that's the cause of the faults? Let me know how the branch goes! @Verylukyguy

This made it worse; now it faults when the accelerator is pressed while engaged.

@sshane
Copy link
Contributor

sshane commented Apr 6, 2022

It looks like this might have always been an issue. We probably need a stock route of an ICE car to see what the ASCM sends to avoid faults as I tried duplicating what it does for the Volt, but that made the issue worse. Doesn't seem to be an issue on the Volts, which is what @twilsonco said the GM port started with.

@Verylukyguy
Copy link
Contributor Author

Verylukyguy commented Apr 6, 2022

It has been an ongoing issue on stock OpenPilot, but it is very strange that the Volt code didn't work, because I am usually on some version of Tim's fork and I do not get those errors. I will attempt to get you some factory GM routes to analyze. Thank you.

@Verylukyguy
Copy link
Contributor Author

Verylukyguy commented Apr 6, 2022

I think that I may have actually found a reference to this issue in #348

@twilsonco
Copy link
Contributor

twilsonco commented Apr 7, 2022

This happens in my '18 Volt with C3 as well; not just Acadia.

Importantly, this does not happen in my fork (tw-0.8.9 branch) GitHub.com/twilsonco/openpilot, or in the opgm fork ('dev' branch) gihub.com/opgm/openpilot.

steps to reproduce on apparently any GM vehicle:

  1. in master, disable the disengage on gas toggle
  2. Drive, engage
  3. Press gas; lateral stays engaged, but you'll have long control with the gas pedal
  4. release gas pedal

Expected behavior: OP resumes long control
Observed behavior: RSOD "Cruise Faulted Take control immediately". This resolves within seconds and you can reengage.

@twilsonco
Copy link
Contributor

twilsonco commented Apr 7, 2022

I'll attempt to fix and submit a PR

edit: nothing prints in tmux to accompany the fault.

@twilsonco
Copy link
Contributor

twilsonco commented Apr 7, 2022

I'm stumped for the time being. Reaching out to @JasonJShuler and @qadmus for help. Here's from discord:

steering stays engaged while you press the gas, but then ACC faults right after you release the pedal. In these plots I got from Shane of stock GM cruise doing it's no-disengage-on-gas thing, I notice that, at t=148s when gasPressed goes to 1, the acc keeps sending a non-"zero" GasRegenCmd for ~1s before it drops to "zero", then it comes back to the 1900 value ~0.5s before the gas is released.

I thought maybe this meant there had to be a non-"zero" gas command present when op-long resumes control, but I don't think that makes sense now...
More likely, we're missing a frame in the ACC commands in one of two ways. When the user releases the gas, op sends the first true GasRegenCmdActive signal either:

  1. a frame too early, overlapping with the gas pedal and causing a fault, or
  2. a frame too late, resulting in missing frames and causing a fault.

@sshane sshane linked a pull request Apr 7, 2022 that will close this issue
@pd0wm pd0wm added bug car vehicle-specific and removed car bug labels Jun 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug car vehicle-specific
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants