Added cr7ial support #33

dniewinski · 2017-07-20T14:27:48Z

Added a cr7ia support library and a urdf and moveit configuration for a cr7ial. Tested on a real arm and in roboguide

gavanderhoorn · 2017-07-20T20:31:28Z

Thanks again for the PR.

Unfortunately, Travis is failing:

FAILURE:
[/root/catkin_ws/src/fanuc_experimental/fanuc_cr7ia_support/tests/roslaunch_test.xml]:
	while processing /root/catkin_ws/src/fanuc_experimental/fanuc_cr7ia_support/launch/load_cr7ia.launch:
Invalid roslaunch XML syntax: [Errno 2] No such file or directory: u'/root/catkin_ws/src/fanuc_experimental/fanuc_cr7ia_support/launch/load_cr7ia.launch'

It should probably be load_cr7ial.launch?

gavanderhoorn

I've pushed some fixup commits already for the minor things. There are some others I've commented on here, could you take a look?

Thanks again for taking the time to contribute this.

gavanderhoorn · 2017-07-21T08:59:16Z

fanuc_cr7ia_support/package.xml

+    </p>
+    <p><b>Specifications</b>:</p>
+    <ul>
+      <li>CR-7iA/L - Normal Range</li>


Just making sure: have you verified this? What did you choose in the initial robot setup?

Verifying this now. J3 limits on Fanuc change depending on J2, and since it's linked-arm, the J3 value is also tied to J2. Is there any way to have the limits of J3 updated based on the current value of J2? Unless the limits are set small enough that they are always within the actual limits, there will always be cases where you can plan outside of actual limits

re: limits based on J2-J3: true, but normal range only refers to the range of motion of J1 in this case.

Limits have been update as of manual. I don't see any reference to max joint speeds in the manual though

No, Fanuc doesn't do that for their CR range. Where'd you get the joint speed limits that you have in there now?

Could you get me a copy of all sy*.va files on the controller? Either simulated or real, doesn't matter. A zip file to my email would be very much appreciated.

FTP is probably easiest.

I will get a backup to you shortly

gavanderhoorn · 2017-07-21T09:01:05Z

fanuc_cr7ia_support/package.xml

+    </ul>
+    <p>
+      Joint limits and maximum joint velocities are based on the information in
+      the FANUC Robot CR-7iA Mechanical Unit Operator's Manual.


Could you please be exact about which version of the manual you used? Fanuc documents are versioned, so I'd like this to be as specific as possible. It should be something like B-NNNNNLL/VV (with NNNNN the doc id, LL the language and VV the revision).

gavanderhoorn · 2017-07-21T09:02:13Z

fanuc_cr7ia_support/package.xml

+  </description>
+  <author>Dave Niewinski</author>
+  <maintainer email="dniewinski@clearpathrobotics.com">dniewinski@clearpathrobotics.com</maintainer>
+  <maintainer email="g.a.vanderhoorn@tudelft.nl">G.A. vd. Hoorn (TU Delft Robotics Institute)</maintainer>


I've added myself as maintainer, as we do that for all ROS-I packages.

Are you actually planning to maintain this? I'd very much welcome that, but it is a committment.

I can maintain, but you would likely be better-suited. I'll just contribute

gavanderhoorn · 2017-07-21T09:03:57Z

fanuc_cr7ial_moveit_config/.setup_assistant

+  URDF:
+    package: fanuc_cr7ia_support
+    relative_path: urdf/cr7ial.xacro
+    use_jade_xacro: true


I've added this to make MoveIt enable Jade+ xacro on Indigo.

gavanderhoorn · 2017-07-21T09:05:10Z

fanuc_cr7ial_moveit_config/config/fanuc_cr7ial.srdf

+        <joint name="joint_4" value="0" />
+        <joint name="joint_5" value="-1.4311" />
+        <joint name="joint_6" value="0" />
+    </group_state>


This group_state seems rather specific to your setup on the Ridgeback, could you remove it from this config?

I believe that is standard shipping position on this arm. Took it from the default ship program

I'll remove this

gavanderhoorn · 2017-07-21T09:05:33Z

fanuc_cr7ial_moveit_config/config/fanuc_cr7ial.srdf

+        <chain base_link="base_link" tip_link="tool0" />
+    </group>
+    <!--GROUP STATES: Purpose: Define a named state for a particular group, in terms of joint values. This is useful to define states like 'folded arms'-->
+    <group_state name="all-zeroes" group="manipulator">


I'm not a native speaker, but I believe the plural of zero is zeros. zeroes would be the verb.

Yup. English is a wonderful language

gavanderhoorn · 2017-07-21T09:06:17Z

fanuc_cr7ial_moveit_config/config/fanuc_cr7ial.srdf

+        <joint name="joint_6" value="0" />
+    </group_state>
+    <!--VIRTUAL JOINT: Purpose: this element defines a virtual joint between a robot link and an external frame of reference (considered fixed with respect to the robot)-->
+    <virtual_joint name="MountPoint" type="fixed" parent_frame="arm_mount_point" child_link="base_link" />


Could I ask you to change the name of the virtual_joint to FixedBase? That is a ROS-I convention.

Additionally, the parent_frame would be world.

If this is the convention, not a problem. I'm not a fan of the mounting point being called "world" because a robot's world isn't necessarily there (ie. fanuc), but that is just a personal opinion

It's really just a convention, and I don't expect people to re-use MoveIt configs as-is anyway, apart from some initial testing maybe (who has a single robot in complete isolation to plan for).

The "world" frame that you are referring to is what Fanuc calls the World Frame, which is modelled with the base frame in the urdf.

gavanderhoorn · 2017-07-21T09:06:51Z

fanuc_cr7ial_moveit_config/config/kinematics.yaml

+  kinematics_solver: kdl_kinematics_plugin/KDLKinematicsPlugin
+  kinematics_solver_search_resolution: 0.005
+  kinematics_solver_timeout: 0.005
+  kinematics_solver_attempts: 3


I would actually recommend you use the trac_ik plugin for this setup.

Just curious, but what are the differences between the two? I've had decent success with KDL so far

Please see traclabs/trac_ik.

In my very minimal testing, KDL seems less likely to switch arm configurations than trac, or when it does, they are more minor swings

You are probably using the Speed solve_type. You'll want to configure Distance. See the trac_ik docs.

gavanderhoorn · 2017-07-21T09:08:03Z

fanuc_cr7ial_moveit_config/config/ompl_planning.yaml

+    - PRMkConfigDefault
+    - PRMstarkConfigDefault
+  projection_evaluator: joints(joint_1,joint_2)
+  longest_valid_segment_fraction: 0.05


Could you please add a default_planner_config entry here? See fanuc_lrmate200ib_moveit_config for an example.

Not a problem

gavanderhoorn · 2017-07-21T09:10:13Z

fanuc_cr7ial_moveit_config/package.xml

+  </description>
+  <author email="dniewinski@clearpathrobotics.com">Dave Niewinski</author>
+  <maintainer email="dniewinski@clearpathrobotics.com">Dave Niewinski</maintainer>
+  <maintainer email="g.a.vanderhoorn@tudelft.nl">G.A. vd. Hoorn (TU Delft Robotics Institute)</maintainer>


See earlier comment about maintainer in fanuc_cr7ia_support.

… a cr7ial. Tested on a real arm and in roboguide. Actually made the tests worthwhile

gavanderhoorn · 2017-07-21T12:39:50Z

@dniewinski: I think you just reverted all my changes. Was that intentional?

dniewinski · 2017-07-21T12:43:41Z

@gavanderhoorn That was not intentional. I had the tests fixed up last night and forgot to push. Pushed right when I got in without checking first. Sorry about that. Feel free to blow mine away, they were very minor

gavanderhoorn · 2017-07-21T12:46:04Z

I'll see if I can rebase mine.

Would be nice if you could look at my remaining comments in the mean time.

dniewinski · 2017-07-21T12:59:06Z

Welcome back changes :) Sorry again about that

gavanderhoorn · 2017-07-21T13:13:09Z

fanuc_cr7ia_support/urdf/cr7ial_macro.xacro

@@ -1,5 +1,6 @@
 <?xml version="1.0" ?>
 <robot xmlns:xacro="http://ros.org/wiki/xacro">
+  <xacro:property name="pi" value="3.1415926535897931" />


Not sure this should've been part of this commit?

Is it still necessary on your machine? Jade+ xacro should already have this defined.

I'm using Indigo, so I need it there for testing

If you start xacro with the --inorder command line option (which I added: 4618626, and the load_cr7ial.launch already had) then it uses Jade+ xacro. Does that not work for you?

dniewinski · 2017-07-21T14:36:19Z

I think I got all of the requested changes

dniewinski · 2017-07-25T13:36:17Z

Just following up. Is there anything left to be done with this before it is merged?

gavanderhoorn · 2017-07-25T13:37:29Z

I just want to test it under Indigo before I merge.

gavanderhoorn · 2017-07-25T13:38:36Z

One more thing btw: the joint position limits for J2 and J3 that you set in b80aab5 seem to be different from the ones in the system variables. Where did you find those values?

dniewinski · 2017-07-25T13:47:36Z

The joint limits in the controller won't match the URDF because their kinematics are different. When you move J2, the J3 link should also move, but it doesn't. This means that the value of that joint won't match, same with the limits. Since the URDF is not setup as a linked-arm, it makes the J3 limits easier to do, but they won't match what the controller "thinks" its limits are. As far as J2, the limits I have set are from the manual, not from my machine, just in case my robot is different

gavanderhoorn · 2017-07-28T15:26:18Z

I'm not too happy with the limits of joint_3, as in all other support pkgs we have specified the actual limits. I understand the linked kinematics problem, but I feel the joint limits in a xacro are not the place to work around that. For now we'll leave it like this though.

Testing was successful, so I'll wait on Travis to be happy and then I'll merge.

Thanks for the contribution 👍

gavanderhoorn · 2017-07-28T15:27:16Z

We'll probably have to fix-up the collision meshes for link_2 and possibly link_4, similar to what was reported for the basic 200iD (#34), but I'll do that later.

gavanderhoorn · 2017-07-28T15:39:58Z

Everything passed. Merging.

…tal#33)

gavanderhoorn requested changes Jul 21, 2017

View reviewed changes

Added a cr7ia support library and a urdf and moveit configuration for…

1b6a519

… a cr7ial. Tested on a real arm and in roboguide. Actually made the tests worthwhile

gavanderhoorn added 9 commits July 21, 2017 14:55

cr7ia: make 'use_gui' a private param.

2490e04

cr7ia: be specific about the exact variant supported.

3e04145

cr7ia: add at least one ROS-I maintainer.

66397a3

cr7ia: add variant suffix to roslaunch test file.

7441351

cr7ia: when using Jade+ xacro, pi is already defined.

4618626

cr7ial_moveit: add readme.

9c4fadb

cr7ia: roslaunch test the correct file.

dc97aaf

cr7ial_moveit: fixup manifest.

99ee7d7

cr7ia: fixup variant name in launch files.

3d0ef53

dniewinski added 2 commits July 21, 2017 09:00

updated default_planner_config to RRTConnectkConfigDefault

5ecc3af

Removed myself as a maintainer

73efdec

gavanderhoorn reviewed Jul 21, 2017

View reviewed changes

dniewinski added 3 commits July 21, 2017 09:18

Fixed a typo in zeros and removed shipping state

be8988a

Updated joint limits and the manual used to get them

b80aab5

Updated the kinematics to use trac_ik

505ac1e

gavanderhoorn added 3 commits July 23, 2017 21:50

cr7ia: set /L limits based on sysvars.

1e43651

cr7ia_moveit: update MoveIt limits as well.

898dcff

cr7ia_moveit: don't depend on metapackage for kin plugin.

d50561f

dniewinski and others added 3 commits July 26, 2017 16:20

Fixed world virtual joint

ccc54fd

cr7ial_moveit: saner RViz cfg.

60f88ba

cr7ia: this is not an LR Mate.

90a0208

gavanderhoorn approved these changes Jul 28, 2017

View reviewed changes

gavanderhoorn merged commit af1f83f into ros-industrial:indigo-devel Jul 28, 2017

gavanderhoorn pushed a commit to gavanderhoorn/fanuc that referenced this pull request Feb 28, 2019

Add experimental support for CR-7iA/L (ros-industrial/fanuc_experimen…

d5d42c9

…tal#33)

Added cr7ial support #33

Added cr7ial support #33

Conversation

dniewinski commented Jul 20, 2017

gavanderhoorn commented Jul 20, 2017

gavanderhoorn left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gavanderhoorn Jul 21, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gavanderhoorn Jul 21, 2017 • edited Loading

Choose a reason for hiding this comment

gavanderhoorn Jul 21, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gavanderhoorn commented Jul 21, 2017

dniewinski commented Jul 21, 2017 • edited Loading

gavanderhoorn commented Jul 21, 2017

dniewinski commented Jul 21, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gavanderhoorn Jul 21, 2017 • edited Loading

Choose a reason for hiding this comment

dniewinski commented Jul 21, 2017

dniewinski commented Jul 25, 2017

gavanderhoorn commented Jul 25, 2017

gavanderhoorn commented Jul 25, 2017

dniewinski commented Jul 25, 2017

gavanderhoorn commented Jul 28, 2017

gavanderhoorn commented Jul 28, 2017

gavanderhoorn commented Jul 28, 2017

gavanderhoorn Jul 21, 2017 •

edited

Loading

gavanderhoorn Jul 21, 2017 •

edited

Loading

gavanderhoorn Jul 21, 2017 •

edited

Loading

dniewinski commented Jul 21, 2017 •

edited

Loading

gavanderhoorn Jul 21, 2017 •

edited

Loading