ros-industrial / motoman

ROS-Industrial Motoman support (http://wiki.ros.org/motoman)
143 stars 194 forks source link

Point streaming and multiple robot groups #586

Open rr-mark opened 4 months ago

rr-mark commented 4 months ago

Context:

I am taking over from @rr-dave, trying to set up our GP25 and 2-axis positioner with a YRC1000 controller as described in https://github.com/ros-industrial/motoman/issues/585.

I have fixed the various MoveIt connections which were causing most of the issues in https://github.com/ros-industrial/motoman/issues/585, such that we don't need https://github.com/ros-industrial/motoman/pull/259 or https://github.com/ros-industrial/motoman/pull/488.

We can now control the GP25 and positioner to move along independent trajectories. (We are only interested in moving one and then the other for now; we will hopefully have migrated to ROS2 by the time we need simultaneous movement).

The problem:

I am now working on getting point streaming working for systems with multiple robot groups. We only need point streaming for the robot, we do not need point streaming for the positioner at this stage.

For consistency, I have updated our other robots (a GP4 and a GP8, both with no Yaskawa external axes) to use a multi-group setup (with only one group on the controller).

The point streaming we are using is https://github.com/ros-industrial/motoman/pull/37 (plus some rebasing and other (hopefully unrelated) changes).

I tried adding point streaming to multi-group setups by adding

sub_joint_command = this->node_.subscribe(
            ns_str + "/" + name_str + "/joint_command", 0, &JointTrajectoryInterface::jointCommandCB, this);

this->sub_joint_commands_[robot_id] = sub_joint_command;

to JointTrajectoryInterface::init(..., robot_groups, ...), to mirror that in JointTrajectoryInterface::init(..., joint_names, ...), but point streaming fails immediately with the error

Aborting POINT STREAM operation.  Failed to send point (#47): Invalid message (3) : Invalid robot ID (3004)

My questions:

rr-mark commented 4 months ago

Looks like MotomanJointTrajectoryStreamer::create_message uses a setRobotID method, which is presumably getting the wrong id from somewhere. I'll see if I can find out where and how to fix it.

rr-mark commented 4 months ago

Okay, I think I understand what's happening. JointTrajectoryStreamer::jointCommandCB calls MotomanJointTrajectoryStreamer::create_message(..., JointTrajectoryPoint, ...), which uses setRobotId(robot_id_).

What I think I need is MotomanJointTrajectoryStreamer::create_message(..., DynamicJointsGroup, ...), which uses setRobotId(pt.group_number).

So I guess I need to write a JointTrajectoryStreamer::jointCommandCB(DynamicJointTrajectoryConstPtr).

rr-mark commented 4 months ago

That seems to work. My code is here https://github.com/rivelinrobotics/rivelin_motoman/pull/1/commits/d34b4ad90cfa99960143bd9a195c22679d8ab4a6 if anyone is interested in cherry-picking it into a PR.

gavanderhoorn commented 4 months ago

Thanks for reporting on your progress + solution.

I noticed https://github.com/rivelinrobotics/rivelin_motoman/pull/1 was closed, not merged.

Did you discover something that was not correct in the end, or was the PR just for running CI?

gavanderhoorn commented 4 months ago

I have fixed the various MoveIt connections which were causing most of the issues in #585, such that we don't need #259 or #488.

late perhaps, but I would really recommend you do in fact incorporate the changes from #488.

The multi-group implementation in HEAD is incomplete, and #488 does fix some of those issues.

rr-mark commented 4 months ago

Thanks for reporting on your progress + solution.

:+1:

I noticed rivelinrobotics#1 was closed, not merged. Did you discover something that was not correct in the end, or was the PR just for running CI?

This was just because the branching in our fork is a mess. We decided to make a new master branch, rather than updating our kinetic-devel branch. We are now using the contents of that PR.

If we need to keep using motoman1 for much longer we will need to tidy up our fork, but we're hoping to migrate to motoros2 before that is necessary.

gavanderhoorn commented 4 months ago

we're hoping to migrate to motoros2 before that is necessary.

in this context feedback/input on Yaskawa-Global/motoros2#186 would be highly welcome.

rr-mark commented 4 months ago

I have fixed the various MoveIt connections which were causing most of the issues in #585, such that we don't need #259 or #488.

late perhaps, but I would really recommend you do in fact incorporate the changes from #488.

The multi-group implementation in HEAD is incomplete, and #488 does fix some of those issues.

Our current solution appears to be working, so we're reluctant to make more changes unless something stops working. If we end up sticking with motoros1 for much longer and come across any problems we will look at #488 again.

rr-mark commented 4 months ago

we're hoping to migrate to motoros2 before that is necessary.

in this context feedback/input on Yaskawa-Global/motoros2#186 would be highly welcome.

Sure thing. We are about half way through our migration to ROS2, and we haven't got to the motoman migration yet. No doubt we will have lots of feedback/input when we get to that stage.

rr-mark commented 4 months ago

Actually, you're right, that discussion does look relevant to us at this stage. I'll discuss with the team, and hopefully someone will contribute soon.