[shin4141]

Related issue: ROS 2 Driver #2012.

Coordinated dependency

This library change does not resolve #2012 by itself: with the unmodified Driver, shutdown still took 10.158 s because its in-flight dashboard client was not visible to stop(). The companion Driver PR #2013 publishes that client before the blocking call. Conversely, the Driver change alone still took 10.235 s with the old URCL welcome receive. Both changes are required for the observed prompt exit. This remains draft until the cross-repository path is reviewed.

Change

For G5's initial dashboard welcome line, poll receive in 100 ms slices while preserving bytes across slices and the former ten-second per-character allowance. A deliberate disconnect() now ends this receive without entering the timeout retry path. Restore the configured receive timeout on success, ordinary failure, and interruption. Normal command-response behavior and the PolyScope X implementation are not changed.

Adds a robot-free CTest regression: a loopback server accepts port 29999 but sends no welcome, then the client is disconnected while connect(1) waits. The test requires the connection thread to finish within three seconds and return false.

Validation

• Built from current URCL master 35da05b1aa2067991c059610f8bb2a658384912c in an isolated ROS Rolling/Linux container.
• New URCL regression and the dashboard mock suite: 16/16 PASS. Existing G5 connect-timeout test: PASS.
• With the companion Driver branch based on main d113bc33045142c8dbd98091cab14a1cfb9dd260, the 29999 silent-welcome SIGINT fixture exited 0 in 0.126 s on this fresh build (the earlier qualification run was 0.146 s). Normal dashboard connection, genuine startup-failure, Primary retry, and autoconnect-disabled controls passed. The new Driver launch test and #2006's two startup launch tests passed.
• Negative controls from the same issue: unmodified 10.521 s; Driver-only 10.235 s; URCL-only 10.158 s.

Only loopback peers were used. No real robot, URSim, Windows/macOS, PolyScope X, or full upstream suite was tested. This PR deliberately does not use an auto-closing Fixes keyword because its merge alone is insufficient.

[urfeex]

All in for an interruptible read here. Two things I would like to add, though:

• I think the timeval setup outside of the try-catch block is now unused and can therefore be removed.
• This isn't a thing changed by this PR, but I think it would be good to address this, anyway: On a TimeOutException when no stop is requested, there is a warning printed and the whole loop is retried. This will then fail, as we will attempt to connect to an already connected socket. We should probably disconnect in the catch block, as otherwise there is no way to retry establishing a connection from the outside after this, as connect will always fail then. We should also fail connect in that case immediately.

[urfeex]

This is looking good so far. @shin4141 if you would make this ready for review, I would like to merge this. I see this independent on the ROS PR.

[urfeex]

RUN_SERIAL is probably overkill, as our tests currently require to be run in serial altogether. But it won't hurt to have it here, so feel free to ignore this comment.

[urfeex]

@shin4141 Is there something you would like to add or can we mark this ready for review?

[codecov]

Codecov Report

❌ Patch coverage is 82.14286% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 83.76%. Comparing base (35da05b) to head (6b79937).
⚠️ Report is 4 commits behind head on master.
✅ All tests successful. No failed tests found.

Files with missing lines	Patch %	Lines
src/ur/dashboard_client_implementation_g5.cpp	82.14%	1 Missing and 4 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #588      +/-   ##
==========================================
+ Coverage   83.24%   83.76%   +0.51%     
==========================================
  Files         121      126       +5     
  Lines        7737     8167     +430     
  Branches     3561     3757     +196     
==========================================
+ Hits         6441     6841     +400     
- Misses        912      922      +10     
- Partials      384      404      +20     

Flag	Coverage Δ
python_scripts	75.90% <ø> (ø)
start_ursim	91.34% <ø> (-2.29%)	⬇️
unit	59.32% <82.14%> (+2.68%)	⬆️
ur5-3.14.3	80.08% <82.14%> (+0.99%)	⬆️
ur5e-10.11.0	75.69% <82.14%> (+1.58%)	⬆️
ur5e-10.12.0	76.70% <82.14%> (+1.49%)	⬆️
ur5e-10.7.0	75.25% <82.14%> (+1.62%)	⬆️
ur5e-5.9.4	80.21% <82.14%> (+0.86%)	⬆️
ur7e-10.13.0	77.29% <82.14%> (+1.50%)	⬆️
ur7e-10.14.0	77.68% <82.14%> (+1.53%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

[cursor]

PR Summary

Medium Risk
Changes connection handshake and timeout/disconnect behavior on a widely used client path; mitigated by focused tests and unchanged post-connect command I/O.

Overview
G5 dashboard connect() no longer blocks for up to 10s on the welcome line, which fixes slow shutdown when the peer accepts TCP but never sends the greeting (e.g. ROS 2 Driver #2012).

Instead of a single long read() with a 10s socket timeout and a retry loop on timeout, the welcome is read one byte at a time with a 100ms receive timeout, a rolling 10s deadline, and a loop that exits when disconnect() / stop is requested. On welcome timeout the client logs, disconnects, and returns false (no silent retry). The configured receive timeout is restored on success, failure, and interruption.

Robot-free regressions use a loopback TestableTcpServer: one test asserts disconnect() during a silent welcome finishes in under 3s; another asserts a timed-out welcome tears down the socket so a later connect() can complete the handshake. CTest timeouts and RUN_SERIAL were added for those cases.

^{Reviewed by Cursor Bugbot for commit 6b79937. Bugbot is set up for automated code reviews on this repo. Configure here.}

[cursor]

Cursor Bugbot has reviewed your changes using default effort and found 2 potential issues.

^{Reviewed by Cursor Bugbot for commit 6b79937. Configure here.}

[urfeex]

This is looking good. Thank you @shin4141

We might want to consider moving the interruptible read down to TCPSocket::read() in the future, but for now let's merge it in as is.

[mergify]

This pull request does not currently match the merge queue conditions, so it cannot be queued from here. The box comes back if it matches again.