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

Copter: Excluded target velocity from slowdown calculations in FOLLOW mode #26142

Merged
merged 1 commit into from
Feb 20, 2024

Conversation

Georacer
Copy link
Contributor

@Georacer Georacer commented Feb 4, 2024

Summary

This PR fixes a bug where Copter would not reach its target in FOLLOW mode.

The issue

Currently, when the Copter would be in FOLLOW mode, following a moving target, it would never actually reach its desired position; instead there would be a steady-state position error.

The reason is that there are slowdown calculations in place in ArduCopter/mode_follow.cpp, in order to overwrite the position controller velocity setpoint and slow down the UA as it approaches its target, and prevent an overshoot.
However, these slowdown calculations would be applied over the absolute velocity, not the velocity relative to the target.
Effectively, the absolute velocity would be required to be zero above the moving target, which is inconsistent with a moving target.

This issue likely blocks the implementation of an effective Copter landing on a moving base.

The changes

Before

ArduCopter/mode_follow.cpp:L65 and beyond carries out the calculations for the desired velocity.
In order, the steps taken would be:

  1. Get the position offset from the target. (L67)
  2. Use FOLL_POS_P gain to calculate approach velocity and add target velocity. (L69-73)
  3. Apply the maximum velocity limits. (L75-85)
  4. Carry out the slow-down calculations to limit lateral velocity based on distance from target. (L94-112).

After

With this PR, the order of the steps is slightly changed.

  1. Get the position offset from the target. (L67)
  2. Use FOLL_POS_P gain to calculate approach velocity without adding target velocity. (L69-73). This yields the desired relative approach velocity.
  3. Carry out the slow-down calculations to limit lateral velocity based on distance from target. (L75-93)
  4. Add the target velocity to obtain the absolute desired velocity. (L95-98)
  5. Apply the maximum velocity limits. (L100-110)

Testing

A following scenario similar to that of #24720 was simulated.
A truck would drive around at ~6m/s and the Copter was tasked to follow it.

Before, the Copter could get no closer than ~15m.
Screenshot from 2024-02-04 23-01-19

After, the Copter could effectively nullify the tracking error:
Screenshot from 2024-02-04 23-02-11

The simulated flight logs are available upon request. Let me know where I should upload them.

ℹ️ A script was running during the tests, but I don't think it alters the presented results.

@IamPete1
Copy link
Member

IamPete1 commented Feb 5, 2024

This seems like a slightly different version of #26138? This does have documented testing tho, so points for that ;)

@Georacer
Copy link
Contributor Author

Georacer commented Feb 5, 2024

@IamPete1 well, two people tried to solve the same problem at the same day, after years of this code being live. What are the odds!

The only comment-worthy difference with @tridge 's code is that in his PR he applies the maximum speed limit (e.g. WPNAV_SPEED or RTL_SPEED) before adding the target's velocity. So there's a good chance the UA will actually overspeed and fail to limit itself to the specified value.

Other than that, tridge's PR has some nicer vector math.

@tridge
Copy link
Contributor

tridge commented Feb 6, 2024

The only comment-worthy difference with @tridge 's code is that in his PR he applies the maximum speed limit (e.g. WPNAV_SPEED or RTL_SPEED) before adding the target's velocity. So there's a good chance the UA will actually overspeed and fail to limit itself to the specified value.

I fixed that in my PR this morning :-)

@IamPete1
Copy link
Member

IamPete1 commented Feb 6, 2024

@Georacer Can you review the latest version of #26138, We think the maths should now be the same?

@Georacer
Copy link
Contributor Author

Georacer commented Feb 6, 2024

@Georacer Can you review the latest version of #26138, We think the maths should now be the same?

@IamPete1 I have done so. I have requested some changes though, it's not 100% equivalent yet.

Copy link
Contributor

@peterbarker peterbarker left a comment

Choose a reason for hiding this comment

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

Commit message needs fixing

@Georacer
Copy link
Contributor Author

Commit message needs fixing

@peterbarker I re-wrote it. Is this what you expected? I went by this guideline: https://ardupilot.org/dev/docs/submitting-patches-back-to-master.html

@peterbarker
Copy link
Contributor

Commit message needs fixing

@peterbarker I re-wrote it. Is this what you expected? I went by this guideline: https://ardupilot.org/dev/docs/submitting-patches-back-to-master.html

Yep, that's good, thanks.

desired_velocity_neu_cms.z += -vel_of_target.z * 100.0f;

// scale desired velocity to stay within horizontal speed limit
float desired_speed_xy = safe_sqrt(sq(desired_velocity_neu_cms.x) + sq(desired_velocity_neu_cms.y));
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we can replace this block with:

  desired_velocity_neu_cms.xy().limit_length(pos_control->get_max_speed_xy_cms());

The slowdown calculations should apply only to the relative velocity,
not the absolute one. Thus the target baseline velocity should be added
afterwards.
Naturally the absolute velocity limits should be applied afterwards.
@tridge
Copy link
Contributor

tridge commented Feb 20, 2024

I tested in SITL, looks good!

@rmackay9 rmackay9 merged commit b2898ca into ArduPilot:master Feb 20, 2024
72 checks passed
@rmackay9
Copy link
Contributor

Merged, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: 4.5.0-beta3
Development

Successfully merging this pull request may close these issues.

6 participants