Skip to content
This repository was archived by the owner on May 31, 2025. It is now read-only.

Sleep in rospy wait_for_service even if exceptions raised#1025

Merged
dirk-thomas merged 1 commit intoros:lunar-develfrom
AlexReimann:patch-3
May 22, 2017
Merged

Sleep in rospy wait_for_service even if exceptions raised#1025
dirk-thomas merged 1 commit intoros:lunar-develfrom
AlexReimann:patch-3

Conversation

@AlexReimann
Copy link
Copy Markdown

@AlexReimann AlexReimann commented Apr 6, 2017

If a service is not actually up, wait_for_service will throw an exception at the contact_service call and skip the sleeping.

This leads to the loop being executed without sleep, burning a lot of CPU.

This makes sure we always sleep. Sleep is good for your health.

if contact_service(resolved_name, timeout_t-time.time()):
return
time.sleep(0.3)
return
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please remove the added trailing whitespaces.

Same below.

if first:
first = False
rospy.core.logerr("wait_for_service(%s): failed to contact [%s], will keep trying"%(resolved_name, uri))
time.sleep(0.3)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With the sleep being moved here the call is not retried immediately but during the 0.3s sleep the call is not being retried yet. That seems to be not the idea of the code. Which exception is being reused in your case and what timeout are you using?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The rosgraph.MasterException from the nested contact_service function.
The problem is the immediately re-trying of the call. This spams the ros master with service lookups without any sleep.

For me the original logic seems:

  • If can contact the service, return immediately. Else sleep.
  • Ohh, we should add some exception checking somewhere here

@dirk-thomas
Copy link
Copy Markdown
Member

Thanks for the patch.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants