-
Notifications
You must be signed in to change notification settings - Fork 337
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
Removed launch files, moved to ur_bringup
#246
Conversation
@ipa-hsd I believe you should target |
@miguelprada: you should be able to do that as well. I've just updated the target branch. @ipa-hsd: should we consider doing this the other way around? So keeping the launch files here in With the change suggested in this PR we again tightly couple |
@ipa-hsd: the PR is failing Travis as you didn't update this line. |
I wasn't sure what the rules of etiquette were regarding this. I will do it next time.
This sounds very reasonable. |
thanks for being so considerate. I would say it will depend a bit on the exact circumstances, but in most cases it should be more expedient for you to change the target branch. |
I am sorry but I usually prefer to keep the driver packages as simple as possible and don't include launch or configuration (yaml) files. See my comment ros-industrial/ros_industrial_issues#49 (comment) I usually divide the "bringup" layer into 3 types of packages (grouped in metapackages):
Also a note: Now that we want to support also the E-series, probably we will need more yaml, launch, URDFs and test files that will imply more complexity for this repo and makes much difficult keep a stable release version. |
@ipa-nhg What you say makes sense, but that would entail adding yet another repository/metapackage with the bringup package, separate from Having layers 1 and 3 of your proposed structure in one repository and layer 2 in another doesn't make much sense to me. In this setting both repositories have a circular dependency, which is not very nice, even if the packages themselves do not. Unless you want to add a third repository, I vote in favor of @gavanderhoorn's suggestion of keeping launch files in |
Totally agree. The only alternative I can propose is merge the layers 1 and 2. That means create a new metapackage (this repo probably) to hold the packages: And then the universal_robot repo should contain similar to the schunk approach : https://github.com/ipa320/schunk_modular_robotics and https://github.com/ipa320/schunk_robots. NOTE: if the urdf introduces a dependency to gazebo(?not sure..) and is not needed to start the driver, the description packages can also be part of the universal_robot repository |
That would tie together the description and msgs packages with the driver, which isn't very nice either. Also, the I still think that the best compromise is to keep layers 1 (i.e.
I believe the URDF is not used by the driver per-se right now (although it is included in the launch files for the individual controllers to use), but if we ever plan to include joint limit enforcement to the driver, which I think we should, then it will be required. |
Closing due to inactivity and because it's slowly drifting towards a more general discussion that would fit ros-industrial/ros_industrial_issues#49 better. |
No description provided.