Made rmw_qos_profile argument optional #133

esteve · 2015-10-19T20:00:55Z

Moved rmw_qos_profile argument around to make it optional.

dirk-thomas · 2015-10-19T20:04:11Z

Why is the order of the topic name and callback changes too?

esteve · 2015-10-19T20:06:04Z

I tried to make it consistent so that the rmw_qos_profile argument always comes after the topic name, but I don't really have a strong opinion on where the callback argument should go.

wjwwood · 2015-10-19T20:07:29Z

In order to keep in line with the order from ROS 1 (topic, queue size, callback), I think at least we should keep callback after the topic. At most I'm not sure we want to allow users to create publishers and subscriptions without specifying at least queue size. Maybe we do, but I think we should have some rational as to why or why not.

tfoote · 2015-10-19T20:10:43Z

Since that API is a convenience for similar backwards compatibility it would be good to keep it consistent with the old signature.

wjwwood · 2015-10-19T20:15:37Z

Since that API is a convenience for similar backwards compatibility it would be good to keep it consistent with the old signature.

How is that a convenience for backwards compatibility? There is no similar signature in ROS 1 that I'm aware of.

esteve · 2015-10-19T20:16:22Z

I'm reordering the arguments so that the callback comes after the topic name.

tfoote · 2015-10-19T20:21:36Z

create_subscription with (topic, queue size, callback) instead of the new qos_profile I consider a convenience method for backwards compatibility. We only expose the one qos dimention previously in ROS1. It's the analog to NodeHandle::subscribe

tfoote · 2015-10-19T20:23:46Z

rclcpp/include/rclcpp/node_impl.hpp

@@ -309,9 +309,9 @@ Node::create_subscription(
 template<typename MessageT, typename CallbackT>
 typename rclcpp::subscription::Subscription<MessageT>::SharedPtr
 Node::create_subscription(
+  CallbackT callback,
  const std::string & topic_name,
  size_t qos_history_depth,


@wjwwood this is the one i'm talking about

wjwwood · 2015-10-19T20:24:27Z

@tfoote is talking about this one: https://github.com/ros2/rclcpp/pull/133/files#diff-809285dba1df7726afedd63625b6a6ceR311

Which I agree should end up as (topic, queue size, callback) and not (topic, callback, queue size).

esteve · 2015-10-19T20:44:47Z

I've changed it so that the callback comes after the topic name for the signature of create_signature where the rmw_qos_profile is optional. And for the other signature, I moved it after the queue size.

esteve · 2015-10-19T20:52:28Z

CI jobs:

http://ci.ros2.org/job/ros2_batch_ci_windows/504/
http://ci.ros2.org/job/ros2_batch_ci_linux/447/
http://ci.ros2.org/job/ros2_batch_ci_osx/359/

wjwwood · 2015-10-19T20:56:20Z

lgtm

esteve · 2015-10-19T21:36:59Z

Since this is just a change in the create_subscription signature and the Linux and OSX jobs pass, I've cancelled the Windows CI job.

Made rmw_qos_profile argument optional

* Debug output for failed node_names test * Give test nodes unique names * Put test name in printout

* ros2GH-138 Move calculation of bag size - previously in rosbag2::Info - now in storage plugin * ros2GH-130 Add rosbag2_bag_v2_plugins package -This package will contain storage and converter plugins * ros2GH-131 don't build plugins on Windows * ros2GH-129 Add function to be generated - massive if/else between all message types - will be generated similar to ros1_bridge plugin * ros2GH-138 Write storage plugin for rosbag v2 bags * ros2GH-138 Make sure that no attempt to create a converter is made when trying to read a rsbag v2 bag file * ros2GH-138 Add play end-to-end test for rosbag v2 plugin * ros2GH-138 Use cmake files to find ros1 packages - Use files from ros1_bridge via PkgConfig * ros2GH-138 Add generator code * ros2GH-141 Add initial version of vendor package * ros2GH-141 Improve vendor package to build on Mac * ros2GH-138 Cleanup CMakeLists * ros2GH-141 Use unmanaged Instance of class-loader - managed instance somehow isn't available for gcc 6.3 * ros2GH-141 Reduce patch and copy new toplevle CMakeLists by hand * ros2GH-141 Fix Shared Instance usage * ros2GH-141 Improve maintainability of vendor package - Document what patches do and why changes are necessary - Load ros1 packages through cmake macro - Do not export ros1 packages via ament - use commit hash of current master which is more stable than using melodic-devel * ros2GH-138 Link against rclcpp - necessary for ros1_bridge * ros2GH-138 Avoid crash when trying to play v2 bags which contain unknown message types * ros2GH-138 Add CLI -s <storage_id> option to ros2 bag info and use it in rosbag2::info - this allows ros2 bag info to work also when the yaml metadata file does not exsist - this is always the case for rosbag1 bagfiles - it could also happen for sqlite or other storage based bagfiles * ros2GH-138 Add end-to-end info test for rosbag v2 files * ros2GH-138 Add unit tests to rosbag_storage * ros2GH-138 Add method to extract filename from path to FilesystemHelpers * ros2GH-138 Add proper logging for topics which cannot be converted * ros2GH-138 Improve finding dependencies of ros1 * ros2GH-141 Explicitly import transitive dependencies of vendor package * ros2GH-138 Skip tests via ament if ros1 is not available * ros2GH-133 First split of plugins * ros2GH-133 Write serialized rosbag message * ros2GH-133 Improve converter plugin - move generation templates outside of plugin folders as both plugins need it - use ros::serialization routines to deserialize the ros message * ros2GH-133 Add plugin to be found by pluginlib * ros2GH-133 Remove empty check in converter - With the rosbag_v2_converter_plugin, we don't need to treat rosbag_v2 storage any different * ros2GH-133 Assert serialization format in unit tests for storage * ros2GH-133 Delete superfluous include folder - Only needed if we want to link against the library * ros2GH-133 get_all_topics_and_types returns only valid ros2 types - This is necessary as the information is used by rosbag2_transport - ros2 bag info still shows all topics and types - rosbag::View::getConnections() can return multiple connections corresponding to the same topic * ros2GH-133 Improve end to end test - use a bagfile with messages not known to ros2 * ros2GH-133 Reformulate info message in case of missing ros1-ros2 mapping for a topic * ros2GH-14 Find messages first * Explicitly print message when on Windows Co-Authored-By: Martin-Idel-SI <external.Martin.Idel@bosch-si.com> * ros2GH-14 Refactor rosbag_storage vendor package - Improve toplevel CMakeLists - Put all patches into a resource subfolder * ros2GH-14 Reflect renames of converter interfaces * ros2GH-156 Workaround for path problems * ros2GH-156 Add documentation for plugin * ros2GH-156 Fix the pluginlib version to greater 2 * ros2GH-156 Prohibit CMake from declaring paths as system paths This switches the order of ros2 and ros1 directories resulting in build failures * ros2GH-156 Prohibit system include paths for rosbag plugins This can lead to switching ros1 and ros2 include paths resulting in missing symbols as the wrong pluginlib gets included * ros2GH-14 Split patches * make README more verbose * add plugin specific readme * more readme for bag_v2 plugin

…os2#133) Co-authored-by: Mauro Passerino <mpasserino@irobot.com>

esteve added the in progress Actively being worked on (Kanban column) label Oct 19, 2015

tfoote reviewed Oct 19, 2015
View reviewed changes

esteve force-pushed the optional-qos-profile branch from 4151bcb to e563dd2 Compare October 19, 2015 20:40

Made rmw_qos_profile argument optional

c845e9b

esteve force-pushed the optional-qos-profile branch from e563dd2 to c845e9b Compare October 19, 2015 20:45

esteve added a commit that referenced this pull request Oct 19, 2015

Merge pull request #133 from ros2/optional-qos-profile

ad609d7

Made rmw_qos_profile argument optional

esteve merged commit ad609d7 into master Oct 19, 2015

esteve removed the in progress Actively being worked on (Kanban column) label Oct 19, 2015

esteve deleted the optional-qos-profile branch October 19, 2015 21:37

nnmm pushed a commit to ApexAI/rclcpp that referenced this pull request Jul 9, 2022

Debug info for test_rcl_get_node_names (ros2#133)

fc75c58

* Debug output for failed node_names test * Give test nodes unique names * Put test name in printout

mauropasse added a commit to mauropasse/rclcpp that referenced this pull request Nov 30, 2023

Actions: Use ipc_setting = rclcpp::IntraProcessSetting::NodeDefault (r…

da09f92

…os2#133) Co-authored-by: Mauro Passerino <mpasserino@irobot.com>

mauropasse added a commit to mauropasse/rclcpp that referenced this pull request Aug 8, 2024

Actions: Use ipc_setting = rclcpp::IntraProcessSetting::NodeDefault (r…

8d2524a

…os2#133) Co-authored-by: Mauro Passerino <mpasserino@irobot.com>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Made rmw_qos_profile argument optional #133

Made rmw_qos_profile argument optional #133

esteve commented Oct 19, 2015

dirk-thomas commented Oct 19, 2015

esteve commented Oct 19, 2015

wjwwood commented Oct 19, 2015

tfoote commented Oct 19, 2015

wjwwood commented Oct 19, 2015

esteve commented Oct 19, 2015

tfoote commented Oct 19, 2015

tfoote Oct 19, 2015

wjwwood commented Oct 19, 2015

esteve commented Oct 19, 2015

esteve commented Oct 19, 2015

wjwwood commented Oct 19, 2015

esteve commented Oct 19, 2015

Made rmw_qos_profile argument optional #133

Made rmw_qos_profile argument optional #133

Conversation

esteve commented Oct 19, 2015

dirk-thomas commented Oct 19, 2015

esteve commented Oct 19, 2015

wjwwood commented Oct 19, 2015

tfoote commented Oct 19, 2015

wjwwood commented Oct 19, 2015

esteve commented Oct 19, 2015

tfoote commented Oct 19, 2015

tfoote Oct 19, 2015

Choose a reason for hiding this comment

wjwwood commented Oct 19, 2015

esteve commented Oct 19, 2015

esteve commented Oct 19, 2015

wjwwood commented Oct 19, 2015

esteve commented Oct 19, 2015