Giter Club home page Giter Club logo

Comments (11)

dirk-thomas avatar dirk-thomas commented on September 13, 2024

I can reproduce the problem. It looks like this is a regression of #227. The topic names are being split into the basename and the partition. Then only the basename is being used to lookup the topic type (

topic_description = participant->lookup_topicdescription(topic_str);
).

I will try to come up with a fix for this. I think it is significant enough to be made for the beta 2 even though we are in code freeze.

from rmw_connext.

dirk-thomas avatar dirk-thomas commented on September 13, 2024

Reading more about partitions and the API in Connext it looks to me that the topic description (which is identifying the type) is being looked up by DDSDomainParticipant::lookup_topicdescription. That function only takes the topic name but not the partition as an arguments. Might it be the case that the same topic in different partitions must have the same type (at least in Connext)?

If that is the case I fear that the concept of partitions can't be used to implement the concept of ROS namespace??? (I hope I am missing something here.) @Karsten1987 @wjwwood Maybe either of you has some more insight on this since you were most involved in this part.

from rmw_connext.

dhood avatar dhood commented on September 13, 2024

I haven't read the details yet, but if you're correct that the types must match for topics in different partitions: there might still be a way we can use partitions to implement namespaces, if we can somehow "trick" connext into using FQNs for the topic type lookup. It never complained in beta1 when doing psuedo-namespacing by putting "test/chatter" as the topic name before the namespace implementation was introduced (that is, there's no restriction on using slashes in topic names in connext). This isn't a concrete suggestion at the moment, just something to get some ideas rolling while I have time to read up.

from rmw_connext.

Karsten1987 avatar Karsten1987 commented on September 13, 2024

So from what I understood the actual problem comes from create_topic which only matches the topic with the type and returns NULL when they don't match. Now, the problem I haven't figured out a solution yet is why this match is done on participant level and not on publisher. That's the reason why the namespaces just work fine between two different applications/nodes, but not within the same node. I may have misunderstood the concept of partitions, but IMO you should be able to create multiple instances of the same topic on different partitions.

I'll have to look at bit more into what options we have.

from rmw_connext.

dhood avatar dhood commented on September 13, 2024

"Participant level" seems to be the key here: you can run the following with no complaints from connext, since they're different participants:

RMW_IMPLEMENTATION=rmw_connext_cpp ros2 topic pub chatter std_msgs/String
RMW_IMPLEMENTATION=rmw_connext_cpp ros2 topic pub chatter std_msgs/Int32

Then the two [topic, type] pairs work fine independently.

RMW_IMPLEMENTATION=rmw_ext_cpp ros2 topic echo chatter std_msgs/Int32
RMW_IMPLEMENTATION=rmw_connext_cpp ros2 topic echo chatter std_msgs/String

while in a single process/node/participant like the modified talker demo, namespaces or not, there is the "wrong type writer" issue if you try to publish different types to the same topic.

I don't mean to say that you should be allowed to publish different types to the same topic. My point is that we can probably get around our current issue in the same way: by using different participants.

from rmw_connext.

dhood avatar dhood commented on September 13, 2024

Looks like it's recommended to use as few participants as possible, so my previous suggestion is not likely to be an ideal solution for the namespacing issue: https://community.rti.com/best-practices/create-few-domainparticipants-possible

from rmw_connext.

dhood avatar dhood commented on September 13, 2024

From reading RTI responses on forum (1, 2, 3) I agree that it looks like connext is not supposed to support different types on the same topic from within a participant, and namespaces aren't taken into consideration with that.

I did find a workaround though that's less dramatic than creating separate participants:

We can get around the '1 type per topic' restriction if we "just" embed the name of the topic type into the ROS -> DDS topic name mangling. Not pretty, but it gets hidden below the RMW layer for the most part.

For interoperability we have to apply the same thing across all DDS rmw implementations, but we already have mangling in our topics names anyway.

With no modification to the above example (but changes to rmw_connext_cpp + rmw_fastrtps_cpp to embed the topic type into the topic name), it now works fine:

$ RMW_IMPLEMENTATION=rmw_connext_cpp ros2 run demo_nodes_py talker
RTI Data Distribution Service EVAL License issued to OSRF [email protected] For non-production use only.
Expires on 16-Jul-2017 See www.rti.com for more information.
Publishing: "Hello World: 1"
Publishing: "Hello World: 2"
Publishing: "Hello World: 3"
RMW_IMPLEMENTATION=rmw_connext_cpp ros2 topic echo chatter std_msgs/String
RTI Data Distribution Service EVAL License issued to OSRF [email protected] For non-production use only.
Expires on 16-Jul-2017 See www.rti.com for more information.
data: 'Hello World: 11'

data: 'Hello World: 12'
$ RMW_IMPLEMENTATION=rmw_fastrtps_cpp ros2 topic echo test/chatter std_msgs/Int64
data: 37

data: 38
$ RMW_IMPLEMENTATION=rmw_fastrtps_cpp ros2 topic list --show-types
/chatter_std_msgs::msg::dds_::String_ [std_msgs/String]
/test/chatter_std_msgs::msg::dds_::Int64_ [std_msgs/Int64]

That's just a hacky prototype. We'd have to use a version of the type names that doesn't have characters not allowed by the spec (not that connext or fastrtps seem to mind), and this will limit users' topic name length, but otherwise I think the only other thing that has to change is the get_topic_names_and_types de-mangling. Since we can infer the part of the topic name to remove from the topic type, we don't have to limit what topic name options/characters are allowed for users, but 'wasting' characters when we already have a length limit may be an issue.

It's not elegant, but this is one possible workaround. By keeping partitions for namespaces we save the implementation of aliasing and dynamically change namespaces easily, so maybe it's worth the trade-off.

from rmw_connext.

Karsten1987 avatar Karsten1987 commented on September 13, 2024

That will certainly work, but at the same time that could also imply that we go back to pack the FQDN "/test/chatter" completely into the topic field with "test__chatter" by applying underscores. If we can't get around modifying the topic string in any case, we may consider keeping it somewhat intuitive so that external DDS components can make sense out of it.

argh, that's a bummer.

Now as for beta2, we could agree on leaving the system as is, stating that creating multiple publisher/subscriber/etc. within the same node, the same topic and using connext will lead to undefined behavior. I do believe it's exactly this combination which happens to fail. Other methods such as loading multiple nodes via composition will work as they have their own participant.

from rmw_connext.

madMAx43v3r avatar madMAx43v3r commented on September 13, 2024

Unfortunately choosing DDS was a huge mistake, you chose ~1000 pages of over-engineered specification that was never fixed and which took ~10 years just to implement version 1. As such you should have expected endless issues like this one.
The success of ROS1 is due to its simplicity and efficiency which you both threw out the window with ROS2. Just ask yourself this, will ROS2 ever be able to transmit ~100 MB/s of data without burning up the entire CPU and having to mess around with hundreds of QoS settings just to get it to work in the first place?

from rmw_connext.

dirk-thomas avatar dirk-thomas commented on September 13, 2024

Unfortunately choosing DDS was a huge mistake, you chose ~1000 pages of over-engineered specification that was never fixed and which took ~10 years just to implement version 1.

Thank you for sharing your opinion. From the feedback we have received so far it seems that many people are disagreeing with that and think choosing DDS as the default option for the RMW implementation makes a lot of sense.

As such you should have expected endless issues like this one.

The problem described in this ticket is actually not really a problem in the DDS spec but due to the way ROS 2 chose to map namespaces to partitions. The next release of ROS 2 will actually use a different approach which resolves this problem.

Just ask yourself this, will ROS2 ever be able to transmit ~100 MB/s of data without burning up the entire CPU and having to mess around with hundreds of QoS settings just to get it to work in the first place?

I am pretty confident that DDS users are already using the software in such scenarios. And since ROS 2 doesn't even expose that many QoS settings the user doesn't have to deal with that "complexity".

from rmw_connext.

dhood avatar dhood commented on September 13, 2024

We've switched back to not using partitions for the implementation of namespace (ros2/ros2#476). Regression test for this issue in ros2/rcl#257

from rmw_connext.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.