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

Ovis moveit minimal #37

Open
wants to merge 67 commits into
base: master
Choose a base branch
from
Open

Ovis moveit minimal #37

wants to merge 67 commits into from

Conversation

Leo-Dan
Copy link
Contributor

@Leo-Dan Leo-Dan commented Mar 2, 2023

This is a draft PR about the work that has been done with our Ovis 6 DOF robot and MoveIT!
The goal of this project was to be able to jog our robot in the cartesian space in realtime with an Xbox controller.
The Cartesian space is a way of describing the pose ( position and orientation) of a robot's end-effector.
We wanted to do position incremental control.
We found out that Cartesian position control is not fit for real-time jogging like we want.
The next step for real time jogging would be cartesian velocity control of the end-effector.

Although, we can now use a RVIZ to dictate a pose to the robot, and he adopts it without issues.
A collision and singularity avoidance algorithm is in place, it makes the IK solutions reliable.
I think it would be usable for approach movements in competition via Rviz directly of via the controls we implemented with the Xbox controller.
The integration of this project to work on remote computer is not yet done.

Two main packages have been created for this project:

  • ovis_moveit package : is the one generated by moveit itself after giving it our robot URDF. It contains the move_group node, which is the main actor in the package.

  • ovis_jog_control: is the node that we created to interface moveit's work with our actual robot. It's like a upper level python interface to interact with the move group node and send out commands to the physical robot via an action server. UI commands would preferably pass through this node and be paralleled with implemented controls so that development can still be made in the future.

This is a top of my head description, so it is incomplete. I hope discussions will enrich this description.

Here is a launch schematic to help you understand the structure Alex put in place to test the robot system IRL and via gazebo simulator, or both at the same time !
MicrosoftTeams-image

Copy link
Contributor

@saxtot saxtot left a comment

Choose a reason for hiding this comment

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

First, let me say that you guys have done a great amount of good work here. I'm out of my league on a lot of things you use, but, from what I can see, this is well done.
As always, I have left multiple comments, but most of them are 0.5 second changes and others are for my own understanding. Don't get discouraged by these, you are 1 or 2 commits away from merging if no real problems are found by the other reviewers.
When time will allow it, I wouldn't say no to taking a couple minutes to talk about implementation decisions and the biggest problems you encountered.
Great effort here, I can't wait to see what you two do next!

@@ -329,7 +329,7 @@ int KinovaAPI::initializeKinovaAPIFunctions(KinovaAPIType connection_type)

setTorqueInactivityType = (int (*)(int))initCommandLayerFunction("SetTorqueInactivityType");

setJointZero = (int (*)(int))initCommandLayerFunction("SetJointZero");
//setJointZero = (int (*)(int))initCommandLayerFunction("SetJointZero");
Copy link
Contributor

Choose a reason for hiding this comment

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

Dead code?

Comment on lines 41 to 51

# ovis/gazebo_ros_control:
# pid_gains:
# arm_joint_1: {p: 100, d: 0.01, i: 0.01, i_clamp: 1}
# arm_joint_2: {p: 100, d: 0.01, i: 0.01, i_clamp: 1}
# arm_joint_3: {p: 100, d: 0.01, i: 0.01, i_clamp: 1}
# arm_joint_4: {p: 100, d: 0.01, i: 0.01, i_clamp: 1}
# arm_joint_5: {p: 100, d: 0.01, i: 0.01, i_clamp: 1}
# arm_joint_6: {p: 100, d: 0.01, i: 0.01, i_clamp: 1}


Copy link
Contributor

Choose a reason for hiding this comment

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

Dead code?

Choose a reason for hiding this comment

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

I don't think we need it, but right now when you launch moveit with gazebo, there are some errors because the PID gains haven't been specified. This code fix the errors, but since the gains haven't been properly adjusted, it doesn't work the way it should.

Copy link
Contributor

Choose a reason for hiding this comment

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

I get it, there are still kinks to fix. Might want to add a comment detailing this with the keyword TODO while its still an issue. Extensions that highlight TODO in comments are pretty common and useful.

# Trajectory Controllers ---------------------------------------
joint_trajectory_controller:
type: position_controllers/JointTrajectoryController
joints:
Copy link
Contributor

Choose a reason for hiding this comment

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

Could the joints be an array? This is more from own knowledge, since it seems like the Kinova code itself also does joint by joint assignments.

</node>

<!-- <include file="$(find ovis_robotiq_gripper)/launch/ovis_gripper.launch" /> -->
Copy link
Contributor

Choose a reason for hiding this comment

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

We could move the gripper's launch elsewhere, you can remove this in the mean time.

Comment on lines +4 to +7
<arg name="ovis" default="true"/>
<arg name="moveit" default="true"/>
<arg name="jog" default="true"/>
<arg name="action" default="true"/>
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do you need the ability to not launch these things?

Choose a reason for hiding this comment

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

For debug (multiple terminal) and because we used to only have one launch file for sim/real.

Examples:

  • You don't want jog, if you don't have a controller (you only want to move with rviz)
  • You don't want action if you use gazebo (action server conflict)
  • You might want to launch ovis_arm in an other terminal so it stay active when testing

Copy link
Contributor

Choose a reason for hiding this comment

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

Might want to consider creating specific launch files or at least commenting this information in the file.

ovis_jog_control/scripts/ovis_control.py Show resolved Hide resolved
Comment on lines 5 to 7


#kinematics_solver: kdl_kinematics_plugin/KDLKinematicsPlugin
Copy link
Contributor

Choose a reason for hiding this comment

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

Dead code?

Choose a reason for hiding this comment

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

IK fast should be faster, but it's always good to have an alternative option for comparison.

Copy link
Contributor

Choose a reason for hiding this comment

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

Might want to add a comment to detail that.

Comment on lines +8 to +9
<author email="[email protected]">Alexandre Lapointe</author>
<maintainer email="[email protected]">Alexandre Lapointe</maintainer>
Copy link
Contributor

Choose a reason for hiding this comment

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

Super ignorable nitpick: the club's email or our personal ones should outlive our institutional one. I don't mind if you use your own, but I doubt you will read the ÉTS one in a couple years, assuming they don't deactivate it.

Copy link
Collaborator

Choose a reason for hiding this comment

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

We used to always put the club's email since after graduation there's about 0.001% chances that you'll touch this code again ;)

Comment on lines 32 to 33
<!-- This package is referenced in the warehouse launch files, but does not build out of the box at the moment. Commented the dependency until this works. -->
<!-- <run_depend>warehouse_ros_mongo</run_depend> -->
Copy link
Contributor

Choose a reason for hiding this comment

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

Are these lines still relevant?

Copy link
Contributor

@GLDuval GLDuval left a comment

Choose a reason for hiding this comment

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

Same here, I'm not too sure what most of the code means, but I still have 2 small remarks for you to check out :) Good job overall!

ovis_bringup/launch/ovis_arm.launch Show resolved Hide resolved
# helper variables
r = rospy.Rate(1)
success = True
deg2rad = 180/3.14159265359
Copy link
Contributor

Choose a reason for hiding this comment

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

Use math.pi

@ahmed-moubtahij
Copy link

I keep receiving emails of replies here on [email protected], and I get an error page when I try to unsubscribe. Can someone remove me from the mailing list? Thanks

@saxtot saxtot removed the request for review from Caprurial March 22, 2023 15:32
@saxtot
Copy link
Contributor

saxtot commented Mar 22, 2023

I keep receiving emails of replies here on [email protected], and I get an error page when I try to unsubscribe. Can someone remove me from the mailing list? Thanks

Oh humm I have no idea why you would get those. I can't see anywhere where you have interacted with this or a reason why you would be subscribed. I removed a useless reviewer that I don't personally know, but that's the extent of where I can help. Maybe open a ticket with GitHub support?

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.

6 participants