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

increase max number of joints in simple_message/joint_data.h #147

Closed
tstoyanov opened this issue Aug 30, 2016 · 16 comments
Closed

increase max number of joints in simple_message/joint_data.h #147

tstoyanov opened this issue Aug 30, 2016 · 16 comments

Comments

@tstoyanov
Copy link

Hi,

I am using the simple_message and industrial_robot_client packages for creating interfaces to the ABB YuMi robot. The packages work rather fine and I have MoveIt and basic ros_control support integrated, over the ros_industrial communication protocol (see https://github.com/rtkg/yumi).

The issue I had to work around though is that currently the maximum payload in a joint message is limited to 10 joints, while YuMi has two 7 DoF manipulators. There are two ways to solve this: either treat the manipulators separately and run two trajectory downloaders, or change the static define in simple_message/joint_data.h, e.g.:
static const industrial::shared_types::shared_int MAX_NUM_JOINTS = 20;
I know this would require a corresponding change on the robot controller side for most supported robots. In case of higher DoF robots however, this is the only option if we want to execute synchronized motion for all joints.

What is the general opinion, would you agree to such a change?

@shaun-edwards
Copy link
Member

@tstoyanov thanks for the question. As our support has grown for dual arm-ed manipulators this limitation has become clear. We created a ROS Enhancement Proposal (REP), here to address this, and we have a prototype implementation that hasn't been released. As you point out, the robot clients would also have to be updated.

As for your specific issue, there are two possible approaches:

  • Update the ABB driver to utilize the new, variable size, protocol outlined in the REP. I could make the prototype code available to you. This would take some amount of work on your part.
  • Fork industrial_core and abb and make the change to the MAX_NUM_JOINTS to meet your needs.

@gavanderhoorn
Copy link
Member

gavanderhoorn commented Aug 31, 2016

Personally, I not sure whether changing that constant in the current version of the industrial_core is a good idea. It's been around for a long time, and many, if not all, drivers assume it has that value. It also corresponds often to the maximum nr of joints per motion group for many robot controllers, which allows for some easy passing of arrays between controller functions and message (de)serialisers.

Secondly: increasing MAX_NUM_JOINTS would mean that the sizes (in bytes) for some messages would almost double. JOINT_FEEDBACK fi would go from it's current 144 to 264 bytes. JOINT_TRAJ_PT_FULL_EX would go from (ex for 2 motion groups) 548 bytes to 788 bytes. Some controllers don't have very sophisticated TCP stacks, and pushing those kind of msgs out at a sufficient rate can lead to quite some CPU usage for (de)serialisation alone, without doing anything interesting.

As @shaun-edwards mentions, there is a REP for an extension of Simple Message that changes the messages used from fixed-size to dynamically-sized, complete with fields for encoding the exact nr of joints in a motion group. While the robot controller may still have a fixed maximum nr, the protocol does not make any assumptions anymore and allows for 2^31-1 (signed 32 bit integer) groups, each with 2^31-1 joints in a single message. Controllers that need it can use as many as they want, while simpler setups would actually see reduced msg sizes and network traffic.

My preference would be for all robot drivers in ROS-Industrial to move to the new messages, instead of upping the value of MAX_NUM_JOINTS to 20 (which is a rather arbitrary limit as well).

@gavanderhoorn
Copy link
Member

ros-industrial/rep#13 is a PR that intends to update REP-I0001 with some changes that are the result of the implementation of the experimental clients.

@tstoyanov
Copy link
Author

@shaun-edwards @gavanderhoorn
Thanks for the tips. I already had solved my problem by forking and everything seems to run quite fine for me with 20 joint messages. The reason I raised the issue here was that this solution would not work for future release of our yumi drivers through the ros build farm.
Of course I agree that 20 is just as random a number as 10 and the best solution would be to have a dynamically allocated buffer size as per the REP. @shaun-edwards If there is code available already, I will gladly test it on my setup.

@shaun-edwards
Copy link
Member

@tstoyanov, the client (ROS) side code is available, but it would require development on the ABB side to handle the new, dynamically sized , simple messages. Would you be willing to do this development?

@gavanderhoorn
Copy link
Member

I have some experience writing the server side for the IRCv2, so I could assist.

@tstoyanov
Copy link
Author

I can give it a try, sure. I have some idea about rapid and I have access to an irc5 for testing. How do I get the ros side code?

@gavanderhoorn
Copy link
Member

@shaun-edwards can provide it to you.

If we do this, I'd very much appreciate it if we can first discuss your implementation approach a bit. Just to streamline development and to avoid unnecessary duplication of effort and frustration later on.

@tstoyanov
Copy link
Author

tstoyanov commented Sep 2, 2016

I'm certainly in favor of avoiding frustration :) Drop me a line at
[email protected] and we can arrange a call to discuss the details.

@shaun-edwards
Copy link
Member

@JeremyZoss, can you help me out with this while I'm on vacation. Let's talk in the office tomorrow (Tuesday).

@JeremyZoss
Copy link
Member

I just added my industrial_core revisions into the industrial_experimental repo. These revisions provide ROS-side support for REP-I0001, which include multi-group support. It has been tested to work previously on a multi-arm Fanuc workcell, but not much beyond that.

Please let me know if you have questions or difficulty with these new packages. It may be a bit tough to dig in, since documentation is slim, but usage is similar to the previous industrial_robot_client library, with the exception of a new controller_joint_map parameter used to map joints/topics/motion-groups.

@Karsten1987
Copy link

Is there any reason which prevents initializing the number of joints within the init function?
https://github.com/ros-industrial/industrial_core/blob/jade-devel/simple_message/include/simple_message/joint_data.h#L88

It could be defaulted to 10 to remain backwards compatible.

@gavanderhoorn
Copy link
Member

Technically I don't think there is a reason, but it wouldn't really address the issue of all the (implicit) assumptions in existing robot drivers and user code we don't know about.

Are you looking to increase the max nr of joints like @tstoyanov?

@shaun-edwards
Copy link
Member

@Karsten1987, are you looking to reuse the message or simply compile a ROS-Industrial client that supports more joints?

I originally made it "hard" to change the number of joints because it would cause issues with standard ROS-I serviers on the controllers (as @gavanderhoorn mentioned above). I assumed if anybody wanted to do anything special, they would fork the library as @tstoyanov did. As mentioned above, we'd like to move toward a the dynamically sized message set.

If you are interested in a more flexible robot client, that could be achieved by setting the number of joints by a ROS param (with 10 as the default). This would also require passing the number of joints through several classes.

@Karsten1987
Copy link

@tstoyanov Yes, I have pretty much exactly the same use case.
I was more wondering why the simple_message package has the number of joints hardcoded.

As for now, I can live with forking the simple_message and yumi package and apply my modifications locally. Nevertheless, I am looking forward to the dynamically sized message.

@gavanderhoorn
Copy link
Member

Seeing as abb_driver is deprecated in favour of abb_robot_driver (which natively supports YuMi and all other ABB robots), I don't see us changing anything in simple_message any more.

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

No branches or pull requests

5 participants