GM: Runaway set speed increase when no acc buttons pressed

[twilsonco]

Describe the bug

While testing for this PR using the following procedure, I encountered a bug where the set speed was increasing in increments of 5mph anytime openpilot was engaged, regardless of ACC button presses.

However, after off-road/on-road cycling my C3, a subsequent attempt of the procedure did not reproduce the bug. This is a preliminary report, and I don't yet know how to reproduce the bug consistently. This has only happened once to me. I suspect the bug is due to me having been holding the res/accel button as openpilot came to a stop behind the lead (which involves a change in ACC cruise state), but again, repeating the below procedure didn't reproduce the bug.

Bug description: Once the bug as been triggered, op increases the set speed as if you're pressing and holding the res/accel button, resulting in 5mph increments to the set speed every half second. Presses of the set/decel button or the res/accel button will result in 1mph decrements or increments to the set speed, in addition to those cased by the persistent phantom res/accel button hold.

Testing procedure :

1. engage and set speed to 5mph
2. Pull behind parked car and as op slows, increase set speed to 90mph (I stopped at 85, but same thing). op will still slow and stop behind parked lead
3. Once stopped, you can use the set/decel button to lower speed 1mph at a time and confirm visually that the dash speed matches the onscreen op set speed.

Which car does this affect?

GM Chevy Volt

Provide a route where the issue occurs

c11fcb510a549332|2022-08-13--10-34-36

openpilot version

67cd27a9e153eef6359a85575c6fe803f9be602a

Additional info

Commit has corresponds to latest commit of the gm-cruise-speed-scaling branch.

[twilsonco]

Here are some plots showing the behavior:

Zoomed segment A:

Here's a zoomed shot of the right half of segment B Matching video:

https://user-images.githubusercontent.com/7284371/184508741-9e9d07cb-119e-4dab-b41e-3da80b3a4438.mp4

Segment C (and surrounding region)

And another of D and E

[twilsonco]

Plots are, from the top down:

1. overlaid set speed (vCruise and CAN dash set speed),
2. ACC button presses (2 = res/accel, 3 = set/decel), VAL_TABLE_ ACCButtons 6 "Cancel" 5 "Main" 3 "Set" 2 "Resume" 1 "None" ;
3. vEgo
4. ACC cruise state from CAN
5. CAN lead car status, friction brake "mode", and friction brake "full stop active" status

Taking us through the images:

Segment A:

is as I slow behind a parked lead, and then press and hold the res/accel button. Note that I'm still holding the button as the vEgo goes to zero at t=170s, accompanied by a brief change in CruiseState, change in the FrictionBrakeMode, and a change in the FullStopActive bit.

I continue to hold the button until set speed climbs to 90mph and then release.

Segment B:

Here you can see me mashing the set/decel button repeatedly (ACC button = 3), which lowers the set speed but then it keeps climbing back up, even though I'm stopped (so the resume button would not increment speed even if I pressed it) and even though I'm not pressing res/accel.

Segment C:

Same as B but I tried to press and hold the set/decel button between t=217.5 and t=219.5s. This resulted in 5mph decrements in set speed but it quickly increased back to 90mph, again absent any res/accel presses.

Segment D/E:

Here's the scary part. Confused. I disengaged, pulled around the parked car, drove off and engaged again (at t=277s), resetting my speed to 25mph, at which point it started climbing again by 5mph as if I were still holding the res/accel button. I try to press the set/decel button to lower the speed, which works, but it's still climbing by 5mph every 0.5s so the car accelerates quickly and I disengage at t=282s with the cancel button (ACC button = 6).

I disengage an engage again at t=287s. You see the set speed incrementing by 5mph absent any ACC button presses (I press set/decel a couple of times, but most of the 5mph increment events are not accompanied by a button press, so it was doing it on its own).

Video starting at t≈380:

Here's plots corresponding to the video

and the video: In case you can't tell, "I'M NOT TOUCHIN' ANY BUTTONS RIGHT NOW!!". I think the acceleration made me excited.

https://user-images.githubusercontent.com/7284371/184507908-98b67139-ead8-4fcc-be3b-542ba4335686.mp4

[twilsonco]

@Verylukyguy informs me he's had this runaway speed behavior occur too. So it's at least Volt + Acadia

[twilsonco]

Here's plots also showing the button events saved to car state for the segments A and B

  # send on change
  struct ButtonEvent {
    pressed @0 :Bool;
    type @1 :Type;

    enum Type {
      unknown @0;
      leftBlinker @1;
      rightBlinker @2;
      accelCruise @3;
      decelCruise @4;
      cancel @5;
      altButton1 @6;
      altButton2 @7;
      altButton3 @8;
      setCruise @9;
      resumeCruise @10;
      gapAdjustCruise @11;
    }
  }

[twilsonco]

So neither the CAN ACC button press for res/accel or the cereal car state res/accel button press is being set in a way that corresponds to the set speed increases.

Indeed, the car state button event type is staying at 4 = decelCruise. the last accelCruise value is seen here in the above image at t=165s.

Taken together, this would seem to mean the the problem is also not occurring in the GM interface:

Nor in car.init:

Which would make this appear to be a global problem, not limited to gm (@adeebshihadeh).

[twilsonco]

I notice that the CS.buttonEvents is only set if the conditional on line 170 of gm/interface.py runs, causing most frames to not have any data about the unpressed status of the buttons.

[twilsonco]

If the problem isn't something to do with ret.buttonEvent not being set each iteration, then it should be in controls or drive helpers:

[sshane]

Thanks for the detailed bug report. Sounds like something similar to the Hyundai issue we fixed. I'll check it out

[sshane]

Looks like you should be able to reproduce this pretty easily if you hold the resume button down, brake to a standstill, then release the resume button. It's because of that special exception for GM to ignore the resume button at a standstill so we don't increment the set speed.

You can see here we send a pressed button event for accelCruise, but we never send the unpressed event.

We should make this generically handled, even though it seems like GM is the only make where pcmCruise=False and you need to also press the resume button.

[JMPZ11]

Do all cars require the resume button for stop-and-go? Even the EUVs?

[twilsonco]

I'm speculating but it seems to be typical for GM ACC, so I would think it applies to all GM.