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

Restructure rclcpp folders #264

Merged
merged 2 commits into from
Apr 17, 2020
Merged

Restructure rclcpp folders #264

merged 2 commits into from
Apr 17, 2020

Conversation

maryaB-osr
Copy link
Contributor

This isn't very important, but the way the rclpy packages are organized makes a nice landing place for the tutorials to point to based on the topic.

So in the python pub/sub tutorial for example, we can link right to rclpy/topics and the reader will see exactly what they need to see and nothing extra. Whereas if I wanted to do the same for the c++ pub/sub tutorial, I have to link to just rclcpp and the reader is bombarded by all these other packages they don't need to see. That's my reasoning, but again, it's not super important.

Also I wasn't sure if minimal_composition, minimal_timer and multithreaded_executor could all be placed in one folder or what that folder would be called.

Signed-off-by: maryaB-osr marya@openrobotics.org

Signed-off-by: maryaB-osr <marya@openrobotics.org>
@clalancette
Copy link
Contributor

So in the python pub/sub tutorial for example, we can link right to rclpy/topics and the reader will see exactly what they need to see and nothing extra. Whereas if I wanted to do the same for the c++ pub/sub tutorial, I have to link to just rclcpp and the reader is bombarded by all these other packages they don't need to see. That's my reasoning, but again, it's not super important.

Looking at this, I actually agree with your reasoning. Putting in the subdirectories gives it a bit of extra clarity that I like.

Also I wasn't sure if minimal_composition, minimal_timer and multithreaded_executor could all be placed in one folder or what that folder would be called.

No, they all deal with different aspects of what rclcpp can do. Going with this new structure, I think the directory structure should be something like this:

- rclcpp
    - actions
        - minimal_action_client
        - minimal_action_server
    - composition
        - minimal_composition
    - executors
        - multithreaded_executor
    - services
        - minimal_client
        - minimal_service
    - timers
        - minimal_timer
    - topics
        - minimal_publisher
        - minimal_subscriber

Separately we should add a "timer" tutorial to the rclpy stuff, but that is follow-up work.

@maryaB-osr Does that make sense to you?

Signed-off-by: maryaB-osr <marya@openrobotics.org>
@maryaB-osr
Copy link
Contributor Author

@clalancette Yup that makes sense to me. "Time", "executors/spinners" and "composition" are all on the list for new tutorials :)

Copy link
Contributor

@clalancette clalancette left a comment

Choose a reason for hiding this comment

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

Looks good to me. I'll run CI here to make sure nothing breaks.

@clalancette
Copy link
Contributor

CI:

  • Linux Build Status
  • Linux-aarch64 Build Status
  • macOS Build Status
  • Windows Build Status

@maryaB-osr maryaB-osr merged commit 50be004 into ros2:master Apr 17, 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.

2 participants