-
Notifications
You must be signed in to change notification settings - Fork 79
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
Support container in frontend #235
Support container in frontend #235
Conversation
Signed-off-by: Kenji Miyake <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Loos pretty good to me, I've left some minor comments
@ivanpauno Thank you for your quick review! I'll fix them. |
Signed-off-by: Kenji Miyake <[email protected]>
Signed-off-by: Kenji Miyake <[email protected]>
Signed-off-by: Kenji Miyake <[email protected]>
Signed-off-by: Kenji Miyake <[email protected]>
Signed-off-by: Kenji Miyake <[email protected]>
Fixed except for #235 (comment) (because there was a problem with it). |
Signed-off-by: Kenji Miyake <[email protected]>
c93d435
to
c119e94
Compare
Signed-off-by: Kenji Miyake <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall LGTM pending green CI
Signed-off-by: Kenji Miyake <[email protected]>
Signed-off-by: Kenji Miyake <[email protected]>
dad664d
to
cb67451
Compare
Signed-off-by: Kenji Miyake <[email protected]>
Signed-off-by: Kenji Miyake <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks @kenji-miyake !!
This PR could use a test case, that could be added here. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM too, and I agree with @ivanpauno this could use a few test cases.
Thank you for your contributions @kenji-miyake!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cool, thanks for adding this! LGTM pending some tests.
Signed-off-by: Kenji Miyake <[email protected]>
@kenji-miyake friendly ping |
@hidmic Oh, I'm sorry. Did you ask me to add tests? I couldn't recognize that from your conversations because there isn't a concrete request, sorry. 😢 |
I'm sorry, I couldn't make time recently. I think I can start to work! |
Yes! Sorry if it wasn't clear. |
Signed-off-by: Kenji Miyake <[email protected]>
db96302
to
7c5ecb9
Compare
@hidmic @ivanpauno @jacobperron I'm sorry to be late. Added tests! Could you review again, please? 😄 The following is my test result. ~/launch_test_ws/src/launch_ros/test_launch_ros support-container-in-frontend
❯ pytest -s test/test_launch_ros/frontend/test_component_container.py
=============================================================================================================================================================================================== test session starts ================================================================================================================================================================================================
platform linux -- Python 3.8.5, pytest-6.2.2, py-1.10.0, pluggy-0.13.0
rootdir: /home/kenji/launch_test_ws/src/launch_ros/test_launch_ros, configfile: pytest.ini
plugins: launch-testing-ros-0.14.2, launch-testing-0.17.0, ament-flake8-0.9.6, ament-lint-0.9.6, ament-pep257-0.9.6, ament-xmllint-0.9.6, ament-copyright-0.9.6, cov-2.11.1, asyncio-0.15.1, dash-1.20.0, repeat-0.9.1, rerunfailures-9.1.1, mock-1.10.4, colcon-core-0.6.1
collected 2 items
test/test_launch_ros/frontend/test_component_container.py [INFO] [launch]: All log files can be found below /home/kenji/.ros/log/2021-06-07-03-55-16-088632-desktop-550780
[INFO] [launch]: Default logging verbosity is set to INFO
1623005716.141647 [0] pytest: selected interface "lo" is not multicast-capable: disabling multicast
[INFO] [component_container-1]: process started with pid [550855]
[component_container-1] 1623005716.156574 [0] component_: selected interface "lo" is not multicast-capable: disabling multicast
[component_container-1] [INFO 1623005716.400550184] [my_container]: Load Library: /opt/ros/foxy/lib/libtalker_component.so
[component_container-1] [INFO 1623005716.401018869] [my_container]: Found class: rclcpp_components::NodeFactoryTemplate<composition::Talker>
[component_container-1] [INFO 1623005716.401033379] [my_container]: Instantiate class: rclcpp_components::NodeFactoryTemplate<composition::Talker>
[component_container-1] [INFO 1623005716.403538496] [my_container]: Load Library: /opt/ros/foxy/lib/liblistener_component.so
[INFO] [launch_ros.actions.load_composable_nodes]: Loaded node '/test_namespace/talker' in container '/my_container'
[component_container-1] [INFO 1623005716.403958001] [my_container]: Found class: rclcpp_components::NodeFactoryTemplate<composition::Listener>
[component_container-1] [INFO 1623005716.403970261] [my_container]: Instantiate class: rclcpp_components::NodeFactoryTemplate<composition::Listener>
[INFO] [launch_ros.actions.load_composable_nodes]: Loaded node '/test_namespace/listener' in container 'my_container'
[component_container-1] [INFO 1623005717.403391317] [test_namespace.talker]: Publishing: 'Hello World: 1'
[component_container-1] [INFO 1623005717.403531039] [test_namespace.listener]: I heard: [Hello World: 1]
[component_container-1] [INFO 1623005718.403373227] [test_namespace.talker]: Publishing: 'Hello World: 2'
[component_container-1] [INFO 1623005718.403473408] [test_namespace.listener]: I heard: [Hello World: 2]
[component_container-1] [INFO 1623005719.403712682] [test_namespace.talker]: Publishing: 'Hello World: 3'
[component_container-1] [INFO 1623005719.403811873] [test_namespace.listener]: I heard: [Hello World: 3]
[component_container-1] [INFO 1623005720.403378523] [test_namespace.talker]: Publishing: 'Hello World: 4'
[component_container-1] [INFO 1623005720.403479034] [test_namespace.listener]: I heard: [Hello World: 4]
[INFO] [component_container-1]: sending signal 'SIGINT' to process[component_container-1]
[component_container-1] [INFO 1623005721.403385760] [test_namespace.talker]: Publishing: 'Hello World: 5'
[component_container-1] [INFO 1623005721.403482981] [test_namespace.listener]: I heard: [Hello World: 5]
[component_container-1] [INFO 1623005721.611731658] [rclcpp]: signal_handler(signal_value=2)
[INFO] [component_container-1]: process has finished cleanly [pid 550855]
.[INFO] [launch]: All log files can be found below /home/kenji/.ros/log/2021-06-07-03-55-16-088632-desktop-550780
[INFO] [launch]: Default logging verbosity is set to INFO
1623005721.951976 [0] pytest: selected interface "lo" is not multicast-capable: disabling multicast
[INFO] [component_container-2]: process started with pid [550887]
[component_container-2] 1623005721.961697 [0] component_: selected interface "lo" is not multicast-capable: disabling multicast
[component_container-2] [INFO 1623005722.206660393] [my_container]: Load Library: /opt/ros/foxy/lib/libtalker_component.so
[component_container-2] [INFO 1623005722.207380071] [my_container]: Found class: rclcpp_components::NodeFactoryTemplate<composition::Talker>
[component_container-2] [INFO 1623005722.207419631] [my_container]: Instantiate class: rclcpp_components::NodeFactoryTemplate<composition::Talker>
[component_container-2] [INFO 1623005722.211512605] [my_container]: Load Library: /opt/ros/foxy/lib/liblistener_component.so
[INFO] [launch_ros.actions.load_composable_nodes]: Loaded node '/test_namespace/talker' in container '/my_container'
[component_container-2] [INFO 1623005722.212243743] [my_container]: Found class: rclcpp_components::NodeFactoryTemplate<composition::Listener>
[component_container-2] [INFO 1623005722.212270203] [my_container]: Instantiate class: rclcpp_components::NodeFactoryTemplate<composition::Listener>
[INFO] [launch_ros.actions.load_composable_nodes]: Loaded node '/test_namespace/listener' in container 'my_container'
[component_container-2] [INFO 1623005723.211132346] [test_namespace.talker]: Publishing: 'Hello World: 1'
[component_container-2] [INFO 1623005723.211273338] [test_namespace.listener]: I heard: [Hello World: 1]
[component_container-2] [INFO 1623005724.211093024] [test_namespace.talker]: Publishing: 'Hello World: 2'
[component_container-2] [INFO 1623005724.211193115] [test_namespace.listener]: I heard: [Hello World: 2]
[component_container-2] [INFO 1623005725.211097102] [test_namespace.talker]: Publishing: 'Hello World: 3'
[component_container-2] [INFO 1623005725.211197673] [test_namespace.listener]: I heard: [Hello World: 3]
[component_container-2] [INFO 1623005726.211102820] [test_namespace.talker]: Publishing: 'Hello World: 4'
[component_container-2] [INFO 1623005726.211201451] [test_namespace.listener]: I heard: [Hello World: 4]
[INFO] [component_container-2]: sending signal 'SIGINT' to process[component_container-2]
[component_container-2] [INFO 1623005727.211125241] [test_namespace.talker]: Publishing: 'Hello World: 5'
[component_container-2] [INFO 1623005727.211216542] [test_namespace.listener]: I heard: [Hello World: 5]
[component_container-2] [INFO 1623005727.421546433] [rclcpp]: signal_handler(signal_value=2)
[INFO] [component_container-2]: process has finished cleanly [pid 550887]
.
================================================================================================================================================================================================ 2 passed in 11.69s ================================================================================================================================================================================================ |
Signed-off-by: Kenji Miyake <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The added test seems reasonable to me!
test_launch_ros/test/test_launch_ros/frontend/test_component_container.py
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM with @ivanpauno comment addressed.
Signed-off-by: Kenji Miyake <[email protected]>
Thanks for the contribution @kenji-miyake !! |
Hello, I've added container support in launch frontend because it seems nothing is going on after #77.
I read https://answers.ros.org/question/333404/launch-composed-nodes-using-the-launch-xml-frontend/ and followed @ivanpauno's suggestion.
@ivanpauno Could you review this, please?
Following is the launch file I used.
How to test
In another terminal,