ros_controllers icon indicating copy to clipboard operation
ros_controllers copied to clipboard

joint_state_ctrlr: should it populate pos/vel/eff if hw doesn't support it?

Open gavanderhoorn opened this issue 6 years ago • 4 comments

According to the sensor_msgs/JointState documentation, the position, velocity and effort lists should only be populated if the joint(s) support reporting that quantity:

The goal is to make each of the fields optional. When e.g. your joints have no effort associated with them, you can leave the effort array empty.

The current implementation of the JointStateController appears to allocate entries in the pos/vel/acc arrays regardless of whether the underlying hw actually is capable of reporting those (here). This would seem to be in "violation" of the semantics of the JointState message.

Is the rationale for the current behaviour known? Was/is there a particular reason to require hardware_interfaces to always register 3 state variables for each joint with the JointStateHandle?

gavanderhoorn avatar Apr 09 '19 15:04 gavanderhoorn

It might be possible to use nullptr as a sentinel value for each of the pos, vel and eff members in the JointStateHandle.

The JointStateController could then see whether handles have != nullptrs registered for each respective quantity, and do-the-right-thing when publishing the message.

A complicating factor is that it's perfectly legal to have heterogeneous joints in a setup, where some for instance support reporting pos and vel, while others only support reporting pos and eff. If there is still only a single JointStateController reporting on all those joints, there would seem to be no way to publish a JointState message, as (from the JointState msg documentation):

All arrays in this message should have the same size, or be empty.

Would it be an option to publish multiple JointState messages from the controller? In which subsets of joints are published that all report the same quantities (so all joints that report pos and eff in one message, and all joints that report pos and vel in another)?

gavanderhoorn avatar Apr 09 '19 15:04 gavanderhoorn

Is the rationale for the current behaviour known? Was/is there a particular reason to require hardware_interfaces to always register 3 state variables for each joint with the JointStateHandle?

The PR2 exposes all of them and this behavior was ported over. And I guess that only one JointStateController is running..

Would it be an option to publish multiple JointState messages from the controller?

For robot_state_publisher and MoveIt you need to have a combined joint state..

(From JointState.msh): The goal is to make each of the fields optional. When e.g. your joints have no effort associated with them, you can leave the effort array empty.

This does not cover hybrid systems.. I would just set the missing values to 0 or NaN.

mathias-luedtke avatar Apr 09 '19 17:04 mathias-luedtke

Is the rationale for the current behaviour known? Was/is there a particular reason to require hardware_interfaces to always register 3 state variables for each joint with the JointStateHandle?

The PR2 exposes all of them and this behavior was ported over.

Ok. So the rationale would be: it was "always" done this way.

And I guess that only one JointStateController is running..

Would it be an option to publish multiple JointState messages from the controller?

For robot_state_publisher and MoveIt you need to have a combined joint state..

Which is possible using joint_state_publisher to coalesce them (MoveIt already does this). Not sure what that does though in this situation.

(From JointState.msh): The goal is to make each of the fields optional. When e.g. your joints have no effort associated with them, you can leave the effort array empty.

This does not cover hybrid systems.. I would just set the missing values to 0 or NaN.

Both 0 and NaN have different semantics: a zero velocity means "no motion". Not sure what a NaN velocity would imply. NaN is essentially a sentinel value for floating point number, signifying an undefined value (or "unrepresentable"). I don't think it would work to use that would work here in this case, unless we want to redefine the semantics of NaN in the context of the JointState message.


Edit: it's actually not absolutely necessary for robot_state_publisher to have a combined joint state: it's legal to have multiple robot_state_publishers each responsible for a part of the tree. If each of those instances receives the JointStates they need, that will work. Consumers of TF broadcasts will automatically merge msgs from multiple sources in a coherent view of the tree.

It's slightly more complex, but can actually be very efficient and more robust.

gavanderhoorn avatar Apr 09 '19 18:04 gavanderhoorn

Indeed, as lame as it sounds, "it's been like that on PR2". If you have a proposal for changing it I think there's no reason why we shouldn't support it.

bmagyar avatar Apr 09 '19 20:04 bmagyar