-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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
ur_robot_driver: 2.1.0-1 in 'noetic/distribution.yaml' [bloom] #35593
Conversation
version: master | ||
release: | ||
packages: | ||
- controller_stopper |
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_stopper
is 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 into ros_control
development 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.ignore
the controller_stopper
in this first release and continue with the other packages - and then resolve the issue around controller_stopper
in 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.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Closing in favor of #35923 |
Increasing version of package(s) in repository
ur_robot_driver
to2.1.0-1
:noetic/distribution.yaml
0.11.2
null
controller_stopper
ur_calibration
ur_dashboard_msgs
ur_robot_driver