-
Notifications
You must be signed in to change notification settings - Fork 1.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
2022 Honda Civic Torque Tuning #1616
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for contributing to opendbc! In order for us to review your PR as quickly as possible, check the following:
- Convert your PR to a draft unless it's ready to review
- Read the contributing docs
- Before marking as "ready for review", ensure:
- the goal is clearly stated in the description
- all the tests are passing
- include a route or your device' dongle ID if relevant
I'm not sure about this change yet. Some notes: Your actuator delay of 0.5 seems very excessive. The prior delay of 0.1 looks just about right, even 0.2 looks to be too much. I could believe that your actuator delay setting might produce a subjectively appealing turn-in experience, but at the level of controls tuning, we need to faithfully execute the lateral plan we get from the model. steerRatio is wandering around, often quite far from your initial default value, and tireStiffnessFactor is running away as described in commaai/openpilot#33074. This is not your fault, but it's going to make tuning a bit of a challenge. The torque learner is applying a value considerably higher than your configured value of 1.25, so in the real world this is going to drive a lot different for the first 30 minutes or so after a reset/reinstall. I've found that applying the live-learned values at this stage of tuning may be counterproductive. For now I'd suggest removing that change in openpilot to make it so that its learned value isn't applied live, but it's always running and observing anyway, and you can update the manual config based on its logs later. TL;DR: I think this one needs to go back to the lab. FWIW, I think the lateral accel torque controller (and maybe paramsd as well) may just not get along well with progressive ratio steering. Volkswagen is also stuck on the old PIF controller due to basically the same problems you're having. I have ideas for how we might adapt the controller to work better for us, but it's going to take some work. |
@jyoung8607 made a new branch https://github.com/MarcoTheDingo/opendbc/tree/civic-tune with just the better tuning parameters if that looks good enough to merge or advice on how to improve. Still has some oscillation at lower speeds, but generally a bit better. Thanks! |
The only change between that branch and master is a lateral accel factor that you're not using. That route was actually driven on the PIF controller. Let's take this back to Discord until it's in a better state of readiness for review. |
Updated the 11th gen with better tuning and torqued live tuning. Then changed actuator delay to 0.5 to make it match planner. Much better responsiveness overall, but especially at low speeds.
Stock route: 5130484aa8069bad/00000000--5da80373ec
Long torqued route with 0.4 delay: 5130484aa8069bad/0000004e--ab53601964
Short torqued route with 0.5 delay: 5130484aa8069bad/00000002--a454a60ba7
Requires: commaai/openpilot#34341