add timeout to service_discovery.spec_index_from_service

[bit-pirate]

I'm using this in the app manager and are currently forced to add an extra timeout (wait_for_service).

Would be nicer (and cleaner?), if this would be handled by service_discovery.

[wjwwood]

Upgraded to a pull request.

[coveralls]

Changes Unknown when pulling ae1645e3d11f6d663dff1e86770811d7cbdf445f on issue_39 into \ on master**.

[bit-pirate]

William, I like the simple implementation. However, there are now two ways of error handling: parsing the returned errors and catching the timeout exception.

Could that be merged into one?

[wjwwood]

So, you would like timeouts to raise rospy.ServiceException or would you like me to make a new error which will I will raise on rospy.ServiceException or a timeout exception?

[bit-pirate]

spec_index_from_service() returns the spec index and a list of errors (it's actually the _spec_loader returning this).

So far all possible errors have been caught inside spec_index_from_service, but now with the timeout in place I have to add my own try: ... catch: ... to handle the timeout exception.

Could the timeout exception be merged with the other errors and handled by the spec_loader (or spec_index_from_service)?

[wjwwood]

I see what you mean now.

I could do that, but if a timeout or ServiceException occurs then I won't have anything to return for the spec_index, so I would have to return None or something like that. The list of errors that come with the spec_index are sort of soft errors, which do not prevent me from creating the index, if a timeout occurs or if a service call fails then I have no information to act on. In the end I am willing to do anything you like, but I feel like these sets of errors are fundamentally different.