search

jsk-ros-pkg / jsk_roseus

ROS EusLisp Client
http://wiki.ros.org/roseus/Tutorials
17 stars 56 forks source link

add call-trigger-service and call-set-bool-service, copied from #705

Closed k-okada closed 2 years ago

k-okada commented 2 years ago
Affonso-Gui commented 2 years ago

A few questions:

k-okada commented 2 years ago

Any insights on why we are defaulting persistent to true here and to nil in the standard service call?

if you call service call multiple times with high rate, we need to set persistent t. See test code https://gist.github.com/k-okada/d930a741d1f0eb0fddb4e0c615c24b44.

In robotics application, most of code runs in 10-500 Hz, and they use service call within the loop (ex. https://github.com/jsk-ros-pkg/jsk_tendon_robot/issues/979 ), Although I basically in favor of using topic for this purpose, I thought setting persistent t will be useful.

But if I read http://wiki.ros.org/roscpp/Overview/Services#Persistent_Connections carefully, they said we need to add reconnect logic. So maybe we'll run into trouble in ether way. Now, I think persistent nil might be ok.

Why we have ros-debug on call-trigger and ros-info on call-set-bool?

fixed

There is no way to differentiate a call failure (service not found) from a failed call (service responds with :success nil) in call-trigger and call-set-bool. Maybe returning the whole return object could be more useful?

OK, fixed to return whole objects, if it is ok, please fix you code > @sktometometo https://github.com/jsk-ros-pkg/jsk_robot/blob/9282d6b2903223d3880507be34aa646e1e048506/jsk_spot_robot/spoteus/spot-interface.l

Affonso-Gui commented 2 years ago

But if I read http://wiki.ros.org/roscpp/Overview/Services#Persistent_Connections carefully, they said we need to add reconnect logic. So maybe we'll run into trouble in ether way. Now, I think persistent nil might be ok.

I also think that defaulting the persistent flag to false is ok. Persistent clients are useful, but should be handled more carefully, so I think that they are more suited as a more advanced alternative option.