GM sends nonzero gas while preEnabled

[nworb-cire]

Describe the bug

GM's carcontroller sends nonzero gas in the preEnable state, i.e. gas is commanded despite the user's foot being on the brake pedal. This is due to only checking if CC.longActive == True, even though this may be the case while CS.brakePressed == True (in other words, while pre-enabled).

I haven't looked closely into if/how OP handles the preEnable state for other makes, but I believe that this has not been reported earlier because many GM vehicles have their minEnableSpeed set unnecessarily high, despite most being capable of enabling from zero.

This contributes to, though is not the sole cause of, the cruise fault we see when enabling from standstill-- the ECM faults if gas or brake commands are sent while user brake is active.

Allowing nonzero gas while user brake is pressed appears to be the root cause on the openpilot side of the same safety issue that I observed a few months ago.

Which car does this affect?

Chevrolet Volt 2018

Provide a route where the issue occurs

806907cd33ef2647|2023-06-24--09-43-33

openpilot version

0.9.2

Additional info

This bug affects the Acadia, Escalade and Escalade ESV in release. The enable speed is set unnecessarily high on most other GM vehicles, including my own, so I had to lower it in order to reproduce the issue. (I suppose I could have achieved the same result by manually fingerprinting as one of those.) I also needed to remove the dashcam flag, since the dashcamOverride param was removed in https://github.com/commaai/openpilot/pull/27509. This is why git is dirty in the route I uploaded.

[sshane]

Can you get a route with the stock system (ASCM activated) and in dashcam mode to find out what it does in these situations:

1. coming to a stop while enabled
2. enabling behind a car with foot on brake

my guess is it applies brakes with your foot on brake, if it didn't you may roll forward or backward on hills

[sshane]

https://github.com/commaai/openpilot/pull/25453 also accounts for this + brakes earlier. Can you try it? Right now we don't brake until we get to -1 m/s^2 which means the car will likely roll forward if you took your foot off the brake after engaging.

[sshane]

This does not affect the Acadia, quite possibly only Volt is affected, but also having @twilsonco test to make sure it's not related to the re-flashed ECM.

[twilsonco]

Here's a route showing this error several times in the Volt ('18 Premier w/ ACC). Reproduce error by pressing set or resume while stopped with foot on brake. It also faulted once when I tried to engage without brakes pressed but on a steep incline where it wasn't creeping. Release brake pedal and set/release will engage property so long as there's any amount of forward velocity is seems. @sshane @nworb-cire https://connect.comma.ai/c11fcb510a549332/1687963320196/1687963569703 Also relates to https://github.com/commaai/openpilot/pull/28703

Here it's just as @nworb-cire describes. I press button, op sends gas while brakes are pressed, and ACC faults in response

[twilsonco]

@sshane Not only https://github.com/commaai/openpilot/pull/25453 but also https://github.com/commaai/openpilot/pull/25485 will help with this issue. The latter will help with it being able to engage from 0 on inclines.

[sshane]

They are both unrelated to this issue, but the first produces the desirable behavior of requesting brake at low speeds earlier, which happens to fix this fault. Thanks for the tests, looks like the same fault. Now we just need one more test where gas is always inactive to make sure it doesn't fault and that it is the cause.

Did you say it faults if you engage while moving backwards? This should've caught that https://github.com/commaai/openpilot/blob/799c6f513d4e4146d69446613b39310c0c6fed59/selfdrive/car/gm/interface.py#L279

Or are you saying it faults if you are stood still with no user brake?

Regardless, let's fix each fault one by one.

[nworb-cire]

Now we just need one more test where gas is always inactive to make sure it doesn't fault and that it is the cause.

I have done this previously and there is no fault if INACTIVE_REGEN is continually sent.

[nworb-cire]

I was thinking, perhaps this issue is more general than just GM. On all makes, while the vehicle is stationary, and until it receives the CruiseControl.resume command, should the long controller not always send CP.stopAccel as the actuators.accel?

[twilsonco]

Did you say it faults if you engage while moving backwards? This should've caught that https://github.com/commaai/openpilot/blob/799c6f513d4e4146d69446613b39310c0c6fed59/selfdrive/car/gm/interface.py#L279

One of the faults in the route I posted above was without brakes pressed, but with vehicle moving slightly backwards. It's at the beginning of the route.

[nworb-cire]

One suggestion for how to fix this:

if not CC.longActive:
  # ASCM sends max regen when not enabled
  self.apply_gas = self.params.INACTIVE_REGEN
  self.apply_brake = 0
elif at_full_stop and not CC.cruiseControl.resume:
  self.apply_gas = self.params.INACTIVE_REGEN
  self.apply_brake = self.params.MAX_BRAKE
else:
  self.apply_gas = int(round(interp(actuators.accel, self.params.GAS_LOOKUP_BP, self.params.GAS_LOOKUP_V)))
  self.apply_brake = int(round(interp(actuators.accel, self.params.BRAKE_LOOKUP_BP, self.params.BRAKE_LOOKUP_V)))

[sshane]

@twilsonco @nworb-cire let me know if you hit any other issues similar to this. Still looking to get https://github.com/commaai/openpilot/pull/25453 merged if we can get some test routes soon