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

PID rate controller - Add controller gain to support Ideal PID form (ISA standard) #12472

Merged
merged 3 commits into from
Jul 16, 2019

Conversation

bresch
Copy link
Member

@bresch bresch commented Jul 12, 2019

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

@bresch bresch added this to the Release v1.9.2 milestone Jul 12, 2019
@bresch bresch self-assigned this Jul 12, 2019
@bresch bresch added this to In progress in Multicopter via automation Jul 12, 2019
@bresch bresch changed the title PID rate controller - Add controller gain to support Ideal PID form ISA standard) PID rate controller - Add controller gain to support Ideal PID form (ISA standard) Jul 12, 2019
Copy link
Contributor

@jlecoeur jlecoeur left a 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();
Copy link
Contributor

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.

Copy link
Contributor

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?

Copy link
Member Author

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.

Copy link
Contributor

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

Multicopter automation moved this from In progress to Review in progress Jul 12, 2019
@bresch
Copy link
Member Author

bresch commented Jul 15, 2019

@jlecoeur Thanks for your review, that's way more readable now! Can you review again please?

Multicopter automation moved this from Review in progress to Reviewer approved Jul 15, 2019
@bresch bresch merged commit d24c415 into master Jul 16, 2019
Multicopter automation moved this from Reviewer approved to Done Jul 16, 2019
@bresch bresch deleted the dev-ideal-pid branch July 16, 2019 08:24
@davids5
Copy link
Member

davids5 commented Jul 31, 2019

@hamishwillee Question from dev-call has this been added to the user docs?

@hamishwillee
Copy link
Contributor

@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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Multicopter
  
Done
Development

Successfully merging this pull request may close these issues.

PID implementation - Parallel vs Ideal
4 participants