Initial commit of port of naive to ros 2 #8
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
griswaldbrooks
requested changes
Jul 13, 2022
ros2/naive/src/incrementer.cpp
Outdated
| : node_{std::move(node)}, | ||
| pub_{node_->create_publisher<std_msgs::msg::Int64>(out_topic,QUEUE_SIZE)}, | ||
| sub_{node_->create_subscription<std_msgs::msg::Int64>(in_topic, QUEUE_SIZE, [this](const std_msgs::msg::Int64::UniquePtr msg) | ||
| { |
There was a problem hiding this comment.
Formatting is wrong
ros2/naive/src/incrementer.cpp
Outdated
| Incrementer::Incrementer(std::shared_ptr<rclcpp::Node> node, const std::string& in_topic, const std::string& out_topic) | ||
| : node_{std::move(node)}, | ||
| pub_{node_->create_publisher<std_msgs::msg::Int64>(out_topic,QUEUE_SIZE)}, | ||
| sub_{node_->create_subscription<std_msgs::msg::Int64>(in_topic, QUEUE_SIZE, [this](const std_msgs::msg::Int64::UniquePtr msg) |
There was a problem hiding this comment.
Does
Suggested change
| sub_{node_->create_subscription<std_msgs::msg::Int64>(in_topic, QUEUE_SIZE, [this](const std_msgs::msg::Int64::UniquePtr msg) | |
| sub_{node_->create_subscription<std_msgs::msg::Int64>(in_topic, QUEUE_SIZE, [this](const auto& msg) |
work?
There was a problem hiding this comment.
Nope, in the ROS1 implementation the function parameter is auto&, but when I tried to keep it in the ROS2 implementation, this is the error that I got (I think the error is saying it can't deduce the type?):
In file included from /opt/ros/galactic/include/rclcpp/client.hpp:32,
from /opt/ros/galactic/include/rclcpp/callback_group.hpp:23,
from /opt/ros/galactic/include/rclcpp/any_executable.hpp:20,
from /opt/ros/galactic/include/rclcpp/memory_strategy.hpp:25,
from /opt/ros/galactic/include/rclcpp/memory_strategies.hpp:18,
from /opt/ros/galactic/include/rclcpp/executor_options.hpp:20,
from /opt/ros/galactic/include/rclcpp/executor.hpp:36,
from /opt/ros/galactic/include/rclcpp/executors/multi_threaded_executor.hpp:26,
from /opt/ros/galactic/include/rclcpp/executors.hpp:21,
from /opt/ros/galactic/include/rclcpp/rclcpp.hpp:156,
from /home/bilal/ws/src/ros2-testing-templates/naive/include/naive/incrementer.hpp:38,
from /home/bilal/ws/src/ros2-testing-templates/naive/src/incrementer.cpp:1:
/opt/ros/galactic/include/rclcpp/function_traits.hpp: In instantiation of ‘struct rclcpp::function_traits::function_traits<Incrementer::Incrementer(std::shared_ptr<rclcpp::Node>, const string&, const string&)::<lambda(const auto:37&)> >’:
/opt/ros/galactic/include/rclcpp/subscription_traits.hpp:97:8: required from ‘struct rclcpp::subscription_traits::has_message_type<Incrementer::Incrementer(std::shared_ptr<rclcpp::Node>, const string&, const string&)::<lambda(const auto:37&)>, std::allocator<void>, void, void, void, void>’
/opt/ros/galactic/include/rclcpp/node.hpp:220:5: required by substitution of ‘template<class MessageT, class CallbackT, class AllocatorT, class CallbackMessageT, class SubscriptionT, class MessageMemoryStrategyT> std::shared_ptr<SubscriptionT> rclcpp::Node::create_subscription(const string&, const rclcpp::QoS&, CallbackT&&, const rclcpp::SubscriptionOptionsWithAllocator<AllocatorT>&, typename MessageMemoryStrategyT::SharedPtr) [with MessageT = std_msgs::msg::Int64_<std::allocator<void> >; CallbackT = Incrementer::Incrementer(std::shared_ptr<rclcpp::Node>, const string&, const string&)::<lambda(const auto:37&)>; AllocatorT = std::allocator<void>; CallbackMessageT = <missing>; SubscriptionT = <missing>; MessageMemoryStrategyT = <missing>]’
/home/bilal/ws/src/ros2-testing-templates/naive/src/incrementer.cpp:15:84: required from here
/opt/ros/galactic/include/rclcpp/function_traits.hpp:51:9: error: decltype cannot resolve address of overloaded function
51 | using arguments = typename tuple_tail<
| ^~~~~~~~~
/opt/ros/galactic/include/rclcpp/function_traits.hpp:57:9: error: decltype cannot resolve address of overloaded function
57 | using argument_type = typename std::tuple_element<N, arguments>::type;
| ^~~~~~~~~~~~~
/opt/ros/galactic/include/rclcpp/function_traits.hpp:59:9: error: decltype cannot resolve address of overloaded function
59 | using return_type = typename function_traits<decltype( &FunctionT::operator())>::return_type;
| ^~~~~~~~~~~
/home/bilal/ws/src/ros2-testing-templates/naive/src/incrementer.cpp: In constructor ‘Incrementer::Incrementer(std::shared_ptr<rclcpp::Node>, const string&, const string&)’:
/home/bilal/ws/src/ros2-testing-templates/naive/src/incrementer.cpp:15:84: error: no matching function for call to ‘rclcpp::Node::create_subscription<std_msgs::msg::Int64>(const string&, const int&, Incrementer::Incrementer(std::shared_ptr<rclcpp::Node>, const string&, const string&)::<lambda(const auto:37&)>)’
15 | })} {}
| ^
In file included from /opt/ros/galactic/include/rclcpp/executors/single_threaded_executor.hpp:28,
from /opt/ros/galactic/include/rclcpp/executors.hpp:22,
from /opt/ros/galactic/include/rclcpp/rclcpp.hpp:156,
from /home/bilal/ws/src/ros2-testing-templates/naive/include/naive/incrementer.hpp:38,
from /home/bilal/ws/src/ros2-testing-templates/naive/src/incrementer.cpp:1:
/opt/ros/galactic/include/rclcpp/node.hpp:229:3: note: candidate: ‘template<class MessageT, class CallbackT, class AllocatorT, class CallbackMessageT, class SubscriptionT, class MessageMemoryStrategyT> std::shared_ptr<SubscriptionT> rclcpp::Node::create_subscription(const string&, const rclcpp::QoS&, CallbackT&&, const rclcpp::SubscriptionOptionsWithAllocator<AllocatorT>&, typename MessageMemoryStrategyT::SharedPtr)’
229 | create_subscription(
| ^~~~~~~~~~~~~~~~~~~
/opt/ros/galactic/include/rclcpp/node.hpp:229:3: note: substitution of deduced template arguments resulted in errors seen above
/home/bilal/ws/src/ros2-testing-templates/naive/src/incrementer.cpp:15:85: error: no matching function for call to ‘std::shared_ptr<rclcpp::Subscription<std_msgs::msg::Int64_<std::allocator<void> > > >::shared_ptr(<brace-enclosed initializer list>)’
15 | })} {}
| ^
In file included from /usr/include/c++/9/memory:81,
from /home/bilal/ws/src/ros2-testing-templates/naive/include/naive/incrementer.hpp:37,
from /home/bilal/ws/src/ros2-testing-templates/naive/src/incrementer.cpp:1:
/usr/include/c++/9/bits/shared_ptr.h:367:7: note: candidate: ‘std::shared_ptr<_Tp>::shared_ptr(const std::weak_ptr<_Tp>&, std::nothrow_t) [with _Tp = rclcpp::Subscription<std_msgs::msg::Int64_<std::allocator<void> > >]’
367 | shared_ptr(const weak_ptr<_Tp>& __r, std::nothrow_t)
| ^~~~~~~~~~
/usr/include/c++/9/bits/shared_ptr.h:367:7: note: candidate expects 2 arguments, 1 provided
/usr/include/c++/9/bits/shared_ptr.h:358:2: note: candidate: ‘template<class _Alloc, class ... _Args> std::shared_ptr<_Tp>::shared_ptr(std::_Sp_alloc_shared_tag<_Tp>, _Args&& ...)’
358 | shared_ptr(_Sp_alloc_shared_tag<_Alloc> __tag, _Args&&... __args)
| ^~~~~~~~~~
/usr/include/c++/9/bits/shared_ptr.h:358:2: note: template argument deduction/substitution failed:
/usr/include/c++/9/bits/shared_ptr.h:307:17: note: candidate: ‘constexpr std::shared_ptr<_Tp>::shared_ptr(std::nullptr_t) [with _Tp = rclcpp::Subscription<std_msgs::msg::Int64_<std::allocator<void> > >; std::nullptr_t = std::nullptr_t]’
307 | constexpr shared_ptr(nullptr_t) noexcept : shared_ptr() { }
| ^~~~~~~~~~
/usr/include/c++/9/bits/shared_ptr.h:307:17: note: conversion of argument 1 would be ill-formed:
/usr/include/c++/9/bits/shared_ptr.h:290:2: note: candidate: ‘template<class _Yp, class _Del, class> std::shared_ptr<_Tp>::shared_ptr(std::unique_ptr<_Up, _Ep>&&)’
290 | shared_ptr(unique_ptr<_Yp, _Del>&& __r)
| ^~~~~~~~~~
/usr/include/c++/9/bits/shared_ptr.h:290:2: note: template argument deduction/substitution failed:
/usr/include/c++/9/bits/shared_ptr.h:282:2: note: candidate: ‘template<class _Yp, class> std::shared_ptr<_Tp>::shared_ptr(std::auto_ptr<_Up>&&)’
282 | shared_ptr(auto_ptr<_Yp>&& __r);
| ^~~~~~~~~~
/usr/include/c++/9/bits/shared_ptr.h:282:2: note: template argument deduction/substitution failed:
/usr/include/c++/9/bits/shared_ptr.h:275:11: note: candidate: ‘template<class _Yp, class> std::shared_ptr<_Tp>::shared_ptr(const std::weak_ptr<_Yp>&)’
275 | explicit shared_ptr(const weak_ptr<_Yp>& __r)
| ^~~~~~~~~~
/usr/include/c++/9/bits/shared_ptr.h:275:11: note: template argument deduction/substitution failed:
/usr/include/c++/9/bits/shared_ptr.h:263:2: note: candidate: ‘template<class _Yp, class> std::shared_ptr<_Tp>::shared_ptr(std::shared_ptr<_Yp>&&)’
263 | shared_ptr(shared_ptr<_Yp>&& __r) noexcept
| ^~~~~~~~~~
/usr/include/c++/9/bits/shared_ptr.h:263:2: note: template argument deduction/substitution failed:
/usr/include/c++/9/bits/shared_ptr.h:254:7: note: candidate: ‘std::shared_ptr<_Tp>::shared_ptr(std::shared_ptr<_Tp>&&) [with _Tp = rclcpp::Subscription<std_msgs::msg::Int64_<std::allocator<void> > >]’
254 | shared_ptr(shared_ptr&& __r) noexcept
| ^~~~~~~~~~
/usr/include/c++/9/bits/shared_ptr.h:254:7: note: conversion of argument 1 would be ill-formed:
/usr/include/c++/9/bits/shared_ptr.h:246:2: note: candidate: ‘template<class _Yp, class> std::shared_ptr<_Tp>::shared_ptr(const std::shared_ptr<_Yp>&)’
246 | shared_ptr(const shared_ptr<_Yp>& __r) noexcept
| ^~~~~~~~~~
/usr/include/c++/9/bits/shared_ptr.h:246:2: note: template argument deduction/substitution failed:
/usr/include/c++/9/bits/shared_ptr.h:234:2: note: candidate: ‘template<class _Yp> std::shared_ptr<_Tp>::shared_ptr(const std::shared_ptr<_Yp>&, std::shared_ptr<_Tp>::element_type*)’
234 | shared_ptr(const shared_ptr<_Yp>& __r, element_type* __p) noexcept
| ^~~~~~~~~~
/usr/include/c++/9/bits/shared_ptr.h:234:2: note: template argument deduction/substitution failed:
/usr/include/c++/9/bits/shared_ptr.h:212:2: note: candidate: ‘template<class _Deleter, class _Alloc> std::shared_ptr<_Tp>::shared_ptr(std::nullptr_t, _Deleter, _Alloc)’
212 | shared_ptr(nullptr_t __p, _Deleter __d, _Alloc __a)
| ^~~~~~~~~~
/usr/include/c++/9/bits/shared_ptr.h:212:2: note: template argument deduction/substitution failed:
/usr/include/c++/9/bits/shared_ptr.h:193:2: note: candidate: ‘template<class _Yp, class _Deleter, class _Alloc, class> std::shared_ptr<_Tp>::shared_ptr(_Yp*, _Deleter, _Alloc)’
193 | shared_ptr(_Yp* __p, _Deleter __d, _Alloc __a)
| ^~~~~~~~~~
/usr/include/c++/9/bits/shared_ptr.h:193:2: note: template argument deduction/substitution failed:
/usr/include/c++/9/bits/shared_ptr.h:173:2: note: candidate: ‘template<class _Deleter> std::shared_ptr<_Tp>::shared_ptr(std::nullptr_t, _Deleter)’
173 | shared_ptr(nullptr_t __p, _Deleter __d)
| ^~~~~~~~~~
/usr/include/c++/9/bits/shared_ptr.h:173:2: note: template argument deduction/substitution failed:
/usr/include/c++/9/bits/shared_ptr.h:156:2: note: candidate: ‘template<class _Yp, class _Deleter, class> std::shared_ptr<_Tp>::shared_ptr(_Yp*, _Deleter)’
156 | shared_ptr(_Yp* __p, _Deleter __d)
| ^~~~~~~~~~
/usr/include/c++/9/bits/shared_ptr.h:156:2: note: template argument deduction/substitution failed:
/usr/include/c++/9/bits/shared_ptr.h:139:2: note: candidate: ‘template<class _Yp, class> std::shared_ptr<_Tp>::shared_ptr(_Yp*)’
139 | shared_ptr(_Yp* __p) : __shared_ptr<_Tp>(__p) { }
| ^~~~~~~~~~
/usr/include/c++/9/bits/shared_ptr.h:139:2: note: template argument deduction/substitution failed:
/usr/include/c++/9/bits/shared_ptr.h:129:7: note: candidate: ‘std::shared_ptr<_Tp>::shared_ptr(const std::shared_ptr<_Tp>&) [with _Tp = rclcpp::Subscription<std_msgs::msg::Int64_<std::allocator<void> > >]’
129 | shared_ptr(const shared_ptr&) noexcept = default;
| ^~~~~~~~~~
/usr/include/c++/9/bits/shared_ptr.h:129:7: note: conversion of argument 1 would be ill-formed:
/usr/include/c++/9/bits/shared_ptr.h:127:17: note: candidate: ‘constexpr std::shared_ptr<_Tp>::shared_ptr() [with _Tp = rclcpp::Subscription<std_msgs::msg::Int64_<std::allocator<void> > >]’
127 | constexpr shared_ptr() noexcept : __shared_ptr<_Tp>() { }
| ^~~~~~~~~~
/usr/include/c++/9/bits/shared_ptr.h:127:17: note: candidate expects 0 arguments, 1 provided
make[2]: *** [CMakeFiles/naive.dir/build.make:76: CMakeFiles/naive.dir/src/incrementer.cpp.o] Error 1
make[1]: *** [CMakeFiles/Makefile2:78: CMakeFiles/naive.dir/all] Error 2
make: *** [Makefile:141: all] Error 2
---
Failed <<< naive [1.71s, exited with code 2]
ros2/naive/src/incrementer.cpp
Outdated
|
|
||
| Incrementer::Incrementer(std::shared_ptr<rclcpp::Node> node, const std::string& in_topic, const std::string& out_topic) | ||
| : node_{std::move(node)}, | ||
| pub_{node_->create_publisher<std_msgs::msg::Int64>(out_topic,QUEUE_SIZE)}, |
There was a problem hiding this comment.
Does
Suggested change
| pub_{node_->create_publisher<std_msgs::msg::Int64>(out_topic,QUEUE_SIZE)}, | |
| pub_{node_->create_publisher(out_topic, QUEUE_SIZE)}, |
work? Probably not but can you check?
There was a problem hiding this comment.
Yup, the compiler does not like that. More template nastiness.
--- stderr: naive
/home/bilal/ws/src/ros2-testing-templates/naive/src/incrementer.cpp: In constructor ‘Incrementer::Incrementer(std::shared_ptr<rclcpp::Node>, const string&, const string&)’:
/home/bilal/ws/src/ros2-testing-templates/naive/src/incrementer.cpp:13:52: error: no matching function for call to ‘rclcpp::Node::create_publisher(const string&, const int&)’
13 | pub_{node_->create_publisher(out_topic,queue_size)},
| ^
In file included from /opt/ros/galactic/include/rclcpp/executors/single_threaded_executor.hpp:28,
from /opt/ros/galactic/include/rclcpp/executors.hpp:22,
from /opt/ros/galactic/include/rclcpp/rclcpp.hpp:156,
from /home/bilal/ws/src/ros2-testing-templates/naive/include/naive/incrementer.hpp:38,
from /home/bilal/ws/src/ros2-testing-templates/naive/src/incrementer.cpp:1:
/opt/ros/galactic/include/rclcpp/node.hpp:200:3: note: candidate: ‘template<class MessageT, class AllocatorT, class PublisherT> std::shared_ptr<PublisherT> rclcpp::Node::create_publisher(const string&, const rclcpp::QoS&, const rclcpp::PublisherOptionsWithAllocator<AllocatorT>&)’
200 | create_publisher(
| ^~~~~~~~~~~~~~~~
/opt/ros/galactic/include/rclcpp/node.hpp:200:3: note: template argument deduction/substitution failed:
/home/bilal/ws/src/ros2-testing-templates/naive/src/incrementer.cpp:13:52: note: couldn’t deduce template parameter ‘MessageT’
13 | pub_{node_->create_publisher(out_topic,queue_size)},
| ^
/home/bilal/ws/src/ros2-testing-templates/naive/src/incrementer.cpp:19:59: error: no matching function for call to ‘std::shared_ptr<rclcpp::Publisher<std_msgs::msg::Int64_<std::allocator<void> > > >::shared_ptr(<brace-enclosed initializer list>)’
19 | })} {}
| ^
In file included from /usr/include/c++/9/memory:81,
from /home/bilal/ws/src/ros2-testing-templates/naive/include/naive/incrementer.hpp:37,
from /home/bilal/ws/src/ros2-testing-templates/naive/src/incrementer.cpp:1:
/usr/include/c++/9/bits/shared_ptr.h:367:7: note: candidate: ‘std::shared_ptr<_Tp>::shared_ptr(const std::weak_ptr<_Tp>&, std::nothrow_t) [with _Tp = rclcpp::Publisher<std_msgs::msg::Int64_<std::allocator<void> > >]’
367 | shared_ptr(const weak_ptr<_Tp>& __r, std::nothrow_t)
| ^~~~~~~~~~
/usr/include/c++/9/bits/shared_ptr.h:367:7: note: candidate expects 2 arguments, 1 provided
/usr/include/c++/9/bits/shared_ptr.h:358:2: note: candidate: ‘template<class _Alloc, class ... _Args> std::shared_ptr<_Tp>::shared_ptr(std::_Sp_alloc_shared_tag<_Tp>, _Args&& ...)’
358 | shared_ptr(_Sp_alloc_shared_tag<_Alloc> __tag, _Args&&... __args)
| ^~~~~~~~~~
/usr/include/c++/9/bits/shared_ptr.h:358:2: note: template argument deduction/substitution failed:
/usr/include/c++/9/bits/shared_ptr.h:307:17: note: candidate: ‘constexpr std::shared_ptr<_Tp>::shared_ptr(std::nullptr_t) [with _Tp = rclcpp::Publisher<std_msgs::msg::Int64_<std::allocator<void> > >; std::nullptr_t = std::nullptr_t]’
307 | constexpr shared_ptr(nullptr_t) noexcept : shared_ptr() { }
| ^~~~~~~~~~
/usr/include/c++/9/bits/shared_ptr.h:307:17: note: conversion of argument 1 would be ill-formed:
/usr/include/c++/9/bits/shared_ptr.h:290:2: note: candidate: ‘template<class _Yp, class _Del, class> std::shared_ptr<_Tp>::shared_ptr(std::unique_ptr<_Up, _Ep>&&)’
290 | shared_ptr(unique_ptr<_Yp, _Del>&& __r)
| ^~~~~~~~~~
/usr/include/c++/9/bits/shared_ptr.h:290:2: note: template argument deduction/substitution failed:
/usr/include/c++/9/bits/shared_ptr.h:282:2: note: candidate: ‘template<class _Yp, class> std::shared_ptr<_Tp>::shared_ptr(std::auto_ptr<_Up>&&)’
282 | shared_ptr(auto_ptr<_Yp>&& __r);
| ^~~~~~~~~~
/usr/include/c++/9/bits/shared_ptr.h:282:2: note: template argument deduction/substitution failed:
/usr/include/c++/9/bits/shared_ptr.h:275:11: note: candidate: ‘template<class _Yp, class> std::shared_ptr<_Tp>::shared_ptr(const std::weak_ptr<_Yp>&)’
275 | explicit shared_ptr(const weak_ptr<_Yp>& __r)
| ^~~~~~~~~~
/usr/include/c++/9/bits/shared_ptr.h:275:11: note: template argument deduction/substitution failed:
/usr/include/c++/9/bits/shared_ptr.h:263:2: note: candidate: ‘template<class _Yp, class> std::shared_ptr<_Tp>::shared_ptr(std::shared_ptr<_Yp>&&)’
263 | shared_ptr(shared_ptr<_Yp>&& __r) noexcept
| ^~~~~~~~~~
/usr/include/c++/9/bits/shared_ptr.h:263:2: note: template argument deduction/substitution failed:
/usr/include/c++/9/bits/shared_ptr.h:254:7: note: candidate: ‘std::shared_ptr<_Tp>::shared_ptr(std::shared_ptr<_Tp>&&) [with _Tp = rclcpp::Publisher<std_msgs::msg::Int64_<std::allocator<void> > >]’
254 | shared_ptr(shared_ptr&& __r) noexcept
| ^~~~~~~~~~
/usr/include/c++/9/bits/shared_ptr.h:254:7: note: conversion of argument 1 would be ill-formed:
/usr/include/c++/9/bits/shared_ptr.h:246:2: note: candidate: ‘template<class _Yp, class> std::shared_ptr<_Tp>::shared_ptr(const std::shared_ptr<_Yp>&)’
246 | shared_ptr(const shared_ptr<_Yp>& __r) noexcept
| ^~~~~~~~~~
/usr/include/c++/9/bits/shared_ptr.h:246:2: note: template argument deduction/substitution failed:
/usr/include/c++/9/bits/shared_ptr.h:234:2: note: candidate: ‘template<class _Yp> std::shared_ptr<_Tp>::shared_ptr(const std::shared_ptr<_Yp>&, std::shared_ptr<_Tp>::element_type*)’
234 | shared_ptr(const shared_ptr<_Yp>& __r, element_type* __p) noexcept
| ^~~~~~~~~~
/usr/include/c++/9/bits/shared_ptr.h:234:2: note: template argument deduction/substitution failed:
/usr/include/c++/9/bits/shared_ptr.h:212:2: note: candidate: ‘template<class _Deleter, class _Alloc> std::shared_ptr<_Tp>::shared_ptr(std::nullptr_t, _Deleter, _Alloc)’
212 | shared_ptr(nullptr_t __p, _Deleter __d, _Alloc __a)
| ^~~~~~~~~~
/usr/include/c++/9/bits/shared_ptr.h:212:2: note: template argument deduction/substitution failed:
/usr/include/c++/9/bits/shared_ptr.h:193:2: note: candidate: ‘template<class _Yp, class _Deleter, class _Alloc, class> std::shared_ptr<_Tp>::shared_ptr(_Yp*, _Deleter, _Alloc)’
193 | shared_ptr(_Yp* __p, _Deleter __d, _Alloc __a)
| ^~~~~~~~~~
/usr/include/c++/9/bits/shared_ptr.h:193:2: note: template argument deduction/substitution failed:
/usr/include/c++/9/bits/shared_ptr.h:173:2: note: candidate: ‘template<class _Deleter> std::shared_ptr<_Tp>::shared_ptr(std::nullptr_t, _Deleter)’
173 | shared_ptr(nullptr_t __p, _Deleter __d)
| ^~~~~~~~~~
/usr/include/c++/9/bits/shared_ptr.h:173:2: note: template argument deduction/substitution failed:
/usr/include/c++/9/bits/shared_ptr.h:156:2: note: candidate: ‘template<class _Yp, class _Deleter, class> std::shared_ptr<_Tp>::shared_ptr(_Yp*, _Deleter)’
156 | shared_ptr(_Yp* __p, _Deleter __d)
| ^~~~~~~~~~
/usr/include/c++/9/bits/shared_ptr.h:156:2: note: template argument deduction/substitution failed:
/usr/include/c++/9/bits/shared_ptr.h:139:2: note: candidate: ‘template<class _Yp, class> std::shared_ptr<_Tp>::shared_ptr(_Yp*)’
139 | shared_ptr(_Yp* __p) : __shared_ptr<_Tp>(__p) { }
| ^~~~~~~~~~
/usr/include/c++/9/bits/shared_ptr.h:139:2: note: template argument deduction/substitution failed:
/usr/include/c++/9/bits/shared_ptr.h:129:7: note: candidate: ‘std::shared_ptr<_Tp>::shared_ptr(const std::shared_ptr<_Tp>&) [with _Tp = rclcpp::Publisher<std_msgs::msg::Int64_<std::allocator<void> > >]’
129 | shared_ptr(const shared_ptr&) noexcept = default;
| ^~~~~~~~~~
/usr/include/c++/9/bits/shared_ptr.h:129:7: note: conversion of argument 1 would be ill-formed:
/usr/include/c++/9/bits/shared_ptr.h:127:17: note: candidate: ‘constexpr std::shared_ptr<_Tp>::shared_ptr() [with _Tp = rclcpp::Publisher<std_msgs::msg::Int64_<std::allocator<void> > >]’
127 | constexpr shared_ptr() noexcept : __shared_ptr<_Tp>() { }
| ^~~~~~~~~~
/usr/include/c++/9/bits/shared_ptr.h:127:17: note: candidate expects 0 arguments, 1 provided
make[2]: *** [CMakeFiles/naive.dir/build.make:76: CMakeFiles/naive.dir/src/incrementer.cpp.o] Error 1
make[1]: *** [CMakeFiles/Makefile2:78: CMakeFiles/naive.dir/all] Error 2
make: *** [Makefile:141: all] Error 2
---
Failed <<< naive [2.35s, exited with code 2]
|
@bgill92 don't forget to use Assignees |
ChrisThrasher
suggested changes
Jul 14, 2022
ros2/naive/src/incrementer.cpp
Outdated
Comment on lines
1
to
5
| #include "naive/incrementer.hpp" | ||
|
|
||
| #include <memory> | ||
| #include "rclcpp/rclcpp.hpp" | ||
| #include "std_msgs/msg/int64.hpp" |
There was a problem hiding this comment.
Why use quoted includes instead of <> includes here?
There was a problem hiding this comment.
Good question. Should #include "naive/incrementer.hpp" remain a quoted include because it is a header file that is a part of this project?
There was a problem hiding this comment.
No, because the path to that header is not relative to the path of the .cpp file including it. The CppCoreGuidelines recommends triangle brackets in this instance. Basically the only time you use quotes is when the header is in the same directory as the .cpp file.
ros2/naive/src/incrementer.cpp
Outdated
| #include "std_msgs/msg/int64.hpp" | ||
|
|
||
| Incrementer::Incrementer(std::shared_ptr<rclcpp::Node> node, const std::string& in_topic, const std::string& out_topic) | ||
| : node_{std::move(node)}, |
There was a problem hiding this comment.
Is moving the shared pointer a valuable thing to do? The overhead of incrementing and decrementing the ref count is pretty miniscule.
There was a problem hiding this comment.
As far as I know, it's canonical to do it this way
https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#Rr-sharedptrparam-owner
ros2/naive/src/node.cpp
Outdated
Comment on lines
7
to
19
| rclcpp::init(argc,argv); | ||
|
|
||
| auto const node = std::make_shared<rclcpp::Node>("incrementer"); | ||
|
|
||
| Incrementer incrementer{node, "/in", "/out"}; | ||
|
|
||
| rclcpp::spin(node); | ||
|
|
||
| rclcpp::shutdown(); | ||
|
|
||
| return 0; | ||
|
|
||
| } |
There was a problem hiding this comment.
All these extra newlines, especially the newline after return is harming readability. And don't forget to add a trailing newline at the end of the file.
| std::shared_ptr<rclcpp::Node> node_; | ||
| std::shared_ptr<rclcpp::Publisher<std_msgs::msg::Int64>> pub_; | ||
| std::shared_ptr<rclcpp::Subscription<std_msgs::msg::Int64>> sub_; | ||
| }; |
There was a problem hiding this comment.
Added. I didn't realize the importance of the final newline.
There was a problem hiding this comment.
I'm not sure what purpose it serves but Git and GitHub will warn you about omitting it so I make sure it's always there. You might have noticed in previous PR diffs how there was a little red symbol on the bottom line to indicate this.
ros2/naive/src/node.cpp
Outdated
| rclcpp::spin(node); | ||
| rclcpp::shutdown(); | ||
|
|
||
| return 0; |
There was a problem hiding this comment.
For what it's worth, this is unnecessary. It's specified that if a program reaches the end, it will always return 0 so it's safe to remove this.
griswaldbrooks
approved these changes
Sep 14, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Resolves # (6)
The naive implementation from the ros1 directory was ported to ros2.
Testing
No official tests exist yet. Local testing was performed by publishing to /in topic and subscribing to the /out topic and seeing if the expected output was published.
Notes
Be sure to not only request reviewers but also assign someone to the pull request so it is clear when a review is required.