search

ros / roslisp_common

Clone of the old roslisp_common SVN repo from code.ros.org.
8 stars 14 forks source link

Exec- and Preempt-Timeouts as client members #45

Open fairlight1337 opened 8 years ago

fairlight1337 commented 8 years ago

In order to choose individual timeouts when calling actionlib services, action-client could have additional fields for the respective values.

As discussed with @airballking, global timeout variables in actionlib-lisp are too intrusive when working with multiple semantically different components using actionlib (e.g. arm movement / base navigation). Those components might have different requirements when it comes to maximum execution time.

gaya- commented 8 years ago

Yes, but the duration is not only different between arm movements and base navigation, but also between different base navigation tasks, if I know that the distance is very big I might increase the timeout, but I wouldn't want to use the same big timeout for a very short distance. In my opinion, having the timeouts as arguments to the action call is perfectly fine.

The *action-server-timeout*, on the other hand, is a variable that defines how long to wait for the server, not how long to wait for the action to succeed. That is supposedly more or less the same on a given machine. I'm talking about the old implementation, BTW. If your issue only relates to the new implementation, I don't have any opinions there.

Another aspect to consider is, that we agreed to keep the APIs as similar as possible to the APIs from other languages, so you might want to check with the Python or C++ client first.

fairlight1337 commented 8 years ago

I agree on the point of different actions on the same module having different time requirements. On the other hand, how do you set a proper time limit here for a task? Seems that then there is no universal solution but only hard-coded values for individual use-cases.

I don't know about the C++ interface and have not been involved with the reimplementation of the actionlib client. Other than that, I'd just like an easy access to the timeout.

The main reason is this issue. Any ideas on a simple yet powerful way to set the proper timeout here? Hard-coding doesn't make the cut for different situations, but putting it in the designators feels wrong as well. One way would be to put an exported timeout variable into pr2_navigation_process_module and setting that before each call. But again, this seems very messy - and at which point do you set that value?

Any great ideas here, guys?

gaya- commented 8 years ago

Well, one idea would be to have some kind of heuristic that would estimate the required time. For example, taking the euclidean distance from the current robot pose to the goal pose and dividing that by some average speed. But, I guess, there is no general solution to this. You either use a heuristic, or learn average time that robot needs to travel from A to B, etc. So, at this point, if you don't want to put too much effort into this, hard-coded values might be fine, no? Also, another approach would be to have a smaller timeout and if the action doesn't succeed in that short time, retry again for multiple times using failure handling. It's also a matter of what you are trying to optimize, speed, robustness, code cleanness. I'm fine with any solution, actually, in the cases where the pretty solution would take too much effort, hard-coded values or heuristics are fine too.

fairlight1337 commented 8 years ago

Thanks a lot for your input.

Yeah, hard-coded values might do it for now. I'll think of an elegant way of doing that.

gaya- commented 8 years ago

Actually, putting the estimated duration into the designator could also be ok, because this is one of the parameters that you can learn from experience etc.

airballking commented 8 years ago

I interpret @fairlight1337's comments like this: For every specific action-client he has timeout values which are fine for, let's say 80% of the calls. For those he would like to have a convenience mechanism within actionlib-lisp to provide them as defaults.

So, basically this is a feature request. So, the main question is whether we can accommodate this request without breaking API and causing misleading behaviour for other users.

fairlight1337 commented 8 years ago

My original issue was closed (see https://github.com/cram2/cram_pr2/commit/4f4c1d969bd88bd08c1608d65699bea43334165d), but this is of course an individual solution for an isolated case. A general way to specify the timeout would be nice, because I can see myself implementing the same for CRAM MoveIt! already ;).