Skip to content
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

Add picknik_controllers #37736

Merged
merged 3 commits into from
Jun 24, 2023
Merged

Conversation

moriarty
Copy link
Contributor

Please add these packages to be indexed in the rosdistro.

For Distributions

Humble, Iron, Rolling

Package names:

  • picknik_reset_fault_controller
  • picknik_twist_controller

The source is here:

https://github.com/PickNikRobotics/picknik_controllers

Purpose of using this:

The purpose of picknik_controllers repository is hold controllers that we are in the process of open sourcing and up-streaming.

The controllers in this repository have been prefixed with picknik_ so that if they are accepted upstream we will drop the prefix and release the prefixed versions with deprication warnings.
As we make changed requested upstream in ros-control/ros2_controllers we may need to keep both versions around during the transition phase.

These do not have a gbp-release package and have not been release they are currently source only.

Following the steps outlined here: https://docs.ros.org/en/rolling/How-To-Guides/Releasing/First-Time-Release.html This PR is for step 2

  1. Be part of a release team: Updates PickNik team membership ros2-gbp/ros2-gbp-github-org#281 I've added @MarqRazz and @moriarty
  2. https://docs.ros.org/en/rolling/How-To-Guides/Releasing/Release-Team-Repository.html#create-a-new-release-repository

Note:

I found the instructions were unclear here:

Create a new release repository
If your repository is new to the ROS community, you should first open a pull request on ros/rosdistro adding a source entry for your repository. The review process for the rosdistro database will ensure your repository and packages conform to the REP 144 package naming conventions and other requirements before release. Once your package name has been approved and merged, fill in the Add New Release Repositories issue issue template if you don’t have a release repo for your project yet.

Unclear in that there were no existing examples of how to create a source entry for your repository. and still have the names of the packages in the pull request to discuss. So I added them here but maybe they were only meant to be discussed in the pull request comments not actually in the code.

Checks

  • All packages have a declared license in the package.xml
  • This repository has a LICENSE file
  • This package is expected to build on the submitted rosdistro

The purpose of picknik_controllers repository is hold controllers that
we are in the process of open sourcing and upstreaming.

The controllers in this repository have been prefixed with `picknik_` so
that if they are accepted upstream we will drop the prefix and release
the prefixed versions with deprication warnings.

- picknik_reset_fault_controller has been discussed here https://roscontrol.slack.com/archives/C01HA09KDLH/p1686332900429009
- picknik_twist_controller ros-controls/ros2_controllers#300

These do not have a gbp-release package and have not been release they are currently source only.

Following the steps outlined here: https://docs.ros.org/en/rolling/How-To-Guides/Releasing/First-Time-Release.html
This PR is for step 2

1. Be part of a release team: ros2-gbp/ros2-gbp-github-org#281
2. https://docs.ros.org/en/rolling/How-To-Guides/Releasing/Release-Team-Repository.html#create-a-new-release-repository

Signed-off-by: Alex Moriarty <alex.moriarty@picknik.ai>
@github-actions github-actions bot added humble Issue/PR is for the ROS 2 Humble distribution iron Issue/PR is for the ROS 2 Iron distribution rolling Issue/PR is for the ROS 2 Rolling distribution labels Jun 20, 2023
Signed-off-by: Alex Moriarty <alex.moriarty@picknik.ai>
- PickNikRobots -> PickNikRobotics

Signed-off-by: Alex Moriarty <alex.moriarty@picknik.ai>
@nuclearsandwich
Copy link
Member

New package review:

@nuclearsandwich
Copy link
Member

Unclear in that there were no existing examples of how to create a source entry for your repository. and still have the names of the packages in the pull request to discuss. So I added them here but maybe they were only meant to be discussed in the pull request comments not actually in the code.

Thanks for sharing this feedback. The process here is still developing and so the feedback definitely helps us improve things. When reviewing new packages for inclusion in ROS distributions we get most of the information, including package names and license info, from the source repository linked. It definitely helps to see it in the PR description as it let's us know that the submitter has considered whether their package meets these requirements explicitly which at least for me, makes the review more straightforward. In my wishlist are some added checks which can check the submitted source repository and collect this information in an automated check to provide feedback immediately. Some repositories will have complicated licensing setups that require a human to confirm and REP-144 naming is not an automatable task but checking package sources and licenses and setting them in the PR somewhere is doable.

@nuclearsandwich nuclearsandwich merged commit bf35fe5 into ros:master Jun 24, 2023
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
humble Issue/PR is for the ROS 2 Humble distribution iron Issue/PR is for the ROS 2 Iron distribution rolling Issue/PR is for the ROS 2 Rolling distribution
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants