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

Add function to calculate the orientation at which the footprint sweeps the smallest area #45

Closed
wants to merge 2 commits into from

Conversation

corot
Copy link

@corot corot commented Jul 29, 2022

@corot corot force-pushed the js/minSweepingAreaOrientation branch from ca0d7f0 to 3fa85bc Compare July 29, 2022 09:43
@corot corot requested a review from a team July 29, 2022 09:43
for (unsigned int i = 0; i < footprint.size() - 1; ++i)
{
double edge_dist = distanceToLine(0, 0, footprint[i].x, footprint[i].y, footprint[i + 1].x, footprint[i + 1].y);
if (edge_dist < min_dist)
Copy link
Member

Choose a reason for hiding this comment

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

Q: I don't understand how distance to the center will give you the smallest sweeping area.
A rectangle like the one below will give you 90 degrees as output (because the left side is the closest to the center). But 90 degrees would sweep the largest area
rectangle

Copy link
Author

Choose a reason for hiding this comment

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

if +x is the horizontal, rotating the rectangle 90 deg will minimize the sweeping area; so 90 deg is the right answer

Copy link
Member

Choose a reason for hiding this comment

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

but for a "kite" shape it doesn't work, because it should be 90 deg too, and it is not:
image

Copy link
Author

Choose a reason for hiding this comment

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

the answer won't be 90, true
but it will be correct anyway: will sweep the same area as with 90

in any case that fp violates the 2nd assumption I added to the function (true, the assumptions are a bit ad-hoc,,, but are true for every polygonal robot I know

Copy link
Member

Choose a reason for hiding this comment

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

hum, true. But then the function should check the violation. You don't need to loop all edges if you assume the closest one is parallel to the x-axis

Comment on lines +96 to +100
// return the orientation of the closest edge, directed from back to front (+x axis direction)
std::sort(closest_edge.begin(), closest_edge.end(),
[](const geometry_msgs::Point& p1, const geometry_msgs::Point& p2) { return p1.x < p2.x; });
return orientation(closest_edge.front().x, closest_edge.front().y, closest_edge.back().x, closest_edge.back().y);
}

Choose a reason for hiding this comment

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

This logic won't work. You are trying to compute the direction in which the sweep is minimum and not the edge which minimizes the sweep (the closest edge may not be the one either as @renan028 mentioned).

You will need to implement something like this https://en.wikipedia.org/wiki/Rotating_calipers and figure out the normal to the min width of the polygon.

Copy link
Author

@corot corot Aug 1, 2022

Choose a reason for hiding this comment

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

oh,,, yes, yes;
I forgot to add the assumptions I'm doing here:

  • the footprint is symmetric wrt the x axis
  • the closest edge is approximately parallel to either x or y axis

Then the logic makes sense

Copy link
Author

Choose a reason for hiding this comment

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

added

@corot corot requested review from ayushgaud and renan028 August 1, 2022 08:25
@corot corot force-pushed the js/minSweepingAreaOrientation branch from 67e2141 to 2fb17ce Compare September 7, 2022 07:04
@corot
Copy link
Author

corot commented Sep 14, 2022

replaced by #52

@corot corot closed this Sep 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants