ros-industrial / industrial_core

ROS-Industrial core communication packages (http://wiki.ros.org/industrial_core)
154 stars 181 forks source link

Fix timing issues #216

Closed pbeeson closed 4 years ago

pbeeson commented 5 years ago

In our use of a Motoman in production, we were often seeing the case where the ROS-I driver would report that the goal objective was met, our higher-level program would then compute and send a new trajectory, but the client would not yet be in the IDLE state, so it would complain about "Splicing not yet allowed".

The simple solution is to simply call trajectoryStop(), then just try the new trajectory. If its empty, nothing happens. If its not empty, it will fail on a Motoman unless the starting point is within 1e-5 radians on all joints to the current robot joints, so it is very safe to just try to run the next trajectory. This minor change solved many many issues where different threads with different values of joint states can get of out sync for a short time but have big consequences for a robot that needs to keep going.

I realize that sending this through may not work for any robot in general, so I'm willing to work on this PR to come up with some way to maybe allow new trajecotires to go through by setting parameter versus maybe not (default).

gavanderhoorn commented 5 years ago

The simple solution is to simply call trajectoryStop(), then just try the new trajectory. If its empty, nothing happens. If its not empty, it will fail on a Motoman unless the starting point is within 1e-5 radians on all joints to the current robot joints, so it is very safe to just try to run the next trajectory.

The general idea of the PR seems ok, but as you note yourself:

I realize that sending this through may not work for any robot in general, so I'm willing to work on this PR to come up with some way to maybe allow new trajecotires to go through by setting parameter versus maybe not (default).

There is no requirement for drivers to implement the start-state check and not many have it, so I wouldn't be comfortable with the removal of that return there for the general case.

pbeeson commented 5 years ago

I agree. I'd like to have a parameter that allows this and maybe defaults to the existing behavior but allows this behavior if the user wants. But, I wanted to get this posted so it can be discussed about the best way to do that is people don't like parameters for that.

pbeeson commented 4 years ago

The meat of this PR was already pulled in elsewhere at some point, so this PR can close.