Skip to content
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

Enable QoS overrides #657

Merged
merged 5 commits into from
May 20, 2021
Merged

Enable QoS overrides #657

merged 5 commits into from
May 20, 2021

Conversation

audrow
Copy link

@audrow audrow commented Apr 29, 2021

This is adding QoS overrides, which were added in ros2/rclcpp#1408.

This PR relies on

Signed-off-by: Audrow Nash <audrow@hey.com>
@audrow audrow marked this pull request as ready for review April 29, 2021 18:49
Signed-off-by: Audrow Nash <audrow@hey.com>
@audrow
Copy link
Author

audrow commented May 19, 2021

@SteveMacenski, the required PR has been merged in. I believe this is ready to merge.

src/ros_filter.cpp Outdated Show resolved Hide resolved
Signed-off-by: Audrow Nash <audrow@hey.com>
@audrow
Copy link
Author

audrow commented May 19, 2021

I'm now using rclcpp::QosOverridingOptions::with_default_policies(), which removes much duplication. It doesn't enable all of the QoS overrides, but after some discussion with the ROS 2 team, it was determined that not all the options are desirable: ros-perception/image_pipeline#651 (comment).

@audrow audrow requested a review from SteveMacenski May 19, 2021 22:39
paudrow added 2 commits May 19, 2021 15:45
Signed-off-by: Audrow Nash <audrow@hey.com>
Signed-off-by: Audrow Nash <audrow@hey.com>
@SteveMacenski SteveMacenski merged commit 2816e92 into cra-ros-pkg:ros2 May 20, 2021
@audrow audrow deleted the audrow/add-qos-overrides branch May 20, 2021 16:13
@SteveMacenski
Copy link
Collaborator

SteveMacenski commented May 25, 2021

@audrow Some change here or in your message filters PR is failing to build robot_localization with galactic ros2/message_filters#56.

https://build.ros2.org/job/Gbin_uF64__robot_localization__ubuntu_focal_amd64__binary/7/console


23:08:22 /tmp/binarydeb/ros-galactic-robot-localization-3.2.2/src/ros_robot_localization_listener.cpp: In constructor ‘robot_localization::RosRobotLocalizationListener::RosRobotLocalizationListener(rclcpp::Node::SharedPtr, rclcpp::SubscriptionOptions)’:
23:08:31 /tmp/binarydeb/ros-galactic-robot-localization-3.2.2/src/ros_robot_localization_listener.cpp:90:26: error: no matching function for call to ‘message_filters::Subscriber<nav_msgs::msg::Odometry_<std::allocator<void> > >::Subscriber(rclcpp::Node::SharedPtr&, const char [14], rmw_qos_profile_t&, rclcpp::SubscriptionOptions&)’
23:08:31    90 |   tf_listener_(tf_buffer_)
23:08:31       |                          ^
23:08:31 In file included from /tmp/binarydeb/ros-galactic-robot-localization-3.2.2/include/robot_localization/ros_robot_localization_listener.hpp:37,
23:08:31                  from /tmp/binarydeb/ros-galactic-robot-localization-3.2.2/src/ros_robot_localization_listener.cpp:51:
23:08:31 /opt/ros/galactic/include/message_filters/subscriber.h:129:3: note: candidate: ‘message_filters::Subscriber<M>::Subscriber() [with M = nav_msgs::msg::Odometry_<std::allocator<void> >]’
23:08:31   129 |   Subscriber() = default;
23:08:31       |   ^~~~~~~~~~
23:08:31 /opt/ros/galactic/include/message_filters/subscriber.h:129:3: note:   candidate expects 0 arguments, 4 provided
23:08:31 /opt/ros/galactic/include/message_filters/subscriber.h:121:3: note: candidate: ‘message_filters::Subscriber<M>::Subscriber(rclcpp::Node*, const string&, rmw_qos_profile_t) [with M = nav_msgs::msg::Odometry_<std::allocator<void> >; std::string = std::__cxx11::basic_string<char>; rmw_qos_profile_t = rmw_qos_profile_t]’
23:08:31   121 |   Subscriber(rclcpp::Node* node, const std::string& topic, const rmw_qos_profile_t qos = rmw_qos_profile_default)
23:08:31       |   ^~~~~~~~~~
23:08:31 /opt/ros/galactic/include/message_filters/subscriber.h:121:3: note:   candidate expects 3 arguments, 4 provided
23:08:31 /opt/ros/galactic/include/message_filters/subscriber.h:116:3: note: candidate: ‘message_filters::Subscriber<M>::Subscriber(rclcpp::Node::SharedPtr, const string&, rmw_qos_profile_t) [with M = nav_msgs::msg::Odometry_<std::allocator<void> >; rclcpp::Node::SharedPtr = std::shared_ptr<rclcpp::Node>; std::string = std::__cxx11::basic_string<char>; rmw_qos_profile_t = rmw_qos_profile_t]’
23:08:31   116 |   Subscriber(rclcpp::Node::SharedPtr node, const std::string& topic, const rmw_qos_profile_t qos = rmw_qos_profile_default)
23:08:31       |   ^~~~~~~~~~
23:08:31 /opt/ros/galactic/include/message_filters/subscriber.h:116:3: note:   candidate expects 3 arguments, 4 provided
23:08:31 /tmp/binarydeb/ros-galactic-robot-localization-3.2.2/src/ros_robot_localization_listener.cpp:90:26: error: no matching function for call to ‘message_filters::Subscriber<geometry_msgs::msg::AccelWithCovarianceStamped_<std::allocator<void> > >::Subscriber(rclcpp::Node::SharedPtr&, const char [22], rmw_qos_profile_t&, rclcpp::SubscriptionOptions&)’
23:08:31    90 |   tf_listener_(tf_buffer_)
23:08:31       |                          ^
23:08:31 In file included from /tmp/binarydeb/ros-galactic-robot-localization-3.2.2/include/robot_localization/ros_robot_localization_listener.hpp:37,
23:08:31                  from /tmp/binarydeb/ros-galactic-robot-localization-3.2.2/src/ros_robot_localization_listener.cpp:51:
23:08:31 /opt/ros/galactic/include/message_filters/subscriber.h:129:3: note: candidate: ‘message_filters::Subscriber<M>::Subscriber() [with M = geometry_msgs::msg::AccelWithCovarianceStamped_<std::allocator<void> >]’
23:08:31   129 |   Subscriber() = default;
23:08:31       |   ^~~~~~~~~~
23:08:31 /opt/ros/galactic/include/message_filters/subscriber.h:129:3: note:   candidate expects 0 arguments, 4 provided
23:08:31 /opt/ros/galactic/include/message_filters/subscriber.h:121:3: note: candidate: ‘message_filters::Subscriber<M>::Subscriber(rclcpp::Node*, const string&, rmw_qos_profile_t) [with M = geometry_msgs::msg::AccelWithCovarianceStamped_<std::allocator<void> >; std::string = std::__cxx11::basic_string<char>; rmw_qos_profile_t = rmw_qos_profile_t]’
23:08:31   121 |   Subscriber(rclcpp::Node* node, const std::string& topic, const rmw_qos_profile_t qos = rmw_qos_profile_default)
23:08:31       |   ^~~~~~~~~~
23:08:31 /opt/ros/galactic/include/message_filters/subscriber.h:121:3: note:   candidate expects 3 arguments, 4 provided
23:08:31 /opt/ros/galactic/include/message_filters/subscriber.h:116:3: note: candidate: ‘message_filters::Subscriber<M>::Subscriber(rclcpp::Node::SharedPtr, const string&, rmw_qos_profile_t) [with M = geometry_msgs::msg::AccelWithCovarianceStamped_<std::allocator<void> >; rclcpp::Node::SharedPtr = std::shared_ptr<rclcpp::Node>; std::string = std::__cxx11::basic_string<char>; rmw_qos_profile_t = rmw_qos_profile_t]’
23:08:31   116 |   Subscriber(rclcpp::Node::SharedPtr node, const std::string& topic, const rmw_qos_profile_t qos = rmw_qos_profile_default)
23:08:31       |   ^~~~~~~~~~

Is this something you can patch? It looks like you may have removed (?) the default constructor

@audrow
Copy link
Author

audrow commented May 25, 2021

Hey @SteveMacenski, I believe that this is because the message filters PR didn't get into Galactic (it was done after the freeze). I'll ask the ROS boss what to do.

@audrow
Copy link
Author

audrow commented May 25, 2021

Since the change wasn't meant to make it in Galactic, I made a PR reverting the change on the galactic branch: #670.

SteveMacenski pushed a commit that referenced this pull request May 25, 2021
* Remove try-catch blocks around declare_parameter (#663)

Try-catches were added in #631 due to a new rclcpp feature enforcing static parameter.
The behavior was later patched for this particular use-case in ros2/rclcpp#1673, so now we can avoid having to try-catch.

Signed-off-by: Jacob Perron <jacob@openrobotics.org>

* Add missing message_filters dependency (#666)

Headers from message_filters are included here:
https://github.com/cra-ros-pkg/robot_localization/blob/67098c2341b5d1ccbcceb8eede60e79db74814a6/include/robot_localization/ros_robot_localization_listener.h\#L41-L42

Signed-off-by: Jacob Perron <jacob@openrobotics.org>

* Revert "Enable QoS overrides (#657)"

This reverts commit 2816e92.

Co-authored-by: Jacob Perron <jacob@openrobotics.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants