commaai / openpilot

openpilot is an operating system for robotics. Currently, it upgrades the driver assistance system in 275+ supported cars.
https://comma.ai/openpilot
MIT License
49.61k stars 9.02k forks source link

possible invalid use of steeringAngleDeg without calibration #33667

Open garrettpall opened 3 days ago

garrettpall commented 3 days ago

Describe the bug

Issue Car struggles to accelerate smoothly when overtaking a car. Car also does not speed up to set speed when not lead visible.

Attempted Troubleshooting Reflash (flash.comma.ai) and reinstall Turn off radar tracks and run VOACC Update to latest master-ci Run Longitudinal Maneuvers Testing Tool

accel Pulsing

Screenshot 2024-09-28 at 3 12 50 PM

image

vEgo not hitting setSpeed

Screenshot 2024-09-28 at 3 21 54 PM

Provide a route where the issue occurs

162796f1469f2f1b/00000001--b37821a3c3

openpilot version

3095fa79279e8c6424571a0195d04f1fcce579fa

Additional info

Disclaimer My car does not have long support upstream due to using extra panda, but code is still nearly identical to camera cars. Code changes should not have this affect on longitudinal plan (https://github.com/garrettpall/openpilot/commits/gm-long-test/)

garrettpall commented 2 days ago

After a bunch of looking around, I have determined the culprit to be limit_accel_in_turns. My steering angle is not 0 when driving straight but actually ~5.5 degrees (I probably need an alignment). limit_accel_in_turns looks only at sm['carState'].steeringAngleDeg and not angleOffsetAverageDeg. Peaks in accel are resulting from when limit_accel_in_turns is not limiting and accel drops to zero when it is limiting.

Accel and Steer Angle image

Measured and actual steer angle image

garrettpall commented 2 days ago

At 80 mph, this is the limits I calculated for my car:


Steering Angle: 5.1, Accel allowed: 1.08
Steering Angle: 5.2, Accel allowed: 0.97
Steering Angle: 5.3, Accel allowed: 0.84
Steering Angle: 5.4, Accel allowed: 0.69
Steering Angle: 5.5, Accel allowed: 0.49
Steering Angle: 5.6, Accel allowed: 0.09
Steering Angle: 5.7, Accel allowed: 0.00
Steering Angle: 5.8, Accel allowed: 0.00
Steering Angle: 5.9, Accel allowed: 0.00
Steering Angle: 6.0, Accel allowed: 0.00
Steering Angle: 6.1, Accel allowed: 0.00
Steering Angle: 6.2, Accel allowed: 0.00
Steering Angle: 6.3, Accel allowed: 0.00
Steering Angle: 6.4, Accel allowed: 0.00
Steering Angle: 6.5, Accel allowed: 0.00
Steering Angle: 6.6, Accel allowed: 0.00
Steering Angle: 6.7, Accel allowed: 0.00
Steering Angle: 6.8, Accel allowed: 0.00
Steering Angle: 6.9, Accel allowed: 0.00```

As seen above, we were definitely going above and below the 5.5 is degree cutoff
jyoung8607 commented 1 day ago

@garrettpall Thank you for the report! This is exactly the sort of thing we hope to flush out with the longitudinal maneuver test tool.

@haraschax This looks like a real issue to me. The angle offset isn't necessarily horrible, depending on platform. We don't even complain until the offset reaches ten degrees. I tried to report on the distribution but the commaCarSegments dataset doesn't contain liveParameters.

I'm told the uncalibrated usage in limit_accel_in_turns is probably going away soonbut there's another uncalibrated usage in paramsd which probably isn't doing us any favors. I'm not filing a PR right away because this will require some careful validation even if correct.

This is a very easy bug to write. It might be a good idea to sweep over xx and make sure nothing like this has crept into training or testing, something that might manifest later as ping-pong or hugging.

jyoung8607 commented 1 day ago

but there's another uncalibrated usage in paramsd which probably isn't doing us any favors

Correction, I think this usage is probably intentional, but it wasn't immediately obvious and I still think it's an easy bug to write.

garrettpall commented 19 hours ago

For what it’s worth, this is what I am running now to fix it: https://github.com/garrettpall/openpilot/commit/894f2afca56b4f908abfba957da150c7692b2150

@twilsonco wrote this commit