Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added watchdog configuration for the reverse socket #178

Merged
merged 11 commits into from Sep 22, 2023

Conversation

urmahp
Copy link
Collaborator

@urmahp urmahp commented Sep 11, 2023

This enables the possibility to configure the read timeout for the reverse socket running in the external control script

Separated control modes into realtime and non realtime to have different possible configurations for the watchdog

Added test to verify the changes

Updated test and examples to coincide with the new changes

This enables the possibility to configure the read timeout for the reverse socket running in the external control script

Separated control modes into realtime and non realtime to have different possible configurations for the watchdog

Added test to verify the changes

Updated test and examples to coincide with the new changes
@urmahp urmahp requested review from urrsk and fmauch September 11, 2023 13:26
@codecov
Copy link

codecov bot commented Sep 12, 2023

examples/spline_example.cpp Show resolved Hide resolved
include/ur_client_library/comm/control_mode.h Outdated Show resolved Hide resolved
include/ur_client_library/ur/watchdog.h Outdated Show resolved Hide resolved
src/ur/ur_driver.cpp Outdated Show resolved Hide resolved
src/ur/ur_driver.cpp Outdated Show resolved Hide resolved
Moved timeout check to the reverse interface

Changed timeout from float to std::chrono::milliseconds
@urmahp
Copy link
Collaborator Author

urmahp commented Sep 13, 2023

I have updated the PR based on your comments.

@fmauch fmauch added this to the Release 1.3.4 milestone Sep 18, 2023
examples/full_driver.cpp Outdated Show resolved Hide resolved
include/ur_client_library/ur/watchdog.h Outdated Show resolved Hide resolved
include/ur_client_library/ur/watchdog.h Outdated Show resolved Hide resolved
include/ur_client_library/ur/watchdog.h Outdated Show resolved Hide resolved
@urrsk
Copy link
Member

urrsk commented Sep 20, 2023

We should also add a test, that verifies that the reverse interface timeout as well with in the expected time.

@urmahp
Copy link
Collaborator Author

urmahp commented Sep 20, 2023

We should also add a test, that verifies that the reverse interface timeout as well with in the expected time.

As the timeout is configured in the script the reverse interface will not timeout, the script will timeout. We could add a test that verifies that the script times out, but that would be a test under the UrDriver class, as it would require robot communication.

@urrsk
Copy link
Member

urrsk commented Sep 20, 2023

We should also add a test, that verifies that the reverse interface timeout as well with in the expected time.

As the timeout is configured in the script the reverse interface will not timeout, the script will timeout. We could add a test that verifies that the script times out, but that would be a test under the UrDriver class, as it would require robot communication.

True. And yes that is a good idea

Copy link
Collaborator

@fmauch fmauch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is looking really good and it reads much better with the RobotReceiveTimeout :-)

@urmahp a quick response to my comments would be nice, then I think we are good to merge this, as most of them is mainly aesthetics.

src/control/reverse_interface.cpp Outdated Show resolved Hide resolved
src/ur/robot_receive_timeout.cpp Outdated Show resolved Hide resolved
src/ur/robot_receive_timeout.cpp Outdated Show resolved Hide resolved
src/ur/robot_receive_timeout.cpp Outdated Show resolved Hide resolved
src/ur/robot_receive_timeout.cpp Outdated Show resolved Hide resolved
src/ur/robot_receive_timeout.cpp Outdated Show resolved Hide resolved
tests/test_control_mode.cpp Outdated Show resolved Hide resolved
urmahp and others added 4 commits September 21, 2023 07:59
Co-authored-by: Felix Exner (fexner) <felix_mauch@web.de>
Usually it happens if two UrDriver objects are created to close to each other, this change should catch this
@urmahp
Copy link
Collaborator Author

urmahp commented Sep 21, 2023

I have updated the PR in relation to the comments and added a better test coverage of the changes.

@urrsk urrsk requested a review from fmauch September 22, 2023 06:46
Copy link
Collaborator

@fmauch fmauch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me! I'll deploy it on my test system and merge if everything goes fine on the real robot.

@fmauch fmauch merged commit c3bead7 into UniversalRobots:master Sep 22, 2023
17 of 19 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants