Skip to content
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

Highlander Tuning #690

Open
wants to merge 2 commits into
base: devel
from

Conversation

Projects
None yet
3 participants
@daehahn
Copy link
Contributor

commented Jun 7, 2019

kpV, kiV: With Phil Lee's PR - #464
kf: Tested with many curves more than several months
stiffnessFactor: It's now live parameter, but change with more realistic init value

Highlander Tuning
kpV, kiV: With Phil Lee's PR - #464
kf: Tested with many curves more than several months
stiffnessFactor: It's now live parameter, but change with more realistic init value

@geohot geohot added the tuning label Jun 7, 2019

@geohot

This comment has been minimized.

Copy link
Contributor

commented Jun 7, 2019

Hmm, so it's hard for me to read this and know if it's good. Though it's simple and does remove a "not optimized yet" comment, so I'm inclined to like this.

@rbiasini can we come up with a procedure to merge tuning PRs?

ret.mass = 4607 * CV.LB_TO_KG + std_cargo #mean between normal and hybrid limited
ret.lateralTuning.pid.kpV, ret.lateralTuning.pid.kiV = [[0.6], [0.05]]
ret.lateralTuning.pid.kf = 0.00006
ret.lateralTuning.pid.kpV, ret.lateralTuning.pid.kiV = [[0.18], [0.0075]] # from Phil Lee's PR

This comment has been minimized.

Copy link
@rbiasini

rbiasini Jun 7, 2019

Contributor

Ki seems way too low (as is, it does nothing). I propose to use the same tuning as the Avalon here, [0.17], [0.03].

Kf at 0.00012 is fine.

This comment has been minimized.

Copy link
@daehahn

daehahn Jun 7, 2019

Author Contributor

I will try with your suggested values and get back here. Thanks Riccardo!

This comment has been minimized.

Copy link
@daehahn

daehahn Jun 11, 2019

Author Contributor

[0.17], [0.03] is better than [0.6], [0.05]. But still it has jerk movement (shaking left-right) with [0.17], [0.03] and [0.17], [0.025] under 35mph. I will try with [0.17], [0.02] today and report it again.

This comment has been minimized.

Copy link
@daehahn

daehahn Jun 12, 2019

Author Contributor

@rbiasini, [0.17], [0.02] also has same jerk movement, but [0.17], [0.0015] was much better. So I updated with old stable parameters, which is [0.18], [0.015]. I used this parameters more than 4 months without any issues. Please review and merge this request.

@rbiasini

This comment has been minimized.

Copy link
Contributor

commented Jun 7, 2019

Yeah, as discussed, we are going to merge tuning PRs as long as:

  • they get consensus from 3 different people using this car
  • the proposed tuning makes physical sense and does not introduce obvious deficiencies (just to iron out extreme placebo effects...)
Update kiV value
Since [0.17], [0.03] ~ [0.02] has jerk movement under 30mph, back to stable old parameter.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.