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

Update default parameters #12543

Merged
merged 7 commits into from
Jul 29, 2019
Merged

Update default parameters #12543

merged 7 commits into from
Jul 29, 2019

Conversation

RomanBapst
Copy link
Contributor

@RomanBapst RomanBapst commented Jul 23, 2019

See #12361 for context.

Also see individual commit message for some explanations.

fixes #12361

- generally better attitude tracking
- much better altitude control immediately after takeoff (hover throttle offset)
- faster landing detection

Signed-off-by: RomanBapst <bapstroman@gmail.com>
- we have set the maximum yaw-rate MC_YAWRATE_MAX to 200 degrees and it
makes senses to leave a bit of margin between what the user can demand
and what the limits of the vehicle are

Signed-off-by: RomanBapst <bapstroman@gmail.com>
- vehicle will fly less aggressive
- it does not make sense to set the lower acceleration limit to something
that exceeds gravity if most of the vehicles do not support reverse thrust

Signed-off-by: RomanBapst <bapstroman@gmail.com>
Signed-off-by: RomanBapst <bapstroman@gmail.com>
Signed-off-by: RomanBapst <bapstroman@gmail.com>
Signed-off-by: RomanBapst <bapstroman@gmail.com>
@RomanBapst RomanBapst requested review from Stifael and MaEtUgR July 23, 2019 15:36
@bresch bresch changed the title Pr defaults Update default parameters Jul 23, 2019
Copy link
Member

@bresch bresch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@Stifael
Copy link
Contributor

Stifael commented Jul 23, 2019

Just one comment to think about:
Given the fact that each of the tuning parameters is very specific to the type of vehicle, I was wondering if it would not make more sense to use MPC_POS_MODE=0 as the default where all the higher-level tuning parameters (MPC_ACC_HOR, MPC_ACC_UP_MAX...) have no effect. This way the user will directly experience how well the more important PID tuning parameters are set and will not be misled by the higher-level tuning.

@bresch
Copy link
Member

bresch commented Jul 24, 2019

@Stifael I agree that in an ideal world this would be correct, but most likely the reaction of the user will be: "Oh, it feels jerky (we hear that a lot), the firmware is bad". We want to make sure that the user experience is good the first time. The experienced user knows what to do and the user that has a first good experience might take time to further tune the vehicle.

@Stifael
Copy link
Contributor

Stifael commented Jul 24, 2019

@Stifael I agree that in an ideal world this would be correct

But that is exactly my point. I think in an ideal world you can find default parameters that make sense, but in reality, it is pretty much impossible to find values where the user will have an out-of-the-box good experience. How did you come up with MPC_ACC_HOR = 3 vs 10? Is there a reference platform that a new user can refer to?

@bresch
Copy link
Member

bresch commented Jul 24, 2019

@Stifael Those parameters used to design the trajectories are good for a drone up to ~25kg with average thrust/weight ratio (~2:1). It might feel "too smooth" for a small drone, but we rarely heard someone complaining that the drone flies too smoothly. Why manual and auto accelerations of 5 and 3m/s2, simply because this limits the demanded tilt to about 30 and 20 degrees, respectively. 10m/s2 generates a tilt of ~45 degrees and the maximum tilt MPC_TILTMAX_AIR is 45 degrees; you don't leave any space for the controller.
We could have chosen 4, 6 or something else; it's king of arbitrary, but this gives a good feeling with a wide variety of platforms. We tested those values on F450, F550, QAV-250, BabyShark, Tarot 960. Would you prefer an other value?

@Stifael
Copy link
Contributor

Stifael commented Jul 24, 2019

Would you prefer an other value?

No, I don't have a strong opinion, Just know that 3 is quite low and that it is not only weight dependent. I personally can't really contribute much here anyway, given that I can't evaluate what reasonable good default values are.

@Stifael Stifael removed their request for review July 24, 2019 11:46
@bresch
Copy link
Member

bresch commented Jul 24, 2019

@PX4/testflights Can you please test those parameters on several vehicles please? (not necessarily different autopilots, just different drones)

@jorge789
Copy link

Tested on PixRacer V4:

Modes Tested

Position Mode: Good.
Altitude Mode: Good.
Stabilized Mode: Good.
Mission Plan Mode (Automated): Good.
RTL: Good.

- Procedure
Arm and Take off in position mode, after flying for approximately one minute, switched to altitude then stabilized mode proceed to switch to mission plan mode then make sure that vehicle follows all waypoints as shown in QGC, once completed all waypoin activate RTL and see landing behaviour.

Notes:
The vehicle on the flight showed a little slower (smooth)
No issues noted, good flight in general.

Logs

https://review.px4.io/plot_app?log=7c30a7fe-b46c-4e5c-b942-daf341d21799

https://review.px4.io/plot_app?log=5267c015-5f01-4ecc-bcee-d5c0e5e768d9

Tested on Pixhawk 2 Cube V3:

Modes Tested

Position Mode: Good.
Altitude Mode: Good.
Stabilized Mode: Good.
Mission Plan Mode (Automated): Good.
RTL: Good.

- Procedure
Arm and Take off in position mode, after flying for approximately one minute, switched to altitude then stabilized mode proceed to switch to mission plan mode then make sure that vehicle follows all waypoints as shown in QGC, once completed all waypoin activate RTL and see landing behaviour.

Notes:
The vehicle on the flight showed a little slower (smooth)
No issues noted, good flight in general.

Logs
https://review.px4.io/plot_app?log=8e5c002a-d0b7-4e8d-9b49-335f158a1795

https://review.px4.io/plot_app?log=ba59d64e-20cb-4876-b505-0863acdaffb1

@dagar dagar merged commit b75d2ce into master Jul 29, 2019
@dagar dagar deleted the pr-defaults branch July 29, 2019 14:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

List of default params that need an update
5 participants