Scripting: allow for aerobatics in AUTO mode in plane #19039

tridge · 2021-10-25T04:39:01Z

This allows for a NAV_SCRIPTING command that is a verify mission item implemented in a script. It is implemented in plane to allow for aerobatics controlled by a lua script.
The mission item has:

a command ID (interpreted by the script)
a timeout in seconds
two script controlled float arguments

This is an example mission:

That mission works with libraries/AP_Scripting/examples/plane_aerobatics.lua. That script implements both loops and rolls

the passes arguments 2, 10, 80, 90 to the script in WP 4. The params are:

2 == an integer argument the script interprets to know what manoeuvrer to do. For the example script 1 is an axial roll, 2 is a loop
10 is the timeout in seconds. If the script doesn't complete the mission item in that time then the mission continues with the next item
80 and 90 are float arguments interpreted by the script. In this case it is the pitch rate and the throttle level

Demo video:
https://www.youtube.com/watch?v=rqFUpj78fKE

Link to related mavlink PR: ArduPilot/mavlink#228

IamPete1 · 2021-10-25T09:42:15Z

ArduPlane/Attitude.cpp

+    } else if (control_mode == &mode_auto &&
+               mission.get_current_nav_cmd().id == MAV_CMD_NAV_SCRIPTING) {
+        // scripting is in control of roll and pitch rates and throttle
+        const float aileron = rollController.get_rate_out(nav_scripting.roll_rate_dps, speed_scaler);
+        const float elevator = pitchController.get_rate_out(nav_scripting.pitch_rate_dps, speed_scaler);
+        SRV_Channels::set_output_scaled(SRV_Channel::k_aileron, aileron);
+        SRV_Channels::set_output_scaled(SRV_Channel::k_elevator, elevator);


I think its better to add support for guided as a auto sub-mode like MAV_CMD_NAV_GUIDED_ENABLE, then all the existing guided options would be available to the script for navigation. We would have to add that to plane, but it should already work for cotper and rover.

we still need a way to handle the verify logic. Is there already a mission item to do that? I didn't see one that waits for completion, only "DO" commands

IamPete1 · 2021-10-25T09:42:55Z

ArduPlane/Plane.h

@@ -1113,6 +1126,10 @@ class Plane : public AP_Vehicle {
    float get_throttle_input(bool no_deadzone=false) const;
    float get_adjusted_throttle_input(bool no_deadzone=false) const;

+    // support for NAV_SCRIPTING mission command
+    bool nav_scripting_active(void) override;
+    bool nav_scripting_update(bool completed, float throttle_pct, float roll_rate_dps, float pitch_rate_dps, float yaw_rate_dps) override;


I think its better to add support for this as a guided command.

Does this API mean that controlling roll/pitch/yaw rates directly from LUA would be available only in an active mission? Would there be a way to access it in normal scripting, e.g. in response to other events or in response to user input?

@arikrupnik
It depends a bit upon the vehicle but in general a lua script can always control the vehicle in Guided mode and a script can also change the vehicle's flight mode so a script could change the vehicle to Guided mode and control it in a variety of different ways.

This PR is adding support for Lua scripts to control the vehicle (a Plane) not only while in Guided mode but also within Auto mode.

I mean as it stands, doesn't the function take essentially a mission item as an argument? How would you call this from a regular timer-style function?

@arikrupnik, by "regular timer-style function" I guess you mean a C++ function/method within the regular vehicle code (not a script)? Some custom method that a developer has written that runs at a regular interval because it is within the Scheduler array? If I understand the question correctly then I don't think that this method could or should make use of this new code in this PR.

@rmackay9 I think I expressed myself poorly. I imagine using this new API similarly to set_target_angle_and_climbrate(), from anywhere in a LUA script. It appears that @tridge's intent is to allow its use only in response to a mission item. There must be a reason not to generalize its use to allow it from anywhere. I'd like to understand it. The intent is explicit in several places, including commands_logic.cpp:

if (!nav_scripting_active()) { return false; }

IamPete1 · 2021-10-25T09:46:59Z

libraries/AP_Scripting/examples/plane_aerobatics.lua

+end
+
+function update()
+   if vehicle:nav_scripting_active() then


Surely better to revive the command directly into the script like MAV_CMD_DO_SEND_SCRIPT_MESSAGE

It might be worth re-working how the buffer there works a little, so a script has to ask for a buffer, then the mission item can just be skipped if nothing has resisted to registered the commands.

I think the main C++ firmware needs to keep control of the mission sequencing. We need the active() accessor so the script knows when it should be doing things. The main fw needs to be able to implement the timeout

Right, maybe I droped my comment on the wrong line. What I mean is that is is a little clunky to have to read it out of mission storage. The do scripting command returns the start time and params as nulable arguments, so it would look like this:

local time_ms, param1, param2, param3, param4 = nav_mission_receive() if time_ms then if param1 == 1 then do_axial_roll(param2, param3, param4) elseif param1 == 2 then do_loop(param2, param3, param4) end end

That would also give us the possibility to send as a command long rather just from mission.

I agree the script needs to be able to tell, but if we implement via guided we could add a in_guided_mode vehicle method.

For the continue we should look at how guided within mission currently works for copter/rover.

I've changed to the tuple method as suggested, with a 16 bit ID to fix the race condition

rmackay9 · 2021-10-25T23:11:08Z

We have support for companion computers (or lua scripts) to control Copters using the NAV_GUIDED_ENABLED mission command which also has safety limits for time and distance. I'd like to be involved in the discussion of why we need both before this gets merged if possible. Maybe we need both, maybe not.. but let's think it through first.

tridge · 2021-10-26T07:21:55Z

Maybe we need both, maybe not.. but let's think it through first.

copying my comment from mavlink PR:

there are key differences:

NAV_GUIDED_ENABLE doesn't have any way for the offboard or script control to hand back to continue the mission. Doing a "set waypoint" to continue is full or race conditions
there NAV_GUIDED_ENABLE takes no parameters. I want the script to know which command is wanted. So the mission can do a loop, then a roll, then an immelman turn, then a spin, then a figure 8 etc, all with normal waypoints to get lined up for the aerobatic manoeuvrer in between

IamPete1 · 2021-10-26T23:35:19Z

I'm not suggesting we don't need a command, Just that the new command should trigger guided within auto the same as NAV_GUIDED_ENABLE does, it continues to run the mission, it just accepts guided commands. This means we don't have to re-implement all those guided commands for this new control path.

Then we would add a new method to allow the mission to continue from that state. We would have a direct scripting binding for this use case and a new mavlink message so that users of NAV_GUIDED_ENABLE can benefit too.

rmackay9 · 2021-10-30T02:19:44Z

To consolidate our understanding of the two types of mission items I think we should stick with the terms, "navigation commands" (commands that affect the physical movement of the vehicle) and "do commands" (commands that affect systems including the camera that do not affect movement of the vehicle) instead of "verify mission items". The reason is that the latter (which is equivalent to what the wiki refers to as "navigation commands") makes it appear that only navigation commands have a "verify" .. a "verify" meaning that the command must return a true to indicate that it has completed. In fact both "navigation" and "do" commands have a verify but perhaps plane doesn't implement these ever? They all need a verify because the order in which "do" commands are executed can be important. For example a "do wait" of say 15 seconds could be placed before a "do camera take snapshot" command. If there were no "verify" the camera command would not be delayed for 15 seconds.

libraries/AP_Scripting/AP_Scripting.cpp

rmackay9 · 2021-10-30T03:48:37Z

I agree with @IamPete1's comments that Guided should be implemented as an Auto submode and then the script can make use of all the existing method's to control the vehicle just like it can in Guided mode. I think this would allow NAV_SCRIPTING to be used in a wider variety of situations.

I think we could also keep it very general purpose by not specifying the use of any of the command's fields. Here's how it might be done:

add a structure (to the vehicle code or AP_Mission?) that holds the active command information. Note; the structure below is an illustration of what I mean, it probably won't actually compile and some details may be incorrect.

    struct NavScriptCmd {
        bool active;  // true if a NAV_SCRIPT command is currently active
        uint8_t active_id;     // a unique id that is randomly generated or implemented as a counter to avoid race conditions
        uint16_t cmd_id;     // the command's position in the mission list (this may or may not be required)
        uiint8_t p1;             // p1 extracted from the nav command
        float param1;
        ...
        float param7;
        Location loc; // this is a convenience member which could make it easier for scripts to get the Location if it has been provided.  The vehicle code would populate this as it does for other commands including changing and alt of "0" to be the current altitude.
    } nav_script_cmd;

add a method (to the vehicle code or AP_Mission?) to allow the script to access the above structure.
- bool get_nav_script_cmd(uint8_t &active_id, uint8_t &p1, float *param1, etc..); // returns true and populates arguments if there is an active NAV_SCRIPT command
add a method to allow the script to indicate it has completed the command
- void set_nav_script_done(uint8_t active_id); // active_id is provided by the script to avoid race conditions including the user manually moving the active command to the next item in the command list which is also a NAV_SCRIPT command.

There is still a small race condition like this:

a. NAV_SCRIPT command 22 is running and the script has grabbed the contents of nav_script_cmd at time 10sec
b. user manually moves the active command to 23 which is also a NAV_SCRIPT
c. script from step a completes and sends control updates to the vehicle code. These are accepted because the Auto sub-mode is still Guided.

This is probably not dangerous though because the script will likely run soon afterwards and consume the new command's fields and update any targets.

It is a little worse if at step b the user switches to Guided mode instead. If the script is providing velocity or rate commands this is not a big problem because worst case the script doesn't provide any updates and Guided will timeout after 3 seconds (in copter) and the vehicle will come to a stop anyway. If the script was providing a low-rate position targets (which don't timeout) then a copter (at least) would not stop like the user would expect it to after switching to Guided mode. Instead it would keep flying to that last target sent by the script. These race conditions could be remove if we created duplicates of the guided mode controls which also accepted the unique id.

By the way, "NAV_SCRIPTING" is very general purpose name, if we were going to go with the approach in this PR we'd probably need to give it a more specific name like "NAV_SCRIPT_RATE_CONTROL" or something like that. If we go with the more general approach we might want to call the command "NAV_SCRIPT" (drop the "ING" part) because why use more characters when they add no clarity?

rmackay9 · 2021-10-30T03:56:25Z

By the way, I can imagine that users might want to implement either DO_ commands or NAV_ commands with scripts. If we wanted to handle both I think the suggestion above would work but we would need two of the nav_script_cmd structures.

tridge · 2021-10-31T07:41:17Z

For example a "do wait" of say 15 seconds could be placed before a "do camera take snapshot" command.

there is no DO_WAIT command. There is CONDITION_DELAY and NAV_DELAY.
All commands that are not instantaneous are either NAV_ or CONDITION_.
This is why the prefixes DO_, CONDITION_ and NAV_ were added in mavlink. It has been this way from the beginning. It is in the name so it is easy to know which commands stop and wait for completion and which have immediate effect then continue just by looking at the mission command list.

then the script can make use of all the existing method's to control the vehicle just like it can in Guided mode

there is absolutely nothing stopping us using all those guided commands in copter with NAV_SCRIPTING.
The point of adding NAV_SCRIPTING is to do the following:

it gives us a timeout per mission item for the time we expect this mission item to take
it gives us a command number so we can say "at wp 17 do a loop, at wp 25 do a cuban-8, at wp 34 do a loop" etc etc. So you can build up a full aerobatic set, or do a completely different set of navigation actions at different points in the mission
it gives us two floating point arguments to each waypoint of this type, for things like roll rate, height of anything else.

You cannot do any of this with NAV_GUIDED_ENABLE. We could extend NAV_GUIDED_ENABLE by taking some of the unused parameters, but that would change existing behaviour. We'd also need to work with px4 on the change as it is in common.

struct NavScriptCmd {
bool active; // true if a NAV_SCRIPT command is currently active
uint8_t active_id; // a unique id that is randomly generated or implemented as a counter to avoid race conditions
uint16_t cmd_id; // the command's position in the mission list (this may or may not be required)
uiint8_t p1; // p1 extracted from the nav command
float param1;
...
float param7;
Location loc; // this is a convenience member which could make it easier for scripts to get the Location if it has been provided. The vehicle code would populate this as it does for other commands including changing and alt of "0" to be the current altitude.
} nav_script_cmd;

this won't work as we don't store general parameters in AP_Mission. Our serialisation is very specific to each command as as have the 12 byte data limit (10 bytes for commands above 255). I carefully made NAV_SCRIPTING fit this limit while covering what needs to be covered for these types of items. It has a command number (so the script knows what type of manoeuvrer is wanted), it has a timeout (which depends on the time of manoeuvrer) and it has two command specific arguments, so you can easily adjust key properties of the manoeuvrer.

1. add a method (to the vehicle code or AP_Mission?) to allow the script to access the above structure.

again it doesn't work as the structure is a different shape for different mission items.

By the way, "NAV_SCRIPTING" is very general purpose name, if we were going to go with the approach in this PR we'd probably need to give it a more specific name like "NAV_SCRIPT_RATE_CONTROL"

the mavlink command is deliberately general purpose. It is in no way tied to rate control. The available scripting bindings in plane is what limits it to rate control at the moment.

I'll change the binding to be nav_rpy_rate_throttle_pct() to make the name clearer, but I really would like to get the mavlink command in soon so I can more sanely edit missions without having to be on a separate branch in mavproxy or use the "UNKNOWN" method in MissionPlanner.

rmackay9 · 2021-11-01T01:21:54Z

.. maybe have a peek at the MAV_CMD_DO_GUIDED_LIMITS mission command ..

tridge · 2021-11-01T01:33:24Z

.. maybe have a peek at the MAV_CMD_DO_GUIDED_LIMITS mission command ..

I've seen it. It really doesn't suit what we're doing for aerobatic planes. The timeout could be used, but not nearly as conveniently as doing it in the mission item for the manoeuvrer.
It is also a problem mavlink command as ArduPilot copter didn't implement it as per the mavlink xml. The xml says the height limits are AMSL. Copter implements it as above origin.
We may implement a similar set of DO commands as modifiers to the aerobatics in future in plane, but for now what I want is a NAV_ item that takes arguments as I've done in the mavlink PR, so we have a command ID, timeout and arguments. If you don't want to implement that for copter that is fine, although I do think you should consider it as it would make for a much easier way to setup an aerobatic missions.
I likely will want to try this on helicopters once we get fixed wing aerobatics woking really well. As well as being an aerobatic fixed wing pilot Andy Palmer is a 3D heli pilot. We've started discussing how we can do full aerobatic displays for helis.

tridge · 2021-11-01T01:41:25Z

Note that what I'm trying to do is different in a fairly key way from the guided sub-mode in AUTO. What I'm trying to achieve is to allow for scripting to truly create new mission items. It isn't a "stop auto mode and go over to this other thing for a while". It is adding new AUTO navigation items as lua scripts.
So I could have implemented all this in C++ by adding a NAV_AEROBATICS command, and then implemented in C++ an axial roll, a loop, a cuban-8 etc etc. I wanted instead to get the effect of doing this but inside lua scripting. So it isn't a sub-mode of AUTO, it is just another mission item in AUTO mode, and it has parameters to that mission item just like we do for NAV_LOITER_TURNS, NAV_LOITER_TO_ALT etc.
I'd also please ask that you concentrate on my mavlink PR. That is what is blocking me and causing issues with working on the aerobatics with Andy.

rmackay9 · 2021-11-01T03:32:40Z

Putting the timeout in the mission command is not as convenient but it also means you lose a field (because we can only record a few fields) and you lose flexibility. Sure, you want a timeout for the moment but 10min from now someone wants a distance limit.

The problem with me approving the mavlink change is that it's closely tied to this implementation because it defines what some of the fields are.

I understand that you're frustrated and want to make forward progress but I'm frustrated that you're trying to railroad in a method that I don't agree with.

tridge · 2021-11-01T04:28:18Z

Putting the timeout in the mission command is not as convenient but it also means you lose a field (because we can only record a few fields) and you lose flexibility. Sure, you want a timeout for the moment but 10min from now someone wants a distance limit.

it can't be a distance limit as we only have 8 bits available. That is OK for a timeout in seconds, but not reasonable for a distance limit. The xml is carefully thought out to make the most of the 10 bytes that are available.

The problem with me approving the mavlink change is that it's closely tied to this implementation because it defines what some of the fields are.

the 10 byte limit really constrains things. We need a command ID, and I want 2 floating point arguments to adjust the command. That leaves 8 bits. I think a timeout is a good use of those 8 bits.

rmackay9 · 2021-11-01T06:20:12Z

it can't be a distance limit as we only have 8 bits available.

The reason for having GUIDED_LIMITS is because all the desired types of limits couldn't be squeezed into a single mission command. I would also love it if we could fit a time limit and/or a distance limit into a single command but it just didn't fit so when I originally implemented the Guided-within-Auto feature for Copter my choice was to add the GUIDED_LIMITS command (a "do"command). This PR instead skips some functionality with the benefit of squeezing it all into a single message. I agree that either choice is valid but when I got to this point (7 years ago) I made the other choice.

I see your point that we can't providing all param1 to param7 command values in a single NAV_SCRIPTING command because it won't fit. The choice to only provide two floats though seems very limiting for some uses. Surely some lua-script-mission-commands will want a location.

By the way, I'm not stuck on forcing this PR to use the existing GUIDED_ENABLED and GUIDED_LIMITS commands. We can create new commands with different names and different levels of ease-of-use and fields but let's put a bit of thought into what the full set might be.

I think we will eventually need at least two NAV_SCRIPT_* commands. One for implementing DO commands and one for NAV commands. For the NAV commands, some will have locations and some won't so we might need two forms of this although my suggested implementation would essentially leave it up to the script to decide if the last three fields were a location or something else.

Note that what I'm trying to do is different in a fairly key way from the guided sub-mode in AUTO. What I'm trying to achieve is to allow for scripting to truly create new mission items. It isn't a "stop auto mode and go over to this other thing for a while". It is adding new AUTO navigation items as lua scripts.

What's the difference between these two? I think the answer to this question is that the lua script has the ability to tell the main flight code when it has completed the mission item so that the vehicle can move onto the next command. My suggested implementation has this feature as well.

rmackay9 · 2021-11-01T06:54:47Z

I think some of our disagreement on this PR comes from the long standing limitation of having fixed number of bytes available for each command. How about this plan to sidestep the issue:

rename the mavlink command to NAV_SCRIPT, remove any definition of column headers and merge (just the mavlink change) so you can continue your development with AndyP.
fund a developer to complete the enhance to allow flexible length storage of mission commands.

tridge · 2021-11-01T07:43:25Z

The choice to only provide two floats though seems very limiting for some uses. Surely some lua-script-mission-commands will want a location.

we could add a separate NAV_SCRIPT_WP mission item for that, but I don't think we need to add that now as we don't have a user for it. That would facilitate displaying on a map which would make sense for some types of actions, but not really for the aerobatics Andy Palmer and I are working on now.

I think we will eventually need at least two NAV_SCRIPT_* commands. One for implementing DO commands and one for NAV commands.

we already have MAV_CMD_DO_SEND_SCRIPT_MESSAGE for the DO command. What we are missing is the NAV commands that take arguments. A NAV_SCRIPT and NAV_SCRIPT_WP would be good, but I don't think I'll add NAV_SCRIPT_WP until we need it.

my suggested implementation would essentially leave it up to the script to decide if the last three fields were a location or something else.

that doesn't actually work. Mission commands are marked with a boolean hasLocation in the XML, along with a isDestination boolean. That is used to determine if the fields are lat/lon and should be displayed on the map. It also controls if you can drag it around on the map. So you can't have free fields that are sometimes lat/lon and sometimes not. We do have one exception to that, where 0/0 means "current location", but it only works for that special case.

What's the difference between these two? I think the answer to this question is that the lua script has the ability to tell the main flight code when it has completed the mission item so that the vehicle can move onto the next command.

the first difference is in how it presents to users. Can you imagine if NAV_LOITER_ALT had to have a command in front of it each time you wanted it? Or NAV_WP had to have a set of constraints as a separate item before it? It would make mission construction very unwieldy.
The second difference is a matter of trust. I want lua scripts to be able to be first class citizens in ArduPilot. I want to extend our C++ with lua in a flexible manner. The NAV_GUIDED_ENABLE with a DO_GUIDED_LIMITS is putting the command in a box. For normal mission commands we do that with a geofence. I'd far prefer to do the NAV_SCRIPT for aerobatics with a geofence as well. I want to trust the lua, and present a good user interface for creating complex aerobatic missions. The method you are suggesting will create a poor user interface and relegates lua mission items to second class citizens.

tridge · 2021-11-01T07:46:12Z

1. ename the mavlink command to NAV_SCRIPT, remove any definition of column headers and merge (just the mavlink change) so you can continue your development with AndyP.

I want the column headers as they are part of the user interface. You can't just treat them as lat/lon. The XML just doesn't work that way. To get a good user experience I want the column headers to show up correctly in the mission editor.

fund a developer to complete the enhance to allow flexible length storage of mission commands.

it isn't a matter of lack of funding, or even lack of time. The problem is lack of an idea on how to actually do this in a reasonable manner. The data structure challenges are significant. The actual code is likely to be fairly simple, but we need to know what compromises we are willing to make.

rmackay9 · 2021-11-01T07:54:03Z

I think "NAV_SCRIPTING" or "NAV_SCRIPT" is too generic a name for such a specific implementation. Something more like "NAV_SCRIPT_RATE" or "NAV_SCRIPT_ACROBATIC" would be better.

NAV_SCRIPT_WP is a good name for the NAV command that takes a location. From my point of view, it's fine not to implement it for now because we often don't implement things before we need them and I'm not attempting to scope creep you.

By the way, when GUIDED_LIMITS are not specified then there are no limits applied meaning offboard companion computers are also first class citizens by default. The limits are only applied if the user wants them applied.

tridge · 2021-11-01T08:01:08Z

Something more like "NAV_SCRIPT_RATE" or "NAV_SCRIPT_ACROBATIC" would be better.

It isn't in any way tied to rate control. There is nothing in the XML that mentions rates or aerobatics in any way.
I'll make it NAV_SCRIPT_ACROBATIC if I have to in order for you to approve, but I think it is a mistake. This can be used for a lot more than aerobatics.

rmackay9 · 2021-11-01T08:06:50Z

It appears that there are some parts that are specific and some are generic. The code in AP_Mission is more generic (e.g. arg1, arg2) while the interface to scripting seems more specific:

bool nav_scripting_update(bool completed, float throttle_pct, float roll_rate_dps, float pitch_rate_dps, float yaw_rate_dps) override;

Re naming, another possible name might be NAV_SCRIPT_TIME which fits with NAV_LOIT_TIME and adds some specificity to the command.

tridge · 2021-11-01T09:54:54Z

It appears that there are some parts that are specific and some are generic. The code in AP_Mission is more generic (e.g. arg1, arg2) while the interface to scripting seems more specific:

right. I was planning to have several scripting bindings that could be used with the mission item. I'll rename the one I have in this PR to be more specifically rpy rates, but I plan on adding some others.

tridge · 2021-11-01T09:55:34Z

another possible name might be NAV_SCRIPT_TIME which fits with NAV_LOIT_TIME and adds some specificity to the command.

thanks, I'll change it to NAV_SCRIPT_TIME

rmackay9 · 2021-11-01T10:43:17Z

libraries/AP_Scripting/examples/plane_aerobatics.lua

+
+function update()
+   if vehicle:nav_scripting_active() then
+      local cnum = mission:get_current_nav_index()


Isn't this a (small) race condition where the active mission command could have been changed since vehicle:nav_scripting_active() was called? As a minimum the script probably needs to check that the mission item's command is NAV_SCRIPT_TIME but more generally I agree with @IamPete1's comments above that it would be better instead to make arg1, arg2 (and whatever other fields go into NAV_SCRIPT_TIME) via another interface rather than asking the script to go to the mission object.

IamPete1 · 2021-11-01T11:39:41Z

I think the scripting command should be kept generic. The command should not try and guess what scripting might want to do. Although maybe it does make sense to have one with a location and one without.

I don't think there should be any scripting commands specific to scripting mission items. When controlling the vehicle in a mission scripting should use the existing guided command set. If there is no guided command for a particular thing we should add it. This does not necessarily limit us to commands that are available over mavlink, we can add new ones only available to scripting but they should work for both guided and mission items. If they prove usefull we could consider adding a mavlink command in the future.

rmackay9 · 2021-11-01T12:12:37Z

I don't think there should be any scripting commands specific to scripting mission items

To clarify, in the line copied above, I think you mean that any NAV_SCRIPT_xxx commands we come up with should be able to be embedded within a mission to be executed in Auto mode OR sent in real-time (from the GCS to the vehicle) and be accepted while the vehicle is in Guided mode. Am I interpreting this line correctly?

IamPete1 · 2021-11-01T12:16:18Z

Correct

for supporting aerobatics in scripts

tridge · 2021-11-08T04:14:43Z

this PR has been redone to use NAV_SCRIPT_TIME, and restructured to better align with the existing guided commands. It also uses an ID to prevent race conditions, and uses the tuple structure suggested by @IamPete1

tridge · 2021-11-08T04:17:22Z

When controlling the vehicle in a mission scripting should use the existing guided command set.

just to be absolutely clear here, the existing guided command set does not do what I want. I cannot do the aerobatics with that command set. I have added a new set_target_throttle_rate_rpy() command instead.

arikrupnik · 2021-11-08T04:48:51Z

When controlling the vehicle in a mission scripting should use the existing guided command set.

just to be absolutely clear here, the existing guided command set does not do what I want. I cannot do the aerobatics with that command set. I have added a new set_target_throttle_rate_rpy() command instead.

It appears that set_target_throttle_rate_rpy() is valid only when executing a script mission item, e.g.:

    } else if (control_mode == &mode_auto &&
               mission.get_current_nav_cmd().id == MAV_CMD_NAV_SCRIPT_TIME) {

Is there an inherent limitation that prevents script control of rates outside a mission, e.g. in response to an auxiliary switch?

tridge · 2021-11-08T06:13:39Z

Is there an inherent limitation that prevents script control of rates outside a mission, e.g. in response to an auxiliary switch?

that could be done, but isn't part of this PR

rmackay9 · 2021-11-08T07:38:26Z

libraries/AP_Vehicle/AP_Vehicle.h

@@ -201,6 +201,10 @@ class AP_Vehicle : public AP_HAL::HAL::Callbacks {
    virtual bool set_target_velaccel_NED(const Vector3f& target_vel, const Vector3f& target_accel, bool use_yaw, float yaw_deg, bool use_yaw_rate, float yaw_rate_degs, bool yaw_relative) { return false; }
    virtual bool set_target_angle_and_climbrate(float roll_deg, float pitch_deg, float yaw_deg, float climb_rate_ms, bool use_yaw_rate, float yaw_rate_degs) { return false; }

+    // command throttle percentage and roll, pitch, yaw target
+    // rates. For use with scripting controllers
+    virtual bool set_target_throttle_rate_rpy(float throttle_pct, float roll_rate_dps, float pitch_rate_dps, float yaw_rate_dps) { return false; }


I would probably call this "set_target_rpy_rates_and_thrust()" but it's not a blocker. I'm happy to see it is right below the "set_target_angle_and_climbrate" which is it's closest relative.

It's actually used as a direct throttle demand percentage to the throttle servo, not a thrust demand that then gets converted to a throttle so I think the existing name is OK.

rmackay9

This looks much better to me. The two separate scripting calls, "set_target_throttle_rate_rpy" and "nav_script_time_done" has really improved it from my point of view.

priseborough · 2021-11-08T09:32:54Z

I'm satisfied these changes to the C++ code won't change exisiting behaviours for users. This is a starting point for something that is likely to build its own core of users and develop significantly from the simple angular rate and throttle demands as currently implemented. I vote we get this in and then focus on the changes that are going to be required to fly accurate aerobatic maneouvres such as support for an acceleration vector demand controller for the lua script that can track a mathematical function defining the desired trajectory, addition of yaw rate control to the autopilot, etc

tridge added Plane Scripting WIP labels Oct 25, 2021

tridge force-pushed the pr-plane-nav-scripting branch from f0381bc to ae80e66 Compare October 25, 2021 06:37

tridge added the DevCallTopic label Oct 25, 2021

tridge force-pushed the pr-plane-nav-scripting branch 2 times, most recently from 2574acb to b7a0c1f Compare October 25, 2021 07:48

IamPete1 reviewed Oct 25, 2021

View reviewed changes

CraigElder removed the DevCallTopic label Oct 26, 2021

tridge force-pushed the pr-plane-nav-scripting branch from 98d9e7b to 437fcb5 Compare October 29, 2021 22:56

rmackay9 reviewed Oct 30, 2021

View reviewed changes

libraries/AP_Scripting/AP_Scripting.cpp Outdated Show resolved Hide resolved

tridge removed the WIP label Oct 30, 2021

rmackay9 reviewed Nov 1, 2021

View reviewed changes

rmackay9 mentioned this pull request Nov 1, 2021

ardupilot: added NAV_SCRIPT_TIME mission item ArduPilot/mavlink#228

Merged

mavlink: added NAV_SCRIPT_TIME

c83e0e1

tridge force-pushed the pr-plane-nav-scripting branch from 437fcb5 to 67f3f1b Compare November 6, 2021 23:45

tridge added 5 commits November 8, 2021 15:06

AP_Mission: support NAV_SCRIPT_TIME

c3e3e2b

Plane: added support for NAV_SCRIPT_TIME

d0d21d8

AP_Vehicle: added nav_script_time methods

99ba7d7

for supporting aerobatics in scripts

AP_Scripting: added bindings for NAV_SCRIPT_TIME support

cad959e

AP_Scripting: added plane aerobatics example

df0104a

tridge force-pushed the pr-plane-nav-scripting branch from 67f3f1b to df0104a Compare November 8, 2021 04:10

tridge added the DevCallTopic label Nov 8, 2021

rmackay9 reviewed Nov 8, 2021

View reviewed changes

rmackay9 approved these changes Nov 8, 2021

View reviewed changes

priseborough approved these changes Nov 8, 2021

View reviewed changes

tridge merged commit ad29135 into ArduPilot:master Nov 8, 2021

CraigElder removed the DevCallTopic label Nov 15, 2021

Scripting: allow for aerobatics in AUTO mode in plane #19039

Scripting: allow for aerobatics in AUTO mode in plane #19039

Conversation

tridge commented Oct 25, 2021 • edited by rmackay9 Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rmackay9 Oct 30, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

IamPete1 Oct 25, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rmackay9 commented Oct 25, 2021 • edited Loading

tridge commented Oct 26, 2021

IamPete1 commented Oct 26, 2021 • edited Loading

rmackay9 commented Oct 30, 2021

rmackay9 commented Oct 30, 2021 • edited Loading

rmackay9 commented Oct 30, 2021

tridge commented Oct 31, 2021

rmackay9 commented Nov 1, 2021

tridge commented Nov 1, 2021

tridge commented Nov 1, 2021

rmackay9 commented Nov 1, 2021 • edited Loading

tridge commented Nov 1, 2021

rmackay9 commented Nov 1, 2021

rmackay9 commented Nov 1, 2021

tridge commented Nov 1, 2021

tridge commented Nov 1, 2021

rmackay9 commented Nov 1, 2021 • edited Loading

tridge commented Nov 1, 2021

rmackay9 commented Nov 1, 2021 • edited Loading

tridge commented Nov 1, 2021

tridge commented Nov 1, 2021

rmackay9 Nov 1, 2021 • edited Loading

Choose a reason for hiding this comment

IamPete1 commented Nov 1, 2021

rmackay9 commented Nov 1, 2021

IamPete1 commented Nov 1, 2021

tridge commented Nov 8, 2021

tridge commented Nov 8, 2021

arikrupnik commented Nov 8, 2021

tridge commented Nov 8, 2021

rmackay9 Nov 8, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rmackay9 left a comment

Choose a reason for hiding this comment

priseborough commented Nov 8, 2021 • edited Loading

tridge commented Oct 25, 2021 •

edited by rmackay9

Loading

rmackay9 Oct 30, 2021 •

edited

Loading

IamPete1 Oct 25, 2021 •

edited

Loading

rmackay9 commented Oct 25, 2021 •

edited

Loading

IamPete1 commented Oct 26, 2021 •

edited

Loading

rmackay9 commented Oct 30, 2021 •

edited

Loading

rmackay9 commented Nov 1, 2021 •

edited

Loading

rmackay9 commented Nov 1, 2021 •

edited

Loading

rmackay9 commented Nov 1, 2021 •

edited

Loading

rmackay9 Nov 1, 2021 •

edited

Loading

rmackay9 Nov 8, 2021 •

edited

Loading

priseborough commented Nov 8, 2021 •

edited

Loading