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: object avoidance gets configurable margin #6080

Merged
merged 5 commits into from
Apr 20, 2017

Conversation

rmackay9
Copy link
Contributor

This PR improves copter's object avoidance in a few ways:

  • new AVOID_MARGIN parameter allow users to specify (in meters) the minimum distance that vehicles should maintain away from objects (default is 2m)
  • send the range of upward facing lidar to the ground station using the DISTANCE_SENSOR message (same as is used for horizontal lidar)
  • fix bug in AC_Proximity library's MAVlink backend so that it reports it is healthy even if only a single upward facing range has been received.

Copy link
Member

@OXINARF OXINARF left a comment

Choose a reason for hiding this comment

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

One of the comments shows as outdated and it is, since in the overall diff it is correct. In the third commit it says "No functional change", but it actually has a (wrong) functional change, so it would be nice to correct it.

Otherwise looks good to me!

bool AP_Proximity::get_horizontal_distances(Proximity_Distance_Array &prx_dist_array) const
{
if ((drivers[primary_instance] == nullptr) || (_type[primary_instance] == Proximity_Type_None)) {
return 0.0f;
Copy link
Member

Choose a reason for hiding this comment

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

return false

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed, thanks!

for (uint8_t i=0; i<count_max; i++) {
if (g2.proximity.get_horizontal_distances(dist_array)) {
Copy link
Member

Choose a reason for hiding this comment

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

Wrong rebase?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't remember having any rebase issues. I think the final result is ok..

Copy link
Contributor Author

@rmackay9 rmackay9 Apr 20, 2017

Choose a reason for hiding this comment

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

Oh, I see what you're saying.. So, yes, at some point I tried to split out the changes into two commits but it didn't work out quite right. It's too painful to fix this now so I'm going to leave it.

@@ -264,13 +272,13 @@ void AC_Avoid::adjust_velocity_proximity(float kP, float accel_cmss, Vector2f &d
// get boundary from proximity sensor
uint16_t num_points;
const Vector2f *boundary = _proximity.get_boundary_points(num_points);
adjust_velocity_polygon(kP, accel_cmss, desired_vel, boundary, num_points, false);
adjust_velocity_polygon(kP, accel_cmss, desired_vel, boundary, num_points, false, _margin);
Copy link
Contributor

Choose a reason for hiding this comment

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

what's the difference between _margin and get_margin()?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fence.get_margin() gets the minimum distance the vehicle should maintain from the edge of the fence. The _margin (which refers to the AVOID_MARGIN parameter) is the distance the vehicle should maintain from objects. It's possible that the Fence margin should move into the Avoidance library but I think keeping it in the fence library (where it has been) is more intuitive for people during setup.
We could argue that the object avoidance margin should be moved out of the Avoidance library and into the proximity library but I think the placement is not terrible. The Proximity library is really meant just to gather up the distances from whatever sensors.

@@ -298,6 +306,9 @@ void AC_Avoid::adjust_velocity_polygon(float kP, float accel_cmss, Vector2f &des
safe_vel.y = desired_vel.y * _ahrs.cos_yaw() - desired_vel.x * _ahrs.sin_yaw(); // forward
}

// calc margin in cm
float margin_cm = MAX(_margin * 100.0f, 0);
Copy link
Contributor

Choose a reason for hiding this comment

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

you pass in "margin" but then use "_margin".. is that right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

nice catch, thanks!

@@ -50,6 +50,12 @@ class AP_Proximity
Proximity_Good
};

// structure holding distances in 8 directions. used for sending distances to ground station
struct Proximity_Distance_Array {
uint8_t orientation[8]; // orientation (i.e. rough direction) of the distance (see MAV_SENSOR_ORIENTATION)
Copy link
Contributor

Choose a reason for hiding this comment

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

should 8 be a define/const?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants