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

Binding a Fast RTPS publisher with a ROS 2 Dashing subscriber node fails #829

Closed
TSC21 opened this issue Oct 27, 2019 · 32 comments
Closed

Comments

@TSC21
Copy link

TSC21 commented Oct 27, 2019

I am pushing ros2/rmw_fastrtps#251 here, as I am not sure at this point if this is a problem on the ROS 2 node or a problem on the agent code.

System specifications:

  • OS: Ubuntu 18.04
  • Fast-RTPS version: 1.8.2
  • Fast-RTPS-Gen version: 1.0.2
  • ROS 2 version: Dashing
  • rmw_fastrtps version 0.7.6
  • Setup: px4_ros_com built from source. Launching micrortps_bridge -t UDP in a console and sensor_combined_listener in another.

Code snippets:

  • The agent publisher code:
/*!
 * @file SensorCombined_Publisher.cpp
 * This file contains the implementation of the publisher functions.
 *
 * This file was adapted from the fastcdrgen tool.
 */


#include <fastrtps/participant/Participant.h>
#include <fastrtps/attributes/ParticipantAttributes.h>
#include <fastrtps/publisher/Publisher.h>
#include <fastrtps/attributes/PublisherAttributes.h>

#include <fastrtps/Domain.h>


#include "SensorCombined_Publisher.h"


SensorCombined_Publisher::SensorCombined_Publisher() : mp_participant(nullptr), mp_publisher(nullptr) {}

SensorCombined_Publisher::~SensorCombined_Publisher() { Domain::removeParticipant(mp_participant);}

bool SensorCombined_Publisher::init()
{
    // Create RTPSParticipant
    ParticipantAttributes PParam;
    PParam.rtps.builtin.domainId = 0;
    PParam.rtps.builtin.leaseDuration = c_TimeInfinite;
    PParam.rtps.setName("SensorCombined_publisher");  //You can put here the name you want
    mp_participant = Domain::createParticipant(PParam);
    if(mp_participant == nullptr)
        return false;

    // Register the type
    Domain::registerType(mp_participant, static_cast<TopicDataType*>(&myType));

    // Create Publisher
    PublisherAttributes Wparam;
    Wparam.topic.topicKind = NO_KEY;
    Wparam.topic.topicDataType = myType.getName();  //This type MUST be registered
    Wparam.topic.topicName = "rt/SensorCombined_PubSubTopic";
    mp_publisher = Domain::createPublisher(mp_participant, Wparam, static_cast<PublisherListener*>(&m_listener));
    if(mp_publisher == nullptr)
        return false;
    //std::cout << "Publisher created, waiting for Subscribers." << std::endl;
    return true;
}

void SensorCombined_Publisher::PubListener::onPublicationMatched(Publisher* pub, MatchingInfo& info)
{
    if (info.status == MATCHED_MATCHING)
    {
        n_matched++;
        std::cout << "Publisher matched" << std::endl;
    }
    else
    {
        n_matched--;
        std::cout << "Publisher unmatched" << std::endl;
    }
}

void SensorCombined_Publisher::run()
{
    while(m_listener.n_matched == 0)
    {
        std::this_thread::sleep_for(std::chrono::milliseconds(250)); // Sleep 250 ms
    }

    // Publication code
    px4_msgs::msg::SensorCombined st;

    /* Initialize your structure here */

    int msgsent = 0;
    char ch = 'y';
    do
    {
        if(ch == 'y')
        {
            mp_publisher->write(&st);  ++msgsent;
            std::cout << "Sending sample, count=" << msgsent << ", send another sample?(y-yes,n-stop): ";
        }
        else if(ch == 'n')
        {
            std::cout << "Stopping execution " << std::endl;
            break;
        }
        else
        {
            std::cout << "Command " << ch << " not recognized, please enter \"y/n\":";
        }
    }while(std::cin >> ch);
}

    void SensorCombined_Publisher::publish(px4_msgs::msg::SensorCombined* st)
{
    mp_publisher->write(st);
}
  • The subscriber ROS 2 node:
/**
 * @brief Sensor Combined uORB topic listener example
 * @file sensor_combined_listener.cpp
 * @addtogroup examples
 * @author Nuno Marques <nuno.marques@dronesolutions.io>
 * @author Vicente Monge
 */

 #include <rclcpp/rclcpp.hpp>
 #include <px4_msgs/msg/sensor_combined.hpp>

/**
 * @brief Sensor Combined uORB topic data callback
 */
class SensorCombinedListener : public rclcpp::Node
{
public:
	explicit SensorCombinedListener() : Node("sensor_combined_listener") {
		subscription_ = this->create_subscription<px4_msgs::msg::SensorCombined>(
			"SensorCombined_PubSubTopic",
			[this](const px4_msgs::msg::SensorCombined::UniquePtr msg) {
			std::cout << "\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n";
			std::cout << "RECEIVED SENSOR COMBINED DATA"   << std::endl;
			std::cout << "============================="   << std::endl;
			std::cout << "ts: "          << msg->timestamp    << std::endl;
			std::cout << "gyro_rad[0]: " << msg->gyro_rad[0]  << std::endl;
			std::cout << "gyro_rad[1]: " << msg->gyro_rad[1]  << std::endl;
			std::cout << "gyro_rad[2]: " << msg->gyro_rad[2]  << std::endl;
			std::cout << "gyro_integral_dt: " << msg->gyro_integral_dt << std::endl;
			std::cout << "accelerometer_timestamp_relative: " << msg->accelerometer_timestamp_relative << std::endl;
			std::cout << "accelerometer_m_s2[0]: " << msg->accelerometer_m_s2[0] << std::endl;
			std::cout << "accelerometer_m_s2[1]: " << msg->accelerometer_m_s2[1] << std::endl;
			std::cout << "accelerometer_m_s2[2]: " << msg->accelerometer_m_s2[2] << std::endl;
			std::cout << "accelerometer_integral_dt: " << msg->accelerometer_integral_dt << std::endl;
		});
	}

private:
	rclcpp::Subscription<px4_msgs::msg::SensorCombined>::SharedPtr subscription_;
};

int main(int argc, char *argv[])
{
	std::cout << "Starting sensor_combined listener node..." << std::endl;
	setvbuf(stdout, NULL, _IONBF, BUFSIZ);
	rclcpp::init(argc, argv);
	rclcpp::spin(std::make_shared<SensorCombinedListener>());

	rclcpp::shutdown();
	return 0;
}

A WireShark record from localhost with both agent and ROS 2 node subscriber running: https://drive.google.com/open?id=1QTx6xjRQZ1Rh1F-sibek40AvvSsF3hw6.

@richiware @LuisGP @MiguelCompany any tips will help, as this is currently a development blocker. Thank you!

@TSC21
Copy link
Author

TSC21 commented Oct 30, 2019

@richiprosima friendly ping :)

@TSC21
Copy link
Author

TSC21 commented Oct 31, 2019

I think I might have a clue of what is happening. Using $ ros2 topic echo /SensorCombined_PubSubTopic I get:

Cannot echo topic '/SensorCombined_PubSubTopic', as it contains more than one type: [px4_msgs::msg::SensorCombined, px4_msgs/msg/SensorCombined]

Where px4_msgs::msg::SensorCombined is the registered type for the SensorCombined microRTPS agent plublisher, and px4_msgs/msg/SensorCombined is the type expected by the sensor_combined_listener ROS 2 listener node, as it can be seen bellow:

$ ros2 node info /sensor_combined_listener
/sensor_combined_listener
  Subscribers:
    /SensorCombined_PubSubTopic: px4_msgs/msg/SensorCombined
    /parameter_events: rcl_interfaces/msg/ParameterEvent
  Publishers:
    /parameter_events: rcl_interfaces/msg/ParameterEvent
    /rosout: rcl_interfaces/msg/Log
  Services:
    /sensor_combined_listener/describe_parameters: rcl_interfaces/srv/DescribeParameters
    /sensor_combined_listener/get_parameter_types: rcl_interfaces/srv/GetParameterTypes
    /sensor_combined_listener/get_parameters: rcl_interfaces/srv/GetParameters
    /sensor_combined_listener/list_parameters: rcl_interfaces/srv/ListParameters
    /sensor_combined_listener/set_parameters: rcl_interfaces/srv/SetParameters
    /sensor_combined_listener/set_parameters_atomically: rcl_interfaces/srv/SetParametersAtomically

This leads me to think that the way that the agent and the rmw_fastrtps registers the types are different now. My question is: how can I match the types here, and what changes do I have to do on the microRTPS agent so to properly register the type?

This is an issue, but I suspect that I might have to adjust the QoS params as well, unless rmw_fastrtps assumes default QoS params.

Thanks in advance for your help!

@TSC21
Copy link
Author

TSC21 commented Oct 31, 2019

@dirk-thomas do you have any tips on the above?

@richiware
Copy link
Member

Dashing has its own IDL generator for FastRTPS. But px4_ros_com is using fastrtpsgen to generate type support. It seems these generators generate different typename. The best solution will be px4_ros_com uses current Dashing IDL generator to generate typesupport instead of fastrtpsgen.

@TSC21
Copy link
Author

TSC21 commented Oct 31, 2019

Dashing has its own IDL generator for FastRTPS. But px4_ros_com is using fastrtpsgen to generate type support. It seems these generators generate different typename. The best solution will be px4_ros_com uses current Dashing IDL generator to generate typesupport instead of fastrtpsgen.

@richiware I am using the IDL's generated from the ROS IDL generator on fastrtpsgen to generate the typesupport. How do you suggest to use the ROS typesupport generator? I mean, px4_msgs already generates the typesupport for FastRTPS, but how can one use it on the microRTPS instead of the ones generated by fastrtpsgen? Does one have to replace https://github.com/PX4/px4_ros_com/blob/master/cmake/GenerateMicroRTPSAgent.cmake#L175-L178 with the typesupport headers generated by px4_msgs?

@TSC21
Copy link
Author

TSC21 commented Oct 31, 2019

Update: Looking at SensorCombinedPubSubTypes.cpp and to dds_fastrtps/sensor_combined__type_support.cpp, they seem to be quite different.

One main difference between what was being done on Crystal and now on Dashing for px4_ros_com is that for Crystal and before, I was using the IDL's generated from rosidl_generate_dds_interfaces() CMake directive, and now for Dashing I am using rosidl_generate_interfaces(), which already generates the IDL files as well. I am not sure at this point what to use and why so if someone can clarify it that would be better.

@TSC21
Copy link
Author

TSC21 commented Nov 2, 2019

Unfortunately I am not able to use the IDL generator given by rosidl_generate_dds_interfaces() as well:

[ 13%] Built target px4_msgs__rosidl_generator_c
[ 13%] Generating C introspection for ROS interfaces
No terminal defined for 'u' at line 1 col 1      

uint64 timestamp	# time since system sta
^

Expecting: {'CONST', 'MODULE', 'ENUM', 'STRUCT', 'TYPEDEF', '__ANON_9', 'AT'}
 /home/nuno/PX4/px4_ros_com_ros2/src/px4_msgs/msg/ActuatorArmed.msg
Error processing idl file: /home/nuno/PX4/px4_ros_com_ros2/src/px4_msgs/msg/ActuatorArmed.msg
Traceback (most recent call last):               
  File "/opt/ros/dashing/lib/rosidl_generator_dds_idl/rosidl_generator_dds_idl", line 41, in <module>
    sys.exit(main())
  File "/opt/ros/dashing/lib/rosidl_generator_dds_idl/rosidl_generator_dds_idl", line 36, in main
    args.additional_service_templates,
  File "/opt/ros/dashing/lib/python3.6/site-packages/rosidl_generator_dds_idl/__init__.py", line 51, in generate_dds_idl
    generate_files(generator_arguments_file, mapping, additional_context, keep_case=True)
  File "/home/nuno/PX4/rosidl_ws/install/rosidl_cmake/lib/python3.6/site-packages/rosidl_cmake/__init__.py", line 94, in generate_files
    raise(e)
  File "/home/nuno/PX4/rosidl_ws/install/rosidl_cmake/lib/python3.6/site-packages/rosidl_cmake/__init__.py", line 73, in generate_files
    idl_file = parse_idl_file(locator)
  File "/home/nuno/PX4/rosidl_ws/install/rosidl_parser/lib/python3.6/site-packages/rosidl_parser/parser.py", line 64, in parse_idl_file
    content = parse_idl_string(string, png_file=png_file)
  File "/home/nuno/PX4/rosidl_ws/install/rosidl_parser/lib/python3.6/site-packages/rosidl_parser/parser.py", line 72, in parse_idl_string
    tree = get_ast_from_idl_string(idl_string)
  File "/home/nuno/PX4/rosidl_ws/install/rosidl_parser/lib/python3.6/site-packages/rosidl_parser/parser.py", line 89, in get_ast_from_idl_string
    return _parser.parse(idl_string)
  File "/usr/lib/python3/dist-packages/lark/lark.py", line 306, in parse
    return self.parser.parse(text, start=start)
  File "/usr/lib/python3/dist-packages/lark/parser_frontends.py", line 182, in parse
    return self._parse(text, start)
  File "/usr/lib/python3/dist-packages/lark/parser_frontends.py", line 54, in _parse
    return self.parser.parse(input, start, *args)
  File "/usr/lib/python3/dist-packages/lark/parsers/earley.py", line 293, in parse
    self._parse(stream, columns, to_scan, start_symbol)
  File "/usr/lib/python3/dist-packages/lark/parsers/xearley.py", line 137, in _parse
    to_scan = scan(i, to_scan)
  File "/usr/lib/python3/dist-packages/lark/parsers/xearley.py", line 114, in scan
    raise UnexpectedCharacters(stream, i, text_line, text_column, {item.expect.name for item in to_scan}, set(to_scan))
lark.exceptions.UnexpectedCharacters: No terminal defined for 'u' at line 1 col 1

uint64 timestamp	# time since system sta
^

Expecting: {'CONST', 'MODULE', 'ENUM', 'STRUCT', 'TYPEDEF', '__ANON_9', 'AT'}

@dirk-thomas
Copy link
Contributor

I don't have any input in this beyond the content that you need to use compatible types.

Atm the ROS typesupport for FastRTPS doesn't use fastrtpsgen (maybe @richiwarr can comment why that is the case and why those two result in incompatible types).

I also don't know enough about micro and why it would not just use the same types already existing in ROS.

@TSC21
Copy link
Author

TSC21 commented Nov 3, 2019

Atm the ROS typesupport for FastRTPS doesn't use fastrtpsgen (maybe @richiware can comment why that is the case and why those two result in incompatible types).

@richiware can you please comment on this? I mean, at this stage, I don't understand as well why would this result on incompatible types. I was with the idea that any Fast-RTPS participant would be capable of communicating between each other, and while ROS is using rmw_fastrtps with specific typesupport, I don't see the reason why should fastrtpsgen follow the same type generation, since from what I can see on the Fast-RTPS examples, all of them use the PubSubTypes generated by fastrtpsgen.

This only means that currently, any Fast-RTPS application using fastrtpsgen to generate its typesupport for its messages is basically incompatible with with any ROS2 node. Which ends up being quite awkward, given that Fast-RTPS started as the default middleware for ROS2.

For the implementation of the microRTPS to work, at this stage, with Dashing, it requires that the full code to be ported as a ROS 2 node, which at this stage is not on the plans and it was not something that one should have to be concerned about, as by default we would expect that two participants on a DDS domain, even more from the same vendor, would be communicating correctly between each other.

@richiprosima @LuisGP (or any other eProsima team member) please advise when able. Thank you!

Note: Keep in mind that I can correctly bind a Publisher generated with fastrtpsgen of Fast-RTPS version 1.7.2 and a ROS2 Crystal Subscriber node. The same I cannot say with Fast-RTPS-Gen v1.0.2 (now out of the Fast-RTPS repo) and the same node but running under ROS2 Dashing interfaces and typesupport.

@TSC21 TSC21 changed the title Bind a Fast RTPS publisher with a ROS 2 subscriber fails Binding a Fast RTPS publisher with a ROS 2 Dashing subscriber node fails Nov 3, 2019
@richiware
Copy link
Member

I was investigating a little more, fastrtpsgen should generate the type's name like the ROS2 IDL generator. fastrtpsgen also creates the type's name using ::. I don't know why ros2 topic echo is finding a type with type's name using / as separator.

@richiware
Copy link
Member

I can give you more information. New rosidl_adapter generates from SensorCombined.msg next SensorCombined.idl file:

module px4_msgs
{
    module msg
    {
        struct SensorCombined
        {
            . . .
        };
    };
};

With this IDL fastrtpsgen generates a type with type's name px4_msgs::msg::SensorCombined as expected. But ROS2 rmw_fastrtps_cpp internally uses the type's name px4_msgs::msg::dds_::SensorCombined_. Surely it is to maintain retrocompatibility.

But for Connext and Opensplice, ROS2 is still generating the old-way IDL files. For SensorCombined.msg generates SensorCombined_idl file:

module px4_msgs
{
    module msg
    {
        module dds_
        {
            struct SensorCombined_
            {
                . . .
            };
        };
    };
};

@dirk-thomas Any suggestion?

@dirk-thomas
Copy link
Contributor

There are two conflicting parts in the IDL generation / type naming:

  • you want to use the same type name across different vendors because you want them to be interoperable
  • the type names for each vendor (e.g. in C++) must be different from each other otherwise the generated files collide with each other as well as with the ROS specific C++ files

But for Connext and Opensplice, ROS2 is still generating the old-way IDL files.

That is why the IDL files generated for each vendor use the extra module dds_. That is not the old-way but a necessity to avoid the collision with the ROS 2 types. For each vendor that module name gets rewritten into something vendor specific, e.g. dds_connext.

@richiware
Copy link
Member

Thanks, @dirk-thomas. @TSC21 For Crystal you should be using the IDL files generated by rosidl_dds for Opensplice to generate code with fastrtpsgen, shouldn't you? In this case, for Dashing you should use also these IDL files instead of the ones generated by rosidl_adapter.

@TSC21
Copy link
Author

TSC21 commented Nov 6, 2019

@TSC21 For Crystal you should be using the IDL files generated by rosidl_dds for Opensplice to generate code with fastrtpsgen, shouldn't you?

@richiprosima no I am not. As you can see on the history of this issue, in Crystal and bellow I use rosidl_generate_dds_interfaces() to generate the IDL files from the msg files and store them under the px4_msgs/msg/dds_fastrtps msg dir - https://github.com/PX4/px4_msgs/blob/master/CMakeLists.txt#L54-L58. As far as I know, this has nothing to do with OpenSplice.
Also, as reported in ros2/rosidl_dds#52, rosidl_generate_dds_interfaces() in Dashing doesn't support passing msg files but rather expects one to pass IDL files so to be converted to the right format.

That is why the IDL files generated for each vendor use the extra module dds_. That is not the old-way but a necessity to avoid the collision with the ROS 2 types. For each vendor that module name gets rewritten into something vendor specific, e.g. dds_connext.

@dirk-thomas where exactly are these being stored? I don't see any dds_fastrtps folder being created where the IDL files are stored, similar to what rosidl_generate_dds_interfaces() was doing for Crystal and bellow if one sets up OUTPUT_SUBFOLDERS dds_fastrtps.

@dirk-thomas
Copy link
Contributor

where exactly are these being stored? I don't see any dds_fastrtps folder being created where the IDL files are stored

The type support packages for FastRTPS don't need custom .idl files and are being generated by the parsed information of the generic .idl files coming from rosidl_adapter.

You should find the exact path of the .idl files in the install spaces of any message package which invokes rosidl_generate_interfaces.

@TSC21
Copy link
Author

TSC21 commented Nov 6, 2019

where exactly are these being stored? I don't see any dds_fastrtps folder being created where the IDL files are stored

The type support packages for FastRTPS don't need custom .idl files and are being generated by the parsed information of the generic .idl files coming from rosidl_adapter.

You should find the exact path of the .idl files in the install spaces of any message package which invokes rosidl_generate_interfaces.

I know. But then we get back to the exact same problem that is being reported.

@richiware
Copy link
Member

@dirk-thomas Changing the type name in code, adding ::dds_, it's creating a different type than the type defined in the generated IDL file from rosidl_adapter. This interoperability problem will exist with any vendor IDL generator (not only fastrtpsgen) when a user generates source code using the generated IDL from rosidl_adapter.

I don't' know if it was a decision you made, but if you want to solve this interoperability problem I think the types defined in the IDL file generated by rosidl_adapter should match with the type name used when registering the type in the DDS participant.

@TSC21
Copy link
Author

TSC21 commented Nov 13, 2019

@dirk-thomas is there a way we can find a compatible solution for this? Otherwise, it's impossible to have a bind between ROS 2 DDS participants and other participants from Fast-RTPS applications.
Until Crystal, this was completely fine when one used rosidl_generate_dds_interfaces() to generate the IDL file. But now, one can't even use it anymore.

@dirk-thomas
Copy link
Contributor

It is unlikely that I will have the bandwidth to drive that development. But I am more than happy to review any proposals and help out.

Any proposed solution still needs to make sure that cross-vendor communication continues to work and that the generated types (from ROS and the different vendors) can coexist.

@TSC21
Copy link
Author

TSC21 commented Nov 14, 2019

Any proposed solution still needs to make sure that cross-vendor communication continues to work and that the generated types (from ROS and the different vendors) can coexist.

Considering that the problem here is that we have incompatible types for the same vendor, I think that's reason enough to consider it already a problem. Besides, rosidl_generate_dds_interfaces() API and usage was changed and that broke what it was used for in versions before Dashing, so by itself it lead to a problem on the implementation of the px4_msgs package.

@dirk-thomas I understand that you may not have the time, but considering that this circumstance actually breaks in-vendor compatibility under the same DDS domain, I think this is of the most importance to actually fix it - otherwise the problem will continue to exist and will never be solved. At least some guidance on how to approach it would be fine (readding the previous behaviour for rosidl_generate_dds_interfaces() to make the types match would be good).

@richiprosima do you have anything else to add here as well?

Thank you both!

@TSC21
Copy link
Author

TSC21 commented Dec 4, 2019

@dirk-thomas @tfoote @richiware as Crystal is becoming EOL, it's becoming essential that this gets solved as soon as possible.

This reported problem is, IMO, a direct violation of the wire protocol interoperability that should be respected for a DDS compliant implementation, given that RTPS is not being made compatible between two implementations and typesupport generations for the same vendor.

If you don't have time to fix this issue, I ask you to please provide insights on how you propose this to be solved. I suggested above that, at least, rosidl_generate_dds_interfaces() can continue to provide the previous API for generating the required interfaces that can be made compatible with the ones generated from fastrtpsgen, as it was being done previous to Dashing. This is probably not the ideal solution, but at least we could make sure that the generated typesupport and type naming was compatible between the two generators.

If you feel there's another way of quickly solving this, please let me know. In the mean time, I ask you to please to provide some insights on how we could handle this incompatibility in a way we don't have to completely change the microRTPS bridge implementation. It's also our priority to support and use what ROS 2 has to offer, but with Crystal becoming EOL, that will be at risk.

Thanks in advance!

@LorenzMeier, @thomasgubler, @jkflying, FYI.

@thomasgubler
Copy link

@dirk-thomas @tfoote

@TSC21/Auterion are happy to support implementation but we want to avoid implementing in the wrong direction that afterwards can not get merged.

Could you provide guidance? Would you maybe be available for a call this Thu or Fr your morning (PST)?

@JaimeMartin
Copy link
Member

@thomasgubler @tfoote

If I understand correctly the problem is because of the generated type name. I can think about several workarounds, but for the future, the way to go is using Micro-ROS as the Bridge.

I am available also to attend the call.

@TSC21
Copy link
Author

TSC21 commented Dec 4, 2019

the way to go is using Micro-ROS as the Bridge.

@JaimeMartin we are not planning at this point to move to microROS. The paradigm will completely change in terms of what we currently view as the standard bridging between the companion computer and PX4 (it wouldn't be a bridge between uORB and RTPS, but would rather tend to replace uORB) and we are not planning to replace uORB, at least from what I know and discussed with the dev team and the stakeholders.

@TSC21
Copy link
Author

TSC21 commented Dec 4, 2019

@JaimeMartin if you know about "workarounds", I ask you to please suggest one so we can work through it. Thank you!

@JaimeMartin
Copy link
Member

@TSC21 The Micro-ROS client does not replace the uORB, that is not the idea. We developed micro-RTPS bridge as the initial PoC for what is now Micro-ROS. In that sense, it is outdated, and the way to go is Micro-ROS.

Also, for your issue, you have functions as https://github.com/eProsima/Fast-RTPS/blob/1.9.x/include/fastrtps/TopicDataType.h#L88 you could use to change the name.

@TSC21
Copy link
Author

TSC21 commented Dec 7, 2019

@TSC21 The Micro-ROS client does not replace the uORB, that is not the idea. We developed micro-RTPS bridge as the initial PoC for what is now Micro-ROS. In that sense, it is outdated, and the way to go is Micro-ROS.

Where exactly is all of this documented? If we are to consider using micro-ROS and the Micro-XRCE capabilities, I need to first understand if it makes sense to have an MVP based on how well documented the API is so it can be used and integrated in short time. Otherwise, it doesn't make sense to replace the bridge. Also, it's not deprecated, it has actually been maintained and updated to our needs. What was a PoC is no longer a a PoC and it's actually performing well and fitting what we require.

Also, for your issue, you have functions as https://github.com/eProsima/Fast-RTPS/blob/1.9.x/include/fastrtps/TopicDataType.h#L88 you could use to change the name.

Right, thanks for this. I am going to try this and let you know. Though, I am still convinced that the rosidl_adapter should be kept consistent with the Fast-RTPS generators and not the other way around. That's why I was focusing the solution in the ROS 2 side and not in the Fast-RTPS side.

@richiware
Copy link
Member

@TSC21 Here you have some links:

@TSC21
Copy link
Author

TSC21 commented Jan 5, 2020

@dirk-thomas I was able to change the Type name of the agent side. Though, weirdly enough, I get the following when trying to echo the topic:

$ ros2 topic echo /SensorCombined_PubSubTopic 
Cannot echo topic '/SensorCombined_PubSubTopic', as it contains more than one type: [px4_msgs/msg/SensorCombined, px4_msgs/msg/SensorCombined]

?? What's happening here? As far as I can tell the types look exactly the same...

@ldg810
Copy link

ldg810 commented Jan 10, 2020

@TSC21

@dirk-thomas I was able to change the Type name of the agent side. Though, weirdly enough, I get the following when trying to echo the topic:

$ ros2 topic echo /SensorCombined_PubSubTopic 
Cannot echo topic '/SensorCombined_PubSubTopic', as it contains more than one type: [px4_msgs/msg/SensorCombined, px4_msgs/msg/SensorCombined]

?? What's happening here? As far as I can tell the types look exactly the same...

Did you found any solution of this problem?

I found a trick to solve the problem from here.
My environment is Fast-RTPS master, ROS2 dashing, Ubuntu 18.04

I used px4_ros_com repository.
After generate micrortps_agent source, you should modify Name of the topic. as follows.
In case of SensorCombined Data, (in SensorCombinedPubSubTypes.cpp)
setName("px4_msgs::msg::SensorCombined"); to setName("px4_msgs::msg::dds_::SensorCombined_");

Then, you can echo the topic!

root@ldg810-X299-WU8:/home/user/git/PX4_rtps/px4_ros_com_ros2# ros2 topic echo /SensorCombined_PubSubTopic
timestamp: 2681980000
gyro_rad: [-0.0014800801873207092, -0.0010418224846944213, -0.0023238887079060078]
gyro_integral_dt: 4000
accelerometer_timestamp_relative: 0
accelerometer_m_s2: [-0.3590507507324219, 0.21467521786689758, -9.883310317993164]
accelerometer_integral_dt: 4000
---
timestamp: 2681988000
gyro_rad: [0.0009716902859508991, -0.006787224672734737, -0.00034484994830563664]
gyro_integral_dt: 4000
accelerometer_timestamp_relative: 0
accelerometer_m_s2: [-0.18192364275455475, 0.13727518916130066, -9.877113342285156]
accelerometer_integral_dt: 4000

@TSC21
Copy link
Author

TSC21 commented Jan 10, 2020

@ldg810 thank you! I should have found that source earlier.

The solution (even though not ideal) was added in PX4/PX4-Autopilot#13907. Thanks for the support. Closing.

@TSC21
Copy link
Author

TSC21 commented Mar 27, 2020

For everyone still having this issue, I added a fix to the FastRTPSGen tool that through an argument passed to the fastrrpsgen script (-typeros2), the type naming will be generated with the structure compatible with ROS2 topics. So now there is no need to change the naming manually after generating the code.

Hope this helps!

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

No branches or pull requests

6 participants