I've been trying out ROS 2 and I'm seeing unexpected behavior when using the bridge.</

Thank you for the issue report <a class="user-mention notranslate" data-hovercard-type

<a class="user-mention notranslate" data-hovercard-type="user" data-hovercard-url="/us

This issue should be now fixed by the <a class="issue-link js-issue-link" data-error-t

Bidirectional bridge duplicating messages with FastRTPS about ros1_bridge HOT 11 CLOSED

ros2 commented on August 17, 2024

Bidirectional bridge duplicating messages with FastRTPS

from ros1_bridge.

Comments (11)

dhood commented on August 17, 2024

Thank you for the issue report @vilhjalmur89

I can confirm this issue happens for both static_bridge and simple_bridge. With the simple_bridge you get some additional logging that shows this in action:

dhood@osrf-esteve:~ [~]$ simple_bridge__rmw_opensplice_cpp 
I heard from ROS 1 from: [/rostopic_17524_1481068220810]
I heard from ROS 1: [static_bridge__rmw_opensplice_cpp is running]
Passing along to ROS 2: [static_bridge__rmw_opensplice_cpp is running]
^Csignal_handler(2)
dhood@osrf-esteve:~ [~]$ simple_bridge__rmw_fastrtps_cpp
I heard from ROS 1 from: [/rostopic_17588_1481068239197]
I heard from ROS 1: [static_bridge__rmw_fastrtps_cpp is running]
Passing along to ROS 2: [static_bridge__rmw_fastrtps_cpp is running]
  I heard from ROS 2: [static_bridge__rmw_fastrtps_cpp is running]
  Passing along to ROS 1: [static_bridge__rmw_fastrtps_cpp is running]
    I heard from ROS 1 from myself

This doesn't seem to be an issue for the dynamic_bridge. Hopefully you can find that useful until we get around to looking into this. If you are interested in investigating yourself, we would gladly welcome a pull request. My current suspicion is that the logic to ignore publishers/subscribers from the bridge itself that exists in the dynamic bridge (e.g.

ros1_bridge/src/dynamic_bridge.cpp

Line 380 in cc1479b

// ignore publishers from the bridge itself

) does not seem to be implemented in the static bridge.

from ros1_bridge.

dirk-thomas commented on August 17, 2024

This is also a problem with the parameter_bridge. Additional filtering based on the node name of the bridge etc. is only implemented in the dynamic_bridge atm.

from ros1_bridge.

ArkadiuszNiemiec commented on August 17, 2024

Ok, so the problem is that ros2_callback() sends messages back to ROS1 without any filtering:

ros1_bridge/include/ros1_bridge/factory.hpp

Lines 145 to 159 in d7954c9

 static 

 void ros2_callback( 

 typename ROS2_T::SharedPtr ros2_msg, 

 ros::Publisher ros1_pub, 

 const std::string & ros1_type_name, 

 const std::string & ros2_type_name) 

 { 

 ROS1_T ros1_msg; 

 convert_2_to_1(*ros2_msg, ros1_msg); 

 RCUTILS_LOG_INFO_ONCE_NAMED( 

 "ros1_bridge", 

 "Passing message from ROS 2 %s to ROS 1 %s (showing msg only once per type)", 

 ros1_type_name.c_str(), ros2_type_name.c_str()); 

 ros1_pub.publish(ros1_msg); 

 }

ros1_callback does it perfectly but I don't think that there's currently a way to get connection_header or caller_id in ROS2 rclcpp. There's also no way to check by timestamp as a lot of msgs do not have header included.

I have implemented simple logic that checks if current ROS2 message equals to the last message published by ros1_callback but it's not allowing to send two exactly equal messages in a row.

ros1_bridge/include/ros1_bridge/factory.hpp

Lines 104 to 143 in d7954c9

 protected: 

 static 

 void ros1_callback( 

 const ros::MessageEvent<ROS1_T const> & ros1_msg_event, 

 rclcpp::PublisherBase::SharedPtr ros2_pub, 

 const std::string & ros1_type_name, 

 const std::string & ros2_type_name) 

 { 

 typename rclcpp::Publisher<ROS2_T>::SharedPtr typed_ros2_pub; 

 typed_ros2_pub = 

 std::dynamic_pointer_cast<typename rclcpp::Publisher<ROS2_T>>(ros2_pub); 

 if (!typed_ros2_pub) { 

 throw std::runtime_error("Invalid type for publisher"); 

 } 

 const boost::shared_ptr<ros::M_string> & connection_header = 

 ros1_msg_event.getConnectionHeaderPtr(); 

 if (!connection_header) { 

 printf(" dropping message without connection header\n"); 

 return; 

 } 

 std::string key = "callerid"; 

 if (connection_header->find(key) != connection_header->end()) { 

 if (connection_header->at(key) == "/ros_bridge") { 

 return; 

 } 

 } 

 const boost::shared_ptr<ROS1_T const> & ros1_msg = ros1_msg_event.getConstMessage(); 

 auto ros2_msg = std::make_shared<ROS2_T>(); 

 convert_1_to_2(*ros1_msg, *ros2_msg); 

 RCUTILS_LOG_INFO_ONCE_NAMED( 

 "ros1_bridge", 

 "Passing message from ROS 1 %s to ROS 2 %s (showing msg only once per type)", 

 ros1_type_name.c_str(), ros2_type_name.c_str()); 

 typed_ros2_pub->publish(ros2_msg); 

 }

Please tell me if you have an idea how to solve it, maybe I missed something and there's a way to get some metadata about the message?

from ros1_bridge.

dirk-thomas commented on August 17, 2024

I have implemented simple logic that checks if current ROS2 message equals to the last message published by ros1_callback but it's not allowing to send two exactly equal messages in a row.

That approach wouldn't be acceptable since dropping "duplicate" messages would be changing the data flow and is therefore not desired.

Please tell me if you have an idea how to solve it, maybe I missed something and there's a way to get some metadata about the message?

As mentioned above the dynamic_bridge already implements the filtering based on the node name of the source of the message (see e.g. https://github.com/ros2/ros1_bridge/blob/master/src/dynamic_bridge.cpp#L504-L507). This logic should be moved into a function which can then be reused by the different variants of the bridge.

from ros1_bridge.

ArkadiuszNiemiec commented on August 17, 2024

The lines that you linked are filtering messages that were published on ROS1 by the bridge. ROS2 part is here:

ros1_bridge/src/dynamic_bridge.cpp

Lines 658 to 672 in 48a373f

 auto publisher_count = ros2_node->count_publishers(topic_name); 

 auto subscriber_count = ros2_node->count_subscribers(topic_name); 

 // ignore publishers from the bridge itself 

 if (bridges_1to2.find(topic_name) != bridges_1to2.end()) { 

 if (publisher_count > 0) { 

 --publisher_count; 

 } 

 } 

 // ignore subscribers from the bridge itself 

 if (bridges_2to1.find(topic_name) != bridges_2to1.end()) { 

 if (subscriber_count > 0) { 

 --subscriber_count; 

 } 

 }

but to be honest I don't quite get it how to translate that to ros2_callback in factory.hpp

from ros1_bridge.

wjwwood commented on August 17, 2024

but I don't think that there's currently a way to get connection_header or caller_id in ROS2 rclcpp.

Not sure this is exactly what you're looking for, but you can get the message info from our C api and that contains the publisher GUID. You can also get the GUID for any publishers that you have a reference to, so you could compare if messages are from a publisher in the same node/process.

This is the function that returns the message info:

https://github.com/ros2/rcl/blob/e48a445f46f1da21ff0c4bf3d52bec9120ab7b57/rcl/include/rcl/subscription.h#L254-L257

This is the message info structure:

https://github.com/ros2/rmw/blob/a82424cfc32312182a775d8607d15c7af71e170f/rmw/include/rmw/types.h#L227

These are the functions that which allow you to get a GUID for a publisher and then compare two GUID:

https://github.com/ros2/rmw/blob/a82424cfc32312182a775d8607d15c7af71e170f/rmw/include/rmw/rmw.h#L368-L376

rclcpp uses it to find intra process pairs:

https://github.com/ros2/rclcpp/blob/45dcd0c6e58bf54d771ae8635d30cc91f8a08e2d/rclcpp/include/rclcpp/subscription.hpp#L186-L192

And then allows you to get it in a callback:

https://github.com/ros2/rclcpp/blob/45dcd0c6e58bf54d771ae8635d30cc91f8a08e2d/rclcpp/include/rclcpp/any_subscription_callback.hpp#L154-L177

I didn't follow this issue very closely, but maybe it would help.

from ros1_bridge.

ArkadiuszNiemiec commented on August 17, 2024

@wjwwood Good to know, thank you! I was hoping to access GUID in this callback:

ros1_bridge/include/ros1_bridge/factory.hpp

Line 146 in d7954c9

void ros2_callback(

but after hours of untangling this code I still don't quite understand how. And it looks like I can't use dispatch() either.

from ros1_bridge.

wjwwood commented on August 17, 2024

@ArkadiuszNiemiec this callback here needs to have the message info argument as well:

ros1_bridge/include/ros1_bridge/factory.hpp

Lines 95 to 99 in d7954c9

 auto callback = 

 [this, ros1_pub, ros1_type_name, ros2_type_name](const typename ROS2_T::SharedPtr msg) { 

 return this->ros2_callback( 

 msg, ros1_pub, ros1_type_name, ros2_type_name); 

 };

So that the create_subscription call right after that:

ros1_bridge/include/ros1_bridge/factory.hpp

Line 100 in d7954c9

return node->create_subscription<ROS2_T>(

Knows to pass it into the callback function. You can then pass the message info from the above linked lambda into the ros2_callback function you referenced just now.

from ros1_bridge.

ArkadiuszNiemiec commented on August 17, 2024

Thank you for explaining this. I have just created a pull request.

from ros1_bridge.

ArkadiuszNiemiec commented on August 17, 2024

This issue should be now fixed by the #113.

from ros1_bridge.

wjwwood commented on August 17, 2024

I'll close, please comment if the problem isn't fixed by #113.

from ros1_bridge.

Bidirectional bridge duplicating messages with FastRTPS about ros1_bridge HOT 11 CLOSED

Comments (11)

Related Issues (20)

Recommend Projects

React

Vue.js

Typescript

TensorFlow

Django

Laravel

D3

Recommend Topics

javascript

web

server

Machine learning

Visualization

Game

Recommend Org

Facebook

Microsoft

Google

Alibaba

D3

Tencent

	static
	void ros2_callback(
	typename ROS2_T::SharedPtr ros2_msg,
	ros::Publisher ros1_pub,
	const std::string & ros1_type_name,
	const std::string & ros2_type_name)
	{
	ROS1_T ros1_msg;
	convert_2_to_1(*ros2_msg, ros1_msg);
	RCUTILS_LOG_INFO_ONCE_NAMED(
	"ros1_bridge",
	"Passing message from ROS 2 %s to ROS 1 %s (showing msg only once per type)",
	ros1_type_name.c_str(), ros2_type_name.c_str());
	ros1_pub.publish(ros1_msg);
	}

	protected:
	static
	void ros1_callback(
	const ros::MessageEvent<ROS1_T const> & ros1_msg_event,
	rclcpp::PublisherBase::SharedPtr ros2_pub,
	const std::string & ros1_type_name,
	const std::string & ros2_type_name)
	{
	typename rclcpp::Publisher<ROS2_T>::SharedPtr typed_ros2_pub;
	typed_ros2_pub =
	std::dynamic_pointer_cast<typename rclcpp::Publisher<ROS2_T>>(ros2_pub);

	if (!typed_ros2_pub) {
	throw std::runtime_error("Invalid type for publisher");
	}

	const boost::shared_ptr<ros::M_string> & connection_header =
	ros1_msg_event.getConnectionHeaderPtr();
	if (!connection_header) {
	printf(" dropping message without connection header\n");
	return;
	}

	std::string key = "callerid";
	if (connection_header->find(key) != connection_header->end()) {
	if (connection_header->at(key) == "/ros_bridge") {
	return;
	}
	}

	const boost::shared_ptr<ROS1_T const> & ros1_msg = ros1_msg_event.getConstMessage();

	auto ros2_msg = std::make_shared<ROS2_T>();
	convert_1_to_2(ros1_msg, ros2_msg);
	RCUTILS_LOG_INFO_ONCE_NAMED(
	"ros1_bridge",
	"Passing message from ROS 1 %s to ROS 2 %s (showing msg only once per type)",
	ros1_type_name.c_str(), ros2_type_name.c_str());
	typed_ros2_pub->publish(ros2_msg);
	}

	auto publisher_count = ros2_node->count_publishers(topic_name);
	auto subscriber_count = ros2_node->count_subscribers(topic_name);

	// ignore publishers from the bridge itself
	if (bridges_1to2.find(topic_name) != bridges_1to2.end()) {
	if (publisher_count > 0) {
	--publisher_count;
	}
	}
	// ignore subscribers from the bridge itself
	if (bridges_2to1.find(topic_name) != bridges_2to1.end()) {
	if (subscriber_count > 0) {
	--subscriber_count;
	}
	}

	auto callback =
	[this, ros1_pub, ros1_type_name, ros2_type_name](const typename ROS2_T::SharedPtr msg) {
	return this->ros2_callback(
	msg, ros1_pub, ros1_type_name, ros2_type_name);
	};