PX4 / PX4-Autopilot

PX4 Autopilot Software
https://px4.io
BSD 3-Clause "New" or "Revised" License
8.16k stars 13.36k forks source link

VTOL backtransition acceptance radius bug #14426

Open RomanBapst opened 4 years ago

RomanBapst commented 4 years ago

Please have a look at the simple mission shown in the picture below. The mission consists of the following items: 1) Vtol takeoff 2) Waypoint 3) Transition to Hover 4) Land

The expectation is that the vehicle finishes the transition to hover just when it reaches the transition waypoint (3) which shares the position of the previous waypoint(2). (QGC ties the transition waypoint and the previous waypoint together to make this clear). The acceptance radius of the transition waypoint is calculated dynamically, taking into account a target deceleration parameter (VT_B_DEC_MSS) and the ground speed of the vehicle.

However, what I'm seeing is the following: Somehow the calculation of the backtransition acceptance radius is not executed anymore, the prinft in the relevant section does not appear in the console. I also don't understand anymore how this should have ever worked. The acceptance radius of the backtransition item (3) should be associated to the previous waypoint (2), as it represents the position of the backtransition item. Looking at the code I don't really see where this is happening.

image

RomanBapst commented 4 years ago

@dagar Do you still remember how this used to work?

ThomasRigi commented 4 years ago

Hi @RomanBapst, we have come across this problem on a real drone which overshot a lot when doing a back transition in this way. Some more information here: https://discuss.px4.io/t/quadplane-back-transition-improvements/15054

What you experience here (not entirely sure if something might have changed with your new transition PR https://github.com/PX4/Firmware/pull/14405 - I haven't yet had time to test it unfortunately): The back transition logics are not the same if you use a transition waypoint attached to a normal waypoint instead of a VTOL transition and land waypoint. VT_B_DEC_MSS is only active with a VTOL transition and land waypoint. In this case, in mission.cpp the lines starting at https://github.com/PX4/Firmware/blob/ca0b5700ab0ba6dd2c088564dc13988af27d3263/src/modules/navigator/mission.cpp#L815 are executed. In particular, https://github.com/PX4/Firmware/blob/ca0b5700ab0ba6dd2c088564dc13988af27d3263/src/modules/navigator/mission.cpp#L838 is responsible that the acceptance radius calculation in mission_block.cpp is adapted according to VT_B_DEC_MSS: https://github.com/PX4/Firmware/blob/ca0b5700ab0ba6dd2c088564dc13988af27d3263/src/modules/navigator/mission_block.cpp#L299-L314

If you attach a transition command to a normal waypoint as you do it here, the logics are as follows: As soon as the normal position waypoint (2 in your example mission) is reached with the default acceptance radius, the mission advances to the transition command (3 in your example mission), which is then sent straight away. To my understanding in this case you can't use VT_B_DEC_MSS to adapt the acceptance radius of WP 2 because a priori the drone doesn't know that there is a transition command upcoming (WP 3) until it has reached WP 2. Would be great if you have an idea on how to make the drone aware that there is an upcoming transition.

Now, in my eyes there is an additional problem with the logics of doing a back transition this way: Suppose the drone initiated the back transition too late (since the acceptance radius was not adapted) and finished the transition only at the drone's position in your screenshot. It would move back in MC mode to the coordinates of WP 2 before advancing the mission and fly to WP 4 where it should land. For me this doesn't make any sense as WP2 has already been accepted previously. The corresponding lines are https://github.com/PX4/Firmware/blob/ca0b5700ab0ba6dd2c088564dc13988af27d3263/src/modules/navigator/mission.cpp#L997-L1019 that I would like to see removed. I can open a seperate PR on this if you agree with me.

stale[bot] commented 4 years ago

This issue has been automatically marked as stale because it has not had recent activity. Thank you for your contributions.