openpilot icon indicating copy to clipboard operation
openpilot copied to clipboard

VW MQB: EA driver inactivity workaround

Open jyoung8607 opened this issue 3 years ago • 9 comments

Resolves #23274. We're now filtering and resending LH_EPS_03, so a process_replay delta is expected.

  • [x] requires commaai/panda#955
  • [x] end user testing

jyoung8607 avatar Jun 02 '22 14:06 jyoung8607

An affected end user reports this fix was successful. This PR and the associated Panda PR are ready for review.

Screenshot from 2022-06-03 23-05-30

jyoung8607 avatar Jun 04 '22 03:06 jyoung8607

Returning to draft for the moment. Another test user is still managing to set off EA on a long level stretch where openpilot is keeping the car straight with less than 0.5 Nm of control output. Will add an interp to scale up the openpilot part of the augmented driver torque signal in lower ranges and retest.

image

jyoung8607 avatar Jun 06 '22 16:06 jyoung8607

My attempt to be clever and sum driver input and openpilot output turns out to do weird things. Inertia combined with high frequency PIF tune control, or driver hands-on-wheel resistance, resulted in opposing (and thus cancelling) outputs and inputs, which then weren't large enough to reset inactivity detection.

We're currently testing a version that stops trying to get fancy, and just sends the greater of actual driver input and doubled openpilot output. The output doubling is because VW's inactivity detection is a little too aggressive, and will sometimes trigger even in stock form with gentle inputs on straight level roads.

In this case, we care little about reality/fidelity, and only about activity and magnitude.

jyoung8607 avatar Jul 31 '22 20:07 jyoung8607

The EA mitigation in this PR seems functional, but I also want to throw up a flag when EA pops off (maybe accFaulted, maybe something new) so that we can rely on comma's MTBF reporting to tell us if the mitigation needs further tuning.

  • [x] Depends on EA signal support from commaai/openpilot#27757

jyoung8607 avatar Mar 30 '23 23:03 jyoung8607

@adeebshihadeh once the HCA cleanup is in, I want to add detection of stock EA kicking in. I have a workaround (in this PR) that I believe will be effective, but I want to verify it always works for everyone.

The condition to monitor, on bus 2, is: bool(HCA_01.EA_Ruckfreigabe) or HCA_01.EA_ACC_Sollstatus > 0.

What's the easiest way to surface that in comma's MTBF reports? Should I just add something to carState (like stockFcw and stockAeb), or do you need a full Event? At this time I don't need openpilot to do anything, I just want to know if you ever see segments with that condition. I considered overloading accFaulted, but that would jumble it in with things like the stock ACC radar being occluded by snow or ice.

jyoung8607 avatar Apr 15 '23 19:04 jyoung8607

Out of draft, ready for review. If possible, I'd like this to be reviewed in time for 0.9.4. Complaints tracing back to this issue have gone up quite a lot as MY2023 has brought EA to several VW and Audi models in North America.

The core problem is documented in #23274. Summary of this PR:

  • When openpilot is engaged, pacify EA by sending simulated driver input toward the stock LKA camera
  • Monitor efficacy of the solution by setting CarState.carFaultedNonCritical if we detect EA activation

This PR should fix the problem while openpilot is engaged, and has been ad-hoc tested by several users. If we see segments with carFaultedNonCritical while openpilot is engaged, we'll know it needs further tuning.

This PR does NOT attempt to fully restore stock EA functionality while openpilot is disengaged. It's technically possible, but would involve selective pass-through of stock LKA functionality, which is another discussion entirely.

jyoung8607 avatar Jul 16 '23 17:07 jyoung8607

This PR has had no activity for 30 days. It will be automatically closed in 7 days if there is no activity.

github-actions[bot] avatar Dec 10 '23 01:12 github-actions[bot]

This PR has been automatically closed due to inactivity. Feel free to re-open once activity resumes.

github-actions[bot] avatar Dec 17 '23 01:12 github-actions[bot]

Moved to Draft for the moment, but still moving forward. I've asked for feedback from a few more affected drivers. If the current version of this change works for them over the next few days, I'll release for review.

jyoung8607 avatar Dec 20 '23 22:12 jyoung8607

User feedback has been good, releasing from Draft. I request this PR be considered for 0.9.6-release. It should be low risk, and this is by far the most frequently reported problem with VW on stock openpilot.

@adeebshihadeh I would like to be notified if you see carFaultedNonCritical on VW similar to steerTimeLimit.

jyoung8607 avatar Jan 08 '24 22:01 jyoung8607

Instead of continuing work on EA, can’t we just add the option to enable/disable EA in vw_mqb_config.py? I could even try myself to write the code for this, just that I can only test it on Skoda Kodiaq 2018.

andiradulescu avatar Jan 10 '24 08:01 andiradulescu