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

Allow model_instances for MBP::CalcJacobianCenterOfMassTranslationalVelocity() #15387

Conversation

mitiguy
Copy link
Contributor

@mitiguy mitiguy commented Jul 13, 2021

New overload for MultibodyPlant::CalcJacobianCenterOfMassTranslationalVelocity() to allow for model_instances
to resolve issue #14916.


This change is Reviewable

@mitiguy mitiguy changed the title [WIP] Allow model_instances for MBP::CalcJacobianCenterOfMassTranslationalVelocity() Allow model_instances for MBP::CalcJacobianCenterOfMassTranslationalVelocity() Jul 19, 2021
Copy link
Contributor Author

@mitiguy mitiguy left a 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)

Copy link
Contributor

@hongkai-dai hongkai-dai left a comment

Choose a reason for hiding this comment

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

:lgtm: 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)

Copy link
Contributor

@hongkai-dai hongkai-dai left a 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)

Copy link
Member

@sherm1 sherm1 left a comment

Choose a reason for hiding this comment

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

Platform :lgtm: 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

Copy link
Contributor Author

@mitiguy mitiguy left a 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.

@sherm1 sherm1 added the status: squashing now https://drake.mit.edu/reviewable.html#curated-commits label Jul 21, 2021
Copy link
Member

@sherm1 sherm1 left a 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: :shipit: complete! all discussions resolved, LGTM from assignees sherm1(platform),hongkai-dai (waiting on @mitiguy)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: multibody plant MultibodyPlant and supporting code priority: medium status: squashing now https://drake.mit.edu/reviewable.html#curated-commits type: feature request unused team: dynamics
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants