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

Plane: support DO_RETURN_PATH_START mission item and command #28574

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

IamPete1
Copy link
Member

Adds support for the DO_RETURN_PATH_START mission item to plane. Alternate to #28571

DO_RETURN_PATH_START is marks that the vehicle can rejoin the mission at any point after that waypoint and before a landing or a DO_LAND_START. The mission will be re-joined on the leg which is closest to the vehicles current location.

I had wanted to get rid of RTL_AUTOLAND and have a AUTO RTL mode as we do in copter. But I have not got to that yet.

@IamPete1 IamPete1 added the Plane label Nov 10, 2024
@Hwurzburg Hwurzburg added the WikiNeeded needs wiki update label Nov 10, 2024
@Hwurzburg
Copy link
Collaborator

can you explain how this differs from do_land_start? if it is used anywhere in the mission an RTL will just jump to the closest mission leg and continue? a bit confused

@IamPete1
Copy link
Member Author

@Hwurzburg I did a demo for copter, at that time the waypoint was going to be called DO_LAND_REJOIN in the end we changed it to DO_RETURN_PATH_START, but the behavior is the same.

https://youtu.be/SoHXRTD0vEs?si=clwC3TLAVPJrNjNF

@peterbarker
Copy link
Contributor

Adds support for the DO_RETURN_PATH_START mission item to plane. Alternate to #28571

Doesn't currently replace the functionality in that PR - doesn't force loops to be considered "done".

Copy link
Contributor

@tridge tridge left a comment

Choose a reason for hiding this comment

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

did anyone measure the CPU cost for max mission size when this went into copter?
we also have larger missions now, as can be on microsd

@@ -1056,11 +1056,27 @@ MAV_RESULT GCS_MAVLINK_Plane::handle_command_int_packet(const mavlink_command_in
case MAV_CMD_DO_GO_AROUND:
return plane.trigger_land_abort(packet.param1) ? MAV_RESULT_ACCEPTED : MAV_RESULT_FAILED;

case MAV_CMD_DO_RETURN_PATH_START:
// attempt to rejoin after the next DO_RETURN_PATH_START command in the mission
if (plane.have_position && plane.mission.jump_to_closest_mission_leg(plane.current_loc)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm a little worried about CPU usage - could we watchdog from calling this with a complex mission? might be worth measuring the CPU cost on a F4 per WP and think about max WP count

@@ -116,12 +115,26 @@ void ModeRTL::navigate()
// return here so we don't change the radius and don't run the rtl update_loiter()
return;
}
// mode change failed, revert force resume flag
plane.mission.set_force_resume(false);
Copy link
Contributor

Choose a reason for hiding this comment

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

is this a separate bug fix? or related somehow?

Copy link
Member Author

@IamPete1 IamPete1 Nov 13, 2024

Choose a reason for hiding this comment

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

Related in that the same pattern is in the new case. If the mode change failed then mission was left with force resume true. This means the next time you enter auto you will resume the mission and not restart ignoring MIS_RESTART 1 if it is set.

edit: I'm sure you spotted it but we set set_force_resume(true) before we try and change mode up on line 113

@tridge tridge removed the DevCallEU label Nov 13, 2024
@IamPete1
Copy link
Member Author

IamPete1 commented Nov 13, 2024

did anyone measure the CPU cost for max mission size when this went into copter? we also have larger missions now, as can be on microsd

I would note that this had been supported for longer than missions on SD card. No CPU measurement were done at the time.

First it scans the mission for DO_RETURN_PATH_START way points. That is a full scan. For each DO_RETURN_PATH_START it scans up to the next 254 way points doing the distance to leg calculation. So I guess a worst case mission is 253 waypoints. Then a DO_RETURN_PATH_START followed by a jump to the start of the mission. Mission item reads would be something like: (n - 253) * 0.5 * 254. So 4367 waypoints would be 522478 waypoint reads.

edit: DO_RETURN_PATH_START can actually have a location, so the worst case mission would actually be one full of DO_RETURN_PATH_START items. (n-254)* 253 + 252 + 251 + 250..... = 1072467

If we decide its a issue it would be quite easy to just consider the first 10 or 20 DO_RETURN_PATH_START that would then be 20 * 254 reads (5080).

@IamPete1
Copy link
Member Author

@tridge I have done some testing. You totally can get it to watchdog if you fill your mission full of DO_RETURN_PATH_START way points. The threshold seems for happiness seems to be about 50 points, watchdogs at 500 or so. 50 consecutive points is 1275 waypoint lookups. The first rejoin searches the 49 remaining points, the next one 48 points ect ect.

Currently the limit is a search of 254 points per DO_RETURN_PATH_START. So 5 points to get us up to the max of 1275 points. I think were probably better to limit to a cumulative way-point lookup of 1000, so you could have DO_RETURN_PATH_START with a path of 1000 or 10 each with a path of 100.

Note that although we don't get a watchdog, we also get long loops on 4367 DO_LAND_START points.

@IamPete1
Copy link
Member Author

I have added that 1000 point limit in #28608

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Plane WikiNeeded needs wiki update
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants