Conversation
|
When this is ready for review, I'll find some time to test it on our robot. Thanks! |
ah, I just saw the related spot_wrapper PR. I'll find some time to review and test them next week |
|
Hey @tcappellari-bdai,This should be ready for review. I just need your opinion onon bdaiinstitute/spot_wrapper#179, specifically regarding the arm velocity control. Do you think the end time should be set by the user or handled internally by the spot_ros2 driver? |
|
Left some other comments on bdaiinstitute/spot_wrapper#179, but I think this example will be super useful to have in soon. Thanks @kabirkedia! |
|
@khughes-bdai this is ready to merge |
|
Hi @kabirkedia! Just tested this out on robot, sorry for the super long delay. The demo is working great! I also tried out just publishing to
Also, can you run |
| # self.logger.info("Standing robot up") | ||
| # result = self.robot.command("stand") | ||
| # if not result.success: | ||
| # self.logger.error("Robot did not stand message was " + result.message) | ||
| # return False |
There was a problem hiding this comment.
can this be deleted/uncommented?
| while time.time() - start_time < ARM_VELOCITY_CMD_DURATION: | ||
| self.pub_arm_vel.publish(arm_cmd) | ||
| time.sleep(0.01) | ||
| self.pub_arm_vel.publish(ArmVelocityCommandRequest()) |
There was a problem hiding this comment.
I think this might be the cause of the error I was seeing in the terminal:
[spot_ros2-1] [ERROR] [1760995022.039689202] [spot_ros2]: Failed to execute arm velocity command: An error occured while trying to move arm
[spot_ros2-1] Exception: bosdyn.api.RobotCommandResponse (InvalidRequestError): Received request with unset command.
This is because ArmVelocityRequest() instead of being a zero command is read as an uninitialized command. I think you will have to explicitly define this as a proto first, then convert to the ros message form. This is just a little more robust than filling in the ros message manually and ensures all of the appropriate fields are captured. I think it would be useful to do this both for the command from the user input as well as the zero command at the end
from bosdyn.api import arm_command_pb2
arm_cmd_request = arm_command_pb2.ArmVelocityCommand.Request()
# then fill in the attributes the same way you are doing above
arm_cmd_request.angular_velocity_of_hand_rt_odom_in_hand.x = end_effector_x
arm_cmd_request.angular_velocity_of_hand_rt_odom_in_hand.y = end_effector_y
...
arm_cmd_request_ros = ArmVelocityCommandRequest()
convert(arm_cmd_request, arm_cmd_request_ros)
# then publish arm_cmd_request_ros
your zero command at the end would be
zero_arm_cmd_request = arm_command_pb2.ArmVelocityCommand.Request()
zero_arm_cmd_request_ros = ArmVelocityCommandRequest()
convert(zero_arm_cmd_request, zero_arm_cmd_request_ros)
| # try: | ||
| os.environ.setdefault("ESCDELAY", "0") | ||
| curses.wrapper(wasd_interface.drive) | ||
| # except Exception as e: | ||
| # wasd_interface.logger.error("WASD has thrown an error:" + str(e)) |
There was a problem hiding this comment.
can these comments be removed?
Co-authored-by: Katie Hughes <157421702+khughes-bdai@users.noreply.github.com>
|
Hey @khughes-bdai , |
Yes the arm locking in the world frame is a weird thing but I implemented a lock arm in place just for that. It should now lock the arm in place using joint commands. |
|
Hi @kabirkedia , sorry for the delay on my end! I just re tested this on robot. There were a few things I had to tweak manually to get it running.
I am also still seeing merge conflicts from the git submodules on this PR. Could you untrack them from your branch? Thanks! |
khughes-bdai
left a comment
There was a problem hiding this comment.
Moved the changes I had to make locally to review format to make it easier to parse!
| if not self.spot_wrapper: | ||
| self.get_logger().info(f"Mock mode, received command vel {data}") | ||
| return | ||
| self.spot_wrapper.velocity_cmd(data.linear.x, data.linear.y, data.angular.z, self.cmd_duration) |
There was a problem hiding this comment.
Forgot to catch this earlier, but this will currently break because of the order of the args from this spot_wrapper function: https://github.com/bdaiinstitute/spot_wrapper/blob/8e6ff4e55817952f5201fe510204324d023825ae/spot_wrapper/wrapper.py#L1288
I would suggest explicitly passing the arg names here to make sure that this is parsed correctly
| self.spot_wrapper.velocity_cmd(data.linear.x, data.linear.y, data.angular.z, self.cmd_duration) | |
| self.spot_wrapper.velocity_cmd(v_x=data.linear.x, v_y=data.linear.y, v_rot=data.angular.z, cmd_duration=self.cmd_duration) |
| timestamp = data.header.stamp.sec + data.header.stamp.nanosec * 1e-9 | ||
| self.spot_wrapper.velocity_cmd( | ||
| v_x=data.linear.x, v_y=data.linear.y, v_rot=data.angular.z, cmd_duration=self.cmd_duration | ||
| data.twist.linear.x, data.twist.linear.y, data.twist.angular.z, timestamp, self.cmd_duration |
There was a problem hiding this comment.
Same comment here about explicitly passing the args:
| data.twist.linear.x, data.twist.linear.y, data.twist.angular.z, timestamp, self.cmd_duration | |
| v_x=data.linear.x, v_y=data.linear.y, v_rot=data.angular.z, timestamp=timestamp, cmd_duration=self.cmd_duration |
| arm_cmd_request.maximum_acceleration.data = ARM_MAXIMUM_ACCELERATION | ||
| arm_cmd_request.command.command_choice = arm_cmd_request.command.COMMAND_CYLINDRICAL_VELOCITY_SET | ||
| arm_cmd_request.command.cylindrical_velocity.linear_velocity.r = r | ||
| arm_cmd_request.command.cylindrical_velocity.linear_velocity.theta = theta | ||
| arm_cmd_request.command.cylindrical_velocity.linear_velocity.z = z | ||
| arm_cmd_request.command.cylindrical_velocity.max_linear_velocity.data = ARM_MAX_LINEAR_VELOCITY |
There was a problem hiding this comment.
Think this was left over from switching from ros msg -> proto
| arm_cmd_request.maximum_acceleration.data = ARM_MAXIMUM_ACCELERATION | |
| arm_cmd_request.command.command_choice = arm_cmd_request.command.COMMAND_CYLINDRICAL_VELOCITY_SET | |
| arm_cmd_request.command.cylindrical_velocity.linear_velocity.r = r | |
| arm_cmd_request.command.cylindrical_velocity.linear_velocity.theta = theta | |
| arm_cmd_request.command.cylindrical_velocity.linear_velocity.z = z | |
| arm_cmd_request.command.cylindrical_velocity.max_linear_velocity.data = ARM_MAX_LINEAR_VELOCITY | |
| arm_cmd_request.maximum_acceleration.value = ARM_MAXIMUM_ACCELERATION | |
| arm_cmd_request.cylindrical_velocity.linear_velocity.r = r | |
| arm_cmd_request.cylindrical_velocity.linear_velocity.theta = theta | |
| arm_cmd_request.cylindrical_velocity.linear_velocity.z = z | |
| arm_cmd_request.cylindrical_velocity.max_linear_velocity.value = ARM_MAX_LINEAR_VELOCITY | |
| self.add_message("Switched to ARM CONTROL mode") | ||
| else: | ||
| self.control_mode = "base" | ||
| self.add_message("Switched to BASE MOVEMENT mode") |
There was a problem hiding this comment.
I think it is more robust to lock the arm in place when you switch modes:
| self.add_message("Switched to BASE MOVEMENT mode") | |
| self.add_message("Switched to BASE MOVEMENT mode") | |
| self.lock_arm_in_position() |
| zero_arm_cmd_request = arm_command_pb2.ArmVelocityCommand.Request() | ||
| zero_arm_cmd_request_ros = ArmVelocityCommandRequest() | ||
| convert(zero_arm_cmd_request, zero_arm_cmd_request_ros) | ||
|
|
||
| self.pub_arm_vel.publish(zero_arm_cmd_request_ros) |
There was a problem hiding this comment.
I think this part can be removed. It gets rid of terminal spew without affecting the performance. I would also reccomend setting the arm_is_unstowed bool to true here as this moves the arm
| zero_arm_cmd_request = arm_command_pb2.ArmVelocityCommand.Request() | |
| zero_arm_cmd_request_ros = ArmVelocityCommandRequest() | |
| convert(zero_arm_cmd_request, zero_arm_cmd_request_ros) | |
| self.pub_arm_vel.publish(zero_arm_cmd_request_ros) | |
| self.is_arm_unstowed = True |
| if self.service_call_in_progress: | ||
| self.add_message("Service call in progress, cannot send velocity command") | ||
| return | ||
| if self.is_arm_unstowed: | ||
| if not self.is_arm_locked_in_position: | ||
| self.service_call_in_progress = True | ||
| result = self.lock_arm_in_position() | ||
| self.service_call_in_progress = False | ||
| if not result: | ||
| self.add_message("Failed to lock arm in position, cannot send velocity command") | ||
| self.is_arm_locked_in_position = False | ||
| return | ||
| else: | ||
| self.is_arm_locked_in_position = True | ||
| else: | ||
| self.is_arm_locked_in_position = False |
There was a problem hiding this comment.
If we handle locking the arm when we switch between arm/base mode, this whole block can be deleted
| if self.service_call_in_progress: | |
| self.add_message("Service call in progress, cannot send velocity command") | |
| return | |
| if self.is_arm_unstowed: | |
| if not self.is_arm_locked_in_position: | |
| self.service_call_in_progress = True | |
| result = self.lock_arm_in_position() | |
| self.service_call_in_progress = False | |
| if not result: | |
| self.add_message("Failed to lock arm in position, cannot send velocity command") | |
| self.is_arm_locked_in_position = False | |
| return | |
| else: | |
| self.is_arm_locked_in_position = True | |
| else: | |
| self.is_arm_locked_in_position = False |
| """Lock arm in current position - placeholder for actual implementation""" | ||
| # This would need to be implemented based on your robot's API |
There was a problem hiding this comment.
Can you update/remove this comment?
Change Overview
Adds better arm control to the driver.
Testing Done
Please create a checklist of tests you plan to do and check off the ones that have been completed successfully. Ensure that ROS 2 tests use
domain_coordinatorto prevent port conflicts. Further guidance for testing can be found on the ros utilities wiki.