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

create_autonomy: 1.3.0-0 in 'indigo/distribution.yaml' [bloom] #18174

Closed
wants to merge 1 commit into from

Conversation

jacobperron
Copy link
Contributor

Increasing version of package(s) in repository create_autonomy to 1.3.0-0:

ca_description

* Migrate to package.xml format 2
  
  Minor linting to package files.
* Update install rules
* Refactor launch files and expose robot base and odometry frame IDs as parameters
* Refactor CMakeLists.txt and package.xml files and add missing install rules
* Contributors: Jacob Perron

ca_driver

* Add explicit dependency on catkin_EXPORTED_TARGETS
  
  This ensures ca_msgs is built before ca_driver.
* Migrate to package.xml format 2
  
  Minor linting to package files.
* Add roslint test and fix lint issues
* find_package libcreate instead of downloading as external project
* Add support for defining and playing songs
* Update install rules
* Refactor launch files and expose robot base and odometry frame IDs as parameters
* Refactor CMakeLists.txt and package.xml files and add missing install rules
* Contributors: Clyde McQueen, Jacob Perron

ca_msgs

* Migrate to package.xml format 2
  
  Minor linting to package files.
* Add support for defining and playing songs
* Refactor CMakeLists.txt and package.xml files and add missing install rules
* Contributors: Clyde McQueen, Jacob Perron

ca_tools

* Migrate to package.xml format 2
  
  Minor linting to package files.
* Update install rules
* Refactor launch files and expose robot base and odometry frame IDs as parameters
* Refactor CMakeLists.txt and package.xml files and add missing install rules
* Contributors: Jacob Perron

create_autonomy

* Migrate to package.xml format 2
  
  Minor linting to package files.
* Contributors: Jacob Perron

@mikaelarguedas
Copy link
Member

While the repository name is explicit, the package names are not.
Please consider renaming them so that users can know what robot they apply to just by reading the package name.
More information can be found on the ROS Naming Conventions wiki page

@jacobperron
Copy link
Contributor Author

jacobperron commented Jun 11, 2018

@mikaelarguedas The current ca_ naming scheme arose because otherwise it might clash with the existing create_description and create_driver. I could rename ca_driver to create_base, but I'm not sure what to do with the description package.

Another possibility is to expand the prefix to create_autonomy_. ie:

  • create_autonomy_description
  • create_autonomy_driver
  • create_autonomy_msgs
  • create_autonomy_tools

Originally, I thought that this may be too verbose. Thoughts?

@tfoote
Copy link
Member

tfoote commented Jun 11, 2018

It does feel long to type out, but I'd rather error on the side of being more verbose than less. Most of the time you won't type the whole thing out anyway, with tab completion, code auto complete, or using namespaces. I'd suggest going ahead with those names.

For some of them the name clash begs the question. Can they be merged/collapsed? For example could your create_autonomy_description be merged into the current create_description and not need the semi-duplicated package.

Also note that the _tools repo seems to be launch files so it would more classically be called _bringup following REP 144

@jacobperron
Copy link
Contributor Author

jacobperron commented Jun 12, 2018

@tfoote Thanks for the feedback.

It seems like a good idea to merge the description packages. It would involve adding URDF/mesh files for Create 2.

But the existing create_driver package is a Python module (used by turtlebot_node in the package create_node) whereas the create_autonomy driver is a ROS node for Create/Roomba platforms (no turtlebot specifics). Seems like the create_node package would be more aptly named turtlebot_node.

Perhaps more trouble than it's worth, but I would refactor the turtlebot packages to make use of either the create_autonomy driver or its underlying libcreate...

For now, I'll proceed with merging the description packages and renaming create_autonomy's packages.

@tfoote
Copy link
Member

tfoote commented Jun 13, 2018

@jacobperron thanks for being willing to do the renaming. adding the new descriptions will definitely be a good thing.

The create_node was designed to be used in the TurtleBot but it's specifically implemented to be reusable outside the context of the TurtleBot.

It would be great to update the TurtleBot support to be able to use either implementation if you have the time. Unfortunately I don't have the create hardware available for testing.

@tfoote
Copy link
Member

tfoote commented Jun 19, 2018

@jacobperron I'm going to close this and the other PRs for lunar and kinetic for now pending the rename. Once the packages are renamed, bloom can open new prs automatically.

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.

None yet

3 participants