Skip to content

AMCL TransformListener reuses node rather than creating its own#538

Merged
2 commits merged intoros-navigation:masterfrom
sloretz:amcl-use-only-one-node
Jan 30, 2019
Merged

AMCL TransformListener reuses node rather than creating its own#538
2 commits merged intoros-navigation:masterfrom
sloretz:amcl-use-only-one-node

Conversation

@sloretz
Copy link
Copy Markdown
Contributor

@sloretz sloretz commented Jan 28, 2019 •
edited
Loading

Basic Info

Info Please fill out this column
Ticket(s) this addresses ros2/rcl#375
Primary OS tested on Ubuntu
Robotic platform tested on None

Description of contribution in a few bullet points

Future work that may be required in bullet points

  • None in this repo

@sloretz
Copy link
Copy Markdown
Contributor Author

sloretz commented Jan 28, 2019

Travis failure test_system_node failed in another PR #536 too. Unrelated to this change? https://travis-ci.org/ros-planning/navigation2/builds/484151523?utm_source=github_status&utm_medium=notification

@mkhansenbot mkhansenbot requested a review from a user January 28, 2019 21:11
@mkhansenbot
Copy link
Copy Markdown
Collaborator

mkhansenbot commented Jan 28, 2019

I'm pretty sure all the PRs are failing the test because of the same issue. I like this change but want @crdelsey to review, as it would probably trump #532. Also, I suspect there are other nodes with the same issue besides AMCL, which is why the test is still failing.

@sloretz
Copy link
Copy Markdown
Contributor Author

sloretz commented Jan 28, 2019

At least one more place experiencing the same issue is

 ros2 run nav2_navfn_planner navfn_planner __node:=navfn_planner

There is an extra node created here

https://github.com/ros-planning/navigation2/blob/1ae4a5493d7d9ddc3d0d1cc814050da8699b2e0d/nav2_tasks/include/nav2_tasks/service_client.hpp#L28-L32

Traceback here 
#0  __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:51
#1  0x00007ffff671f801 in __GI_abort () at abort.c:79
#2  0x00007ffff6d748b7 in ?? () from /usr/lib/x86_64-linux-gnu/libstdc++.so.6
#3  0x00007ffff6d7aa06 in ?? () from /usr/lib/x86_64-linux-gnu/libstdc++.so.6
#4  0x00007ffff6d7aa41 in std::terminate() () from /usr/lib/x86_64-linux-gnu/libstdc++.so.6
#5  0x00007ffff6d7ac74 in __cxa_throw () from /usr/lib/x86_64-linux-gnu/libstdc++.so.6
#6  0x00007ffff75f209c in rclcpp::exceptions::throw_from_rcl_error (ret=1, prefix="failed to initialize rcl node", error_state=0x7ffff7fc2b88, reset_error=0x7ffff7076cd4 <rcutils_reset_error>) at /home/sloretz/ws/ros2/src/ros2/rclcpp/rclcpp/src/rclcpp/exceptions.cpp:72
#7  0x00007ffff7628958 in rclcpp::node_interfaces::NodeBase::NodeBase (this=0x5555564acb20, node_name="GetCostmap_Node", namespace_="", context=std::shared_ptr<rclcpp::Context> (expired, weak count 0) = {...}, arguments=std::vector of length 0, capacity 0, use_global_arguments=true)
    at /home/sloretz/ws/ros2/src/ros2/rclcpp/rclcpp/src/rclcpp/node_interfaces/node_base.cpp:178
#8  0x00007ffff76237a0 in rclcpp::Node::Node (this=0x555555d27e30, node_name="GetCostmap_Node", namespace_="", context=std::shared_ptr<rclcpp::Context> (expired, weak count 0) = {...}, arguments=std::vector of length 0, capacity 0, 
    initial_parameters=std::vector of length 0, capacity 0, use_global_arguments=true, use_intra_process_comms=false, start_parameter_services=true) at /home/sloretz/ws/ros2/src/ros2/rclcpp/rclcpp/src/rclcpp/node.cpp:65
#9  0x00007ffff762361a in rclcpp::Node::Node (this=0x555555d27e30, node_name="GetCostmap_Node", namespace_="", use_intra_process_comms=false) at /home/sloretz/ws/ros2/src/ros2/rclcpp/rclcpp/src/rclcpp/node.cpp:52
#10 0x00007ffff7b5ec08 in __gnu_cxx::new_allocator<rclcpp::Node>::construct<rclcpp::Node, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > (this=0x7fffffff4947, __p=0x555555d27e30, __args#0=...) at /usr/include/c++/7/ext/new_allocator.h:136
#11 0x00007ffff7b5a6fc in std::allocator_traits<std::allocator<rclcpp::Node> >::construct<rclcpp::Node, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > (__a=..., __p=0x555555d27e30, __args#0=...) at /usr/include/c++/7/bits/alloc_traits.h:475
#12 0x00007ffff7b54a42 in std::_Sp_counted_ptr_inplace<rclcpp::Node, std::allocator<rclcpp::Node>, (__gnu_cxx::_Lock_policy)2>::_Sp_counted_ptr_inplace<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > (this=0x555555d27e20, __a=...)
    at /usr/include/c++/7/bits/shared_ptr_base.h:526
#13 0x00007ffff7b4d3db in std::__shared_count<(__gnu_cxx::_Lock_policy)2>::__shared_count<rclcpp::Node, std::allocator<rclcpp::Node>, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > (this=0x7fffffff4b48, __a=...)
    at /usr/include/c++/7/bits/shared_ptr_base.h:637
#14 0x00007ffff7b45474 in std::__shared_ptr<rclcpp::Node, (__gnu_cxx::_Lock_policy)2>::__shared_ptr<std::allocator<rclcpp::Node>, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > (this=0x7fffffff4b40, __tag=..., __a=...)
    at /usr/include/c++/7/bits/shared_ptr_base.h:1295
#15 0x00007ffff7b3daa3 in std::shared_ptr<rclcpp::Node>::shared_ptr<std::allocator<rclcpp::Node>, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > (this=0x7fffffff4b40, __tag=..., __a=...) at /usr/include/c++/7/bits/shared_ptr.h:344
#16 0x00007ffff7b378ec in std::allocate_shared<rclcpp::Node, std::allocator<rclcpp::Node>, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > (__a=..., __args#0=...) at /usr/include/c++/7/bits/shared_ptr.h:691
#17 0x00007ffff7b32437 in std::make_shared<rclcpp::Node, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > (__args#0=...) at /usr/include/c++/7/bits/shared_ptr.h:707
#18 0x00007ffff7b2cb66 in rclcpp::Node::make_shared<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > (args#0=...) at /home/sloretz/ws/ros2/install/rclcpp/include/rclcpp/node.hpp:70
#19 0x00007ffff7b29cfb in nav2_tasks::ServiceClient<nav2_msgs::srv::GetCostmap>::ServiceClient (this=0x55555576e9f0, name="GetCostmap") at /home/sloretz/navigation/install/nav2_tasks/include/nav2_tasks/service_client.hpp:30
#20 0x00007ffff7b28bd8 in nav2_tasks::CostmapServiceClient::CostmapServiceClient (this=0x55555576e9f0) at /home/sloretz/navigation/install/nav2_tasks/include/nav2_tasks/costmap_service_client.hpp:28
#21 0x00007ffff7b22263 in nav2_navfn_planner::NavfnPlanner::NavfnPlanner (this=0x55555576e920) at /home/sloretz/navigation/navigation2/nav2_navfn_planner/src/navfn_planner.cpp:52
#22 0x0000555555557a78 in __gnu_cxx::new_allocator<nav2_navfn_planner::NavfnPlanner>::construct<nav2_navfn_planner::NavfnPlanner> (this=0x7fffffff5597, __p=0x55555576e920) at /usr/include/c++/7/ext/new_allocator.h:136
#23 0x0000555555557931 in std::allocator_traits<std::allocator<nav2_navfn_planner::NavfnPlanner> >::construct<nav2_navfn_planner::NavfnPlanner> (__a=..., __p=0x55555576e920) at /usr/include/c++/7/bits/alloc_traits.h:475
#24 0x000055555555775c in std::_Sp_counted_ptr_inplace<nav2_navfn_planner::NavfnPlanner, std::allocator<nav2_navfn_planner::NavfnPlanner>, (__gnu_cxx::_Lock_policy)2>::_Sp_counted_ptr_inplace<>(std::allocator<nav2_navfn_planner::NavfnPlanner>) (this=0x55555576e910, __a=...)
    at /usr/include/c++/7/bits/shared_ptr_base.h:526
#25 0x0000555555557457 in std::__shared_count<(__gnu_cxx::_Lock_policy)2>::__shared_count<nav2_navfn_planner::NavfnPlanner, std::allocator<nav2_navfn_planner::NavfnPlanner>>(std::_Sp_make_shared_tag, nav2_navfn_planner::NavfnPlanner*, std::allocator<nav2_navfn_planner::NavfnPlanner> const&) (this=0x7fffffff56f8, __a=...) at /usr/include/c++/7/bits/shared_ptr_base.h:637
#26 0x000055555555730a in std::__shared_ptr<nav2_navfn_planner::NavfnPlanner, (__gnu_cxx::_Lock_policy)2>::__shared_ptr<std::allocator<nav2_navfn_planner::NavfnPlanner>>(std::_Sp_make_shared_tag, std::allocator<nav2_navfn_planner::NavfnPlanner> const&) (this=0x7fffffff56f0, 
    __tag=..., __a=...) at /usr/include/c++/7/bits/shared_ptr_base.h:1295
#27 0x0000555555557258 in std::shared_ptr<nav2_navfn_planner::NavfnPlanner>::shared_ptr<std::allocator<nav2_navfn_planner::NavfnPlanner>>(std::_Sp_make_shared_tag, std::allocator<nav2_navfn_planner::NavfnPlanner> const&) (this=0x7fffffff56f0, __tag=..., __a=...)
    at /usr/include/c++/7/bits/shared_ptr.h:344
#28 0x00005555555570f0 in std::allocate_shared<nav2_navfn_planner::NavfnPlanner, std::allocator<nav2_navfn_planner::NavfnPlanner>>(std::allocator<nav2_navfn_planner::NavfnPlanner> const&) (__a=...) at /usr/include/c++/7/bits/shared_ptr.h:691
#29 0x0000555555556ec7 in std::make_shared<nav2_navfn_planner::NavfnPlanner> () at /usr/include/c++/7/bits/shared_ptr.h:707
#30 0x0000555555556b31 in main (argc=2, argv=0x7fffffff5878) at /home/sloretz/navigation/navigation2/nav2_navfn_planner/src/main.cpp:22

I don't have more time to look into this at the moment. I think this PR is still a good idea, though it looks like #532 will still be needed to make CI green again.

@ghost
Copy link
Copy Markdown

ghost commented Jan 29, 2019

We had a brief meeting and the team prefers this approach over #532. I'll pick up where you left off and see if I can get this to pass CI tomorrow

@ghost
Copy link
Copy Markdown

ghost commented Jan 30, 2019

Well, this gets the system test passing in Travis, but broke everything else. I think the other CI builds are broken because I pushed to @sloretz branch, and now they are confused about what branch to pull from. I believe the build failures are not the fault of the changes in this PR.

@mkhansenbot mkhansenbot mentioned this pull request Jan 30, 2019
Merged
mkhansenbot
mkhansenbot approved these changes Jan 30, 2019
View reviewed changes
Copy link
Copy Markdown
Collaborator

@mkhansenbot mkhansenbot 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 and passed the PR CI. I think it's good to merge.

@ghost ghost mentioned this pull request Jan 30, 2019
Closed
@ghost ghost merged commit 9767501 into ros-navigation:master Jan 30, 2019
@ghost ghost mentioned this pull request Jan 30, 2019
Closed
@ghost ghost mentioned this pull request Feb 11, 2019
Merged
ghost pushed a commit that referenced this pull request Feb 12, 2019
Revert "Merge pull request #538 from sloretz/amcl-use-only-one-node"
a6df9a2 
This reverts commit 9767501, reversing
changes made to 1ae4a54.

Changes are no longer needed since the error this fixed has been turned
into a warning.

(cherry picked from commit 828a388)
mini-1235 pushed a commit to mini-1235/navigation2 that referenced this pull request Feb 5, 2025
@anaelle-sw
Collision monitor circle radius sub new param (ros-navigation#538)
4d461e5 
* update collision monitor configuration guide with new param radius_sub_topic

Signed-off-by: asarazin <anaelle.sarazin@robocc.com>

* update migration guide with new param radius_sub_topic for collision monitor

Signed-off-by: asarazin <anaelle.sarazin@robocc.com>

---------

Signed-off-by: asarazin <anaelle.sarazin@robocc.com>
Co-authored-by: asarazin <anaelle.sarazin@robocc.com>
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mkhansenbot mkhansenbot mkhansenbot approved these changes

@SteveMacenski SteveMacenski Awaiting requested review from SteveMacenski

@mhpanah mhpanah Awaiting requested review from mhpanah

+1 more reviewer

@orduno orduno orduno approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@sloretz @mkhansenbot @orduno