Add tests for launch_utils of controller manager

[saikishor]

Background

We have nice utilities for spawner launching of controllers, however, there are no tests or no usage of them in ros2_control_demos to have them tested via functional tests that we already have.

It would be nice to have tests in place for these launch_utils to avoid situations like in PR : #2146.

Instructions

Hi, this is a good-first-issue issue. This means we've worked to make it more legible to people who either haven't contributed to our codebase before, or even folks who haven't contributed to open source before.

We're interested in helping you take the first step, and can answer questions and help you out along the way. Note that we're especially interested in contributions from underrepresented groups!

We know that creating a pull request is the biggest barrier for new contributors. This issue is for you 💝

If you have contributed before, consider leaving this PR for someone new, and looking through our general bug issues. Thanks!

🤔 What you will need to know.

Nothing. This issue is meant to welcome you to Open Source :) We are happy to walk you through the process.

📋 Step by Step

• [ ] 🙋 Claim this issue: Comment below. If someone else has claimed it, ask if they've opened a pull request already and if they're stuck -- maybe you can help them solve a problem or move it along!

• [ ] 🗄️ Create a local workspace for making your changes and testing following these instructions, for Step 3 use "Download Source Code" section with these instructions.

• [ ] 🍴 Fork the repository using the handy button at the top of the repository page and clone it into ~/ws_ros2_control/src/ros-controls/ros2_control, here is a guide that you can follow (You will have to remove or empty the existing ros2_control folder before cloning your own fork)

• [ ] Checkout a new branch using git checkout -b <branch_name>

• [ ] 🤖 Apply pre-commit auto formatting, by running pip3 install pre-commit and running pre-commit install in the ros2_control repo.

• [ ] 💾 Commit and Push your changes

• [ ] 🔀 Start a Pull Request to request to merge your code into master. There are two ways that you can start a pull request:

1. If you are not familiar with GitHub or how to create a pull request, here is a guide you can follow on how GitHub works.

• [ ] 🏁 Done Ask in comments for a review :)

Is someone else already working on this?

🔗- We encourage contributors to link to the original issue in their pull request so all users can easily see if someone's already started on it.

👥- If someone seems stuck, offer them some help!

🤔❓ Questions?

Don’t hesitate to ask questions or to get help if you feel like you are getting stuck. For example leave a comment below! Furthermore, you find helpful resources here:

• ROS2 Control Contribution Guide
• ROS2 Tutorials
• Robotics Stack Exchange

Good luck with your first issue!

[Ayushman-Saha]

Hi, I'm a 2nd year student with a keen interest in robotics and I'm looking to get started with open source contributions. I saw this "good-first-issue" and it seems like a great opportunity. I'd like to claim it and work on it.

Regards Ayushman Saha

[Mehul0x]

Hi, Can I help with this too?

I suppose ROS2-Testing has enough information to do this?

[saikishor]

Hi, Can I help with this too?

I suppose ROS2-Testing has enough information to do this?

@Mehul0x Unfortunately, we can only assign it to one person. As @Ayushman-Saha approached this first, he gets to work on it and probably he is already on it

[christophfroehlich]

@Ayushman-Saha there is no PR linked to this issue, so I suppose that this isn't fixed and you are not right working on it? @Mehul0x still interested?

[robkwan]

Hello! I am new to this open-source contribution and I would like to ask if it is ok to try working on it as "good first issue" please. My plan is:- 1. create the /ros2_control/control_manager/test_launch_utils folder 2. create the test_launch_utils.py inside the folder to validate if: test_launch_utils.py::test_generate_spawner_from_list_basic PASSED test_launch_utils.py::test_generate_spawner_from_dict_basic PASSED test_launch_utils.py::test_pr_2146_dict_keys_converted_to_list PASSED ← THE KEY TEST! test_launch_utils.py::test_empty_list_handled PASSED test_launch_utils.py::test_empty_dict_handled PASSED 3. Update CMakeLists.txt for the new test_launch_utils.py. Would this be ok pls? Thanks.

[christophfroehlich]

Welcome! Have a look at the launch_utils.py, there are three entry points

• generate_controllers_spawner_launch_description
• generate_controllers_spawner_launch_description_from_dict
• generate_load_controller_launch_description

Copy controller_manager/test/test_ros2_control_node_launch.py and integrate them.

[robkwan]

Thanks @christophfroehlich ! Will look into it as you suggested then.

[robkwan]

Hi @christophfroehlich, I'd like to ask a few questions to clarify pls.

1. I did try to integrate into the controller_manager/test/test_ros2_control_node_launch.py and when I run: pytest src/ros2_control/controller_manager/test/test_ros2_control_node_launch.py -v it would pass ok but it would not work for "colcon test" method. That's why I also added another controller_manager/test/test_launch_utils.py separately, so that the "colcon test --packages-select controller_manager --event-handlers console_direct+" will also cover the test and passed ok. Would this be redundant pls?!

2. My setup environment can work with ROS jazzy only and so I used branch jazzy for the work only. Will it be ok pls?

Not sure if I should clarify these before creating the pull request?!

Thanks & Regards

[christophfroehlich]

I don't understand the first question, what is the difference for colcon? As one launch_testing python file works only with one launch description, you might have to duplicate the existing file and just use different ways in setting up the LD. or you make separate test cases for spawners later on.

you can develop on jazzy, but please rebase your changes on top of master before creating the PR.

[robkwan]

Thanks for prompt reply. My first question is related to this:-

Even though ament_add_pytest_test(test_ros2_control_node test/test_ros2_control_node_launch.py) is already in CMakeLists.txt, "colcon test" approach would not trigger the test_ros2_control_node_launch.py but I can get it working with
ament_add_pytest_test(test_launch_utils test/test_launch_utils.py APPEND_ENV AMENT_PREFIX_PATH=${ament_index_build_path} PYTHONPATH=${CMAKE_CURRENT_BINARY_DIR}:${CMAKE_CURRENT_SOURCE_DIR}:${PYTHONPATH} )
But I know that it could be related to the way of my implementation. I will need to look into it more then. Thanks.

[robkwan]

Hello @christophfroehlich, I made changes to get it working in the controller_manager/test/test_ros2_control_node_launch.py and I did the rebase but I'm still not that confident if I did it correctly or not. Anyway, I just submitted the pull request for your review please. Thanks.

[robkwan]

Oops! I realized that I just rebased for the jazzy branch only and will do that for the master next pls...thanks.

[robkwan]

Hello @christophfroehlich , I just rebased and merged back to the master branch and hope that it would be ok this time. Thanks!

[christophfroehlich]

But you haven't updated your PR?

[robkwan]

Hi @christophfroehlich, sorry that I thought that already did it with "git push --force-with-lease origin master" before but it didn't! Therefore, I redid everything again and clicked "Create Pull Request" from https://github.com/ros-controls/ros2_control/compare/master...robkwan:master. And I hope that it will work correctly this time...Thanks!