search

commaai / openpilot

openpilot is an operating system for robotics. Currently, it upgrades the driver assistance system on 275+ supported cars.
https://comma.ai/openpilot
MIT License
49.83k stars 9.08k forks source link

Unable to activate OP while brake hold active #23095

Open jsegill opened 2 years ago

jsegill commented 2 years ago

Describe the bug

When brake hold is enabled and in effect on a Toyota Corolla HB 2019, OP will not engage.

Before 0.8.11, OP would engage while in a brake hold with a lead car. Not allowing OP to engage while at a stop, makes stop and go traffic disengagements more frustrating. One cannot simply re-enable OP after making a correction resulting in a stop of the car. Also trying to re-enable OP with brake hold active, results in the brake hold being released with no OP engagement.

Stock Toyota system allows for ACC engagement while brake hold is active. Stock Toyota ACC requires a second push of the resume button to resume ACC functions.

I believe this stems from PR #22810 to fix issue Issue #22791. Anyway to prevent the integral build up, mentioned in Issue #22791 so OP may be enabled again when brake hold is active?

What hardware does this issue affect?

comma three, comma two

Which car does this affect?

Toyotas; Toyota Corolla HB 2019

Provide a route where the issue occurs

99725b56a15b5510|2021-12-01--15-16-19

openpilot version

0.8.11

Additional info

No response

jsegill commented 2 years ago

Update: Discovered OP will activate when manually pushing the brake at a complete stop- this mirrors what OP used to do with the brake hold feature. While physically pushing the brake at a complete stop, the resume with another press of the resume button illuminates on the Toyota dash.

Seems like this behavior should be flipped... Activate OP while brake hold is active (car holding brake), not while brake is manually pushed.

image

jsegill commented 1 year ago

May have found a way to fix this car bug with the generous help of @cydia2020 PR soon?

sshane commented 1 year ago

Looked at the history and I don't see any clear reason why it was prevented. We allow it if not openpilot longitudinal control, so perhaps it was a way to avoid faults with openpilot longitudinal since we didn't want to spend the time to verify that this combination of modes worked properly.

We also used to disallow enabling without a lead with brake pressed, so perhaps this can be revisited. Not a priority at the moment though.

jsegill commented 1 year ago

Thank you @sshane for researching the issue and the history of why this may be happening. It's been confusing to find the root cause. Much appreciation!