Add robnux_arm_kinematics_trajectory repo.

[guanfengliu]

Please Add This Package to be indexed in the rosdistro.

robnux_arm_kinematics_trajectory

The source is here:

https://github.com/ROBNUX/robnux_arm_kinematics_trajectory.git

Checks

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

[mjcarroll]

Holding for Rolling sync

[guanfengliu]

Don't know why rosdistro validation failed? I generated 1.0.0 release tag in https://github.com/ROBNUX/arm_kinematics_trajectory. But don't know how to retrigger pipeline.

[guanfengliu]

Left comments for rolling. The same applies to noetic. Addressed. Thanks.

[guanfengliu]

Hmm. still don't know why rosdep checks have errors.

[mjcarroll]

@cottsay can we take a look at why the checks are failing here?

This seems very much unrelated to what is in the PR here.

Error: Package 'clang-native@meta-clang' could not be found for openembedded master on 

Error: Package 'graphviz@meta-ros-common' could not be found for openembedded master on 

[cottsay]

can we take a look at why the checks are failing here? This seems very much unrelated to what is in the PR here.

Indeed. The old automation is unable to disambiguate between lines being added by a PR, and lines which have been removed on master by commits not present in this PR's history. The new rosdistro-reviewer automation solves the issue, but that check hasn't been moved over to the new automation yet.

Rebasing the PR or merging from master should solve the issue. I'd do it myself, but this PR does not allow write access by rosdistro maintainers.

[MichaelOrlov]

@guanfengliu, Can you please rebase your branch on top of the latest master? It might help to fix CI warnings.

[MichaelOrlov]

@Yadunund Friendly ping here to re-iterate on review.

[cottsay]

Alright, I was able to take a look at this repository and after discussing with some other developers, I don't believe that this repository is appropriate for inclusion in the official index in its current state. I have two specific points:

1. All of the packages in this repository are catkin packages, so it can't be included in Rolling at all.
2. The names of the individual packages are FAR too broad for inclusion in the official index, and don't accurately describe the scope of each package. For example, a package simply named Logger is simply not appropriate.

The path forward here is to choose more appropriate names for the packages. For reference, here are the packages I found in the repo:

Logger		(ros.catkin)
intp		(ros.catkin)
kdl_common	(ros.catkin)
kinematics_map	(ros.catkin)
quattro		(ros.catkin)
rob_commands	(ros.catkin)
scurve_lib	(ros.catkin)
trajectory	(ros.catkin)

[guanfengliu]

Alright, I was able to take a look at this repository and after discussing with some other developers, I don't believe that this repository is appropriate for inclusion in the official index in its current state. I have two specific points:

1. All of the packages in this repository are `catkin` packages, so it can't be included in Rolling at all.

2. The names of the individual packages are FAR too broad for inclusion in the official index, and don't accurately describe the scope of each package. For example, a package simply named `Logger` is simply not appropriate.

The path forward here is to choose more appropriate names for the packages. For reference, here are the packages I found in the repo:

Logger		(ros.catkin)
intp		(ros.catkin)
kdl_common	(ros.catkin)
kinematics_map	(ros.catkin)
quattro		(ros.catkin)
rob_commands	(ros.catkin)
scurve_lib	(ros.catkin)
trajectory	(ros.catkin)

Hmm, I understand the naming issue of the packages, will modify that. But what is the issue with catkin based package? Doesn't that be aligned with ros convention?

[cottsay]

...what is the issue with catkin based package? Doesn't that be aligned with ros convention?

Catkin-type packages are used in ROS 1, so they're absolutely correct for Noetic. The Rolling distrubtion is ROS 2 which does not use catkin-type packages. Many of the package dependencies are also different, for example, there is no roscpp package in ROS 2.

Your packages need to be ported to ROS 2 before they can be built in a ROS 2 distribution.

[guanfengliu]

...what is the issue with catkin based package? Doesn't that be aligned with ros convention?

Catkin-type packages are used in ROS 1, so they're absolutely correct for Noetic. The Rolling distrubtion is ROS 2 which does not use catkin-type packages. Many of the package dependencies are also different, for example, there is no roscpp package in ROS 2.

Your packages need to be ported to ROS 2 before they can be built in a ROS 2 distribution.

okay. I will remove the changing to rolling. just leave it into noetic

[guanfengliu]

(1) changed the naming of each package so that they are more intuitive and matching the scope of the package (2) revert back the rolling

[guanfengliu]

(1) changed the naming of each package so that they are more intuitive and matching the scope of the package (2) revert back the rolling

Please help to review again.

[sloretz]

Holding for Noetic sync

[github-actions[bot]]

This PR hasn't been activity in 14 days. If you are still are interested in getting it merged please provide an update. Otherwise it will likely be closed by a rosdistro maintainer following our contributing policy. It's been labeled "stale" for visibility to the maintainers. If this label isn't appropriate, you can ask a maintainer to remove the label and add the 'persistent' label.

[guanfengliu]

Any chance to get reviewed again?

[christophebedard]

Thanks for renaming some packages. I think the trajectory and kdl_common packages are still too generic. I would suggest prefixing them with something, such as your organization name, e.g., robnux_trajectory/robnux_kdl_common. See REP 144: https://www.ros.org/reps/rep-0144.html.

[guanfengliu]

Thanks for renaming some packages. I think the trajectory and kdl_common packages are still too generic. I would suggest prefixing them with something, such as your organization name, e.g., robnux_trajectory/robnux_kdl_common. See REP 144: https://www.ros.org/reps/rep-0144.html.

Changed. Please review again

[christophebedard]

Please rebase your branch on the latest version of the master branch to fix unrelated issues.

[christophebedard]

Checklist:

• [x] At least one of the following must be present
  • [x] Top level license file: https://github.com/ROBNUX/arm_kinematics_trajectory/blob/main/LICENSE
  • [ ] Per package license files:
• [x] License is OSI-approved: MIT
• [x] License correctly listed in package.xmls
  • https://github.com/ROBNUX/arm_kinematics_trajectory/blob/d13e1df0cf66d3ea6375255e9764fd237adf9bb5/dsl_intp/package.xml#L9
  • https://github.com/ROBNUX/arm_kinematics_trajectory/blob/d13e1df0cf66d3ea6375255e9764fd237adf9bb5/quattro_bot/package.xml#L9
  • https://github.com/ROBNUX/arm_kinematics_trajectory/blob/d13e1df0cf66d3ea6375255e9764fd237adf9bb5/rob_motion_commands/package.xml#L9
  • https://github.com/ROBNUX/arm_kinematics_trajectory/blob/d13e1df0cf66d3ea6375255e9764fd237adf9bb5/robnux_kdl_common/package.xml#L9
  • https://github.com/ROBNUX/arm_kinematics_trajectory/blob/d13e1df0cf66d3ea6375255e9764fd237adf9bb5/robnux_kinematics_map/package.xml#L9
  • https://github.com/ROBNUX/arm_kinematics_trajectory/blob/d13e1df0cf66d3ea6375255e9764fd237adf9bb5/robnux_trajectory/package.xml#L9
  • https://github.com/ROBNUX/arm_kinematics_trajectory/blob/d13e1df0cf66d3ea6375255e9764fd237adf9bb5/scurve_lib/package.xml#L9
  • https://github.com/ROBNUX/arm_kinematics_trajectory/blob/d13e1df0cf66d3ea6375255e9764fd237adf9bb5/simple_motion_logger/package.xml#L9
• [x] Public source repo: https://github.com/ROBNUX/arm_kinematics_trajectory
• [x] Source repository contains ROS packages
• [x] Each package meets REP-144 naming conventions
  • I think the names are more reasonable now:
    • dsl_intp
    • quattro_bot
    • rob_motion_commands
    • robnux_kdl_common
    • robnux_kinematics_map
    • robnux_trajectory
    • scurve_lib
    • simple_motion_logger

[christophebedard]

@cottsay do you want to take another look at the package names? See my comment above.

[guanfengliu]

@Yadunund Looks like still needs your approval to be able to included in?

[christophebedard]

Looks like still needs your approval to be able to included in?

We can bypass that, but I noticed that Yadu pointed out you were using a 1.0.0 tag, which is now a branch: https://github.com/ros/rosdistro/pull/44353#discussion_r1941732425. I see that the packages on that branch don't have the same names: https://github.com/ROBNUX/arm_kinematics_trajectory/tree/1.0.0.

Please make sure that the branch you're using (source's version) has the packages with the corrected names. I would actually recommend using main as your version here. Later, when you actually create a new/first release of your packages, you can create a 1.0.0 tag, which will be used as release's version, see this line for example: https://github.com/ros/rosdistro/blob/3802b2dc826ce8adaed1e48ca85f7e10dd809a22/noetic/distribution.yaml#L9290.

[guanfengliu]

Looks like still needs your approval to be able to included in?

We can bypass that, but I noticed that Yadu pointed out you were using a 1.0.0 tag, which is now a branch: #44353 (comment). I see that the packages on that branch don't have the same names: https://github.com/ROBNUX/arm_kinematics_trajectory/tree/1.0.0.

Please make sure that the branch you're using (source's version) has the packages with the corrected names. I would actually recommend using main as your version here. Later, when you actually create a new/first release of your packages, you can create a 1.0.0 tag, which will be used as release's version, see this line for example:

https://github.com/ros/rosdistro/blob/3802b2dc826ce8adaed1e48ca85f7e10dd809a22/noetic/distribution.yaml#L9290 .

Update 1.0.0 branch. Please review it again.