From 6fa5a90c00531a39647b10c0cac11074cd808957 Mon Sep 17 00:00:00 2001 From: Vinicius de Oliveira Date: Thu, 11 Jun 2026 09:59:45 +0000 Subject: [PATCH 01/15] Apply httplib timeouts in DashboardClientImplX MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit cpp-httplib's default read_timeout is 300 seconds. Because the PolyScope X DashboardClientImplX constructor never overrides any timeout, every dashboard call (commandPowerOff, commandStop, commandPlay, etc.) blocks for the full 300 s when the controller becomes unresponsive — e.g. a network partition or paused container in tests. Symptoms include thread-pool exhaustion in callers that dispatch dashboard commands from a bounded executor pool. Two changes: 1. Apply explicit set_connection_timeout(5s), set_read_timeout(1s), and set_write_timeout(5s) in the constructor. The 1 s read default matches the contract already documented on this impl: getConfiguredReceiveTimeout() returns {tv_sec = 1, tv_usec = 0}. 2. Wire setReceiveTimeout(const timeval&) into the underlying httplib::Client instead of being a [[maybe_unused]] no-op. This makes the base-class API work as documented on the X path; callers can now tune the read timeout per their needs. Both changes are backward-compatible: there is no public API change, and the new defaults align with what the impl already advertised via getConfiguredReceiveTimeout(). --- .../ur/dashboard_client_implementation_x.h | 5 ++--- src/ur/dashboard_client_implementation_x.cpp | 17 +++++++++++++++++ 2 files changed, 19 insertions(+), 3 deletions(-) diff --git a/include/ur_client_library/ur/dashboard_client_implementation_x.h b/include/ur_client_library/ur/dashboard_client_implementation_x.h index d99ca7e3a..f242f85a4 100644 --- a/include/ur_client_library/ur/dashboard_client_implementation_x.h +++ b/include/ur_client_library/ur/dashboard_client_implementation_x.h @@ -162,9 +162,8 @@ class DashboardClientImplX : public DashboardClientImpl DashboardResponse commandUpdateProgram(const std::string& file_path) override; DashboardResponse commandDownloadProgram(const std::string& program_name, const std::string& save_path) override; - void setReceiveTimeout([[maybe_unused]] const timeval& timeout) override - { - } + // Defined in the .cpp because httplib::Client is only forward-declared in this header. + void setReceiveTimeout(const timeval& timeout) override; protected: DashboardResponse performProgramUpload( diff --git a/src/ur/dashboard_client_implementation_x.cpp b/src/ur/dashboard_client_implementation_x.cpp index 3826f2f33..a860cba7c 100644 --- a/src/ur/dashboard_client_implementation_x.cpp +++ b/src/ur/dashboard_client_implementation_x.cpp @@ -24,6 +24,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 @@ -49,6 +50,22 @@ DashboardClientImplX::DashboardClientImplX(const std::string& host) : DashboardC // Some targets have changed between versions redirecting to the correct endpoint. For this to // work, we'll have to follow redirects, which is not the default for httplib. cli_->set_follow_location(true); + + // cpp-httplib's default read_timeout is 300 seconds. Applied unchanged, this makes any + // dashboard call hang for 5 minutes when the controller becomes unresponsive (e.g. network + // partition, paused container in tests). Align with the contract documented on the base + // class: getConfiguredReceiveTimeout() returns {tv_sec = 1, tv_usec = 0} on this impl. + cli_->set_connection_timeout(std::chrono::seconds(5)); + cli_->set_read_timeout(std::chrono::seconds(1)); + cli_->set_write_timeout(std::chrono::seconds(5)); +} + +void DashboardClientImplX::setReceiveTimeout(const timeval& timeout) +{ + if (cli_) + { + cli_->set_read_timeout(std::chrono::seconds(timeout.tv_sec) + std::chrono::microseconds(timeout.tv_usec)); + } } std::string DashboardClientImplX::sendAndReceive([[maybe_unused]] const std::string& text) From b1f900de668364da4a906270b4ba86e7a7de237b Mon Sep 17 00:00:00 2001 From: Vinicius de Oliveira Date: Thu, 11 Jun 2026 10:07:48 +0000 Subject: [PATCH 02/15] Use longer read timeout during connect() setup Address review feedback: the new 1 s default applied in the constructor was in effect during the initial openapi.json GET in connect(), unlike the prior 300 s default which gave that first read plenty of headroom. Mirror the G5 pattern (dashboard_client_implementation_g5.cpp connect()): save the configured timeout, temporarily bump the read timeout to 10 s for the setup GET, then restore the configured value before returning. Also consolidate the exit paths so the restore runs on every outcome. --- src/ur/dashboard_client_implementation_x.cpp | 32 ++++++++++++++------ 1 file changed, 23 insertions(+), 9 deletions(-) diff --git a/src/ur/dashboard_client_implementation_x.cpp b/src/ur/dashboard_client_implementation_x.cpp index a860cba7c..8790dcf23 100644 --- a/src/ur/dashboard_client_implementation_x.cpp +++ b/src/ur/dashboard_client_implementation_x.cpp @@ -76,6 +76,16 @@ std::string DashboardClientImplX::sendAndReceive([[maybe_unused]] const std::str bool DashboardClientImplX::connect([[maybe_unused]] const size_t max_num_tries, [[maybe_unused]] const std::chrono::milliseconds reconnection_time) { + // The initial openapi.json fetch can take significantly more time than steady-state + // dashboard calls (larger payload, first-contact handshake). Mirror the G5 pattern: + // temporarily extend the read timeout for setup, then restore the configured value. + timeval configured_tv = getConfiguredReceiveTimeout(); + timeval setup_tv; + setup_tv.tv_sec = 10; + setup_tv.tv_usec = 0; + setReceiveTimeout(setup_tv); + + bool result = false; std::string endpoint = base_url_ + "/openapi.json"; // The PolyScope X Robot API doesn't require any connection prior to making calls. However, this // check call will assure that the endpoint for making Robot API calls exist. This could fail if @@ -85,19 +95,23 @@ bool DashboardClientImplX::connect([[maybe_unused]] const size_t max_num_tries, if (res->status != 200) { URCL_LOG_ERROR("Received non-200 response code when connecting to Robot API: %d", res->status); - return false; } - auto db_res = handleHttpResult(res, false); - auto json_data = json::parse(db_res.message); - if (db_res.ok && json_data.contains("info") && json_data["info"].contains("version") && - json_data["info"]["version"].is_string()) + else { - robot_api_version_ = VersionInformation::fromString(json_data["info"]["version"]); - URCL_LOG_DEBUG("Connected to Robot API version: %s", robot_api_version_.toString().c_str()); - return true; + auto db_res = handleHttpResult(res, false); + auto json_data = json::parse(db_res.message); + if (db_res.ok && json_data.contains("info") && json_data["info"].contains("version") && + json_data["info"]["version"].is_string()) + { + robot_api_version_ = VersionInformation::fromString(json_data["info"]["version"]); + URCL_LOG_DEBUG("Connected to Robot API version: %s", robot_api_version_.toString().c_str()); + result = true; + } } } - return false; + + setReceiveTimeout(configured_tv); + return result; } void DashboardClientImplX::disconnect() From efd9b8ac051df6ca0d55a543a8cb28c983c50f1a Mon Sep 17 00:00:00 2001 From: Vinicius de Oliveira Date: Thu, 11 Jun 2026 10:12:25 +0000 Subject: [PATCH 03/15] Restore read timeout on exceptions in connect() Address review feedback: json::parse and VersionInformation::fromString can throw on malformed or unexpected openapi.json payloads. Without exception handling, the restore call at the end of connect() would be skipped and the client would be left on the 10 s setup read timeout. Wrap the body in try / catch(...) and restore the configured timeout before rethrowing, so the configured value is honored on every exit path. --- src/ur/dashboard_client_implementation_x.cpp | 44 ++++++++++++-------- 1 file changed, 27 insertions(+), 17 deletions(-) diff --git a/src/ur/dashboard_client_implementation_x.cpp b/src/ur/dashboard_client_implementation_x.cpp index 8790dcf23..f5dbec394 100644 --- a/src/ur/dashboard_client_implementation_x.cpp +++ b/src/ur/dashboard_client_implementation_x.cpp @@ -79,6 +79,8 @@ bool DashboardClientImplX::connect([[maybe_unused]] const size_t max_num_tries, // The initial openapi.json fetch can take significantly more time than steady-state // dashboard calls (larger payload, first-contact handshake). Mirror the G5 pattern: // temporarily extend the read timeout for setup, then restore the configured value. + // The restore must run on every exit, including exceptions from json::parse or + // VersionInformation::fromString — hence the catch(...) rethrow guard. timeval configured_tv = getConfiguredReceiveTimeout(); timeval setup_tv; setup_tv.tv_sec = 10; @@ -86,29 +88,37 @@ bool DashboardClientImplX::connect([[maybe_unused]] const size_t max_num_tries, setReceiveTimeout(setup_tv); bool result = false; - std::string endpoint = base_url_ + "/openapi.json"; - // The PolyScope X Robot API doesn't require any connection prior to making calls. However, this - // check call will assure that the endpoint for making Robot API calls exist. This could fail if - // the IP address is wrong or the robot at the IP doesn't have the necessary software version. - if (auto res = cli_->Get(endpoint)) + try { - if (res->status != 200) + std::string endpoint = base_url_ + "/openapi.json"; + // The PolyScope X Robot API doesn't require any connection prior to making calls. However, this + // check call will assure that the endpoint for making Robot API calls exist. This could fail if + // the IP address is wrong or the robot at the IP doesn't have the necessary software version. + if (auto res = cli_->Get(endpoint)) { - URCL_LOG_ERROR("Received non-200 response code when connecting to Robot API: %d", res->status); - } - else - { - auto db_res = handleHttpResult(res, false); - auto json_data = json::parse(db_res.message); - if (db_res.ok && json_data.contains("info") && json_data["info"].contains("version") && - json_data["info"]["version"].is_string()) + if (res->status != 200) { - robot_api_version_ = VersionInformation::fromString(json_data["info"]["version"]); - URCL_LOG_DEBUG("Connected to Robot API version: %s", robot_api_version_.toString().c_str()); - result = true; + URCL_LOG_ERROR("Received non-200 response code when connecting to Robot API: %d", res->status); + } + else + { + auto db_res = handleHttpResult(res, false); + auto json_data = json::parse(db_res.message); + if (db_res.ok && json_data.contains("info") && json_data["info"].contains("version") && + json_data["info"]["version"].is_string()) + { + robot_api_version_ = VersionInformation::fromString(json_data["info"]["version"]); + URCL_LOG_DEBUG("Connected to Robot API version: %s", robot_api_version_.toString().c_str()); + result = true; + } } } } + catch (...) + { + setReceiveTimeout(configured_tv); + throw; + } setReceiveTimeout(configured_tv); return result; From 6e318e6a4faa5a04cd37ec2ff2a5b527b6e9c34b Mon Sep 17 00:00:00 2001 From: Vinicius de Oliveira Date: Thu, 11 Jun 2026 10:14:54 +0000 Subject: [PATCH 04/15] Honor caller-set read timeout across connect() Address review feedback: setReceiveTimeout previously only updated the httplib client, while getConfiguredReceiveTimeout always returned the hardcoded 1 s default. connect() saves and restores via the latter, so any custom read timeout the caller set before (or between) connects was silently overwritten with 1 s. Mirror the G5 pattern: store the caller-provided value in a member recv_timeout_ (null until explicitly set), and have getConfiguredReceiveTimeout return that value when present, falling back to the 1 s documented default otherwise. Now connect()'s save/restore round-trips the caller's value correctly. --- .../ur/dashboard_client_implementation_x.h | 4 ++++ src/ur/dashboard_client_implementation_x.cpp | 14 ++++++++++++-- 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/include/ur_client_library/ur/dashboard_client_implementation_x.h b/include/ur_client_library/ur/dashboard_client_implementation_x.h index f242f85a4..f23a88869 100644 --- a/include/ur_client_library/ur/dashboard_client_implementation_x.h +++ b/include/ur_client_library/ur/dashboard_client_implementation_x.h @@ -184,6 +184,10 @@ class DashboardClientImplX : public DashboardClientImpl std::unique_ptr cli_; VersionInformation robot_api_version_; + // Caller-configured read timeout. Null until setReceiveTimeout() is called explicitly; + // getConfiguredReceiveTimeout() returns the documented 1 s default in that case. Mirrors + // the recv_timeout_ pattern used by DashboardClientImplG5. + std::unique_ptr recv_timeout_; }; } // namespace urcl diff --git a/src/ur/dashboard_client_implementation_x.cpp b/src/ur/dashboard_client_implementation_x.cpp index f5dbec394..1613dca9e 100644 --- a/src/ur/dashboard_client_implementation_x.cpp +++ b/src/ur/dashboard_client_implementation_x.cpp @@ -62,6 +62,7 @@ DashboardClientImplX::DashboardClientImplX(const std::string& host) : DashboardC void DashboardClientImplX::setReceiveTimeout(const timeval& timeout) { + recv_timeout_ = std::make_unique(timeout); if (cli_) { cli_->set_read_timeout(std::chrono::seconds(timeout.tv_sec) + std::chrono::microseconds(timeout.tv_usec)); @@ -132,9 +133,18 @@ void DashboardClientImplX::disconnect() timeval DashboardClientImplX::getConfiguredReceiveTimeout() const { + // If the caller has explicitly configured a receive timeout via setReceiveTimeout, + // return that. Otherwise fall back to the documented 1 s default. Mirrors G5. timeval tv; - tv.tv_sec = 1; - tv.tv_usec = 0; + if (recv_timeout_ != nullptr) + { + tv = *recv_timeout_; + } + else + { + tv.tv_sec = 1; + tv.tv_usec = 0; + } return tv; } From 241c1a2787789b14462781962833c6f84a8b5491 Mon Sep 17 00:00:00 2001 From: Vinicius de Oliveira Date: Thu, 11 Jun 2026 11:28:55 +0000 Subject: [PATCH 05/15] Add send-timeout API and honor commandPowerOn timeout Addresses upstream review on the httplib timeout PR: - Add setSendTimeout / getConfiguredSendTimeout to the base class (DashboardClientImpl), mirroring the existing read-timeout API. getConfiguredSendTimeout has a non-pure default that returns the documented 1 s value, so DashboardClientImplG5 needs no change. - Override both in DashboardClientImplX with a send_timeout_ member parallel to recv_timeout_. The setter updates the underlying httplib::Client; the getter falls back to the 10 s constructor default until a caller configures it explicitly. - Bump the constructor's read/write timeouts from 1 s / 5 s to 10 s / 10 s. The 1 s read default was too tight for legitimate blocking calls (brake_release, commandLoadProgram, program upload/update/download). The 10 s figure covers all non-power-on commands without per-method timeouts; callers can override via setReceiveTimeout / setSendTimeout. - Honor the timeout parameter on commandPowerOn using the same save-bump-restore pattern as connect(): cache the configured read timeout, set it to the caller-supplied value for the PUT, restore on both success and exception (catch(...) rethrow). --- .../ur/dashboard_client_implementation.h | 21 ++++++ .../ur/dashboard_client_implementation_x.h | 9 ++- src/ur/dashboard_client_implementation_x.cpp | 71 ++++++++++++++++--- 3 files changed, 90 insertions(+), 11 deletions(-) diff --git a/include/ur_client_library/ur/dashboard_client_implementation.h b/include/ur_client_library/ur/dashboard_client_implementation.h index 9979f529a..6fda79375 100644 --- a/include/ur_client_library/ur/dashboard_client_implementation.h +++ b/include/ur_client_library/ur/dashboard_client_implementation.h @@ -135,6 +135,27 @@ class DashboardClientImpl */ virtual void setReceiveTimeout([[maybe_unused]] const timeval& timeout) {}; + /*! + * \brief Gets the configured send timeout. If send timeout is unconfigured "normal" socket + * timeout of 1 second will be returned. + * + * \returns configured send timeout + */ + virtual timeval getConfiguredSendTimeout() const + { + timeval tv; + tv.tv_sec = 1; + tv.tv_usec = 0; + return tv; + } + + /*! + * \brief Sets the send timeout for the socket. + * + * \param timeout The timeout to be set + */ + virtual void setSendTimeout([[maybe_unused]] const timeval& timeout) {}; + /*! * \brief Sends command and verifies that a valid answer is received. * diff --git a/include/ur_client_library/ur/dashboard_client_implementation_x.h b/include/ur_client_library/ur/dashboard_client_implementation_x.h index f23a88869..7f541e325 100644 --- a/include/ur_client_library/ur/dashboard_client_implementation_x.h +++ b/include/ur_client_library/ur/dashboard_client_implementation_x.h @@ -164,6 +164,8 @@ class DashboardClientImplX : public DashboardClientImpl // Defined in the .cpp because httplib::Client is only forward-declared in this header. void setReceiveTimeout(const timeval& timeout) override; + void setSendTimeout(const timeval& timeout) override; + timeval getConfiguredSendTimeout() const override; protected: DashboardResponse performProgramUpload( @@ -184,10 +186,11 @@ class DashboardClientImplX : public DashboardClientImpl std::unique_ptr cli_; VersionInformation robot_api_version_; - // Caller-configured read timeout. Null until setReceiveTimeout() is called explicitly; - // getConfiguredReceiveTimeout() returns the documented 1 s default in that case. Mirrors - // the recv_timeout_ pattern used by DashboardClientImplG5. + // Caller-configured timeouts. Null until the corresponding setter is called explicitly; + // the getters fall back to the constructor default (10 s) in that case. Mirrors the + // recv_timeout_ pattern used by DashboardClientImplG5. std::unique_ptr recv_timeout_; + std::unique_ptr send_timeout_; }; } // namespace urcl diff --git a/src/ur/dashboard_client_implementation_x.cpp b/src/ur/dashboard_client_implementation_x.cpp index 1613dca9e..117d36aad 100644 --- a/src/ur/dashboard_client_implementation_x.cpp +++ b/src/ur/dashboard_client_implementation_x.cpp @@ -53,11 +53,16 @@ DashboardClientImplX::DashboardClientImplX(const std::string& host) : DashboardC // cpp-httplib's default read_timeout is 300 seconds. Applied unchanged, this makes any // dashboard call hang for 5 minutes when the controller becomes unresponsive (e.g. network - // partition, paused container in tests). Align with the contract documented on the base - // class: getConfiguredReceiveTimeout() returns {tv_sec = 1, tv_usec = 0} on this impl. + // partition, paused container in tests). + // + // The 10 s default for read/write is chosen to cover blocking calls that legitimately take + // time on real hardware — brake_release, commandLoadProgram (read), commandUploadProgram + // (write), commandUpdateProgram (write), commandDownloadProgram (read) — without forcing + // each one to plumb its own timeout. commandPowerOn already accepts its own (longer) + // timeout parameter. Callers needing different limits can override via setReceiveTimeout. cli_->set_connection_timeout(std::chrono::seconds(5)); - cli_->set_read_timeout(std::chrono::seconds(1)); - cli_->set_write_timeout(std::chrono::seconds(5)); + cli_->set_read_timeout(std::chrono::seconds(10)); + cli_->set_write_timeout(std::chrono::seconds(10)); } void DashboardClientImplX::setReceiveTimeout(const timeval& timeout) @@ -69,6 +74,15 @@ void DashboardClientImplX::setReceiveTimeout(const timeval& timeout) } } +void DashboardClientImplX::setSendTimeout(const timeval& timeout) +{ + send_timeout_ = std::make_unique(timeout); + if (cli_) + { + cli_->set_write_timeout(std::chrono::seconds(timeout.tv_sec) + std::chrono::microseconds(timeout.tv_usec)); + } +} + std::string DashboardClientImplX::sendAndReceive([[maybe_unused]] const std::string& text) { throw NotImplementedException("sendAndReceive is not implemented for DashboardClientImplX."); @@ -134,7 +148,9 @@ void DashboardClientImplX::disconnect() timeval DashboardClientImplX::getConfiguredReceiveTimeout() const { // If the caller has explicitly configured a receive timeout via setReceiveTimeout, - // return that. Otherwise fall back to the documented 1 s default. Mirrors G5. + // return that. Otherwise fall back to the constructor default. See the constructor + // comment for the rationale on the 10 s default (covers brake_release / program + // load/upload/download without per-method timeouts). timeval tv; if (recv_timeout_ != nullptr) { @@ -142,7 +158,23 @@ timeval DashboardClientImplX::getConfiguredReceiveTimeout() const } else { - tv.tv_sec = 1; + tv.tv_sec = 10; + tv.tv_usec = 0; + } + return tv; +} + +timeval DashboardClientImplX::getConfiguredSendTimeout() const +{ + // Mirrors getConfiguredReceiveTimeout. Default of 10 s matches the constructor. + timeval tv; + if (send_timeout_ != nullptr) + { + tv = *send_timeout_; + } + else + { + tv.tv_sec = 10; tv.tv_usec = 0; } return tv; @@ -196,9 +228,32 @@ DashboardResponse DashboardClientImplX::commandPowerOff() return put("/robotstate/v1/state", R"({"action": "POWER_OFF"})"); } -DashboardResponse DashboardClientImplX::commandPowerOn([[maybe_unused]] const std::chrono::duration timeout) +DashboardResponse DashboardClientImplX::commandPowerOn(const std::chrono::duration timeout) { - return put("/robotstate/v1/state", R"({"action": "POWER_ON"})"); + // commandPowerOn can take significantly longer than steady-state dashboard calls (the robot + // boots, runs self-checks, etc.). Bump the read timeout to the caller-supplied value for the + // duration of the PUT, then restore — using the same save-bump-restore pattern as connect(). + // The restore must run on every exit, including exceptions from inside put(), hence the + // catch(...) rethrow guard. + timeval configured_tv = getConfiguredReceiveTimeout(); + timeval pwron_tv; + pwron_tv.tv_sec = static_cast(std::chrono::duration_cast(timeout).count()); + pwron_tv.tv_usec = 0; + setReceiveTimeout(pwron_tv); + + DashboardResponse response; + try + { + response = put("/robotstate/v1/state", R"({"action": "POWER_ON"})"); + } + catch (...) + { + setReceiveTimeout(configured_tv); + throw; + } + + setReceiveTimeout(configured_tv); + return response; } DashboardResponse DashboardClientImplX::commandBrakeRelease() From 63f117367f628380468543f5a0392702d8cbe0b3 Mon Sep 17 00:00:00 2001 From: Vinicius de Oliveira Date: Thu, 11 Jun 2026 12:12:19 +0000 Subject: [PATCH 06/15] Never shrink configured read timeout during connect() setup MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address review feedback: connect() unconditionally set the setup-phase read timeout to 10 s, even when getConfiguredReceiveTimeout() already returned a larger value. That temporarily shrank the in-flight read timeout during the openapi.json GET. The worst case is the lazy-connect path inside commandPowerOn: power on calls setReceiveTimeout(timeout) (typically 300 s by default), then issues a PUT. If robot_api_version_ is still empty, put() lazy-calls connect(), which would previously drop the 300 s back to 10 s for the setup GET — exactly when the controller is booting and the GET most needs the larger budget. Treat the 10 s as a minimum-headroom for the setup GET, not a cap on the caller's preferences: start from the configured value and only bump up to 10 s if it is smaller. The save/restore around the GET still honors whatever the caller had configured before connect() was entered. --- src/ur/dashboard_client_implementation_x.cpp | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/src/ur/dashboard_client_implementation_x.cpp b/src/ur/dashboard_client_implementation_x.cpp index 117d36aad..53d9abaf2 100644 --- a/src/ur/dashboard_client_implementation_x.cpp +++ b/src/ur/dashboard_client_implementation_x.cpp @@ -96,10 +96,19 @@ bool DashboardClientImplX::connect([[maybe_unused]] const size_t max_num_tries, // temporarily extend the read timeout for setup, then restore the configured value. // The restore must run on every exit, including exceptions from json::parse or // VersionInformation::fromString — hence the catch(...) rethrow guard. + // + // The 10 s here is a minimum-headroom for setup, not a cap on the caller's + // preferences. If the caller (or an enclosing call like commandPowerOn) has already + // configured a larger read timeout, never shrink it — a lazy connect() inside such a + // call would otherwise reduce the in-flight deadline during the openapi.json GET. timeval configured_tv = getConfiguredReceiveTimeout(); - timeval setup_tv; - setup_tv.tv_sec = 10; - setup_tv.tv_usec = 0; + constexpr time_t kSetupMinSeconds = 10; + timeval setup_tv = configured_tv; + if (setup_tv.tv_sec < kSetupMinSeconds) + { + setup_tv.tv_sec = kSetupMinSeconds; + setup_tv.tv_usec = 0; + } setReceiveTimeout(setup_tv); bool result = false; From cae74894b15710ea140572926b3d3ea6a3a204e8 Mon Sep 17 00:00:00 2001 From: Vinicius de Oliveira Date: Thu, 11 Jun 2026 12:23:30 +0000 Subject: [PATCH 07/15] Preserve sub-second precision when converting commandPowerOn timeout Address review feedback: commandPowerOn previously mapped its std::chrono::duration argument to timeval via duration_cast, which truncates toward zero, and hardcoded tv_usec = 0. A caller passing e.g. 2.5 s would silently get a 2 s read timeout. Cast to microseconds (the smallest unit timeval can represent) and split into tv_sec + tv_usec so the fractional portion survives. --- src/ur/dashboard_client_implementation_x.cpp | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/ur/dashboard_client_implementation_x.cpp b/src/ur/dashboard_client_implementation_x.cpp index 53d9abaf2..e13713358 100644 --- a/src/ur/dashboard_client_implementation_x.cpp +++ b/src/ur/dashboard_client_implementation_x.cpp @@ -245,9 +245,12 @@ DashboardResponse DashboardClientImplX::commandPowerOn(const std::chrono::durati // The restore must run on every exit, including exceptions from inside put(), hence the // catch(...) rethrow guard. timeval configured_tv = getConfiguredReceiveTimeout(); + // Preserve sub-second precision: duration_cast truncates fractional values, + // so go via microseconds (the smallest unit timeval can represent) and split. + const auto pwron_us = std::chrono::duration_cast(timeout); timeval pwron_tv; - pwron_tv.tv_sec = static_cast(std::chrono::duration_cast(timeout).count()); - pwron_tv.tv_usec = 0; + pwron_tv.tv_sec = static_cast(pwron_us.count() / 1'000'000); + pwron_tv.tv_usec = static_cast(pwron_us.count() % 1'000'000); setReceiveTimeout(pwron_tv); DashboardResponse response; From cdd9988238ae31438e03fdde23df24be5c12f577 Mon Sep 17 00:00:00 2001 From: Vinicius de Oliveira Date: Fri, 12 Jun 2026 12:57:40 +0000 Subject: [PATCH 08/15] Use plain timeval members instead of unique_ptr for X timeouts The std::unique_ptr indirection was carried over from DashboardClientImplG5, where it composes with TCPSocket's existing unique_ptr member. In DashboardClientImplX it bought nothing: no nullable semantics are needed because the constructor now installs a 10 s default that every getter falls back to anyway. Replace recv_timeout_ and send_timeout_ with value-typed timeval members defaulted to {10, 0}. setReceiveTimeout / setSendTimeout assign directly; the getters return the member. Behavior is preserved (verified end-to-end against a stalling-server test that times connect() at the configured value). --- .../ur/dashboard_client_implementation_x.h | 7 ++--- src/ur/dashboard_client_implementation_x.cpp | 28 +++---------------- 2 files changed, 6 insertions(+), 29 deletions(-) diff --git a/include/ur_client_library/ur/dashboard_client_implementation_x.h b/include/ur_client_library/ur/dashboard_client_implementation_x.h index 7f541e325..8b2322988 100644 --- a/include/ur_client_library/ur/dashboard_client_implementation_x.h +++ b/include/ur_client_library/ur/dashboard_client_implementation_x.h @@ -186,11 +186,8 @@ class DashboardClientImplX : public DashboardClientImpl std::unique_ptr cli_; VersionInformation robot_api_version_; - // Caller-configured timeouts. Null until the corresponding setter is called explicitly; - // the getters fall back to the constructor default (10 s) in that case. Mirrors the - // recv_timeout_ pattern used by DashboardClientImplG5. - std::unique_ptr recv_timeout_; - std::unique_ptr send_timeout_; + timeval recv_timeout_ = {10, 0}; + timeval send_timeout_ = {10, 0}; }; } // namespace urcl diff --git a/src/ur/dashboard_client_implementation_x.cpp b/src/ur/dashboard_client_implementation_x.cpp index e13713358..17aee9f7e 100644 --- a/src/ur/dashboard_client_implementation_x.cpp +++ b/src/ur/dashboard_client_implementation_x.cpp @@ -67,7 +67,7 @@ DashboardClientImplX::DashboardClientImplX(const std::string& host) : DashboardC void DashboardClientImplX::setReceiveTimeout(const timeval& timeout) { - recv_timeout_ = std::make_unique(timeout); + recv_timeout_ = timeout; if (cli_) { cli_->set_read_timeout(std::chrono::seconds(timeout.tv_sec) + std::chrono::microseconds(timeout.tv_usec)); @@ -76,7 +76,7 @@ void DashboardClientImplX::setReceiveTimeout(const timeval& timeout) void DashboardClientImplX::setSendTimeout(const timeval& timeout) { - send_timeout_ = std::make_unique(timeout); + send_timeout_ = timeout; if (cli_) { cli_->set_write_timeout(std::chrono::seconds(timeout.tv_sec) + std::chrono::microseconds(timeout.tv_usec)); @@ -160,33 +160,13 @@ timeval DashboardClientImplX::getConfiguredReceiveTimeout() const // return that. Otherwise fall back to the constructor default. See the constructor // comment for the rationale on the 10 s default (covers brake_release / program // load/upload/download without per-method timeouts). - timeval tv; - if (recv_timeout_ != nullptr) - { - tv = *recv_timeout_; - } - else - { - tv.tv_sec = 10; - tv.tv_usec = 0; - } - return tv; + return recv_timeout_; } timeval DashboardClientImplX::getConfiguredSendTimeout() const { // Mirrors getConfiguredReceiveTimeout. Default of 10 s matches the constructor. - timeval tv; - if (send_timeout_ != nullptr) - { - tv = *send_timeout_; - } - else - { - tv.tv_sec = 10; - tv.tv_usec = 0; - } - return tv; + return send_timeout_; } VersionInformation DashboardClientImplX::queryPolyScopeVersion() From e84c7693046fa556ea341eb11fa4fef4bca9c90a Mon Sep 17 00:00:00 2001 From: Vinicius de Oliveira Date: Fri, 12 Jun 2026 13:03:42 +0000 Subject: [PATCH 09/15] Remove save-bump-restore from connect() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reviewer feedback: the save-bump-restore in connect() mimics G5's pattern but for the wrong reason. G5 bumps because it does an unsolicited blocking read for the dashboard server's boot-up message after TCPSocket::setup() — explicitly called out in the G5 source ("The first read after connection can take more time"). PolyScope X's connect() just issues an HTTP GET for /openapi.json (~200 ms over VPN per maintainer measurement); no analogous unsolicited read exists. The original cursor-bot concern that motivated b1f900d was that the 1 s constructor read_timeout left openapi.json under-resourced while in-tree callers raised the timeout only post-connect. That concern was resolved by bumping the constructor default from 1 s to 10 s, which gives openapi.json the same headroom the example wrapper applies manually. The save-bump-restore became dead weight: a no-op for default callers, and it overrides explicit caller values shorter than 10 s — paternalistic behavior the reviewer is rightly pushing back on. Drop the bump, the try/catch(...) rethrow guard, and the restore. connect() returns to its pre-b1f900d shape (early returns from the GET-and-parse pipeline). The recv_timeout_ storage stays because commandPowerOn still uses it for its own per-call save-bump-restore, which is a different and legitimate case (caller-supplied longer deadline for an operation known to take longer). --- src/ur/dashboard_client_implementation_x.cpp | 69 +++++--------------- 1 file changed, 18 insertions(+), 51 deletions(-) diff --git a/src/ur/dashboard_client_implementation_x.cpp b/src/ur/dashboard_client_implementation_x.cpp index 17aee9f7e..507f18e70 100644 --- a/src/ur/dashboard_client_implementation_x.cpp +++ b/src/ur/dashboard_client_implementation_x.cpp @@ -91,61 +91,28 @@ std::string DashboardClientImplX::sendAndReceive([[maybe_unused]] const std::str bool DashboardClientImplX::connect([[maybe_unused]] const size_t max_num_tries, [[maybe_unused]] const std::chrono::milliseconds reconnection_time) { - // The initial openapi.json fetch can take significantly more time than steady-state - // dashboard calls (larger payload, first-contact handshake). Mirror the G5 pattern: - // temporarily extend the read timeout for setup, then restore the configured value. - // The restore must run on every exit, including exceptions from json::parse or - // VersionInformation::fromString — hence the catch(...) rethrow guard. - // - // The 10 s here is a minimum-headroom for setup, not a cap on the caller's - // preferences. If the caller (or an enclosing call like commandPowerOn) has already - // configured a larger read timeout, never shrink it — a lazy connect() inside such a - // call would otherwise reduce the in-flight deadline during the openapi.json GET. - timeval configured_tv = getConfiguredReceiveTimeout(); - constexpr time_t kSetupMinSeconds = 10; - timeval setup_tv = configured_tv; - if (setup_tv.tv_sec < kSetupMinSeconds) - { - setup_tv.tv_sec = kSetupMinSeconds; - setup_tv.tv_usec = 0; - } - setReceiveTimeout(setup_tv); - - bool result = false; - try + std::string endpoint = base_url_ + "/openapi.json"; + // The PolyScope X Robot API doesn't require any connection prior to making calls. However, this + // check call will assurea that the endpoint for making Robot API calls exist. This could fail if + // the IP address is wrong or the robot at the IP doesn't have the necessary software version. + if (auto res = cli_->Get(endpoint)) { - std::string endpoint = base_url_ + "/openapi.json"; - // The PolyScope X Robot API doesn't require any connection prior to making calls. However, this - // check call will assure that the endpoint for making Robot API calls exist. This could fail if - // the IP address is wrong or the robot at the IP doesn't have the necessary software version. - if (auto res = cli_->Get(endpoint)) + if (res->status != 200) { - if (res->status != 200) - { - URCL_LOG_ERROR("Received non-200 response code when connecting to Robot API: %d", res->status); - } - else - { - auto db_res = handleHttpResult(res, false); - auto json_data = json::parse(db_res.message); - if (db_res.ok && json_data.contains("info") && json_data["info"].contains("version") && - json_data["info"]["version"].is_string()) - { - robot_api_version_ = VersionInformation::fromString(json_data["info"]["version"]); - URCL_LOG_DEBUG("Connected to Robot API version: %s", robot_api_version_.toString().c_str()); - result = true; - } - } + URCL_LOG_ERROR("Received non-200 response code when connecting to Robot API: %d", res->status); + return false; + } + auto db_res = handleHttpResult(res, false); + auto json_data = json::parse(db_res.message); + if (db_res.ok && json_data.contains("info") && json_data["info"].contains("version") && + json_data["info"]["version"].is_string()) + { + robot_api_version_ = VersionInformation::fromString(json_data["info"]["version"]); + URCL_LOG_DEBUG("Connected to Robot API version: %s", robot_api_version_.toString().c_str()); + return true; } } - catch (...) - { - setReceiveTimeout(configured_tv); - throw; - } - - setReceiveTimeout(configured_tv); - return result; + return false; } void DashboardClientImplX::disconnect() From 8b9e0f4ba969d0a5afa77aecada23ad879773ded Mon Sep 17 00:00:00 2001 From: Felix Exner Date: Fri, 12 Jun 2026 15:18:47 +0200 Subject: [PATCH 10/15] Apply suggestions from code review Co-authored-by: Felix Exner --- src/ur/dashboard_client_implementation_x.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/ur/dashboard_client_implementation_x.cpp b/src/ur/dashboard_client_implementation_x.cpp index 507f18e70..acde480cd 100644 --- a/src/ur/dashboard_client_implementation_x.cpp +++ b/src/ur/dashboard_client_implementation_x.cpp @@ -61,8 +61,8 @@ DashboardClientImplX::DashboardClientImplX(const std::string& host) : DashboardC // each one to plumb its own timeout. commandPowerOn already accepts its own (longer) // timeout parameter. Callers needing different limits can override via setReceiveTimeout. cli_->set_connection_timeout(std::chrono::seconds(5)); - cli_->set_read_timeout(std::chrono::seconds(10)); - cli_->set_write_timeout(std::chrono::seconds(10)); + cli_->set_read_timeout(std::chrono::seconds(recv_timeout_.tv_sec) + std::chrono::microseconds(recv_timeout_.tv_usec)); + cli_->set_write_timeout(std::chrono::seconds(send_timeout_.tv_sec) + std::chrono::microseconds(send_timeout_.tv_usec)); } void DashboardClientImplX::setReceiveTimeout(const timeval& timeout) @@ -93,7 +93,7 @@ bool DashboardClientImplX::connect([[maybe_unused]] const size_t max_num_tries, { std::string endpoint = base_url_ + "/openapi.json"; // The PolyScope X Robot API doesn't require any connection prior to making calls. However, this - // check call will assurea that the endpoint for making Robot API calls exist. This could fail if + // check call will assure that the endpoint for making Robot API calls exist. This could fail if // the IP address is wrong or the robot at the IP doesn't have the necessary software version. if (auto res = cli_->Get(endpoint)) { From d29eafdb029d652f76fe637273fd5f7caae61248 Mon Sep 17 00:00:00 2001 From: Vinicius de Oliveira Date: Mon, 15 Jun 2026 08:33:20 +0200 Subject: [PATCH 11/15] Update src/ur/dashboard_client_implementation_x.cpp Co-authored-by: Felix Exner --- src/ur/dashboard_client_implementation_x.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/src/ur/dashboard_client_implementation_x.cpp b/src/ur/dashboard_client_implementation_x.cpp index acde480cd..179031e76 100644 --- a/src/ur/dashboard_client_implementation_x.cpp +++ b/src/ur/dashboard_client_implementation_x.cpp @@ -132,7 +132,6 @@ timeval DashboardClientImplX::getConfiguredReceiveTimeout() const timeval DashboardClientImplX::getConfiguredSendTimeout() const { - // Mirrors getConfiguredReceiveTimeout. Default of 10 s matches the constructor. return send_timeout_; } From 1ed89b7e628056642e8f60d422123591a90048f8 Mon Sep 17 00:00:00 2001 From: Vinicius de Oliveira Date: Mon, 15 Jun 2026 08:33:32 +0200 Subject: [PATCH 12/15] Update src/ur/dashboard_client_implementation_x.cpp Co-authored-by: Felix Exner --- src/ur/dashboard_client_implementation_x.cpp | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/ur/dashboard_client_implementation_x.cpp b/src/ur/dashboard_client_implementation_x.cpp index 179031e76..101760f9b 100644 --- a/src/ur/dashboard_client_implementation_x.cpp +++ b/src/ur/dashboard_client_implementation_x.cpp @@ -123,10 +123,6 @@ void DashboardClientImplX::disconnect() timeval DashboardClientImplX::getConfiguredReceiveTimeout() const { - // If the caller has explicitly configured a receive timeout via setReceiveTimeout, - // return that. Otherwise fall back to the constructor default. See the constructor - // comment for the rationale on the 10 s default (covers brake_release / program - // load/upload/download without per-method timeouts). return recv_timeout_; } From be606e8a7ff3618bffb78d611dfeaeb4616ecdca Mon Sep 17 00:00:00 2001 From: Vinicius de Oliveira Date: Mon, 15 Jun 2026 07:13:04 +0000 Subject: [PATCH 13/15] Add tests for DashboardClientImplX timeout setters/getters Co-Authored-By: Claude Opus 4.7 (1M context) --- tests/test_dashboard_client_x.cpp | 52 +++++++++++++++++++++++++++++++ 1 file changed, 52 insertions(+) diff --git a/tests/test_dashboard_client_x.cpp b/tests/test_dashboard_client_x.cpp index 089a8accc..b30b02671 100644 --- a/tests/test_dashboard_client_x.cpp +++ b/tests/test_dashboard_client_x.cpp @@ -451,6 +451,58 @@ TEST_F(DashboardClientTestX, download_program) } } +TEST_F(DashboardClientTestX, set_receive_timeout) +{ + timeval expected_tv; + expected_tv.tv_sec = 30; + expected_tv.tv_usec = 0; + dashboard_client_->setReceiveTimeout(expected_tv); + EXPECT_TRUE(dashboard_client_->connect()); + + timeval actual_tv = dashboard_client_->getConfiguredReceiveTimeout(); + EXPECT_EQ(expected_tv.tv_sec, actual_tv.tv_sec); + EXPECT_EQ(expected_tv.tv_usec, actual_tv.tv_usec); +} + +TEST_F(DashboardClientTestX, set_send_timeout) +{ + timeval expected_tv; + expected_tv.tv_sec = 30; + expected_tv.tv_usec = 0; + dashboard_client_->setSendTimeout(expected_tv); + EXPECT_TRUE(dashboard_client_->connect()); + + timeval actual_tv = dashboard_client_->getConfiguredSendTimeout(); + EXPECT_EQ(expected_tv.tv_sec, actual_tv.tv_sec); + EXPECT_EQ(expected_tv.tv_usec, actual_tv.tv_usec); +} + +TEST_F(DashboardClientTestX, timeouts_round_trip_subsecond) +{ + timeval expected_tv; + expected_tv.tv_sec = 7; + expected_tv.tv_usec = 250000; + dashboard_client_->setReceiveTimeout(expected_tv); + dashboard_client_->setSendTimeout(expected_tv); + + timeval recv_tv = dashboard_client_->getConfiguredReceiveTimeout(); + EXPECT_EQ(expected_tv.tv_sec, recv_tv.tv_sec); + EXPECT_EQ(expected_tv.tv_usec, recv_tv.tv_usec); + + timeval send_tv = dashboard_client_->getConfiguredSendTimeout(); + EXPECT_EQ(expected_tv.tv_sec, send_tv.tv_sec); + EXPECT_EQ(expected_tv.tv_usec, send_tv.tv_usec); +} + +TEST_F(DashboardClientTestX, microsecond_receive_timeout_makes_connect_fail) +{ + timeval tiny_tv; + tiny_tv.tv_sec = 0; + tiny_tv.tv_usec = 1; + dashboard_client_->setReceiveTimeout(tiny_tv); + EXPECT_FALSE(dashboard_client_->connect()); +} + int main(int argc, char* argv[]) { ::testing::InitGoogleTest(&argc, argv); From 4f28df319a40cc827828746884394b2226af60f9 Mon Sep 17 00:00:00 2001 From: Felix Exner Date: Tue, 16 Jun 2026 09:59:28 +0200 Subject: [PATCH 14/15] Fix formatting --- .../ur_client_library/ur/dashboard_client_implementation_x.h | 4 ++-- src/ur/dashboard_client_implementation_x.cpp | 3 ++- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/include/ur_client_library/ur/dashboard_client_implementation_x.h b/include/ur_client_library/ur/dashboard_client_implementation_x.h index 8b2322988..b257e1726 100644 --- a/include/ur_client_library/ur/dashboard_client_implementation_x.h +++ b/include/ur_client_library/ur/dashboard_client_implementation_x.h @@ -186,8 +186,8 @@ class DashboardClientImplX : public DashboardClientImpl std::unique_ptr cli_; VersionInformation robot_api_version_; - timeval recv_timeout_ = {10, 0}; - timeval send_timeout_ = {10, 0}; + timeval recv_timeout_ = { 10, 0 }; + timeval send_timeout_ = { 10, 0 }; }; } // namespace urcl diff --git a/src/ur/dashboard_client_implementation_x.cpp b/src/ur/dashboard_client_implementation_x.cpp index 101760f9b..160160bc9 100644 --- a/src/ur/dashboard_client_implementation_x.cpp +++ b/src/ur/dashboard_client_implementation_x.cpp @@ -62,7 +62,8 @@ DashboardClientImplX::DashboardClientImplX(const std::string& host) : DashboardC // timeout parameter. Callers needing different limits can override via setReceiveTimeout. cli_->set_connection_timeout(std::chrono::seconds(5)); cli_->set_read_timeout(std::chrono::seconds(recv_timeout_.tv_sec) + std::chrono::microseconds(recv_timeout_.tv_usec)); - cli_->set_write_timeout(std::chrono::seconds(send_timeout_.tv_sec) + std::chrono::microseconds(send_timeout_.tv_usec)); + cli_->set_write_timeout(std::chrono::seconds(send_timeout_.tv_sec) + + std::chrono::microseconds(send_timeout_.tv_usec)); } void DashboardClientImplX::setReceiveTimeout(const timeval& timeout) From f04ef2c34e7fb6afac485da02c7d56009fcbf09c Mon Sep 17 00:00:00 2001 From: Felix Exner Date: Tue, 16 Jun 2026 10:01:12 +0200 Subject: [PATCH 15/15] Use long instead of time_t and suseconds_t On Windows timeval contains long datatypes, and the same should compile fine on Linux systems, as well. --- src/ur/dashboard_client_implementation_x.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/ur/dashboard_client_implementation_x.cpp b/src/ur/dashboard_client_implementation_x.cpp index 160160bc9..d728d91d5 100644 --- a/src/ur/dashboard_client_implementation_x.cpp +++ b/src/ur/dashboard_client_implementation_x.cpp @@ -192,8 +192,8 @@ DashboardResponse DashboardClientImplX::commandPowerOn(const std::chrono::durati // so go via microseconds (the smallest unit timeval can represent) and split. const auto pwron_us = std::chrono::duration_cast(timeout); timeval pwron_tv; - pwron_tv.tv_sec = static_cast(pwron_us.count() / 1'000'000); - pwron_tv.tv_usec = static_cast(pwron_us.count() % 1'000'000); + pwron_tv.tv_sec = static_cast(pwron_us.count() / 1'000'000); + pwron_tv.tv_usec = static_cast(pwron_us.count() % 1'000'000); setReceiveTimeout(pwron_tv); DashboardResponse response;