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.

Sign up for GitHub

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

Mikael/remove topic partitions #171

Merged
merged 6 commits into from
Apr 30, 2018
Merged

Mikael/remove topic partitions #171

Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
3a4a777
unnecessary Now
mikaelarguedas Apr 28, 2018
d12dd0e
XRCE DDS -> DDS-XRCE + link
mikaelarguedas Apr 28, 2018
c1c22c4
try to make the sentence sound more naturel
mikaelarguedas Apr 28, 2018
7b8333a
attempt to clarify the 'un-natural' character of using partition
mikaelarguedas Apr 28, 2018
4d93d60
missing whitespace
mikaelarguedas Apr 28, 2018
9fc4805
avoid having same section and subsection title
mikaelarguedas Apr 28, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 8 additions & 8 deletions articles/140_topic_and_service_name_mapping.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -202,7 +202,7 @@ The ROS topic and service name constraints allow more types of characters than t
These must be substituted or otherwise removed during the process of converting the topic or service name to DDS concepts.
Since ROS 2 topic and service names are expanded to fully qualified names, any balanced bracket (`{}`) substitutions and tildes (`~`) will have been expanded.
Additionally any URL related syntax, e.g. the `rostopic://` prefix, will be removed once parsed.
Previously forward slashes (`/`) were disallowed in DDS topic names, now the restriction has been lifted(see [issue](https://issues.omg.org/issues/lists/dds-rtf5#issue-42236) on omg.org) and therefore the ROS topic names are first prefixed with ROS Specific Namespace prefix (described below) are then mapped directly into DDS topic names.
Previously forward slashes (`/`) were disallowed in DDS topic names, now the restriction has been lifted (see [issue](https://issues.omg.org/issues/lists/dds-rtf5#issue-42236) on omg.org) and therefore the ROS topic names are first prefixed with ROS Specific Namespace prefix (described below) are then mapped directly into DDS topic names.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i'd remove now in the sentence.


### ROS Specific Namespace Prefix

Expand Down Expand Up @@ -253,9 +253,8 @@ Since all ROS topics are prefixed when being converted to DDS topic names, it ma
For example, if an existing DDS program is publishing on the `image` topic (and is using the DDS equivalent to the ROS message type) then a ROS program could not subscribe to it because of the name mangling produced by the implicit ROS specific namespace.
Therefore to allow ROS programs to interoperate with "native" DDS topic names the API should provide a way to skip the ROS specific prefixing.

#### Communicating with Non-ROS Topics

Now, There is an option in the API, a boolean `avoid_ros_namespace_convention` in the qos_profile which can be set to `false` to use ROS prefix and `true` to not using ROS namespace prefixing.
There is an option in the API, a boolean `avoid_ros_namespace_convention` in the qos_profile which can be set to `false` to use ROS prefix and `true` to not using ROS namespace prefixing.

For example:

Expand All @@ -264,7 +263,8 @@ For example:
| `rostopic://image` | `false` | `rt/image` |
| `rostopic://image` | `true` | `image` |

Alternative(Idea):
#### Alternative(Idea)

Note that the alternative below is not part of the proposal, but only possible solutions to the issue of communicating with "native" DDS topics.
Another option would be to have some markup in the scheme name, for example:

Expand All @@ -277,7 +277,7 @@ Another option would be to have some markup in the scheme name, for example:

## Compare and Contrast with ROS 1

In order to support a mapping to a little more restrictive DDS topic name rules, these rules are in some ways more constraining than the rules for ROS 1.
In order to support a mapping to the - slightly more - restrictive DDS topic name rules, these rules are in some ways more constraining than the rules for ROS 1.
Other changes have been proposed for convenience or to remove a point of confusion that existed in ROS 1.
In ROS 2, topic and service names differ from ROS 1 in that they:

Expand Down Expand Up @@ -359,10 +359,10 @@ You can read more about partitions in RTI's documentation:

Trade-offs (in comparison to using the whole ROS name along with the namespaces):

- Splitting the ROS name into "namespace" and "base name", and placing the complete namespace into a single partition seemed un-natural.
- Splitting the ROS name into "namespace" and "base name", and placing the complete namespace into a field designed for another purpose seemed incorrect.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So I actually still think that we have to justify this statement more precisely. So one concrete reason why putting the namespace in a partition is incorrect is that the tuple of (partition, topic, type) has to be unique amongst a DDS environment. Fast-RTPS does not enforce this, however RTI does, hence the github issue below.

- In general partitions are recommended to be used as a spare, but using partitions for all ROS names suggested otherwise.
- Major concern was reported in this [issue](https://github.com/ros2/rmw_connext/issues/234), where having two topics with same base name, although different namespace and different ypes caused problem. For example: topicA is `/camera/data` of type `Image` and topicB is `/imu/data` of type `Imu`. The base names for both topicA and topicB is `data`, generated errors as described in the [issue](https://github.com/ros2/rmw_connext/issues/234).
- Newer standards such as XRCE DDS might not have partitions at all.
- Major concern was reported in this [issue](https://github.com/ros2/rmw_connext/issues/234), where having two topics with same base name, although different namespace and different types caused problem. For example: topicA is `/camera/data` of type `Image` and topicB is `/imu/data` of type `Imu`. The base names for both topicA and topicB is `data`, generated errors as described in the [issue](https://github.com/ros2/rmw_connext/issues/234).
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"... with the same base name ... caused a problem."

- Newer standards such as [DDS-XRCE](https://www.omg.org/spec/DDS-XRCE) might not have partitions at all.
- Using the complete ROS name in the later strategy will cause a tighter length limit on base name because the DDS topic name would contain ROS prefix, namespace along with the base name which should not exceed DDS topic name limitation which is 256 characters.

Rationale:
Expand Down