-
Notifications
You must be signed in to change notification settings - Fork 511
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Consider making UNREGISTER_TIMEOUT configurable #320
Comments
Hi @jorgenfb, sounds good! Could you provide a PR? |
jorgenfb
added a commit
to jorgenfb/rosbridge_suite
that referenced
this issue
Feb 8, 2018
Pull request RobotWebTools#247 introduces a 10 second delay to mitigate issue RobotWebTools#138. This change makes this delay configurable by passing an argument either on the command line or when including a launch file. Usage example: ```xml <launch> <include file="$(find rosbridge_server)/launch/rosbridge_websocket.launch"> <arg name="unregister_timeout" value="5.0"/> </include> </launch> ``` Closes RobotWebTools#320
jihoonl
pushed a commit
that referenced
this issue
Feb 24, 2018
Pull request #247 introduces a 10 second delay to mitigate issue #138. This change makes this delay configurable by passing an argument either on the command line or when including a launch file. Usage example: ```xml <launch> <include file="$(find rosbridge_server)/launch/rosbridge_websocket.launch"> <arg name="unregister_timeout" value="5.0"/> </include> </launch> ``` Closes #320
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Pull request #247 introduces a 10 second delay to mitigate issue #138. I propose making this property configurable through parameters. I can provide a PR if we decide it is a good idea.
The reason I want to configure the timeout is that we still see the problem described in #138 at rare occasions and would experiment with other timeouts. I also see forks completely disabling the unregistering from a topic to prevent these problems. Even if you don't agree with their solution, making the timeout configurable would probably allow them to run the official version with a really high timeout value, removing the need to maintain a fork.
The text was updated successfully, but these errors were encountered: