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.85k stars 9.09k forks source link

Hyundai SCC automatic resume is unreliable #2142

Closed gregjhogan closed 4 years ago

gregjhogan commented 4 years ago

Describe the bug

Somewhere in the range of 1% to 10% of the time the car will not accept the resume command from openpilot for 5+ seconds

How to reproduce or log data

Sit behind a parked car, engage, and let openpilot send the resume command repeatedly when the car goes into the state where it says press pedal or accel button to resume.

Expected behavior

The car always accepts the resume command from openpilot within a second and goes back into the state where it is prepared to accelerate immediately when the lead vehicle pulls away.

Device/Version information (please complete the following information):

Additional context

https://github.com/commaai/openpilot/blob/e8c8e9dab25c81fbeefa4fa05bf38166f9abe203/selfdrive/car/hyundai/carcontroller.py#L79-L80

Most of the time the car will accept the very first resume command openpilot sends, but some percentage of the time it takes way too long and you have to manually resume.

What I have tried:

The vehicle sends the button state at 50 Hz so perhaps sometimes the next message from the vehicle will come in very close after openpilot sent a message, overriding what openpilot sent?

I would guess this is an issue for the Sonata and Palisade since they use the same radar and this is related to SCC, and perhaps other vehicles, too.

xps-genesis commented 4 years ago

@gregjhogan give the changes in #2152 a try. seems to be very reliable on my genesis, also have a sonata 2020 user who seems to be happy. you can remove the lead relative velocity check in the PR if you want to just test activating behind a target in a parking lot to see if the resume spam works every time the cluster shows display to it resume.

image

gregjhogan commented 4 years ago

Someone reproduced the issue and verified that before adding this code there is a problem and after this change it is fixed?

xps-genesis commented 4 years ago

I have been working on button spam to auto change set speed. I tried to use the 5Hz spam like it is in Master and it was unreliable or sometimes it will fault out. I played with the logic and this logic in PR worked every time with no issues.

so, in general the new logic in the PR works for button spam. it worked flawlessly for changing set speed every time without any faults. So, i propose the same logic for auto resume to overcome the issue you have mentioned.

i have added the lead vRel check so the resume spam doesn't happen when the lead is creeping forward without the intention to drive off. this ensures resume spam only happens when lead accelerates.(intends to drive off)

xps-genesis commented 4 years ago

@gregjhogan has this been tested?

xx979xx commented 4 years ago

https://github.com/commaai/openpilot/commit/b81f404457b2bfeee1d9c18c86af2aeb167b9403 is bad, revert fix it for me

gregjhogan commented 4 years ago

@xps-genesis @xx979xx I tested both and I still have instances where it takes more than 10 seconds for a resume message to get accepted

xx979xx commented 4 years ago

@gregjhogan the issue, as I understand it, cluster instrument sends clu11 at 50 Hz, SCC has msg count threshold to accept that button pressed and has time window for accepting Rx msg which shifts as soon as valid msg received . it is a timing issue, and defining this time window solve it. the community way is the most effective up to now

gregjhogan commented 4 years ago

I have tested everything suggested here and it would be inaccurate to say anything suggested fixes this issue