-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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 goal pose to CriticData #4812
base: humble
Are you sure you want to change the base?
Add goal pose to CriticData #4812
Conversation
Signed-off-by: redvinaa <redvinaa@gmail.com>
4465836
to
2d61b3f
Compare
Please check implementation and when everything is correct, I'll port to rolling |
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.
Generally looks good to me
@@ -92,13 +92,15 @@ geometry_msgs::msg::TwistStamped MPPIController::computeVelocityCommands( | |||
last_time_called_ = clock_->now(); | |||
|
|||
std::lock_guard<std::mutex> param_lock(*parameters_handler_->getLock()); | |||
geometry_msgs::msg::Pose goal = path_handler_.getTransformedGoal().pose; |
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.
Shouldn't we get the goal as transformed_plan.poses.back()
? There's no reason to do another TF operation I don't think and we can find it on L98 below transformPath()
instead
@@ -119,7 +119,7 @@ void CostCritic::score(CriticData & data) | |||
|
|||
// If near the goal, don't apply the preferential term since the goal is near obstacles | |||
bool near_goal = false; | |||
if (utils::withinPositionGoalTolerance(near_goal_distance_, data.state.pose.pose, data.path)) { |
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 much prefer this API so that its explicit about what information that withinPositionGoalTolerance
is working on instead of handing it everything and then having to reference the implementation to know what data its using. I think you can just update the existing withinPositionGoalTolerance()
method instead - it seems like a pretty trivial change
Basic Info
Description of contribution in a few bullet points
Description of documentation updates required from your changes
Future work that may be required in bullet points
For Maintainers: