-
Notifications
You must be signed in to change notification settings - Fork 1k
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
WIP: Upgrade MoveIt new KinematicBase API #476
WIP: Upgrade MoveIt new KinematicBase API #476
Conversation
This is WIP because I made a fix in this other PR that needs to be copied over here (once that's merged): Having multiple used forks is very complicated... |
It would probably have been sufficient to just submit the PRs here. @fmauch could then rebase his branch and things would've worked out. |
I submitted the PR to the branch used by the official UR driver, which seems to carry more weight than this repo. Thanks for responding to UniversalRobots/Universal_Robots_ROS_Driver#84 |
I'm not sure I follow this. It's also not true. |
I'd like to avoid rebasing the branch being used for the driver currently, as from my experience people not too familiar with git will get confused if they can't do a normal So, in fact, I will vote for merging those here, as well. |
I'm referring to the Where it says:
We are using the new (OEM) UR driver and thus, using this fork. Based on past experience of the ur_modern_driver, I know it can take years to sort out forking of UR-related repositories and so I have no confidence that https://github.com/ros-industrial/universal_robot/ is relevant anymore. However, I know the current situation is not ideal so I've created PRs to both locations in attempt to do the best thing possible. |
@fmauch I think a merge in the branch will be hard to rectify in the future, or at least be ugly in the commit history Maybe we should just wait before merging my PRs until the calibration branch is sorted out. How long will that be? Having end-users work from a feature branch is really not ideal for the reasons your just outlined. |
Ok. |
2bade90
to
6a64eb0
Compare
What's the status of this? |
Since the changes from this PR seem to make the plugin crash, I'll remove it from the milestone. Changing this after the first binary release should not be much of an issue. |
The crash is due to robot_model_ in the plugin shadowing the "newly" * introduced robot_model_ in KinematicsBase, sorry for not opening a PR earlier
|
Closing this in favor of #644 |
@rhaske updated the kinematics API in Melodic, this fixes deprecation warnings
I also submitted this here: fmauch#3