Add fields and states required for robust Volkswagen safety compliance

[jyoung8607]

This is the first in a series of PRs to upstream the Volkswagen, Audi, SEAT, and Škoda community port. However, almost all of these changes could be useful for other manufacturer ports.

Additional PRs will be opened for OpenDBC, Panda safety, and OP updates in due course. I am filing this one first to avoid breaking your upload and ingest pipeline. I will make the corresponding changes in our port if and when you merge this PR.

Add generic (manufacturer-independent) cruise-control button events in support of enhanced steering wheel or third stalk ACC control.

This change is required because our button semantics are different. Our cars DO NOT have the traditional overloaded res/accel or set/coast buttons; the Res and Set buttons behave exactly as labeled whether momentarily pressed or held. Res will not accelerate, and Set will not decelerate. The increase and decrease buttons usually adjust the ACC speed setpoint, but are temporarily mode-switched to following distance if the driver presses the gap adjust button. The speed setpoint CAN BE changed with ACC disengaged, and the act of changing it DOES NOT engage ACC.

In the short term, we need to be able to flow these button press events up into controlsd, if for no other reason than resetting the DM timer at night. We should be able to press the Res button as a DM reset no-op, without having it treated as accel with the wrong behavior. In the long term, when we tackle longitudinal control, we'll need to handle each of these individual buttons correctly. I understand that Comma has no intent to support manual gap adjustment for native longitudinal, but we still need it for stock.

For the reasons above, we cannot (ab)use the existing controlsd behavior for accelCruise and decelCruise Events. I am handling part of it with altButton1/2/3 today, but with poor clarity and obvious functional compromise, since we can only do that with three of the five buttons.

Add generic (manufacturer-independent) gearshift position definitions for Eco and Manumatic.

Definitions exist today for Sport and Low. I am currently lying to OP by saying Eco is Drive and Tiptronic is Sport; we would like to handle this more correctly and clearly. Manumatic is the best generic (manufacturer-independent) name available for manually-shifted automatic transmissions.

Add a preEntry Event state, in support of generic (manufacturer-independent) strict safety model compliance.

Our port obeys Comma's strict safety model, which has conditions beyond stock ACC requirements. I want to make sure failed engage attempts show something on the OP display about why. Doing this "right", preventing stock ACC from ever being engaged if there's a Comma safety model fail for which stock ACC would still work, cannot be done reliably without an architectural tweak (I tried!). The way Honda does it still lets stock ACC engage, even if briefly, and engaging OP before stock creates a timing race for controls mismatch.

My proposal is to add a preEntry state, a sort of companion to preEnable. Any port could raise a buttonEnable event with PRE_ENTRY set, as a signal for controlsd to show NO_ENTRY alerts without necessarily engaging OP at that moment. If this proposal is accepted and merged, I'll submit a follow-up PR to update the Disabled logic in controlsd state_transition().

Update the Volkswagen safety model ID from "vw" to "volkswagen" to be consistent with the existing community port.

No justification other than personal preference, as the guy doing much of the work here, and the fact I'd have to go back and do a ton of renaming. Help me out here? :)

Separately, please consider merging #3 for generic (manufacturer-independent) support of manual transmissions.

Again I can understand they might not be first class Comma supported, but we already have manual transmission users on the community Volkswagen port, and right now we're having to pretend they're in Drive all the time. We'd like to give them better safety handling, and surely your upload and ingest pipeline will appreciate not being lied to... if nothing else, this will give you a handy marker to identify them and toss the data if you don't want it!

[rbiasini]

accelCruise and decelCruise are enough, aren't they?

[jyoung8607]

I'm afraid they aren't. The detailed reasoning is in the wall of text above, but TL;DR:

This change is required because our button semantics are different. Our cars DO NOT have the traditional overloaded res/accel or set/coast buttons; the Res and Set buttons behave exactly as labeled whether momentarily pressed or held.
....

For the reasons above, we cannot (ab)use the existing controlsd behavior for accelCruise and decelCruise Events.

[jyoung8607]

I realize this may be confusing so let me try to add some more clarity around VW driver expectations from their completely factory stock ACC:

• Pressing RES should not accelerate (this will be a very useful no-op for night-time DM reset instead of bumping the steering)
• Pressing SET should not decelerate (doesn't matter that much, but still)
• Pressing INCREASE or DECREASE to change the ACC setpoint should not engage ACC if it's disengaged (this is done all the time in stops or traffic jams)

We can't use accelCruise and decelCruise because the controlsd code treats them as (engage if disengaged) AND (resume or set) AND (accel or coast) functions all-in-one. So, we need to teach controlsd about these different style buttons and their separate functions.

[rbiasini]

I'm happy to change the behavior in controls so you don't have to double define accelCruise and increaseCruise:

• for cars like Honda and GM, in car interface, I'll send both accelCruise and resumeCruise when the button is pressed, for example.
• controlsd will engage on setCruise and resumeCruise, instead of accelCruise and decelCruise.

Makes sense?

[jyoung8607]

In the short term, using stock ACC, that meets my needs and is completely acceptable. I have pushed a commit to withdraw the increase and decrease definitions. In the long term, if we have OP take long control (and I fully intend to), there are a few semantics we'll need to verify. However, that's not a problem I need to solve immediately.

Test case: A driver is cruising on the highway at their ACC setpoint of 65mph. They come upon a stop-and-go traffic jam, during which ACC has slowed the car to 15mph. The driver wants to bring down the ACC setpoint somewhat, so the car doesn't go bonkers accelerating if there's briefly a gap ahead, that another driver might merge into suddenly.

• Setpoint expectation on one touch of decrease: 64mph
• Setpoint expectation on hold of decrease: 60mph (jumps in 5mph increments per second
• What a car with overloaded set/coast semantics would do : 14mph

The same sort of scenarios can apply to accelerating as well as decelerating. I'm not completely sure that's how OP long works today, though, and if it does, your proposal of sending both events together for vehicles with overloaded button semantics should allow us to solve it cleanly.

The increase/decrease description was also a way to use them for following distance adjustment in the future, matching the stock capability. I'm given to understand that's not in Comma's strategy at this time. I have some feelings about that, but they won't be relevant in the near term future, and this PR is not the appropriate vehicle to express them anyway.

TL;DR: I'm cool with your suggestion.

[jyoung8607]

BTW, I had thought about calling this showAlerts instead of preEntry. Up to you. I just need something to tell controlsd to throw up alerts without engaging if the driver tries to engage with the seatbelt unbuckled, etc.

[rbiasini]

mmm... that's the point of the noEntry events. Why can't you use that?
Controls won't try to engage if a noEntry event is present (it won't transition to enabled state). The car might still engage ACC for a fraction of a second just because it reacts to the button press (can't do anything about it).

[jyoung8607]

PR has been brought current with your other work today, conflicts resolved, with fewer changes remaining in scope. Thank you for the feedback and consideration so far!