GM longitudinal pitch compensation #25469
Conversation
missed pitch cleanup
for compensating long actuator delay cleanup revert Road Pitch string missed pitch future future pitch no kalman future pitch reduce smoothing was 0.01 in carstate at 100Hz, now use 0.05 for 20Hz. future_pitch not class member slighlty less pitch smoothing less pitch smoothing add liveParams pitchFutureLong
bump cereal bump cereal bump cereal add pitch accel deadzone
twilsonco
force-pushed
the
pr-gm-long-pitch-compensation
branch
2 times, most recently
from
af6f1fe
to
e88d676
Compare
|
Need to record routes on each PR in the stack. Will be ready for review after that |
using smoothed, future pitch to account for long actuator delay switch signs use existing deadzone function cc cleanup log pitch-adjusted actuatorOutput.accel and correct import update submaster at 20Hz gm use accelPitchCompensated gm use accelPitchCompensated bump cereal only use pitch-compensation to determine gas only use pitch to determine brakes on incline only use pitch-comp accel for brakes at speed
twilsonco
force-pushed
the
pr-gm-long-pitch-compensation
branch
from
872d420
to
a01439e
Compare
|
Routes recorded for other three PRs in the stack (not a real stack), so they're ready for review. Need to record a final route for this PR |
twilsonco
changed the title
|
Generally when proposing a large change, it's best to open a single PR with all your changes so we can see what it would look like. Then if the PR is wanted and it could be split out into smaller PRs, we'd do that at that time. |
|
Route: c11fcb510a549332|2022-08-18--07-27-30--5 |
|
As I mention in the related pull requests. Future pitch is necessary in order to account for long actuator and other delays. Humans don't wait for the speed to drop before applying more gas when climbing a hill. They do it proactively, which avoids the error, avoids overcorrecting for the error, and improves efficiency and passenger comfort. Surely if the model is reliable enough to plan a path to drive on, it's reliable enough to plan a tiny change in acceleration? There are rate checks and caps in place to ensure predicted pitch values are safe and sane. I've been driving on this logic for close to 1000 miles. It works great, consistently. |
|
I'll reimplement a standalone carcontroller version without using model pitch. |
bounds check
bounds check use correct pitch bounds and rates
bounds check use correct pitch bounds and rates
Part of PR stack, but I don't know how to submit a PR to commaai/openpilot that's comparing two branches in the opgm repo, so the layers of the PR stack will include the commits from the previous.
Description
Adds longitudinal pitch compensation to be used in cars that use gas/brake control, where the command specifies an amount of acceleration force rather than an amount of acceleration relative to the road. The implementation is contained in car controller, as a type of passive long feedforward, transparent to the controller.
To allow this to account for longitudinal actuator delay to allow for smoothing of the pitch signal, future model pitch is used to create a smoothed, predictive pitch.
Though not shown here, another benefit of this improvement is that if more than
ACCEL_MAXacceleration is required to maintain speed up a steep hill, stock will not be able to maintain speed. With this improvement that is no longer the case.actuators.accelPitchCompensatedis used to determine the gas command, which specifies an acceleration force. The brake command on gm specifies acceleration relative to the ground, so theactuators.accelvalue is the correct value for friction brakes, even when going downhill.To support this assertion, see that gas command does not match up with aEgo:
While brake command does match with aEgo
I also verified this extensively (though not with proper documentation) when developing "one pedal mode" in my openpilot fork, which involved artificially sending a constant brake command and observing the car's braking behavior. I found that the same brake command produces far more braking force on a decline, and far less (or none) on an incline.
However, when openpilot requests negative acceleration in order to stop behind a lead when going uphill, it would engage the friction brakes sooner (at the same time it would now, without this improvement), despite the negative gravitational acceleration, and resulting in more braking than in necessary, since it's being assisted by gravity.
So you'd like for openpilot to postpone friction brake use when decelerating uphill, but it can become a problem when stopping, as the pitch-compensated acceleration may remain positive even though openpilot would like to stop, resulting in late stopping behind leads.
The solution is to smoothly switch to non-pitch-compensated acceleration for determining brakes at low speed.
I've been using this method on my fork for about a month and it keeps the efficiency and brake-avoidance benefits of pitch-compensation without introducing possible problems when stopping behind leads.
(Any issues caused by the gas and brake signals having different physical meanings will also be minimized by using the correct gas/brake threshold acceleration value)
This should also be used on other makes that specify acceleration force with gas or brake commands
Verification
The result is a complete elimination of long error due to pitch changes. In the following image the plots on the left are without pitch compensation, and on the right with pitch compensation.
See that as I climb up the stepped hill (pitch changing between 0 and ~12% grade), the pitchFutureLone (right side, second plot from top, orange line) predicts the liveParams pitch by ~0.4s, which is the value set for longitudinalActuatorDelayLowerBound (and upper bound), so it's predicting by the correct amount.
While there are large deviations between target and actual acceleration (fourth plot from top, left side) without pitch compensation, the pitch-induced error is completely removed on the right (error stays within ±0.05m/s^2 vs ±0.25m/s^2).
Also the with pitch compensation speed stays within 0±0.1m/s^2 compared to ±0.4m/s^2 (top plot).
Here's another image showing data where I descend down the same stepped hill. Again, with pitch compensation, the speed and acceleration oscillations due to changing pitch are completely removed.
This PR includes three other PRs to 1) add liveParams pitch, 2) add liveParams future predicted pitch based on long actuator delay, and 3) to add calculation of pitch-compensated acceleration, saved to actuators.accelPitchCompensated.
Route: c11fcb510a549332|2022-08-17--13-58-51--0