search

sshane / openpilot

DOES NOT SUPPORT COMMA 3X - Stock Additions (0.8.14): 3/automatic following distance profiles, customizable fork params with opEdit, and a smoother longitudinal experience for TSS2 Toyotas
MIT License
220 stars 85 forks source link

Velocity and Acceleration Error Integrator is Not Used in Dynamic Follow #482

Closed krkeegan closed 3 years ago

krkeegan commented 3 years ago

https://github.com/sshane/openpilot/blob/dc1c69fef352291b3776377eca038dcbdf988575/selfdrive/controls/lib/dynamic_follow/__init__.py#L290-L309

I noticed that the upcoming Openpilot release has redesigned the Long MPC. So I was looking through the dynamic follow code to try and understand if it would need to be adjusted for the new design.

In the process, I found the above. It looks like you created essentially an integrator for Vel and Acc error and the code uses that to modify the TR distance in lines 294 & 295. Decreasing the TR time for acceleration and increasing for deceleration. However, the code throws all this work away in the stop and go logic that starts on 300. TR gets set to a new value on either line 302 or 307 ignoring any prior calculations you made.

I am sure the code is a lot to maintain with everything else you have going on. I just wanted to point this out in case it had been overlooked.

sshane commented 3 years ago

Yep looks like you're right, good catch! The file has become a mess recently. I only added the integration due to jerky braking on my specific car (that was trying to fix tuning with planning, in hindsight. So, a bad idea!) and have never tuned it since, so I missed that. It can probably be cleaned up now, or even have the new integration code reverted to bring the old mods back (just simple relative velocity mods+extras).