Default wrist_tform_tool to identity

[llee-bdai]

Turns out ArmCartesianCommand defaults wrist_tform_tool to be some approximately 20cm offset https://dev.bostondynamics.com/protos/bosdyn/api/proto_reference#armcartesiancommand-request

This PR sets the transform to be identity so that when you send a hand pose, it's commanding the wrist to the pose, and not wrist plus some unknown defaulted offset.

[coveralls]

Pull Request Test Coverage Report for Build 11731269410

Details

• 0 of 0 changed or added relevant lines in 0 files are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage remained the same at 40.848%

---

Totals
Change from base Build 11486927459:	0.0%
Covered Lines:	1870
Relevant Lines:	4578

---

💛 - Coveralls

[amessing-bdai]

@llee-bdai Is this the function that you recently fixed? I am trying to figure out how much of a breaking change this is.

[scastro-bdai]

Given that this function is called hand_pose but really sets the pose of the arm, and also is called in the arm_pose_cmd_callback of the SpotROS node...

how would you feel about

1. Renaming this to set_arm_pose()
2. Actually making this transform an argument of the function instead of hard-coding, but still defaulting it to identity?
3. Making the above default arg an SE3 and converting it to the protobuf inside this function body?

[khughes-bdai]

@scastro-bdai i am nervous that renaming from hand_pose() could be a breaking change to external users -- although there already was a breaking change to this function with #145 and there were no complaints 😅
To your third point, we unfortunately can't use spatialmath SE3 in this repo as it's not a dependency. I think a bosdyn SE3Pose could make sense as a default arg

[llee-bdai]

@amessing-bdai pulling you into comments here because wer're all talking about the same thing. To answer your question as to if this is the function I recently fixed, yes, in terms of fixing it by removing ROS1 dependencies and actually using it. This isn't a breaking change for anything internal, but it would be breaking externally, technically.

@scastro-bdai @khughes-bdai

1. I am not opposed to changing the name. The only reason it is named what it is now is because it has been this way since being written. If we were to rename it, I would agree to set_arm_pose
2. and 3. I would rather not make this an argument for now until we have a reason to make it an argument. We already have so many args into this function, I would hesitate to add more until we need it.

[amessing-bdai]

Hmm, since we recently changed the signature already it's probably ok to further change it, but lets make sure this is the last change to the signature and not make this common practice.

[llee-bdai]

I am also perfectly fine not changing the signature because it's been named this for a while and just very clearly specifying in the function comments that it's to the wr1 link

[jbarry-bdai]

IIRC the offset that it defaults to is the offset from the wrist to the center of the gripper, which is usually the "tool frame" people want.

I think this is probably not a desirable change especially this deep in the code in a repo that lots of people use...

[scastro-bdai]

IIRC the offset that it defaults to is the offset from the wrist to the center of the gripper, which is usually the "tool frame" people want.

I think this is probably not a desirable change especially this deep in the code in a repo that lots of people use...

I guess as a compromise, should we include the optional offset argument, and default it to the "sanctioned" tool frame?

[khughes-bdai]

If you want to be explicitly commanding the wr1 link, it might make sense to make a new function called something like wrist_pose. This would eliminate any confusion with the pre-existing hand_pose function and make it clear that it's commanding a different link of the robot. thoughts?

[llee-bdai]

I'm cool with @khughes-bdai suggestion of adding a whole new wrist pose function. But also, if we add another function, I wouldn't mind just adding in all the extra params required to specify wrist to some offset pose

[llee-bdai]

On second thought, because this change ends up affecting a lot more than I expected, I am going to close this. Thanks for all the comments