GM: fix ASCMGasRegenCmd scale and units

[sshane]

Correct scaling and offset now. Plus checksum is 1 bit larger, which captures that fake inverse bit, removed some set mes.

Opened to preserve, will verify later.

This does not change behavior.

[garrettpall]

This update would allow 2 m/s^2 acceleration on GM trucks. The extra bits added to GasRegenCmd are needed to achieve this.

[sshane]

I would like to understand why the force doesn't match positive aEgo before we merge (Bolt)

[1454]

Is there anything needed from me to support this fix?

[sshane]

Is there any progress on this to help us GM1500 folks with painfully slow acceleration? It's borderline unusable.

You can start by providing a route showing this issue or ideally running the longitudinal maneuver tool.

[garrettpall]

What is "accel from force"/what's it's formula?

[1454]

Is there any progress on this to help us GM1500 folks with painfully slow acceleration? It's borderline unusable.

You can start by providing a route showing this issue or ideally running the longitudinal maneuver tool.

Is there any progress on this to help us GM1500 folks with painfully slow acceleration? It's borderline unusable.

You can start by providing a route showing this issue or ideally running the longitudinal maneuver tool.

I'll get that to you tomorrow or Thursday at the latest. Thanks Shane.

[sshane]

a = F/m

[morrislee]

Is there any progress on this to help us GM1500 folks with painfully slow acceleration? It's borderline unusable.

You can start by providing a route showing this issue or ideally running the longitudinal maneuver tool.

dc7716b32bf25574/00000009--a7db12be52/0

Please check the above route to see if this works 🤗

[garrettpall]

dc7716b32bf25574/00000009--a7db12be52

Route is private

[morrislee]

dc7716b32bf25574/00000009--a7db12be52

Route is private

made public and preserved now

[garrettpall]

a = F/m

Factoring in the gas delay, it all looks reasonable

CoeffDrag = .30 (seems stable)
CoeffRolling = 0.008 (seems stable)
tireRadius = 0.42 (varies by car)
frontalArea = 3.971605 (varies by car)
g = 9.81 (constant)
airDensity = 1.225 (constant)
torque = value (gasRegenCmd)
mass = v1
vego = v2

accel = ((torque / tireRadius) - (.5*CoeffDrag*frontalArea*airDensity*vego*vego) - (CoeffRolling*mass*g))/mass 

return (accel)

XT4:

tireRadius = 0.37
frontalArea = 3.13

Silverado:

tireRadius = 0.42
frontalArea = 3.97

Bolt:

tireRadius = 0.32
frontalArea = 2.85

[garrettpall]

F_rolling isn't doing too much and could probably be removed. To be complete we should also add a F_pitch term to compensate for vehicle pitch going up and down hills.

or

[morrislee]

What else would you need to get this moving again? This PR has been over a year now :( Would love to see some love for GM peeps

[garrettpall]

What else would you need to get this moving again? This PR has been over a year now :( Would love to see some love for GM peeps

Unfortunately until there is a way to tell the Trailblazer and Silverado apart, any tuning efforts have to apply to both. Giving a Trailblazer the same torque command as a Silverado will not act the same.

[garrettpall]

I made a test branch without pitch compensation and linear fitting wheelbase to tire size and frontal area.

Old:

New:

[jyoung8607]

Panda needs a corresponding change, it's also only checking 12 bits wide: https://github.com/commaai/panda/blob/9aec4294cc940964a5b2fa34aa83301ab408b377/board/safety/safety_gm.h#L137

I'm not super stoked the "over-actuation" from people test driving this gets through Panda safety checks. I'm guessing the overflow was still within bounds. Protection against setting undefined bits is a worthwhile but separate conversation.

[garrettpall]

Panda needs a corresponding change, it's also only checking 12 bits wide: https://github.com/commaai/panda/blob/9aec4294cc940964a5b2fa34aa83301ab408b377/board/safety/safety_gm.h#L137

I'm not super stoked the "over-actuation" from people test driving this gets through Panda safety checks. I'm guessing the overflow was still within bounds. Protection against setting undefined bits is a worthwhile but separate conversation.

Since there currently isn’t a good way to check torque in panda since it changes based on vehicle for a given accel, my current test branches impose the same old limits in panda and car controller:

https://github.com/garrettpall/openpilot/blob/d890a448cd2a8d22b5a31e324e73cb875b20bdf2/opendbc_repo/opendbc/car/gm/carcontroller.py#L104

Needing to go over that limit is really only helpful with large vehicles like the Silverados which currently only get 1 m/s/s with the current limits, but the formal scales correctly and gives an accurate response. On frog pilot where we already include the extra bits for more acceleration we changes the panda checks to include the extra bits also:

https://github.com/FrogAi/FrogPilot/blob/0e68dff54ba937c0cfabfd6f0f54856010fd543e/panda/board/safety/safety_gm.h#L246

While yes, there is a way to modify the limits unsafely so it basically has no limit, we are not doing that.

TL/DR: The test branches for the formula have been implanted keeping the current limits intact and the formula is only used for command mapping.

[jyoung8607]

To be completely clear, it's fine that you're testing fixes here, just saying the signal width fix has two parts that need to merge together. If we need to allow higher torque demand for heavier vehicles, we can add safety parameter settings in a follow-up round.