-
Notifications
You must be signed in to change notification settings - Fork 523
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
Modify README for the port to MoveIt! 2.0 #1
Conversation
Dear @vmayoral and @davetcoleman, I'm not sure who decided to perform MoveIt ROS2 development within a new repository clone. This was not discussed in a maintainer meeting and I don't like the idea, because this makes it much harder to cherry-pick changes back and forth between ROS1 and ROS2 development branches. |
Several reasons led us to this decision:
|
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.
A few suggestions
README.md
Outdated
- [Overview of MoveIt! for ROS 2.0](#) | ||
- [Installation Instructions](#) | ||
- [Documentation](#) | ||
- [Get Involved](#) |
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.
these are all dead links, i think they should still point to the previous locations. our intent is not to kill moveit.ros.org
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.
Right, that's a leftover 8b730c8
README.md
Outdated
- [Documentation](http://moveit.ros.org/documentation/) | ||
- [Get Involved](http://moveit.ros.org/documentation/contributing/) | ||
## Legal disclaimer | ||
**WORK IN PROGRESS** |
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.
why is this work in progress?
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.
Solved in the commit described below.
README.md
Outdated
|
||
## Continuous Integration Status | ||
*The material and information contained in this repository is published in good faith and is for the benefit of the general ROS 2.0 community only. You should not rely upon the material or information in this repository as a basis for making any business, legal or any other decisions. Acutronic Robotics makes no representations, claims, promises, guarantees or warranties of any kind, express or implied about the completeness, accuracy, reliability, suitability or adequacy with respect to the software, information or related graphics contained in this repository for any purpose. Acutronic Robotics is not liable for any loss, damage or injury that may arise in connection with this repository. Any action or reliance you take or place based upon the information in this repository is strictly at your own risk.* |
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.
the BSD license covers these issues already, this disclaimer is not needed
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.
README.md
Outdated
|
||
[![Build Status](https://travis-ci.org/AcutronicRobotics/moveit2.svg?branch=ros2)](https://travis-ci.org/AcutronicRobotics/moveit2) |
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.
I've just turned on Travis for moveit2:
https://travis-ci.org/ros-planning/moveit2
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.
f925d83, please review. Note I pointed to master
branch.
Hello @rhaschke,
I'm sorry if we stepped on someone's toes, our intention is to contribute. Our team will be happy to participate in the next developers' session. We started working at https://github.com/AcutronicRobotics/moveit2-original (which should disappear as soon as we stabilize things here, or somewhere else) for the same reasons @davetcoleman provides above, in particular and mainly, to have a clean, easy to manage and accesible (to our team) repository. Overall, to advance faster.
True, it will become harder but arguably, and as far as I can see for now, the difficulty of cherry-picking is somewhat manageable. Changes from MoveIt! in ROS 1 (for now and while we keep things coherent) shouldn't imply much more than: git remote add moveit https://github.com/ros-planning/moveit
git fetch moveit
git cherry-pick ... |
That one valid argument.
So you started the new repo from the current melodic-branch including all history?
Would be great if you could share your ideas in one of the next maintainer meetings 😉
Do you want to relax the reviewing process to improve on this? If not, I don't see how you can progress faster on MoveIt2 than on MoveIt1. The group of people doing reviews stays the same, doesn't it? |
Point to official MoveIt! repos
Co-Authored-By: vmayoral <[email protected]>
Dear @vmayoral I appreciate very much your and your team's involvement in migrating MoveIt to ROS2. Thank you very much for this effort in advance! However, I was surprised and confused that we have chosen the approach of a new repo, because we discussed to keep things in sync as long as possible. Now, I'm afraid that the two repos will quickly diverge. |
@rhaschke, thanks for your quick response.
Well this is certainly concerning. Our team is certainly open to consider other possibilities. Maybe having weekly sync-ups with moveit main branch would help @rhaschke? |
Point to the issue for further discussion. Co-Authored-By: vmayoral <[email protected]>
As you started from MoveIt1's branch, syncing should be indeed feasible. |
You already found the source of this issue being a mistaken git push: moveit/moveit#1356 Overall, your concerns @rhaschke are all valid, I think we clarified some in an email exchange and we can discuss further at next Thu's maintainer meeting |
Convert geometry_msgs/Transform to geometry_msgs/TransformStamped
Refactor Global and Local Planner Components into NodeClasses Add a simple launch test (#1) Try to fix plugin export; add helpful debug when stuck Error if global planning fails READY and AWAIT_TRAJ states are redundant Lock the planning scene as briefly as possible Specify joint group when checking waypoint distance Implement a reset() function in the local planner Detect when the local planner gets stuck
* adding rosdep install and catkin config to instructions * updating visualization tutorial
No description provided.