-
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
Allow model_instances for MBP::CalcJacobianCenterOfMassTranslationalVelocity() #15387
Allow model_instances for MBP::CalcJacobianCenterOfMassTranslationalVelocity() #15387
Conversation
…) with model_instances argument to resolve issue RobotLocomotion#14916
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.
Feature review +@hongkai-dai (you suggested issue #14916, feel free to redirect if needed).
Reviewable status: LGTM missing from assignee hongkai-dai, needs platform reviewer assigned, needs at least two assigned reviewers, commits need curation (https://drake.mit.edu/reviewable.html#curated-commits) (waiting on @hongkai-dai)
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.
could you also add the python binding for this new function?
Reviewed 2 of 5 files at r1, 3 of 3 files at r2, 1 of 1 files at r3.
Reviewable status: needs platform reviewer assigned, needs at least two assigned reviewers, commits need curation (https://drake.mit.edu/reviewable.html#curated-commits) (waiting on @mitiguy)
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.
+@sherm1 for platform review please, thanks!
Reviewable status: LGTM missing from assignee sherm1(platform), commits need curation (https://drake.mit.edu/reviewable.html#curated-commits) (waiting on @sherm1)
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.
Platform pending a few nits. I will fix those since Paul is on a long vacation.
Reviewed 2 of 5 files at r1, 3 of 3 files at r2, 1 of 1 files at r3.
Reviewable status: 2 unresolved discussions, commits need curation (https://drake.mit.edu/reviewable.html#curated-commits) (waiting on @mitiguy)
multibody/plant/test/multibody_plant_jacobians_test.cc, line 313 at r3 (raw file):
bodyB_ = &plant_->AddRigidBody("bodyB", bodyB_instance_, M_Bcm); // bodyA_ = &plant_->AddRigidBody("BodyA", M_Bcm); // bodyB_ = &plant_->AddRigidBody("BodyB", M_Bcm);
nit: delete dead code
multibody/tree/multibody_tree.cc, line 2615 at r3 (raw file):
JacobianWrtVariable with_respect_to, const Frame<T> &frame_A, const Frame<T> &frame_E,
nit: "&" misplaced
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.
Reviewable status: commits need curation (https://drake.mit.edu/reviewable.html#curated-commits) (waiting on @hongkai-dai and @sherm1)
multibody/plant/test/multibody_plant_jacobians_test.cc, line 313 at r3 (raw file):
Previously, sherm1 (Michael Sherman) wrote…
nit: delete dead code
Done.
multibody/tree/multibody_tree.cc, line 2615 at r3 (raw file):
Previously, sherm1 (Michael Sherman) wrote…
nit: "&" misplaced
Done.
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.
Reviewed 2 of 2 files at r4.
Reviewable status: complete! all discussions resolved, LGTM from assignees sherm1(platform),hongkai-dai (waiting on @mitiguy)
New overload for MultibodyPlant::CalcJacobianCenterOfMassTranslationalVelocity() to allow for model_instances
to resolve issue #14916.
This change is