Skip to content
This repository has been archived by the owner on Jan 27, 2020. It is now read-only.

[QUESTION] Naming and location #18

Closed
hidmic opened this issue Feb 28, 2019 · 9 comments
Closed

[QUESTION] Naming and location #18

hidmic opened this issue Feb 28, 2019 · 9 comments
Assignees

Comments

@hidmic
Copy link
Contributor

hidmic commented Feb 28, 2019

As this testing framework matures and attains widespread use throughout ROS 2 packages, a couple questions come up:

  • Should it have a more generic name?
  • Where should it live?
@hidmic
Copy link
Contributor Author

hidmic commented Feb 28, 2019

Ping @wjwwood @mjcarroll @sloretz.

@mjcarroll
Copy link
Contributor

I think it would be best to live in ros2/rostest, if the Apex folks are okay with that.

@pbaughman
Copy link
Contributor

I think @dejanpan can answer for the Apex side of this question.

@wjwwood
Copy link
Contributor

wjwwood commented Feb 28, 2019

Well there's actually two things in this repository right now, launch testing stuff and ROS specific launch testing stuff. This is my invisioned end state:

  • repository ros2/launch:
    • launch
    • launch_testing
  • repository ros2/launch_ros:
    • launch_ros
    • test_launch_ros
    • ros2launch
  • repository ros2/rostest:
    • rostest

Not all of these changes need to be realized right now (or to complete this task), the important thing is that the ROS agnostic code in this repository is moved to the launch or launch_testing packages in the ros2/launch repository, and that the rest of the code here is moved into the ros2 organization.

We don't have to split off the ros2/launch_ros repository yet, but we might as well while we're refactoring.

Just to reinforce why the split is needed, launch_ros and therefore code in this repository use rclpy, so rclpy itself and anything below it like rcl or maybe even rclcpp (if we end up needing it in launch_ros or rostest in the future) cannot use launch_ros or rostest, but they still need some facilities to do testing with launch, so instead they'd depend on and use launch_testing.


I had consider renaming this repository launch_ros_testing to follow the naming scheme in ROS 2 already, but rostest is more concise and you guys seem to prefer that, which I'm fine with. On the other hand, it has more collision with rostest from ROS 1. It's been nice to have some conceptual space between launch/launch_ros and roslaunch from ROS 1.

@hidmic
Copy link
Contributor Author

hidmic commented Mar 1, 2019

I think that makes a lot of sense @wjwwood. @dejanpan thoughts?

@pbaughman
Copy link
Contributor

This PR will remove the last rclpy dependency from apex_launchtest, and put in a CI check that will notify us if it's accidentally reintroduced. The dependency tree will look like this:

$ colcon list --topological-graph --packages-up-to apex_launchtest
ament_flake8        + *****
osrf_pycommon        +   *.
ament_pep257          +****
ament_copyright        + **
ament_index_python      + *
launch                   +*
apex_launchtest           +

@dejanpan
Copy link
Contributor

dejanpan commented Mar 4, 2019

@wjwwood @hidmic @mjcarroll @sloretz
regarding #18 (comment), Apex has no problem wrt to moving this repo into the ros2 org and renaming stuff. We can do it in whatever way as long as it will help make ROS 2 be more tested.

Now, @pbaughman and I think that we do not 100% understand the proposed split so we will try to repeat what @wjwwood said with the links:

And then we also have https://github.com/ApexAI/apex_rostest/tree/master/apex_launchtest_cmake (cmake integration) which would be needed for some of the above packages - which ones?

The 2 remaining Qs then are:

  1. who will do the refactor and split (we can as long as we well define the process)?
  2. will Pete and I have access to above 3 repos in ros2 org?

@wjwwood
Copy link
Contributor

wjwwood commented Mar 5, 2019

No not exactly, in my imagined layout test_launch_ros is a test package (indicated by starting with test_) which tests launch_ros itself. I was thinking that rostest in my above layout would be where https://github.com/ApexAI/apex_rostest/tree/master/apex_launchtest_ros would live. I was copying your name apex_rostest without the apex prefix.

And then we also have https://github.com/ApexAI/apex_rostest/tree/master/apex_launchtest_cmake (cmake integration) which would be needed for some of the above packages - which ones?

I would guess this goes in the rostest package, but it could be separated, that way the launch testing stuff could be used without cmake (e.g. from a pure Python package or something else).

@wjwwood
Copy link
Contributor

wjwwood commented Mar 20, 2019

I created this issue on the ros2/launch repository to track the work on this:

ros2/launch#208

Please have a look to make sure there are no large issues. We're beginning work on this now.

I'm going to close this in favor of that issue.

@wjwwood wjwwood closed this as completed Mar 20, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants