VW MQB: work around standstill limit #2764
Conversation
There was a problem hiding this comment.
Thanks for contributing to opendbc! In order for us to review your PR as quickly as possible, check the following:
- Convert your PR to a draft unless it's ready to review
- Read the contributing docs
- Before marking as "ready for review", ensure:
- the goal is clearly stated in the description
- all the tests are passing
- include a route or your device' dongle ID if relevant
3c55cef to
a9300c3
Compare
a9300c3 to
ce25ac1
Compare
| "ACC_zul_Regelabw_unten": 0.2, # TODO: dynamic adjustment of comfort-band | ||
| "ACC_zul_Regelabw_oben": 0.2, # TODO: dynamic adjustment of comfort-band | ||
| "ACC_zul_Regelabw_unten": 0 if starting else 0.2, # TODO: dynamic adjustment of comfort-band | ||
| "ACC_zul_Regelabw_oben": 0 if starting else 0.2, # TODO: dynamic adjustment of comfort-band |
There was a problem hiding this comment.
I experienced a few cases where openpilot got stuck at standstill because accel never exceeded the comfort band
There was a problem hiding this comment.
I'm sure there's tuning gains to be had, but file that in a separate PR first so I can validate.
There was a problem hiding this comment.
Yeah there's a couple weird (tune-related) behaviors at standstill that I haven't quite figured out yet.
I think I'll focus the work in this PR on standstill fault prevention and may circle back to tuning stop behavior later
There was a problem hiding this comment.
If you get into stop tuning, this is key:
opendbc/opendbc/car/volkswagen/mqbcan.py
Line 118 in 8ac2dc9
That's always been just a placeholder. It's known to be a distance in meters. If you want to figure out the math/code necessary to compute the distance to a nice smooth stop based on the car's current speed and acceleration at the time we transition to stopping, that would help a lot. I think your FtS car will use this as well.
| acc_hold_type = 4 # hold release / startup | ||
| elif esp_hold: | ||
| acc_hold_type = 3 # hold standby | ||
| acc_hold_type = 1 # hold request (at stop) |
There was a problem hiding this comment.
these were flipped from how stock ACC uses them. the car will still respect either value and stop, but sometimes the car is late to release ESP_confirmation (ESP_Haltebestaetigung) when transitioning directly from 3 -> 4.
on cars with EPB this is fine, but on cars without this can easily cause faults if the hold confirmation is retained for too long.
There was a problem hiding this comment.
I'll take a look when you're out-of-draft and ready for real review, but IIRC I got those field descriptions from an authoritative source back in the day. That's not to say we're for-sure using them correctly.
There was a problem hiding this comment.
after further testing, I don't think it matters like I thought it did. I'll probably leave it this way since its more "correct" but I am not entirely sure if the ESP on my car even uses this value
There was a problem hiding this comment.
If needs must, you're welcome to add FtS-specific conditional behavior here.
| acc_control = self.CCS.acc_control_value(CS.out.cruiseState.available, CS.out.accFaulted, longActive) | ||
| accel = float(np.clip(actuators.accel, self.CCP.ACCEL_MIN, self.CCP.ACCEL_MAX) if longActive else 0) | ||
| stopping = longActive and actuators.longControlState == LongCtrlState.stopping | ||
| rollout_in_progress = CS.out.vEgo < self.CP.vEgoStopping and accel > 0 |
There was a problem hiding this comment.
the previous logic would often flip 'starting' to true when slowing to a stop, which is not what the car expects
I'm restricting 'starting' to only positive acceleration here
| # this mimics what would happen if the stock ACC intentionally rolled off after holding for too long | ||
| # the key difference is that the stock ACC also enables freewheel (ACC_Freilauf_Info) during this time | ||
| # but we leave it disabled to reset the timer without actually rolling off | ||
| if longActive and stopping and CS.esp_hold_confirmation and not (self.CP.flags & VolkswagenFlags.PQ) and CS.acc_type == 1: |
There was a problem hiding this comment.
Is CS.acc_type == 1 the correct way to detect ACC Low cars that can't hold at stop?
VAL_ 290 ACC_Typ
0 "Basis_ACC"
1 "ACC_mit_FollowToStop"
2 "ACC_mit_StopAndGo"
3 "ACC_nicht_codiert" ;
There was a problem hiding this comment.
I've seen from discord logs that ACC High disengages and enables parking brake after three minutes at standstill. If this is still the case in 2025, then applying this workaround to all MQB (instead of just ACC Low) might remove that time limit and keep OP enabled indefinitely.
I don't have an ACC high car to test with though, so I can't verify.
There was a problem hiding this comment.
I've seen from discord logs that ACC High disengages and enables parking brake after three minutes at standstill. If this is still the case in 2025, then applying this workaround to all MQB (instead of just ACC Low) might remove that time limit and keep OP enabled indefinitely.
I don't have an ACC high car to test with though, so I can't verify.
I have a ACC High VW Atlas, it holds indefinitely at a stop from my use of stock VW ACC, the max time I have tested is 5 minutes without EPB engaging, but it may be longer
There was a problem hiding this comment.
Is
CS.acc_type == 1the correct way to detect ACC Low cars that can't hold at stop?
We've seen conflicting data in the past, but I believe that ACC Low/High are actually references to max engagement speed. ACC High means you have enough "look-ahead" to track lead cars under German autobahn speeds. I believe that can be obtained either with a LRR, or a MRR combined with a camera.
FtS vs SnG capability are separate from the "High/Low" capability.
There was a problem hiding this comment.
Re: standstill time, I concur with @tealtwo, I tested this a long time ago on my cars. There is indeed a three minute timer, but the stock radar is running it. The car is happy to sit longer for openpilot.
|
before merge the openpilot long controller will have to be adapted to work with PQ as well, the current implementation may not function properly with PQ vehicles running stock openpilot |
| # this mimics what would happen if the stock ACC intentionally rolled off after holding for too long | ||
| # the key difference is that the stock ACC also enables freewheel (ACC_Freilauf_Info) during this time | ||
| # but we leave it disabled to reset the timer without actually rolling off | ||
| if longActive and stopping and CS.esp_hold_confirmation and not (self.CP.flags & VolkswagenFlags.PQ) and CS.acc_type == 1: |
There was a problem hiding this comment.
Is
CS.acc_type == 1the correct way to detect ACC Low cars that can't hold at stop?
We've seen conflicting data in the past, but I believe that ACC Low/High are actually references to max engagement speed. ACC High means you have enough "look-ahead" to track lead cars under German autobahn speeds. I believe that can be obtained either with a LRR, or a MRR combined with a camera.
FtS vs SnG capability are separate from the "High/Low" capability.
| "ACC_zul_Regelabw_unten": 0.2, # TODO: dynamic adjustment of comfort-band | ||
| "ACC_zul_Regelabw_oben": 0.2, # TODO: dynamic adjustment of comfort-band | ||
| "ACC_zul_Regelabw_unten": 0 if starting else 0.2, # TODO: dynamic adjustment of comfort-band | ||
| "ACC_zul_Regelabw_oben": 0 if starting else 0.2, # TODO: dynamic adjustment of comfort-band |
There was a problem hiding this comment.
I'm sure there's tuning gains to be had, but file that in a separate PR first so I can validate.
| # reset standstill timer for MQB w/out EPB while braking | ||
| # this mimics what would happen if the stock ACC intentionally rolled off after holding for too long | ||
| # the key difference is that the stock ACC also enables freewheel (ACC_Freilauf_Info) during this time | ||
| # but we leave it disabled to reset the timer without actually rolling off |
There was a problem hiding this comment.
Are you sure you have a car that actually supports freewheeling? This seems to be rare, potentially dry-clutch DSGs only. IIRC we've only had one show up, and IIRC they might be broken without passing through another flag.
There was a problem hiding this comment.
Are you sure you have a car that actually supports freewheeling?
I have no idea, really. But sending different values for ACC_Freilauf_Info does have a measurable effect.
If I send 0 (freewheel allowed) the car will immediately start rolling. If we're facing uphill, the car will start rolling backward. If I send 2 (freewheel not allowed) the car will remain at a stop for a moment before rolling forward (assuming no brake request, ofc). It won't ever roll backwards, even if I send ACC disabled.
I haven't experimented much with 1 (No transition to coasting allowed) and 3 (Coasting requested) but plan to soon. The stock system sends 0 when forcing FTS disengagement.
| acc_hold_type = 4 # hold release / startup | ||
| elif esp_hold: | ||
| acc_hold_type = 3 # hold standby | ||
| acc_hold_type = 1 # hold request (at stop) |
There was a problem hiding this comment.
I'll take a look when you're out-of-draft and ready for real review, but IIRC I got those field descriptions from an authoritative source back in the day. That's not to say we're for-sure using them correctly.
|
I've been doing some fairly extensive tests on this over the last month or so to try and improve the behavior and reduce faults. The biggest problem right now is hills. If we're on a hill (specifically uphill), we have to send a longer 'start request' before the car will acknowledge. I'm looking for some kind of workaround, but I'm leaning toward just doing the best we can while also asking the user to press brake on steeper hills. (The dashboard has a 'press brake' alert we can use for that) |
| acc_07_values = { | ||
| "ACC_Anhalteweg": 0.3 if stopping else 20.46, # Distance to stop (stopping coordinator handles terminal roll-out) | ||
| "ACC_Freilauf_Info": 2 if acc_enabled else 0, | ||
| "ACC_Folgebeschl": 3.02, # Not using secondary controller accel unless and until we understand its impact |
There was a problem hiding this comment.
not really related, but ACC_Folgebeschl = "acceleration of the car we're following relative to our car"
There was a problem hiding this comment.
Where are you getting that? That's not reflected in docs I've seen (not that they said much useful) and it doesn't match observations from my car's stock ACC.
There was a problem hiding this comment.
At first I thought it was an accel command to our car (and was wondering if I might be able to reset standstill somehow using it - that's the main reason I was investigating in the first place)
Keep in mind I have a sample size of one car, but in testing I made a few key observations of the stock behavior:
- This signal correlates with
ACC_Abstandsindex. When that signal is neutral, so is this one. - This signal is sent even when cruise is not engaged and the car isn't moving. If I'm behind a car at a light, this signal will increase as they drive away, even if I never move and I don't have cruise engaged.
- If I'm following a car moving at a steady pace and I slam on my breaks, the signal goes very negative.
happy to share a few segments if you'd like to investigate/compare
There was a problem hiding this comment.
If you graph it in PJ against the normal accel control signal, such that they're scaled properly, I suspect you'll find they're identical or almost identical while ACC is engaged. However, as you say, it keeps sending projected accelerations even when ACC is not engaged, still tracking the lead car. I have a suspicion it might be used to improve DSG shift predictions. Not sure what else.
There was a problem hiding this comment.
will experiment and follow up in discord
You may find this challenging. One of the differences between FtS and SnG cars is bidirectional rear wheel speed sensors, for detecting when the wheel is rolling backwards, to protect against rollback on hill starts. Essentially the same problem you're having. With that info, I suspect the ESP is able to keep the car still until the powertrain develops enough force to overcome the hill. Same basic thing happens with manual trans hill-hold. I don't know how well FtS cars deal with this.
Not exactly Plan A, but we do have longitudinal pitch data available in carcontroller. I wouldn't trust the car's road-grade signal for this. |
goal
MQB cars without an EPB (often referred to as 'ACC Low') generally will not hold at a standstill for more than a few seconds before faulting. This works around that fault to get full stop-n-go capability without an EPB.
why it works
Stock ACC will only hold at a stop for 1 second before disengaging ACC and rolling. As stock ACC disengages, it sends a short burst of 'start request' to inform the car that it should start rolling.
We can mimic this request to reset the standstill timer. First, we wait for the car to reach standstill and acquire
esp_confirmation. Once we've sat for about a half second, send a one-frame pulse of 'start request'. If the car accepts our request, we'll seeesp_confirmationflip to false about two frames later for one frame. This indicates that the standstill timer has been reset.The signal we send is slightly different from the stock signal mentioned prior. There are two notable differences:
ACC_Freilauf_Infosignal by sending 0 - freewheel allowed. We prevent the car from moving by sending 2 - not released instead.3.01(which is neutral). We instead send an acceleration of0.01with our 'start request'. the reset works either way, but a neutral acceleration will cause the engine RPM to vary slightly at standstill.Sometimes the car will enter a state where it refuses to acknowledge start requests. If this happens we alternate between sending 'roll request' and 'acc disabled' every five frames until the car either accepts one of our requests, or in the worst case, faults.
other related tweaks and notes
ACC_Anforderung_HMSdid not match the stock ACC when approaching a stop. When approaching a stop, stock ACC transitions 0 -> 3 -> 1. The previous logic would transition 0 -> 4 -> 1 -> 3. (possible states are 0 none, 1 hold, 2 park, 3 hold‑standby, 4 drive‑off, 5 ramp‑release)a3d353f2254caee4/00000019--f43b7bfcf7/5for an example.todo
Longitudinal Maneuvers Route
(has lots of standstill, great for seeing the new behavior)
a3d353f2254caee4a3d353f2254caee4/00000016--544d9e5e69Validation (Outdated)
a3d353f2254caee4a3d353f2254caee4/0000000f--3a1c287356