Skip to content

Commit bc114a0

Browse files
authored
Fix rtde client shutdown (UniversalRobots#474)
Destroying an RTDE client with a lost (but not shutdown) connection can hang. This can happen for example due to a server crash or a lost network / plugged cable. This commit adds a test case for that where the fake RTDE server simply stops sending data. Pausing an active RTDE communication before disconnecting shouldn't be strictly necessary and as far as I remember was added as a workaround to mitigate callback issues. These should not be present since we refactored the RTDEClient to no longer use the pipeline (which it never fully used, anyway). Thus, this structure should allow a cleaner implementation with a clearer logic flow.
1 parent 47c18b0 commit bc114a0

2 files changed

Lines changed: 28 additions & 10 deletions

File tree

src/rtde/rtde_client.cpp

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -91,8 +91,6 @@ RTDEClient::~RTDEClient()
9191
{
9292
reconnecting_thread_.join();
9393
}
94-
prod_->stopProducer();
95-
stopBackgroundRead();
9694
disconnect();
9795
}
9896

@@ -510,14 +508,6 @@ bool RTDEClient::setupInputs()
510508

511509
void RTDEClient::disconnect()
512510
{
513-
// If communication is started it should be paused before disconnecting
514-
if (client_state_ == ClientState::RUNNING)
515-
{
516-
pause();
517-
}
518-
if (client_state_ >= ClientState::INITIALIZING)
519-
{
520-
}
521511
if (client_state_ > ClientState::UNINITIALIZED)
522512
{
523513
stream_.disconnect();

tests/test_rtde_client.cpp

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@
4343

4444
#include "fake_rtde_server.h"
4545
#include "ur_client_library/helpers.h"
46+
#include "ur_client_library/log.h"
4647

4748
using namespace urcl;
4849

@@ -419,6 +420,33 @@ TEST_F(RTDEClientTest, get_data_package_fake_server)
419420
client_.reset();
420421
}
421422

423+
TEST_F(RTDEClientTest, destroy_client_after_server_stops_sending)
424+
{
425+
auto fake_rtde_server = std::make_unique<RTDEServer>(g_FAKE_RTDE_PORT);
426+
fake_rtde_server->setStartTime(std::chrono::steady_clock::now() - std::chrono::seconds(42));
427+
client_.reset(new rtde_interface::RTDEClient("localhost", notifier_, resources_output_recipe_,
428+
resources_input_recipe_, 100, false, g_FAKE_RTDE_PORT));
429+
client_->init();
430+
client_->start();
431+
432+
URCL_LOG_INFO("Receiving data package from fake server to verify that connection is working.");
433+
434+
const std::chrono::milliseconds read_timeout{ 100 };
435+
auto data_pkg = rtde_interface::DataPackage(client_->getOutputRecipe());
436+
ASSERT_TRUE(client_->getDataPackage(data_pkg, read_timeout));
437+
438+
double timestamp = 0.0;
439+
EXPECT_TRUE(data_pkg.getData("timestamp", timestamp));
440+
EXPECT_GT(timestamp, 0.0);
441+
442+
URCL_LOG_INFO("Stopping fake server from sending data packages.");
443+
fake_rtde_server->stopSendingDataPackages();
444+
std::this_thread::sleep_for(std::chrono::milliseconds(50));
445+
446+
URCL_LOG_INFO("Destroying client while no more data packages are received from the server.");
447+
client_.reset();
448+
}
449+
422450
TEST_F(RTDEClientTest, reconnect_fake_server_background_read)
423451
{
424452
auto fake_rtde_server = std::make_unique<RTDEServer>(g_FAKE_RTDE_PORT);

0 commit comments

Comments
 (0)