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

Refactor Package Structure #21

Merged
merged 19 commits into from
Apr 27, 2020
Merged

Refactor Package Structure #21

merged 19 commits into from
Apr 27, 2020

Conversation

lukicdarkoo
Copy link
Member

@lukicdarkoo lukicdarkoo commented Apr 23, 2020

Description
This aims to improve the structure of the packages, by renaming, deleting unnecessary code and adding proper documentation before it is released:

  • Now we have epuck_ros2_cpp but it should be language agnostic, so I think it should be renamed to epuck_ros2_driver
  • There is a Python implementation of the driver in epuck_ros2. That implementation doesn't implement as many features as in epuck_ros2_cpp and we have much better performances with epuck_ros2_cpp. To avoid maintaining the code and not to create confusion I think this should be deleted.
  • epuck_ros2 should depend on epuck_ros2_cpp and epuck_ros2_camera, and it should provide a mechanism to launch nodes from these packages.

Related Issues
Fixes #9
Also, this is related to this PR: cyberbotics/webots_ros2#69

Tasks

  • Rename epuck_ros2_cpp to epuck_ros2_driver
  • Delete Python driver implementation from epuck_ros2
  • Create launch file in epuck_ros2 that is able to run nodes from epuck_ros2_driver and epuck_ros2_camera
  • Add documentation
  • epuck_ros2 does not depends on epuck_ros2_camera and epuck_ros2_camera is ignored. This has to be resolved
    • Exclude MMAL related files from build process in CI (if -DAVOID_EPUCK_CAMERA_BUILD is defined MMAL will be excluded from the build process, otherwise no changes)
    • As epuck_ros2_camera is now part of code quality verification resolve linter errors
  • Verify if everything works on Raspberry Pi

Additional Context
ROS2 installation tutorial will be properly written later with #18.

For organizing documentation I followed this package:

  • short documentation in the repo root, with CI and references to more thoughtful documentation,
  • in the main package (epuck_ros2 in our case) there is no README,
  • every other package has a short explanation and
  • most of the documentation is located on the external website.

Building the packages in development on RPi:

colcon build --packages-select epuck_ros2 epuck_ros2_driver epuck_ros2_camera --cmake-args -DBUILD_TESTING:BOOL=OFF

@lukicdarkoo lukicdarkoo added the enhancement New feature or request label Apr 23, 2020
@lukicdarkoo lukicdarkoo self-assigned this Apr 23, 2020
@lukicdarkoo lukicdarkoo marked this pull request as ready for review April 24, 2020 14:05
Copy link
Member

@DavidMansolino DavidMansolino left a comment

Choose a reason for hiding this comment

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

Documentation check only for now.

Copy link
Member

@DavidMansolino DavidMansolino left a comment

Choose a reason for hiding this comment

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

Looks globally very good, just minor cleanups.

Copy link
Member

@DavidMansolino DavidMansolino left a comment

Choose a reason for hiding this comment

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

Thank you.

@lukicdarkoo lukicdarkoo merged commit 5220e8c into master Apr 27, 2020
@lukicdarkoo lukicdarkoo deleted the refactor-package-structure branch April 27, 2020 13:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Development

Successfully merging this pull request may close these issues.

Create documentation
2 participants