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

new_gripper_design #2

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

new_gripper_design #2

wants to merge 2 commits into from

Conversation

zhenyan2001
Copy link
Contributor

updates on the new gripper design

Copy link

@MatteoTschesche MatteoTschesche left a comment

Choose a reason for hiding this comment

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

Thanks for your work! I have a few points I suggest to change:
There is no need for 3 python scripts to update each transform/axis. You can just update the axis positions in the action server and remove a lot of duplicated code and make it easier to understand. You also don't need the cmd_vel topic then to send the information to the other scripts. Same for the static_tf_gripper_origin.py

To Gripper.action in gripper_msgs:

  • remove the outcommented variables
  • feedback is not needed since the transform is given all necessary feedback already

To the README.md:

  • reformat it so the code is copy-able and well structured like the README in gripper_tf_simulator

To gripper_tf_simulator/resource/gripper_tf_simulator

  • remove this empty file/folder

self.transf.transform.rotation.y = 0.0
self.transf.transform.rotation.z = 0.0
self.transf.transform.rotation.w = 1.0

self.tf_broadcaster = TransformBroadcaster(self)
self.prev_time = self.get_clock().now()
self.timer = self.create_timer(0.1, self.tf_timer)

Choose a reason for hiding this comment

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

Call the topic something different than cmd_vel. You don't want to send velocity commands and it is already in use by the robotino driver. Call it cmd_gripper, gripper_commands, gripper_target or something like that.

@@ -51,18 +69,24 @@ def tf_timer(self):
self.position_x += self.linear_x * dt
self.position_y += self.linear_y * dt
self.position_z += self.linear_z * dt

self.yaw += self.angular_yaw * dt

Choose a reason for hiding this comment

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

What is the reason for this for the x axis?

# Update the previous time
self.prev_time = current_time



def main():
rclpy.init()

Choose a reason for hiding this comment

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

remove the try catch block below and just do
rclpy.spin(node)
rclpy.shutdown()

# self.transf.transform.rotation.z = quat[2]
# self.transf.transform.rotation.w = quat[3]

quat = euler2quat(self.angular_roll, self.angular_pitch, self.angular_yaw)

Choose a reason for hiding this comment

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

this code is doubled and it is unclear for what you use it except for setting self.angular_yaw

@@ -34,14 +47,19 @@ def arm_callback(self, msg: Twist):
self.linear_x = msg.linear.x

Choose a reason for hiding this comment

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

It is unclear why you do more than setting self.linear_x in this function.

# feedback_msg.y_angle = goal_handle.request.pitch_target - orientation.y
# feedback_msg.z_angle = goal_handle.request.yaw_target - orientation.z
# feedback_msg.w_angle = goal_handle.request.qua_target - orientation.w
current_yaw = math.atan2(

Choose a reason for hiding this comment

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

Use euler2quat from transforms3d.euler for that instead. Else nobody can follow if this is correct.

1.0 - 2.0 * (orientation.y ** 2 + orientation.z ** 2)
)
target_yaw = math.atan2(goal_handle.request.y_target, goal_handle.request.x_target)
z_angle = target_yaw - current_yaw
goal_handle.publish_feedback(feedback_msg)

Choose a reason for hiding this comment

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

Feedback is not needed since you update the TFs of the axis as feedback for the action clients.

if (abs(feedback_msg.position_x) <= tolerance and
abs(feedback_msg.position_y) <= tolerance and
abs(feedback_msg.position_z) <= tolerance):
abs(z_angle) <= tolerance and

Choose a reason for hiding this comment

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

No need to check it like this. If you have the execution in this script as well, you know when the position is reached.


# Check if the target position is reached (within a small tolerance)
tolerance = 0.01 # Define your tolerance
tolerance = 0.1 # Define your tolerance

Choose a reason for hiding this comment

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

10cm is a very large threshold. Try 0.005 instead.

cmd_vel.linear.x = 3 * feedback_msg.position_x
cmd_vel.linear.y = 3 * feedback_msg.position_y
cmd_vel.linear.z = 3 * feedback_msg.position_z
if goal_handle.request.x_target <0 :

Choose a reason for hiding this comment

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

I will change this mathematical part to what we discussed today.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants