robot_client: joint_trajectory_streamer doesn't check reply msg from ctrlr in streamingThread

[gavanderhoorn]

The state machine in JointTrajectoryStreamer::streamingThread() uses SmplMsgConnection::sendAndReceiveMsg(req, reply, ..) to send JointTrajPtMessage instances to the server program running on the controller, but the returned reply msg is never checked for success of the operation (here). As trajectory streaming is done using (synchronous) SERVICE_REQUEST type msgs, it would seem logical to also check the reply from the controller.

The current implementation makes it impossible for servers to reject or nack an incoming traj pt (or in general: a service request).

[gavanderhoorn]

Do we actually care about this? Should the act of transferring the traj pt be considered the 'service request', and not so much the execution of the associated motion? If that is the case, the fields in the RobotStatus msgs would indirectly give feedback on the success of the motion (no errors on the robot_status topic == success), and there is no need to be able to nack points explicitly.

@shaun-edwards: could you elaborate on the design of the protocol wrt this?

[shaun-edwards]

The success should by tied to the queuing of the motion, since this is often times the only information the controller provides. Most controllers queue up motion separate from program execution. As a result, sending a response, when the motion command is executed/achieved is no easily done. Should anything fail in the execution of a trajectory, the RobotStatus should indicate this.

[gavanderhoorn]

So it wouldn't make much sense to check at service invocation if a traj pt is actually reachable by the manipulator, servers should just queue it and fail whenever the motion is actually attempted?

Depending on buffer size that could mean traj execution fails after quite some time.

Would it perhaps make sense to rename the msgs then? Instead of JOINT_TRAJ_PT something like QUEUE_X (where X is a TRAJ_PT, DYN_JNT_PT etc) would seem to be much clearer as it conveys what is actually the idea behind the service request.

[gavanderhoorn]

This is slightly off-topic for this issue actually, as even if only the queueing itself should be considered the service req, the current client nodes still do not check the response.