From 96b409aeaf3931ae10137f00a693dc6c29135b0f Mon Sep 17 00:00:00 2001 From: Shin <128954611+shin4141@users.noreply.github.com> Date: Fri, 2 Oct 2026 23:29:32 +0900 Subject: [PATCH 1/3] Make G5 dashboard welcome receive interruptible --- src/ur/dashboard_client_implementation_g5.cpp | 40 ++++++++++++++++++- tests/CMakeLists.txt | 2 + tests/test_dashboard_client.cpp | 40 +++++++++++++++++++ 3 files changed, 81 insertions(+), 1 deletion(-) diff --git a/src/ur/dashboard_client_implementation_g5.cpp b/src/ur/dashboard_client_implementation_g5.cpp index 2c2150d32..6cb0857b2 100644 --- a/src/ur/dashboard_client_implementation_g5.cpp +++ b/src/ur/dashboard_client_implementation_g5.cpp @@ -26,6 +26,7 @@ // ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE // POSSIBILITY OF SUCH DAMAGE. +#include #include #include #include @@ -222,16 +223,53 @@ bool DashboardClientImplG5::connect(const size_t max_num_tries, const std::chron { if (TCPSocket::connect(host_, port_, max_num_tries, reconnection_time)) { - URCL_LOG_INFO("%s", read().c_str()); + // The welcome receive must remain interruptible by disconnect(). A ten-second + // blocking recv can outlive a shutdown request even after its socket is closed. + timeval poll_tv; + poll_tv.tv_sec = 0; + poll_tv.tv_usec = 100000; + TCPSocket::setReceiveTimeout(poll_tv); + std::string welcome; + auto deadline = std::chrono::steady_clock::now() + 10s; + while (!isStopRequested()) + { + char character; + size_t read_chars = 0; + if (TCPSocket::read(reinterpret_cast(&character), 1, read_chars)) + { + welcome.push_back(character); + if (character == '\n') + { + break; + } + deadline = std::chrono::steady_clock::now() + 10s; + } + else if (getState() != comm::SocketState::Connected || std::chrono::steady_clock::now() >= deadline) + { + throw TimeoutException("Did not receive answer from dashboard server in time.", tv); + } + } + if (isStopRequested()) + { + TCPSocket::setReceiveTimeout(configured_tv); + return false; + } + URCL_LOG_INFO("%s", welcome.c_str()); ret_val = true; } else { + TCPSocket::setReceiveTimeout(configured_tv); return false; } } catch (const TimeoutException&) { + TCPSocket::setReceiveTimeout(configured_tv); + if (isStopRequested()) + { + return false; + } URCL_LOG_WARN("Did not receive dashboard bootup message although connection was established. This should not " "happen, please contact the package maintainers. Retrying anyway..."); } diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index beda5d0eb..cc7d1f874 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -175,6 +175,8 @@ add_executable(dashboard_client_mock_tests test_dashboard_client.cpp) target_link_libraries(dashboard_client_mock_tests PRIVATE ur_client_library::urcl GTest::gmock_main) gtest_add_tests(TARGET dashboard_client_mock_tests ) +set_tests_properties(DashboardClientWelcomeTest.disconnect_interrupts_silent_welcome + PROPERTIES TIMEOUT 20 RUN_SERIAL TRUE) add_executable(primary_parser_tests test_primary_parser.cpp) target_link_libraries(primary_parser_tests PRIVATE ur_client_library::urcl GTest::gtest_main) diff --git a/tests/test_dashboard_client.cpp b/tests/test_dashboard_client.cpp index 9b8608224..67b46465c 100644 --- a/tests/test_dashboard_client.cpp +++ b/tests/test_dashboard_client.cpp @@ -36,7 +36,11 @@ #include #include +#include +#include #include +#include +#include "test_utils.h" using namespace urcl; @@ -402,6 +406,42 @@ TEST_F(DashboardClientTest, X_program_api) EXPECT_EQ(prog.programState, "FINAL"); } +// A dashboard peer may accept TCP but never send the welcome line. A deliberate +// disconnect must end that receive without waiting for its ten-second timeout. +TEST(DashboardClientWelcomeTest, disconnect_interrupts_silent_welcome) +{ + TestableTcpServer server(29999); + server.start(); + DashboardClientImplG5 client("127.0.0.1"); + bool connected = false; + std::exception_ptr connection_error; + std::thread connection([&]() { + try + { + connected = client.connect(1); + } + catch (...) + { + connection_error = std::current_exception(); + } + }); + + const bool accepted = server.waitForConnectionCallback(5000); + std::this_thread::sleep_for(std::chrono::milliseconds(100)); + const auto started = std::chrono::steady_clock::now(); + client.disconnect(); + connection.join(); + const auto elapsed = std::chrono::steady_clock::now() - started; + + ASSERT_TRUE(accepted) << "The loopback dashboard server did not accept the connection"; + if (connection_error) + { + std::rethrow_exception(connection_error); + } + EXPECT_FALSE(connected); + EXPECT_LT(elapsed, std::chrono::seconds(3)); +} + int main(int argc, char* argv[]) { ::testing::InitGoogleTest(&argc, argv); From 8cfc787158e2ab008613073b480e56549d0ca72c Mon Sep 17 00:00:00 2001 From: Shin <128954611+shin4141@users.noreply.github.com> Date: Mon, 5 Oct 2026 18:42:49 +0900 Subject: [PATCH 2/3] Handle G5 dashboard welcome timeout as failed connection --- src/ur/dashboard_client_implementation_g5.cpp | 85 +++++++++---------- tests/CMakeLists.txt | 2 + tests/test_dashboard_client.cpp | 69 +++++++++++++++ 3 files changed, 109 insertions(+), 47 deletions(-) diff --git a/src/ur/dashboard_client_implementation_g5.cpp b/src/ur/dashboard_client_implementation_g5.cpp index 6cb0857b2..b39c7250f 100644 --- a/src/ur/dashboard_client_implementation_g5.cpp +++ b/src/ur/dashboard_client_implementation_g5.cpp @@ -208,79 +208,70 @@ bool DashboardClientImplG5::connect(const size_t max_num_tries, const std::chron URCL_LOG_ERROR("%s", "Socket is already connected. Refusing to reconnect."); return false; } - bool ret_val = false; - timeval configured_tv = getConfiguredReceiveTimeout(); - - while (!ret_val) + try { - timeval tv; - // The first read after connection can take more time. - tv.tv_sec = 10; - tv.tv_usec = 0; - TCPSocket::setReceiveTimeout(tv); - try + if (TCPSocket::connect(host_, port_, max_num_tries, reconnection_time)) { - if (TCPSocket::connect(host_, port_, max_num_tries, reconnection_time)) + // The welcome receive must remain interruptible by disconnect(). A ten-second + // blocking recv can outlive a shutdown request even after its socket is closed. + timeval poll_tv; + poll_tv.tv_sec = 0; + poll_tv.tv_usec = 100000; + TCPSocket::setReceiveTimeout(poll_tv); + const timeval welcome_tv = { 10, 0 }; + std::string welcome; + auto deadline = std::chrono::steady_clock::now() + 10s; + while (!isStopRequested()) { - // The welcome receive must remain interruptible by disconnect(). A ten-second - // blocking recv can outlive a shutdown request even after its socket is closed. - timeval poll_tv; - poll_tv.tv_sec = 0; - poll_tv.tv_usec = 100000; - TCPSocket::setReceiveTimeout(poll_tv); - std::string welcome; - auto deadline = std::chrono::steady_clock::now() + 10s; - while (!isStopRequested()) + char character; + size_t read_chars = 0; + if (TCPSocket::read(reinterpret_cast(&character), 1, read_chars)) { - char character; - size_t read_chars = 0; - if (TCPSocket::read(reinterpret_cast(&character), 1, read_chars)) - { - welcome.push_back(character); - if (character == '\n') - { - break; - } - deadline = std::chrono::steady_clock::now() + 10s; - } - else if (getState() != comm::SocketState::Connected || std::chrono::steady_clock::now() >= deadline) + welcome.push_back(character); + if (character == '\n') { - throw TimeoutException("Did not receive answer from dashboard server in time.", tv); + break; } + deadline = std::chrono::steady_clock::now() + 10s; } - if (isStopRequested()) + else if (getState() != comm::SocketState::Connected || std::chrono::steady_clock::now() >= deadline) { - TCPSocket::setReceiveTimeout(configured_tv); - return false; + throw TimeoutException("Did not receive welcome message from dashboard server in time.", welcome_tv); } - URCL_LOG_INFO("%s", welcome.c_str()); - ret_val = true; } - else + if (isStopRequested()) { TCPSocket::setReceiveTimeout(configured_tv); return false; } + URCL_LOG_INFO("%s", welcome.c_str()); } - catch (const TimeoutException&) + else { TCPSocket::setReceiveTimeout(configured_tv); - if (isStopRequested()) - { - return false; - } - URCL_LOG_WARN("Did not receive dashboard bootup message although connection was established. This should not " - "happen, please contact the package maintainers. Retrying anyway..."); + return false; } } + catch (const TimeoutException& e) + { + if (isStopRequested()) + { + TCPSocket::setReceiveTimeout(configured_tv); + return false; + } + URCL_LOG_WARN("%s", e.what()); + disconnect(); + TCPSocket::setReceiveTimeout(configured_tv); + return false; + } // Reset read timeout to configured socket timeout TCPSocket::setReceiveTimeout(configured_tv); polyscope_version_ = queryPolyScopeVersion(); - return ret_val; + return true; } void DashboardClientImplG5::disconnect() diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index cc7d1f874..647cb8ee4 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -177,6 +177,8 @@ gtest_add_tests(TARGET dashboard_client_mock_tests ) set_tests_properties(DashboardClientWelcomeTest.disconnect_interrupts_silent_welcome PROPERTIES TIMEOUT 20 RUN_SERIAL TRUE) +set_tests_properties(DashboardClientWelcomeTest.welcome_timeout_allows_external_retry + PROPERTIES TIMEOUT 30 RUN_SERIAL TRUE) add_executable(primary_parser_tests test_primary_parser.cpp) target_link_libraries(primary_parser_tests PRIVATE ur_client_library::urcl GTest::gtest_main) diff --git a/tests/test_dashboard_client.cpp b/tests/test_dashboard_client.cpp index 67b46465c..857b5f4da 100644 --- a/tests/test_dashboard_client.cpp +++ b/tests/test_dashboard_client.cpp @@ -442,6 +442,75 @@ TEST(DashboardClientWelcomeTest, disconnect_interrupts_silent_welcome) EXPECT_LT(elapsed, std::chrono::seconds(3)); } +// A missing welcome is a failed connection attempt, not a connected socket +// that leaves every later external connect() unable to try again. +TEST(DashboardClientWelcomeTest, welcome_timeout_allows_external_retry) +{ + TestableTcpServer server(29999); + server.start(); + DashboardClientImplG5 client("127.0.0.1"); + + const auto first_started = std::chrono::steady_clock::now(); + const bool first_connected = client.connect(1); + const auto first_elapsed = std::chrono::steady_clock::now() - first_started; + const bool first_accepted = server.waitForConnectionCallback(1000); + const bool first_disconnected = server.waitForDisconnectionCallback(1000); + + bool second_connected = false; + std::exception_ptr connection_error; + std::thread second_attempt([&]() { + try + { + second_connected = client.connect(1); + } + catch (...) + { + connection_error = std::current_exception(); + } + }); + + const bool second_accepted = server.waitForConnectionCallback(1000); + bool greeting_sent = false; + bool version_requested = false; + bool version_sent = false; + if (second_accepted) + { + const std::string greeting = "Connected: Universal Robots Dashboard Server\n"; + size_t written = 0; + greeting_sent = server.write(reinterpret_cast(greeting.data()), greeting.size(), written) && + written == greeting.size(); + if (greeting_sent) + { + version_requested = server.waitForMessageCallback(2000); + } + if (version_requested) + { + const std::string version = "URSoftware 5.12.0.1 (validation)\n"; + version_sent = server.write(reinterpret_cast(version.data()), version.size(), written) && + written == version.size(); + } + } + if (!second_accepted || !greeting_sent || !version_requested || !version_sent) + { + client.disconnect(); + } + second_attempt.join(); + + EXPECT_TRUE(first_accepted); + EXPECT_FALSE(first_connected); + EXPECT_LT(first_elapsed, std::chrono::seconds(13)); + EXPECT_TRUE(first_disconnected) << "The timed-out welcome left the socket connected"; + EXPECT_TRUE(second_accepted) << "The next connect() never reached the dashboard server"; + EXPECT_TRUE(greeting_sent); + EXPECT_TRUE(version_requested); + EXPECT_TRUE(version_sent); + if (connection_error) + { + std::rethrow_exception(connection_error); + } + EXPECT_TRUE(second_connected); +} + int main(int argc, char* argv[]) { ::testing::InitGoogleTest(&argc, argv); From 6b7993773091a4799d9c5df734c2fcf3120484a5 Mon Sep 17 00:00:00 2001 From: Felix Exner Date: Mon, 12 Oct 2026 09:42:55 +0200 Subject: [PATCH 3/3] Change log severity Co-authored-by: Felix Exner --- src/ur/dashboard_client_implementation_g5.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ur/dashboard_client_implementation_g5.cpp b/src/ur/dashboard_client_implementation_g5.cpp index b39c7250f..77f442d36 100644 --- a/src/ur/dashboard_client_implementation_g5.cpp +++ b/src/ur/dashboard_client_implementation_g5.cpp @@ -260,7 +260,7 @@ bool DashboardClientImplG5::connect(const size_t max_num_tries, const std::chron TCPSocket::setReceiveTimeout(configured_tv); return false; } - URCL_LOG_WARN("%s", e.what()); + URCL_LOG_ERROR("%s", e.what()); disconnect(); TCPSocket::setReceiveTimeout(configured_tv); return false;