Delay unregister to mitigate !138 #247
Conversation
|
Looks like some tests are failing because they expect deregistration to be instant - can you take a look at those and fix them, please? |
|
@T045T I have modified the tests. Due to the nature of the changes I have added to add sleeps matching the duration of the unregister timeout (currently 10seconds) so the tests need 2-3 minutes to run instead of 1. If that is a problem, the duration of the timeout can be reduced on the unit tests, but I'd rather test the code without any modifications. |
|
Thanks! |
|
Sure if we're not going to treat it as a constant either it's fine. |
Tests will sleep for 10% extra of the timeout to prevent some situations were the test sleep ended right before the unregister timer fired
|
Class-level constants are somewhat unusual, so I'd prefer to either make it a class member (-> lowercase name) or a module constant ( -> uppercase name). In the latter case, it's still possible to access (and modify) it from test code (looks like that was the reason you changed it into a member in the first place). Since we don't really expect users to mess with it, a module constant is probably the way to go. |
|
Great work, thank you! |
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
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
No description provided.