COPTER improvement - separate max ascent and descent speeds in LOITER

[sashan13]

Issue details

Right now we have one parameter PILOT_VELZ_MAX that controls both max ascent and descent speeds in Althold and Loiter. I have it set to 200 so that the copter doesn't descend sickeningly fast. However, that makes ascent very slow. I gave my copter to 2 other people to try and they both agreed that the issue is there.

DJI, for example, has them separated and set to hardcoded -2 to +4 m/s.

I propose to add separate parameters for max ascent and descent speeds. Similar to these but for manual flight: WPNAV_SPEED_DN and WPNAV_SPEED_UP.

Version

3.5.2

Platform

[ ] All
[ ] AntennaTracker
[X] Copter
[ ] Plane
[ ] Rover
[ ] Submarine

Airframe type

4kg Quadcopter

Hardware type

Pixhack by CUAV

Logs

No logs

[robustini]

I agree, missing this option.

[Pedals2Paddles]

Seconded!

[proficnc]

+1 :)

[ChrisBird]

I'm happy to give this one a go - had a quick look at lunch time today and it appears to be within my abilities. Some lessons learnt from the last time, are the following parameters acceptable:
PILOT_VELZ_MAX_UP
PILOT_VELZ_MAX_DN

Or is something else preferred? Should I drop the existing parameter or for backwards compatibility should I leave it and then use the new values if present and fall back to PILOT_VELZ_MAX as a default?

I noticed that sub uses the same logic too, should I change it there too?

As it'll be in the same blocks of code, do we want the same acceleration for both up and down or would we prefer to be able to do asymmetric acceleration too?

[sashan13]

I believe new parameters and fallback are better. As for acceleration I'm not sure but I'd test them separated. Better set two parameters to same values then be unable to separate them. On Sep 11, 2017 06:24, "ChrisBird" <notifications@github.com> wrote: I'm happy to give this one a go - had a quick look at lunch time today and it appears to be within my abilities. Some lessons learnt from the last time, are the following parameters acceptable: PILOT_VELZ_MAX_UP PILOT_VELZ_MAX_DN Or is something else preferred? Should I drop the existing parameter or for backwards compatibility should I leave it and then use the new values if present and fall back to PILOT_VELZ_MAX as a default? I noticed that sub uses the same logic too, should I change it there too? As it'll be in the same blocks of code, do we want the same acceleration for both up and down or would we prefer to be able to do asymmetric acceleration too? — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#6879 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ASzWIluNgD2X0cXVV8A0KBz_0S_ZlcExks5shLXlgaJpZM4PKJbr> .

[Pedals2Paddles]

You'll have to make them PILOT_VELZ_MAX_U and PILOT_VELZ_MAX_D since parameter names are limited to 16 characters. But nonetheless, that gets my vote as well. Keeping the acceleration the same is fine to me. I don't see a use case for them being different, I suspect most keep it default anyway.

[ChrisBird]

I've made a change to alt hold so far, doing some SITL testing it to see if it works. I'll have to do further testing tomorrow to be confident in my change. I also have to have a look at AUTO to see what it does as it's not in my list of modes that is using pilot_velocity_z_max . If all goes well I'll try to get a pull request together towards the end of the week.

@rmackay9 as the copter maintainer can you advise which parameter naming you'd prefer, we are hitting the 16 character limit, so would rather get your input now:
PILOT_VELZ_MAX_U - for up
PILOT_VELZ_MAX_D - for down

Or:
PILOT_VELZ_MAX_A - for ascent rate
PILOT_VELZ_MAX_D - for descent rate

Or:
PILOT_VELZ_MX_UP
PILOT_VELZ_MX_DN

I'll make the variable linked to the parameter the same naming convention (up, dn or ascent, descent).

Also I'll make the change to sub as it could benefit from it too. Quadplane I'll have to have another look but it looks like it would work for it too.

[Pedals2Paddles]

Auto mode uses WPNAV_SPEED_UP and WPNAV_SPEED_DN, not the pilot parameters. RTL and Land mode also use the separate land speed parameters. Guided would use whatever the user specifies in the mavlink commands. So I don't think your change would have any effect on those modes. It will also not apply to stabilize or acro.

[ChrisBird]

@Pedals2Paddles yes I agree, my comment was more about if we should get any of the other flight modes using it but they are covered already and it's the flight modes I'm looking at that have missed out in the past :-)

Interesting GUIDED, uses the pilot values in the guided_vel_control_start(), where the guided_posvel_control_start() uses the wpnav values.....

Testing looks good in SITL for ALTHOLD mode.

The following copter modes will be updated with the change:
ALTHOLD
AUTOTUNE
CIRCLE
LOITER
POSHOLD
SPORT

I'm making the changes to the above modes tonight.

Confident I'll get the above modes tested tomorrow night and I should be able to fold the sub modes in too and take a look quadplane and copter guided mode too.

Pull request will be ready for end of the week on this one.

[ChrisBird]

Since I've removed all the code that uses PILOT_VELZ_MAX (replaced with down and up equivalents) should we be constrained to the naming convention given I will exceed the 16 char limit if I try to add up and dn?

What if I named the parameters (Copter and Sub):
PILOT_SPEED_UP
PILOT_SPEED_DN

For quadplane are we happy with:
Q_VELZ_MAX_UP
Q_VELZ_MAX_DN
or
Q_VELZ_SPEED_UP
Q_VELZ_SPEED_DN
or
Q_SPEED_UP
Q_SPEED_DN

This would bring the copter parameters into line with the WPNAV parameters? @rmackay9, @tridge thoughts? So many naming options.....

I've had a look at the quadplane code and I'll be able to apply this change to it as well.

Can we get Plane and Sub added to the tags?

[sashan13]

Just UP and DN are fine IMO. Get rid of velz.

…

On Sep 13, 2017 16:33, "ChrisBird" ***@***.***> wrote: Since I've removed all the code that uses PILOT_VELZ_MAX should we be constrained to the naming convention given I will exceed the 16 char limit if I try to add up and dn? What if I named the parameters (Copter and Sub): PILOT_SPEED_UP PILOT_SPEED_DN For quadplane are we happy with: Q_VELZ_MAX_UP Q_VELZ_MAX_DN or Q_VELZ_SPEED_UP Q_VELZ_SPEED_DN or Q_SPEED_UP Q_SPEED_DN This would bring the copter parameters into line with the WPNAV parameters? @rmackay9 <https://github.com/rmackay9>, @tridge <https://github.com/tridge> thoughts? So many naming options..... I've had a look at the quadplane code and I'll be able to apply this change to it as well. Can we get Plane and Sub added to the tags? — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#6879 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ASzWIsgSpKg9aPBMC0k67D1gnujNRnj6ks5sh-eygaJpZM4PKJbr> .

[Pedals2Paddles]

I like speed up and down too. Matches the wpnav. Consistency is good.

[ChrisBird]

Its been merged into master so this issue can be closed.