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

Define the term "ROS package" #265

Closed
wants to merge 2 commits into from
Closed

Conversation

rotu
Copy link
Contributor

@rotu rotu commented May 20, 2020

This term is used numerous times in this document, but is left undefined.

rotu added 2 commits May 20, 2020 16:29
This term is used numerous times in this document, but is left undefined.
rep-0149.rst Outdated
@@ -51,6 +51,12 @@ Motivation
This REP covers three separate topics which are described in the following
subsections.

A source tree containing a conformant ``package.xml`` file in its root
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is likely incorrect. I think Dirk will come over and mention that the package.xml has nothing to do with ROS, it has to do with catkin / ament.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the term "ROS Package" has to be this broad for the following statement to be correct:

<build_depend> (multiple)

Declares a rosdep key or ROS package name that this package requires at build-time. For system packages, the rosdep key will normally specify the "development" package, which frequently ends in "-dev".

This REP does use the term "catkin package", which seems to mean "a ROS package that uses build_type = catkin"

Copy link
Contributor

@SteveMacenski SteveMacenski May 20, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that's an argument to remove ROS in those instances, actually. This clarifications you added seem to be to be largely unnecessary and probably incorrect. I think we should correct those errors rather than clarify the potential error for the rest of the doc

Copy link
Contributor Author

@rotu rotu May 20, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think "ROS package" is unambiguous and defined exactly how it is used in the document. After all this is a "ROS Enhancement Proposal". It might be helpful to think of the "ROS" qualifier as "interoperable with the ROS community and associated tools".

I don't think it's sensible to just drop the qualifier. "Package" a very overloaded term, and we need a way to disambiguate between these things are often associated with ROS packages:

For instance rosidl_generator_c:

  • Is contained within a ROS package named rosidl_generator_c
  • Is built using a CMake project named rosidl_generator_c
  • Installs a CMake config-file package named rosidl_generator_c
  • Installs a Python import package named rosidl_generator_c

Copy link
Member

@tfoote tfoote left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Defining a ROS package a little bit more clearly probably makes sense, but this is not the right place. This is defining the format for the package.xml.

This specification was specifically written to be ROS independent. There's nothing related to robots in a package manifest format. This was done to allow related projects to leverage the format and tools but not have to call themselves a ROS package, however adding a generic package.xml can facilitate the ROS ecosystem as well as potentially others to leverage their software.

Somewhere in the ROS Overview documentation is probably the right place as that's where newcomers will find it.

Update: There is already a definition here under Filesystem Concepts: http://wiki.ros.org/Packages

@rotu
Copy link
Contributor Author

rotu commented May 21, 2020

Defining a ROS package a little bit more clearly probably makes sense, but this is not the right place. This is defining the format for the package.xml.

What do you want to call "the thing the package.xml file describes"? (Especially to disambiguate that thing from the language-specific or platform-specific definitions of "package")

I could coin a new thing like "REP-149 Package" or "rospkg Package" (which have ROS in the name, so no more ROS-agnostic) or "package.xml package" (which is objectively terrible).

"Ament package" is out since package.xml predates ament, and there are build_types ament_python, ament_cmake versus (presumably non-ament) python, cmake.

Similarly, "Colcon package" is out since it's used in other tools than Colcon and Colcon recognizes packages that don't use this manifest format. In fact, the colcon documentation doesn't even mention this file type.

This specification was specifically written to be ROS independent. There's nothing related to robots in a package manifest format. This was done to allow related projects to leverage the format and tools but not have to call themselves a ROS package, however adding a generic package.xml can facilitate the ROS ecosystem as well as potentially others to leverage their software.

How do you propose to replace the term "ROS package" in this document? This term is used to define the <*_depend> tags.

@rotu
Copy link
Contributor Author

rotu commented Jun 15, 2020

Closing and moving to issue #276

@rotu rotu closed this Jun 15, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants