-
Notifications
You must be signed in to change notification settings - Fork 9.5k
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: ACC faults #24223
Comments
Diagnosed my Volt ACC faults to a known brake-pressed-sensor vehicle issue.
Narrative: vehicle ACC and openpilot are using different brake-pressed signals. Vehicle is more sensitive, and wants to disengage but openpilot continues with ACC commands. In 0.5 seconds, vehicle ACC faults.
|
I have been getting intermittent RSOD CAN errors when I press the brake when OpenPilot is engaged, but they resolve immediately. This just started happening, maybe since the GM brake signal was changed. |
This PR is causing ACC faults on cars that have a noisy brake pedal: #23712 The car's ACC system actually uses the noisy Ideally we should detect this and alert the user when we disengage to prevent a fault. Looked into if there were any bits that describe that threshold, but there are none. Plot showing a fault occurs shortly after that signal goes to 8, while the filtered/true brake signal (that we use now) stays zero: |
https://github.com/qadmus/openpilot/tree/gm-fix-acc-fault-brake-position Worked this morning with my light-brake-press test method. Traveling this weekend, will make PR if no one gets to it first. |
Looked at all the segments here that we have rlogs available for, most don't seem to be caused by any brake pedal issue: Total segments with faults: 27, would be able to prevent by disengaging with brake pedal: 6
Reverted PR would have caught: 5 # https://github.com/commaai/openpilot/pull/23712
Master would have caught: 0
New PR would have caught: 6 # https://github.com/commaai/openpilot/pull/24589 |
Guys - the 8 threshold is not the same on all cars. The switch away from it originated when I needed to bump the threshold to 11 to accommodate trucks - The suburban, Silverado and Tahoe were getting random disengagements. Adeeb requested finding and using a boolean signal, - both Jason Young and someone at comma confirmed the validity of the brake-pressed signal against lots of data. @sshane - it was my understanding that the stock ACC is controlled by the ASCM, and that the ASCM must be disconnected for all currently supported GM vehicles. The ASCM sends gas and brake commands directly to the PT / Chassis busses. OP completely replaces the functionality of the ASCM - where is this disengagement coming from? We have an experimental VOACC branch that is able to control gas/brake without syncing any sort of ACC state. When the gas command is active, the car applies gas, etc. |
Note PR #24589 is a new source of Brake Pedal Position, from @JasonJShuler I'd guess the ECM disengages, as a redundant test of the brake pedal position. I don't trust the names of these messages though, don't know where they came from. We may need BOTH (and more) brake pressed detections. And if models or worse, individual cars, have calibrated thresholds, which is possible, then finding a boolean signal is really valuable. |
@JasonJShuler I'm not convinced all these faults are caused by the brake pedal issue, it's just a few could have been prevented and we wanted to revert back to a state where we would have caught at least some faults. If you believe different cars use different brake pressed signals/thresholds we should make a safety param flag for that based on the car. We should also figure out what is causing these faults in the process. I checked and blocked messages are also not the cause. Yeah there must be a redundant check in the ECM (/PCM, or is that the ASCM?) where it won't allow you apply brake or gas if the user is pressing the brake, which it thinks they are based on the noisy signal we removed. |
We simply reverted back to avoid regressions for currently supported cars. If these new cars already kind of work, it'll be easier to find a solution if you open a PR and include some routes. |
Quickly estimated how many of these faults would be fixed by #25402. Seems like a pretty sizable chunk of the segments with rlogs. I can likely adapt to work on qlogs later. The criteria was engaged shorter than 600 ms ago with the resume button (since we can be in any part of the route we don't know if we ever engaged after main toggle before this (can be solved with looking at the whole qlog route)):
|
Camera ACC ACC faults:
|
@sshane ready to close or are we still working through some? |
There's still a lot I don't understand from this list. I may have figured out a sizable chunk of the remaining camera ACC faults, so I'll re-check all the ones above with rlogs |
Bolt EUV camera ACC faults:
|
Closing since we've now solved most faults (if not all) in the initial list |
Events while engaged on 0.8.13-release
The text was updated successfully, but these errors were encountered: