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

Trajectory interface #72

Merged
merged 8 commits into from
Jun 15, 2021
Merged

Conversation

fmauch
Copy link
Collaborator

@fmauch fmauch commented Jun 2, 2021

Adds the trajectory interface from the beta-testing branch.

@fmauch fmauch requested a review from t-schnell June 2, 2021 10:34
@fmauch
Copy link
Collaborator Author

fmauch commented Jun 2, 2021

Setting this back to draft, as @stefanscherzinger and I plan another iteration before merging. @t-schnell it would be nice if you could review that state nevertheless, as it reflects the things that you have done before.

@fmauch fmauch marked this pull request as draft June 2, 2021 11:23
@fmauch
Copy link
Collaborator Author

fmauch commented Jun 2, 2021

@stefanscherzinger 5a66723 and UniversalRobots/Universal_Robots_ROS_Driver@01a1cf0 introduce a done callback for trajectory forwarding. Do you think that this would be a way we could base on?

@stefanscherzinger
Copy link
Contributor

@fmauch , I only skimmed over 5a66723 , but UniversalRobots/Universal_Robots_ROS_Driver@01a1cf0 looks good as a basis. I'll see how to get that back into the PassThroughControllers and where/how to best convert the result flags.

@fmauch fmauch marked this pull request as ready for review June 7, 2021 08:54
Copy link
Collaborator

@t-schnell t-schnell left a comment

Choose a reason for hiding this comment

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

Mostly minor changes in documentation, biggest open question is setting the port for the additional socket.

include/ur_client_library/ur/ur_driver.h Outdated Show resolved Hide resolved
include/ur_client_library/ur/ur_driver.h Outdated Show resolved Hide resolved
include/ur_client_library/ur/ur_driver.h Outdated Show resolved Hide resolved
resources/external_control.urscript Outdated Show resolved Hide resolved
resources/external_control.urscript Outdated Show resolved Hide resolved
src/control/trajectory_point_interface.cpp Outdated Show resolved Hide resolved
src/control/trajectory_point_interface.cpp Outdated Show resolved Hide resolved
src/ur/ur_driver.cpp Outdated Show resolved Hide resolved
fmauch and others added 3 commits June 11, 2021 11:29
Thanks @t-schnell

Co-authored-by: t-schnell <tris.schnell@gmail.com>
We should not implicitly open a port that is not exposed in the API interface.
With adding this to the API interface (inside the UrDriver's constructor)
developers will explicitly know that this port will be opened.
@fmauch fmauch requested a review from t-schnell June 11, 2021 10:18
@fmauch
Copy link
Collaborator Author

fmauch commented Jun 11, 2021

@t-schnell I worked in all your suggestions. As you stated that otherwise this PR is alright, I will merge it on Monday if I don't hear back from you.

@t-schnell
Copy link
Collaborator

Looked over it (quickly), everything seems fine and can be merged.

@fmauch fmauch merged commit e528b4e into UniversalRobots:master Jun 15, 2021
@fmauch fmauch deleted the trajectory_interface branch June 15, 2021 08:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants