If no new commands are received, halt#440
If no new commands are received, halt#440urfeex wants to merge 7 commits intoUniversalRobots:masterfrom
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #440 +/- ##
==========================================
+ Coverage 76.28% 76.30% +0.01%
==========================================
Files 105 105
Lines 5537 5537
Branches 593 593
==========================================
+ Hits 4224 4225 +1
+ Misses 1015 1013 -2
- Partials 298 299 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Sounds good, I also have an idea for that in mind. It's a bit more complicated, as we need to generate a PolyScope program and require the EC URCap to run that, but it should be possible to do. I just don't know when I will have the time to spend on this. |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
urfeex
force-pushed
the
halt_on_communication_loss
branch
2 times, most recently
from
8c48d9c to
a10563a
Compare
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
| std::ofstream out_file; | ||
| out_file.open(modified_script_path); | ||
| out_file << prog; | ||
| out_file.close(); |
There was a problem hiding this comment.
Silent write failure in extendScript temp file
Low Severity
extendScript opens ofs on the temp file for error-checking but never writes through it, then opens a second std::ofstream (out_file) to the same file while ofs is still open. On Windows, two simultaneous write handles to the same file cause a sharing violation, so out_file.open() silently fails. There is no check of out_file's error state after open() or operator<<, so the function returns the path to an empty file and the test proceeds with an unusable script. Additionally, the file descriptor returned by mkstemp is discarded via std::ignore, causing a resource leak.
Currently, we only exit the main loop when no new command is received anymore (already taking any configured receive timeout into account). However, if we don't receive a new command anymore, that should always be considerd an unintentional interruption of the control flow. In that case, we should stop program execution, as interrupting the communication was an unexpected error event. If interrupting the communication in order to continue with the rest of the program was intentional, users will send a stop command. This will end the main control loop at another code branch leading to a clean shutdown of the external_control part in order to be able to continue with the rest of the program.
urfeex
force-pushed
the
halt_on_communication_loss
branch
from
a10563a to
7dd910f
Compare
2 participants
Currently, we only exit the main loop when no new command is received anymore (already taking any configured receive timeout into account).
However, if we don't receive a new command anymore, that should always be considered an unintentional interruption of the control flow. In that case, we should stop program execution, as interrupting the communication was an unexpected error event.
If interrupting the communication in order to continue with the rest of the program was intentional, users will send a stop command. This will end the main control loop at another code branch leading to a clean shutdown of the external_control part in order to be able to continue with the rest of the program.
This should be closing #438 but in order to make sense with the ROS drivers, we would need UniversalRobots/Universal_Robots_ROS2_Driver#1678 and potentially UniversalRobots/Universal_Robots_ROS_Driver#767 before merging this.
Note
Medium Risk
Changes the ExternalControl URScript timeout path to actively stop motion and
halt, which can change behavior for users relying on timeout-as-exit. Also tweaks TCP server disconnect callback behavior, which may affect shutdown/cleanup edge cases but is narrowly scoped and covered by updated/new tests.Overview
ExternalControl now treats missing commands as a hard fault. When the
reverse_socketread times out, the URScript no longer just exits the loop; it issuesstopj(STOPJ_ACCELERATION)andhaltto stop the robot program immediately.Test infrastructure was refactored and expanded. A new
TestableTcpServerhelper centralizes connection/message/disconnect waiting and simplifies existing TCP/stream/producer/pipeline/socket tests, and new integration tests (test_external_control_program.cpp, wired intotests/CMakeLists.txt) assert the new timeout-halting behavior while ensuring an explicitstopControl()path still allows the program to continue.TCP server shutdown behavior is slightly tightened.
TCPServer::handleDisconnect()now only firesdisconnect_callback_whilekeep_running_is true, avoiding callbacks during teardown.Written by Cursor Bugbot for commit a10563a. This will update automatically on new commits. Configure here.