-
Notifications
You must be signed in to change notification settings - Fork 95
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
Conversation
53785d2
to
aae9a83
Compare
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. |
@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? |
@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. |
5a66723
to
68fb5ad
Compare
This will increase backwards compatibility.
68fb5ad
to
158dfa0
Compare
There was a problem hiding this 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.
Thanks @t-schnell Co-authored-by: t-schnell <[email protected]>
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.
@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. |
Looked over it (quickly), everything seems fine and can be merged. |
Adds the trajectory interface from the beta-testing branch.