openpilot icon indicating copy to clipboard operation
openpilot copied to clipboard

VW MQB: Fahrzeuglängsbeschleunigungssteuerungseinrichtung

Open jyoung8607 opened this issue 4 years ago • 9 comments
trafficstars

Rough draft support of openpilot longitudinal control for VW MQB vehicles. The tune needs help, particularly for braking behind rapidly slowing lead cars. Right now I'm using some HKG constants I stole and took around the block once. But, it's driving pretty good on the highway, and the state transitions in-and-out of standstill seem to be working.

Test drive: 3cfdec54aa035f3f|2021-11-17--23-33-17

Activated using the DisableRadar_Allow parameter, which enables the OP long control toggle in the UI.

Limitations:

  • Requires a gateway (J533) harness. I hope, but make no promises, to support camera harnesses later.
  • Requires full Stop-and-Go ACC. Will look at Follow-to-Stop vehicles later, but probably can't make them hold at stop.
  • This is VOACC, but we do require factory ACC be present. I can confirm this won't be a restriction forever.

Todo:

  • [x] openpilot prerequisites: #23004
  • [x] DBC cleanup: commaai/opendbc#475
  • [x] Panda Safety: commaai/panda#920
  • [x] openpilot carcontroller cleanup
  • [x] Real-world standstill/startup reverification
  • [x] Show lead car in cluster/HUD
  • [x] Block attempts to use OP long when integrated at camera
  • [x] Detect ACC Low (Follow-to-Stop only) vehicles and decide how to handle
  • [x] ~~Dash indication for Front Assist (FCW/AEB) disabled~~
  • [x] Tuning
  • [x] Upload a test route after tuning

Known bugs:

  • [x] At first startup, with mainswitch on but no ACC setpoint established, the setpoint will show 158MPH instead of ---
  • [x] Some cars will briefly show a FCW-like alert in the cluster when disengaging by brake pedal

jyoung8607 avatar Nov 18 '21 05:11 jyoung8607

Working a problem wherein the car sometimes just doesn't decelerate as-requested. This gets exciting with a lead car. On comparison with stock, it looks like there's yet a third copy of our requested acceleration to deal with, in an ACC_07 signal I originally thought was lead car relative speed, and it's not always active. I'll continue working on it later this weekend.

image

jyoung8607 avatar Nov 19 '21 03:11 jyoung8607

For ease of review, much of the prerequisite work has been submitted separately under #23004. Once that's merged, this PR for actual OP long support will be cleaner.

At this time I'm not going to set the dash indication for Front Assist (FCW/AEB) disabled, because we've been able to verify stock FCW/AEB availability together with openpilot longitudinal control. It does depend on integrating at the gateway (J533); this would not be workable if the radar were silenced for camera (R242) integration. Given the value of retaining stock FCW/AEB I don't know that I'll invest time in camera integration until openpilot has its own quality FCW/AEB.

I'm still working to clean up state transitions around stopping and starting, and trying to come up with a tune that handles slowing lead cars properly.

jyoung8607 avatar Nov 22 '21 20:11 jyoung8607

  • Requires full Stop-and-Go ACC. Will look at Follow-to-Stop vehicles later, but probably can't make them hold at stop.
  • [ ] Detect ACC Low (Follow-to-Stop only) vehicles and decide how to handle

ACC Low (follow to stop) and ACC Basic (for lack of a better known name, down to ~15kph or something) turn out to drive just fine with the accel command structure we're using, if we set the appropriate type enum and don't exceed what that TSK/ESP combo is willing to do for us. We actually had someone drive around a manual trans Golf R just fine, even during shifting.

Main problem will actually be characterizing which type the car supports at startup. I may not have a convenient way to determine this at get_params() time, I may have to detect it on-the-fly at runtime. I think I may also need to teach openpilot some states and events for cars that are willing to slow to zero, but only stay at zero for ~3 seconds, and I'm not yet sure what they do if asked to depart from standstill.

This PR will remain focused on ACC High vehicles; I'll deal with ACC Low and Basic at another time in another PR.

  • [x] Some cars will briefly show a FCW-like alert in the cluster when disengaging by brake pedal

This turns out to be a bit annoying. I'm confident I can fix it, but I'm still experimenting with the best means to do so while preserving stock FCW/AEB. Unfortunately I only have intermittent access to a vehicle with this problem.

jyoung8607 avatar Dec 01 '21 03:12 jyoung8607

"Brake!" alert on disengage for newer cars is fixed, thanks to @hendrik212. That was the last major technical obstacle.

Remaining:

  • [x] Re-file Panda safety PR after the MQB-PQ split
  • [x] ~~Determine the radar firmware breakpoint for needing to receive/send ACC_13~~

jyoung8607 avatar Feb 06 '22 01:02 jyoung8607

  • [ ] Safety check, timely response to stopping request: ACC_07.ACC_Anhalten -> ESP_21.ESP_Anhaltevorgang_ACC_aktiv

jyoung8607 avatar Jul 13 '22 20:07 jyoung8607

Want to get this merged to avoid more merge conflicts? We can merge if we explicitly disable it and ignore DisableRadar.

adeebshihadeh avatar Aug 01 '22 19:08 adeebshihadeh

Want to get this merged to avoid more merge conflicts? We can merge if we explicitly disable it and ignore DisableRadar.

VW PQ long is in a better state of readiness, with some lessons-learned on both openpilot and Panda safety I need to come back and apply here. I'd like to focus on delivering VW PQ long, including cleanup of abstractions and dropping in some of the shared VW car port infrastructure (accel limits/clipping, button enable logic, default tune, etc etc). Then I can apply VW MQB long on top of that.

jyoung8607 avatar Aug 01 '22 19:08 jyoung8607

Just something minor, pass through of acc_04 can be removed. It is not necessary to suppress the disengagement alerts. The original cause were inconsistent values sent to acc_02. This was fixed by sending acc_02 the correct values.

Hendrik212 avatar Sep 09 '22 15:09 Hendrik212

Want to get this merged to avoid more merge conflicts? We can merge if we explicitly disable it and ignore DisableRadar.

I'd like to go ahead with this. This works reasonably well on SNG cars, within the limitations of VOACC/E2E without radar points, but needs more work in a few areas before it's globally enabled.

The empty fingerprint checks are ugly but necessary to avoid docs being generated with non-zero min engage speeds (we fall through to the manual transmission / basic ACC case). LMK if you see a less hacky way to deal with it.

Tasks for a future PR:

  • Need better detection and handling of FTS cars, they drive on this but temporarily go accFaulted after 3 second stop
  • Add explicit fault detection for failure to acknowledge stopping request (seen with stock ACC in comma data)
  • More robust brake pressed detection, some cars have lazy brake light switches and we need stock-like redundancy
  • Restore lead car display in HUD
  • Explicitly validate disengage/timeout-at-standstill cases for SNG cars (I think this works, but needs some rigor)
  • Figure out what to do about harness recommendations, maybe make a run at software disabling the radar

jyoung8607 avatar Oct 14 '22 19:10 jyoung8607