-
Notifications
You must be signed in to change notification settings - Fork 60
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 support for vehicle_width_override property on route #485
Conversation
swri_route_util/src/route_speeds.cpp
Outdated
@@ -355,10 +355,24 @@ void speedsForObstacles( | |||
} | |||
continue; | |||
} | |||
|
|||
// Use lane width as the car radius when smaller |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please elaborate the comment to describe the purpose of this override and replace 'lane width' with the name of the propery
swri_route_util/src/route_speeds.cpp
Outdated
double veh_r = car_r; | ||
if (point.hasProperty("vehicle_width_override")) | ||
{ | ||
ROS_INFO("Speeds for obstacle found vehicle_width_override property"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this will spam the console in most cases. This should probably be removed or set to ROS_DEBUG
Apply Marc's recommendations
// Pick the smaller of the radii | ||
if (veh_r >= width/2.0) | ||
{ | ||
veh_r = width/2.0; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It probably makes sense to issue a warning here if the vehicle width is actually overridden. Doing so won't change the behavior, but it will make it more obvious if it causes problems. Given Marc's earlier comment about spamming the console, a ROS_WARN_THROTTLE(1.0, ...)
is probably appropriate.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added
Add thottled warning for when vehicle width is overriden.
No description provided.