Skip to content

Commit 241c1a2

Browse files
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).
1 parent 6e318e6 commit 241c1a2

3 files changed

Lines changed: 90 additions & 11 deletions

File tree

include/ur_client_library/ur/dashboard_client_implementation.h

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -135,6 +135,27 @@ class DashboardClientImpl
135135
*/
136136
virtual void setReceiveTimeout([[maybe_unused]] const timeval& timeout) {};
137137

138+
/*!
139+
* \brief Gets the configured send timeout. If send timeout is unconfigured "normal" socket
140+
* timeout of 1 second will be returned.
141+
*
142+
* \returns configured send timeout
143+
*/
144+
virtual timeval getConfiguredSendTimeout() const
145+
{
146+
timeval tv;
147+
tv.tv_sec = 1;
148+
tv.tv_usec = 0;
149+
return tv;
150+
}
151+
152+
/*!
153+
* \brief Sets the send timeout for the socket.
154+
*
155+
* \param timeout The timeout to be set
156+
*/
157+
virtual void setSendTimeout([[maybe_unused]] const timeval& timeout) {};
158+
138159
/*!
139160
* \brief Sends command and verifies that a valid answer is received.
140161
*

include/ur_client_library/ur/dashboard_client_implementation_x.h

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -164,6 +164,8 @@ class DashboardClientImplX : public DashboardClientImpl
164164

165165
// Defined in the .cpp because httplib::Client is only forward-declared in this header.
166166
void setReceiveTimeout(const timeval& timeout) override;
167+
void setSendTimeout(const timeval& timeout) override;
168+
timeval getConfiguredSendTimeout() const override;
167169

168170
protected:
169171
DashboardResponse performProgramUpload(
@@ -184,10 +186,11 @@ class DashboardClientImplX : public DashboardClientImpl
184186

185187
std::unique_ptr<httplib::Client> cli_;
186188
VersionInformation robot_api_version_;
187-
// Caller-configured read timeout. Null until setReceiveTimeout() is called explicitly;
188-
// getConfiguredReceiveTimeout() returns the documented 1 s default in that case. Mirrors
189-
// the recv_timeout_ pattern used by DashboardClientImplG5.
189+
// Caller-configured timeouts. Null until the corresponding setter is called explicitly;
190+
// the getters fall back to the constructor default (10 s) in that case. Mirrors the
191+
// recv_timeout_ pattern used by DashboardClientImplG5.
190192
std::unique_ptr<timeval> recv_timeout_;
193+
std::unique_ptr<timeval> send_timeout_;
191194
};
192195

193196
} // namespace urcl

src/ur/dashboard_client_implementation_x.cpp

Lines changed: 63 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -53,11 +53,16 @@ DashboardClientImplX::DashboardClientImplX(const std::string& host) : DashboardC
5353

5454
// cpp-httplib's default read_timeout is 300 seconds. Applied unchanged, this makes any
5555
// dashboard call hang for 5 minutes when the controller becomes unresponsive (e.g. network
56-
// partition, paused container in tests). Align with the contract documented on the base
57-
// class: getConfiguredReceiveTimeout() returns {tv_sec = 1, tv_usec = 0} on this impl.
56+
// partition, paused container in tests).
57+
//
58+
// The 10 s default for read/write is chosen to cover blocking calls that legitimately take
59+
// time on real hardware — brake_release, commandLoadProgram (read), commandUploadProgram
60+
// (write), commandUpdateProgram (write), commandDownloadProgram (read) — without forcing
61+
// each one to plumb its own timeout. commandPowerOn already accepts its own (longer)
62+
// timeout parameter. Callers needing different limits can override via setReceiveTimeout.
5863
cli_->set_connection_timeout(std::chrono::seconds(5));
59-
cli_->set_read_timeout(std::chrono::seconds(1));
60-
cli_->set_write_timeout(std::chrono::seconds(5));
64+
cli_->set_read_timeout(std::chrono::seconds(10));
65+
cli_->set_write_timeout(std::chrono::seconds(10));
6166
}
6267

6368
void DashboardClientImplX::setReceiveTimeout(const timeval& timeout)
@@ -69,6 +74,15 @@ void DashboardClientImplX::setReceiveTimeout(const timeval& timeout)
6974
}
7075
}
7176

77+
void DashboardClientImplX::setSendTimeout(const timeval& timeout)
78+
{
79+
send_timeout_ = std::make_unique<timeval>(timeout);
80+
if (cli_)
81+
{
82+
cli_->set_write_timeout(std::chrono::seconds(timeout.tv_sec) + std::chrono::microseconds(timeout.tv_usec));
83+
}
84+
}
85+
7286
std::string DashboardClientImplX::sendAndReceive([[maybe_unused]] const std::string& text)
7387
{
7488
throw NotImplementedException("sendAndReceive is not implemented for DashboardClientImplX.");
@@ -134,15 +148,33 @@ void DashboardClientImplX::disconnect()
134148
timeval DashboardClientImplX::getConfiguredReceiveTimeout() const
135149
{
136150
// If the caller has explicitly configured a receive timeout via setReceiveTimeout,
137-
// return that. Otherwise fall back to the documented 1 s default. Mirrors G5.
151+
// return that. Otherwise fall back to the constructor default. See the constructor
152+
// comment for the rationale on the 10 s default (covers brake_release / program
153+
// load/upload/download without per-method timeouts).
138154
timeval tv;
139155
if (recv_timeout_ != nullptr)
140156
{
141157
tv = *recv_timeout_;
142158
}
143159
else
144160
{
145-
tv.tv_sec = 1;
161+
tv.tv_sec = 10;
162+
tv.tv_usec = 0;
163+
}
164+
return tv;
165+
}
166+
167+
timeval DashboardClientImplX::getConfiguredSendTimeout() const
168+
{
169+
// Mirrors getConfiguredReceiveTimeout. Default of 10 s matches the constructor.
170+
timeval tv;
171+
if (send_timeout_ != nullptr)
172+
{
173+
tv = *send_timeout_;
174+
}
175+
else
176+
{
177+
tv.tv_sec = 10;
146178
tv.tv_usec = 0;
147179
}
148180
return tv;
@@ -196,9 +228,32 @@ DashboardResponse DashboardClientImplX::commandPowerOff()
196228
return put("/robotstate/v1/state", R"({"action": "POWER_OFF"})");
197229
}
198230

199-
DashboardResponse DashboardClientImplX::commandPowerOn([[maybe_unused]] const std::chrono::duration<double> timeout)
231+
DashboardResponse DashboardClientImplX::commandPowerOn(const std::chrono::duration<double> timeout)
200232
{
201-
return put("/robotstate/v1/state", R"({"action": "POWER_ON"})");
233+
// commandPowerOn can take significantly longer than steady-state dashboard calls (the robot
234+
// boots, runs self-checks, etc.). Bump the read timeout to the caller-supplied value for the
235+
// duration of the PUT, then restore — using the same save-bump-restore pattern as connect().
236+
// The restore must run on every exit, including exceptions from inside put(), hence the
237+
// catch(...) rethrow guard.
238+
timeval configured_tv = getConfiguredReceiveTimeout();
239+
timeval pwron_tv;
240+
pwron_tv.tv_sec = static_cast<time_t>(std::chrono::duration_cast<std::chrono::seconds>(timeout).count());
241+
pwron_tv.tv_usec = 0;
242+
setReceiveTimeout(pwron_tv);
243+
244+
DashboardResponse response;
245+
try
246+
{
247+
response = put("/robotstate/v1/state", R"({"action": "POWER_ON"})");
248+
}
249+
catch (...)
250+
{
251+
setReceiveTimeout(configured_tv);
252+
throw;
253+
}
254+
255+
setReceiveTimeout(configured_tv);
256+
return response;
202257
}
203258

204259
DashboardResponse DashboardClientImplX::commandBrakeRelease()

0 commit comments

Comments
 (0)