ur_robot_driver: 2.1.0-1 in 'noetic/distribution.yaml' [bloom] #35593
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -11253,6 +11253,26 @@ repositories: | |
| url: https://github.com/ros-industrial/ur_msgs.git | ||
| version: melodic-devel | ||
| status: maintained | ||
| ur_robot_driver: | ||
| doc: | ||
| type: git | ||
| url: https://github.com/UniversalRobots/Universal_Robots_ROS_Driver.git | ||
| version: master | ||
| release: | ||
| packages: | ||
| - controller_stopper | ||
| - ur_calibration | ||
| - ur_dashboard_msgs | ||
| - ur_robot_driver | ||
| tags: | ||
| release: release/noetic/{package}/{version} | ||
| url: https://github.com/UniversalRobots/Universal_Robots_ROS_Driver-release.git | ||
| version: 2.1.0-1 | ||
| source: | ||
| type: git | ||
| url: https://github.com/UniversalRobots/Universal_Robots_ROS_Driver.git | ||
|
There was a problem hiding this comment. @fmauch I see a few LICENSE files in the source repository but none at the root. Would you mind adding a copy (or relocating one of the existing ones)? There was a problem hiding this comment. |
||
| version: master | ||
| status: developed | ||
| urdf: | ||
| doc: | ||
| type: git | ||
|
|
||
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
@fmauch
controller_stopperis a bit too generic for a package name. I understand it is to be used along with the http://wiki.ros.org/controller_manager package, which makes me wonder if it shouldn't be over at https://github.com/ros-controls/ros_control instead. @audrow thoughts?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.
Yes, the packages was indeed meant to be a generic node to interact with the
controller_manager. Since I had implemented something similar using a KUKA robot in the past, I thought it would be good to make this a generic node, hence the generic name. However, since I'm not too deeply involved intoros_controldevelopment I think it would be better to keep it here at the moment. I assume the location of this package could be changed again, later.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 do agree that the name may be too generic. If we're going to keep it in this repo, maybe we could change the name to be less generic. If it does move over to
ros_control, then maybe we can go back to the generic name.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.
Thanks for the feedback. Although I do not fully agree, I also understand the concerns raised.
For the record: I've created an issue at ros-controls/ros_control#510 but we'll be thinking about a more suitable name in parallel.
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'm really looking forward to this release 👍
would it be an option to
noetic.ignorethecontroller_stopperin this first release and continue with the other packages - and then resolve the issue aroundcontroller_stopperin the next iteration?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.
Leaving out the controller_stopper is not really an option, as it is a runtime dependency for the driver (at least from its launchfiles).
We've been discussing this internally and we basically see two ways forward: Either we merge the controller_stopper into the driver's package, as we did for ROS2 (as it is a bit more UR-specific there) or we leave it with the current name and publish it like that. The latter would leave the option to migrate the package to ros_control at some point. @audrow do you think the second option would be possible? Otherwise I will go ahead and drop the standalone package.