-
Notifications
You must be signed in to change notification settings - Fork 13.7k
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
PID rate controller - Add controller gain to support Ideal PID form (ISA standard) #12472
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.
Looks good, I have minor suggestions.
// to the ideal (K * [1 + 1/sTi + sTd]) form | ||
const float p_k = _param_mc_rollrate_k.get(); | ||
const float q_k = _param_mc_pitchrate_k.get(); | ||
const float r_k = _param_mc_yawrate_k.get(); |
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.
It would improve readability to use these gains in MulticopterAttitudeControl::control_attitude_rates()
, where the PID is implemented.
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.
Maybe TPA can scale the _K gains instead of the _P, _I and _D gains?
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.
I don't know for TPA, I'm not sure what's its real purpose.
If it's just pure gain scheduling because the dynamic of the vehicle changes at high thrust, we should leave it as is and if it's due to thrust non-linearity, we should scale K.
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.
I am not sure either:
- The documentation suggests that it corrects thrust non-linearity (suggesting TPA should be equal for P I and D components).
- However a search on the codebase shows that the platforms that use TPA (s250aq, zmr250, and qav250) do not scale the P I and D the same way. Maybe scaling K would have been equivalent?
For reference here is the PR that implements per-component TPA: #5861
@jlecoeur Thanks for your review, that's way more readable now! Can you review again please? |
@hamishwillee Question from dev-call has this been added to the user docs? |
@davids5 No, was not aware that this work had moved past proposal. @bresch Do you want to take first shot at it - or can you take some time to discuss? I've assigned issue on this to you PX4/PX4-Devguide#847 |
The introduction of a parameter that controls the total gain of the controller facilitate the tuning of a new airframe as it decouples properly the integral and derivative time constants. A user should be able to take the gains of a similar platform (size/inertia) and simply adjust this gain to have it flying properly.
Until now, with the pure parallel form, if the thrust to weight ratio is different from the reference config, the tuning has to be done almost from scratch.
The default value is 1.0 and basically scales P, I and D gains. This means that the current configurations and tunings are unaffected.
For more background, see this issue: #12362
Closes #12362