VW MQB: fix cruise fault message during openpilot start#3228
VW MQB: fix cruise fault message during openpilot start#3228RJWadley wants to merge 1 commit intocommaai:masterfrom
Conversation
RJWadley
changed the title
Car behavior reportReplays driving segments through this PR and compares the behavior to master. Testing 130 segments for: VOLKSWAGEN_ARTEON_MK1, VOLKSWAGEN_ATLAS_MK1, VOLKSWAGEN_CRAFTER_MK2, VOLKSWAGEN_GOLF_MK7, VOLKSWAGEN_JETTA_MK7, VOLKSWAGEN_PASSAT_MK8, VOLKSWAGEN_PASSAT_NMS, VOLKSWAGEN_POLO_MK6, VOLKSWAGEN_TAOS_MK1, VOLKSWAGEN_TIGUAN_MK2, VOLKSWAGEN_TOURAN_MK2, VOLKSWAGEN_TRANSPORTER_T61, VOLKSWAGEN_TROC_MK1 ✅ 0 changed, 130 passed, 0 errors |
|
The change seems reasonable at a glance. I agree there's a radar init state, and it's even visible for a few seconds in the instrument cluster on some newer MQB. But, I can't recall any other complaints of openpilot displaying a cruise fault, and I'm not sure why ACC type 1 vs 2 would matter. I would like to see routes both before and after the change. Setting to Draft until there's data to look at. |
|
I can confirm now that ACC type 1 vs 2 does not matter, I just don't have an ACC type 2 car to test on so I wasn't sure. Presumably ACC type 0 is the same as well, but I haven't investigated. Here's an example from an ACC type 2 car, notice the TSK is momentarily faulted during the initialization phase: 
Usually openpilot does not start up fast enough to see or display this fault. My theory is that this is a race condition, and if openpilot starts particularly quickly you'll see a fault message on the comma four as well as the dash cluster. Will report back with routes. |
jyoung8607
left a comment
jyoung8607
left a comment
There was a problem hiding this comment.
I agree there's sometimes a brief fault shown in the instrument cluster. Not sure I've seen a Cruise Fault on openpilot at the same time, but I'll admit I don't watch the device all that closely.
That problem has been around since the beginning of VW op long support. I suspect that's caused by failure to match the COUNTER values the powertrain was receiving from the stock radar, that the randomness of the error is based on how far off we happened to be in the COUNTER cycle.
If you feel like writing some code to sync with the stock ACC counter, and you can verify it helps with that instrument cluster error, I'd merge a PR for that. We'd want to do that in a separate PR, and if this PR alone fixes all the problems, maybe we don't need to do it. Either way, I'm really hoping we can get to a place where OP long doesn't fault when card restarts after openpilot settings menu changes.
| ret.cruiseState.enabled = pt_cp.vl["TSK_06"]["TSK_Status"] in (3, 4, 5) | ||
| ret.cruiseState.speed = ext_cp.vl["ACC_02"]["ACC_Wunschgeschw_02"] * CV.KPH_TO_MS if self.CP.pcmCruise else 0 | ||
| ret.accFaulted = pt_cp.vl["TSK_06"]["TSK_Status"] in (6, 7) | ||
| cruise_initialized = ext_cp.vl["ACC_06"]["ACC_Status_ACC"] != 1 |
There was a problem hiding this comment.
Let's make this latching, so we can't inadvertently jump to an init state later in case the radar browns out or crashes, and put a time limit on it. You can just match what we're doing for eps_init_complete.
RJWadley
changed the title
|
this doesn't actually help, I was mislead by openpilot timing differences during my initial tests. different PR coming soon with actual verified fix. there are actually two displayed errors here, and they should probably be fixed separately:
|
2 participants
goal: openpilot should not show "Cruise Fault: Restart the Car" when the car first starts
when the car first starts, braking is always unavailable.
This will temp fault the TSK even when using stock cruise control.
The stock cruise system ignores this fault because it happens while ACC is still initializing.
relevant signals
ESC_TSK_SRBM_nicht_verfuegbaronESP_33: braking unavailableTSK_StatusonTSK_06: TSK fault statusACC_Status_ACConACC_06: ACC init status from stock radarValidation (pending)