-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
Extend MAV_CMD_NAV_VTOL_TAKEOFF #1875
Conversation
- allow to specify waypoint altitude different than transition altitude - added waypoint role enum which allows define the role of the waypoint after the front transition has completed Signed-off-by: RomanBapst <bapstroman@gmail.com>
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 like this approach. It shows a way to cleanly support both the current PX4 and Ardupilot implementation, plus adds the 3rd option that will enable us to use the VTOL_TAKEOFF as a standalone command without a subsequent mission.
Co-authored-by: Silvan Fuhrer <silvan@auterion.com>
message_definitions/v1.0/common.xml
Outdated
@@ -3608,6 +3608,18 @@ | |||
<description>Use the current heading when reaching takeoff altitude (potentially facing the wind when weather-vaning is active).</description> | |||
</entry> | |||
</enum> | |||
<enum name="VTOL_TAKEOFF_WAYPOINT_ROLE"> | |||
<description>Role of the VTOL Takeoff Waypoint</description> | |||
<entry value="0" name="VTOL_TAKEOFF_WAYPOINT_ROLE_IGNORE_POSITION"> |
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.
The first role, before this one, should be "system default" with a value of 0. This allows the recipient to know that the GCS doesn't care or hasn't been updated to the new meaning of the item.
message_definitions/v1.0/common.xml
Outdated
<description>Takeoff from ground using VTOL mode, and transition to forward flight with specified heading. The command should be ignored by vehicles that dont support both VTOL and fixed-wing flight (multicopters, boats,etc.).</description> | ||
<param index="1">Empty</param> | ||
<description>Takeoff from ground using VTOL mode, and transition to forward flight with specified heading. Upon transition completion proceed navigation according to VTOL_TAKEOFF_WAYPOINT_ROLE. The command should be ignored by vehicles that dont support both VTOL and fixed-wing flight (multicopters, boats,etc.).</description> | ||
<param index="1" label="Altitude" units="m">Target altitude after the transition to fixed-wing mode. NAN: Transition altitude is used to define waypoint altitude.</param> |
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.
@sfuhrer Added a comment should this be climbout altitude, which got removed when I merged my update to fix the text.
IMO "altitude" implies final altitude, but would be more than happy with "Final altitude", "Target altitude" or "Climbout altitude".
If possible I would reverse the names - i.e. param 7 being final altitude and param 1 being the transition,. That's because to me param7 altitude is the final target. That might require the "roles" to be redefined though, and might not be possible if the default doesn't match.
The most important there here is that a system that has not updated or does not choose to update should behave in the same way if it gets this command, and that this behaviour should be expected against the new message definition. More precisely,
This is pretty easy if all systems currently assume the param7 altitude is the transition altitude OR if they all assume it is the final altitude. If it is neither we have a breaking change and this will need a little more thought. @WickedShell Can you confirm how ArduPilot treats the lat/lon/alt w.r.t. transition and climbout in a VTOL takeoff? Is this variable in parameters? |
@hamishwillee I will add a "system default" role. In PX4 currently the system default is to fly to the vtol takeoff waypoint at the transition altitude. Afaik in ardupilot the system default is to ignore the position (which is an existing role). I was thinking about making this the same default role for PX4 but thinking about it twice I believe we should prioritize consistency and leave the system default as is.
Are you suggesting to use param 7 as transition altitude only for the "system default" role and swap with param1 for everything else? I also think it's much cleaner to use param1 as transition altitude and param7 for climbout altitude but that of course breaks compatibility. Although, switching altitude params depending on the role is also a bit weird, don't you think? |
…e to climbout altitude Signed-off-by: RomanBapst <bapstroman@gmail.com>
Are you saying that if ardupilot has a mission with MAV_CMD_NAV_VTOL_TAKEOFF by default it will:
What I am saying is:
Does that make sense? |
@RomanBapst OK, so I had a chat to @WickedShell about this. As I understand the situation (note, the ArduPilot bit below should now be accurate): PX4 current behaviour:
Note, I presume PX4 respects the MAV_FRAME in command for ALT? ArduPilot current behaviour:
So ArduPilot treat this as purely an instruction to takeoff. PX4 treat it as an instruction to takeoff and then become a normal waypoint. To me both are not "odd" interpretations. Given there are lat/lon/alt I would assume these to be used, but I would also expect them to be used as the transition point, and for transition to be the completion of the item. Then you would plan your mission as a primitive. Making this a takeoff and then a waypoint seems a bit weird. So where does this leave us? For compatibility (so the same default command does the same thing before and after)
If you do that I think you don't need the roles. If you want to loiter or whatever, add a loiter point as the next item. Thoughts? |
@hamishwillee The main motivation of this PR is to be able to use the VTOL takeoff item as a standalone item! How does the current proposal not maintain compatibility would be my question? The two newly introduced parameters were ignored before, right? |
I don't think this can be the transition alt as PX4 currently is using param 7 for transition alt.
That's possible to do for mission, it's less practical as a guided action as you need to send two commands. Sending one command which defines the entire behavior is much more robust. |
That's why you need to read #1875 (comment) carefully. That states my assumptions, one of which is that PX4 has no transition alt - just a waypoint alt. See:
Now I think you are saying: :
Once you've confirmed the assumptions, only then would I know what makes sense. |
<description>Takeoff from ground using VTOL mode, and transition to forward flight with specified heading. The command should be ignored by vehicles that dont support both VTOL and fixed-wing flight (multicopters, boats,etc.).</description> | ||
<param index="1">Empty</param> | ||
<description>Takeoff from ground using VTOL mode, and transition to forward flight with specified heading. Upon transition completion proceed navigation according to VTOL_TAKEOFF_WAYPOINT_ROLE. The command should be ignored by vehicles that dont support both VTOL and fixed-wing flight (multicopters, boats,etc.).</description> | ||
<param index="1" label="Climbout Altitude" units="m">Climbout altitude after the transition to fixed-wing mode. NAN: Transition altitude is used to define waypoint altitude.</param> |
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.
We'd probably best use use 0 rather than NaN.
- Yes 0 is a valid altitude but it is unlikely to be the desired climbout altitude
- A GCS will be sending 0 by default if it has not been updated. So you want the 0 value to mean "do what you always did"
@RomanBapst @sfuhrer My updated assumption about PX4 existing behaviour:
Is that right? Now you want it to be the same but.
Is that all correct? Please confirm. Why would you want to loiter once? Is it if you know that you're going to loiter on the next command so that you have a good entry path? If so I think you're right - this is not incompatible in that an non-updated system sending the command to a system that has been updated will get the same behaviour. We could add a role "takeoff command only". That would be the same as VTOL_TAKEOFF_WAYPOINT_ROLE_DEFAULT for ArduPilot. |
Discussed with @RomanBapst in MAVLink call last night. We're now on the same page. So essentially the options are
This is waiting on Roman to discuss with Silvan and then update. |
@RomanBapst @sfuhrer We discussed it in the dev call, and generally we think that extending this command makes hard to understand, which is in turn risky for implementers. We might be better off replacing the old message and taking time to do whatever else we might like. Then we could do something like this, which allows the two use cases we actually want - accept immediately on transition, and transition then got to waypoint. <entry value="XXXXX" name="MAV_CMD_NAV_VTOL_TAKEOFF_WAYPOINT" hasLocation="true" isDestination="true">
<description>
Takeoff from ground vertically in VTOL mode.
Transition to forward flight with the specified heading at the transition altitude (param1).
Following transition, fly to climbout waypoint defined by params 5, 6, 7.
If the transition value is set to the invalid value (NaN) a system default transition altitude is used.
If the climbout waypoint values are set to the invalid value, the command will complete immediately after the transition.
Further navigation depends on the value of param3 (VTOL_TAKEOFF_WAYPOINT_ROLE).
The command should be ignored by vehicles that don't support both VTOL and fixed-wing flight (multicopters, boats, etc.).
</description>
<param index="1" label="Transition Altitude" units="m">Transition altitude. NAN: Transition at flight stack default altitude.</param>
<param index="2" label="Transition Heading" enum="VTOL_TRANSITION_HEADING">Front transition heading.</param>
<param index="4" label="Yaw Angle" units="deg">Yaw angle. NaN to use the current system yaw heading mode (e.g. yaw towards next waypoint, yaw to home, etc.).</param>
<param index="5" label="Latitude">Climbout Latitude. NaN/INT32_MAX to use transition latitude.</param>
<param index="6" label="Longitude">Climbout Longitude. NaN/INT32_MAX to use transition latitude.</param>
<param index="7" label="Altitude" units="m">Climbout Altitude. NaN to use transition altitude.</param>
</entry> @auturgy is going to comment on what he'd like to see in a message like this. But note that I don't plan on looking at this again unless you re-engage, and if you haven't done so in a month or so I will close this PR. |
@hamishwillee I'm sorry, I lost track of this. I will make the changes today as discussed before. We agreed that we don't need the loiter role which simplifies a lot. With the loiter role gone the only thing that this PR really adds it the loiter altitude (which I think makes sense in any case). |
@hamishwillee @auturgy I thought about it a bit more and came to the conclusion that any change I'm trying to do to this message bears the risk to cause unexpected behavior in the case where flight controller and ground station don't use the same message definition. The command as it's defined right now leaves a lot of room for interpretation and therefore also PX4 and ardupilot have implemented this command in different ways. I therefore welcome the idea of creating a new command with some more thought put into it. |
@hamishwillee @auturgy This is my first suggestion of how I think the command could look like.
|
New idea: MAV_CMD_NAV_VTOL_TAKEOFF_RELATIVE
|
@RomanBapst The first idea (#1875 (comment)) seems a lot like the intent I had for the proposal above. So that one makes sense to me. I am not sure I fully understand the @auturgy YOu were going to add suggestions on what you need from a VTOL takeoff? |
The problem with specifying a global coordinate in the message is that you are bound to that global coordinate but the actual takeoff location in the field might be a different one. Therefore, we could trash the global position of that command and just specify a transition heading and possibly a loiter bearing and distance. This would mean that wherever you end up doing the takeoff, the experience will be consistent. |
Ah, thanks. YOur original post did not mention distance :-). I'm not opposed but it would need clear specification. |
Yes the takeoff relative to every other takeoff will be consistent flight path, but relative to the rest of the plan it means that the flight path will be different every time. I don't know if it matters, but nor do I know if it "helps". I'd try this out with a few hand draw scenarios. I've been trying to get @auturgy to comment on "what is it that is needed for a VTOL takeoff point", to no luck. To progress this, can I suggest you create a new PR with the latest thinking. That gives us a concrete proposal to agree/argue and we can be selective about what in this ginormous thread we include. I'm going to close this for that reason. |
Motivation
MAV_CMD_NAV_VTOL_TAKEOFF is used to command a VTOL to takeoff from the ground vertically and transition to fixed wing flight via the specified heading. What happens after the transition is not well defined and typically different flight controller implementations did what they considered the "correct" thing to do.
In PX4 the current behavior is to treat the VTOL takeoff waypoint as a standard 3D waypoint once the vehicle transitioned to fixed wing flight. In Ardupilot (please correct me if I'm wrong) the 3D position of the waypoint is ignored once the vehicle transitioned to fixed wing flight.
A ground station like QGC cannot really display any meaningful predicted path if it does not know about the specifics of the implementation.
This is an attempt to extend the definition of MAV_CMD_NAV_VTOL_TAKEOFF in order to allow different behaviors of the vehicle after the transition to fixed wing flight.
Details
Add an altitude field which (together with lat/lon fields) fully defines the 3D location of the waypoint. This altitude is NOT the same as the transition altitude as those two measures don't necessarily need to be the same. Some use cases/air frames require low transition altitudes but a higher climbout altitude. Generally you want to transition as low as possible to conserve energy but you want to loiter high enough to clear potential obstacles.
Define VTOL_TAKEOFF_WAYPOINT_ROLE enum which specifies the role of the 3D vtol takeoff waypoint.
This enum describes what exactly the vehicle will do after the transition to fixed wing flight has completed and what needs to happen in order for the MAV_CMD_NAV_VTOL_TAKEOFF to be considered "reached".
The way-point's only use is to define a transition direction if required by the "Transition Heading" parameter. Other than that the location does not serve any other purpose since the vehicle does not need to reach the position.
Other than potentially defining a transition direction ("Transition Heading") the waypoint is treated as a "standard" waypoint which needs to be "reached" by the vehicle according to the measures implemented by the flight controller (acceptance radius).
This option is useful to obtain a predictable flight path.
The same as VTOL_TAKEOFF_WAYPOINT_ROLE_FLYTHROUGH, however, instead of just reaching the waypoint, the vehicle needs to complete one full loiter circle at the waypoint location. The radius and direction are given by vehicle defaults.
This option is useful to accelerate convergence of wind estimation. This can be very important for vehicle relying on synthetic airspeed computation. It's also useful in cases where there are no other navigation commands following immediately (e.g. VTOL takeoff commanded by the user directly instead of being commanded as part of a mission) since then the vehicle is loitering at a known position and does not needs any immediate action.
Here is an attempt to sketch out the different options:
Other comments
The intent is to extend the message in such a way that it does not break existing implementations. This should be given by the fact that the proposal makes user to the two empty fields and does not alter any existing fields.
Related Links
PX4/PX4-Autopilot#15645
mavlink/qgroundcontrol#8221
Example implementation in QGroundControl:
mavlink/qgroundcontrol#10123
Alternatives