DDS Topic name with '-' leads to BAD PARAMTER error

[AkhilTThomas]

The issue #657 is marked as closed but there is still no resolution.

https://www.omg.org/spec/DDS/1.4/PDF#page=179

The DDS specifications mentions that '-' is a valid character but cyclonedds throws the below error: cyclonedds.core.DDSException: [DDS_RETCODE_BAD_PARAMETER] Bad parameter value. Occurred upon initialisation of a cyclonedds.topic.Topic

[eboasson]

The argument in #657 — many thanks for making the effort of looking it up! — holds just as much today as it did then, so it was to be expected.

How big is the problem you are facing? If you are dealing with a system originally built using one of those sloppy (😁) implementations, and are now trying to gradually replace that implementation with Cyclone (or if you have integrate with such a system), I can see how this could be a real problem and we might have to relent a bit.

[AkhilTThomas]

I'm trying to define DDS topic names based on the VSS specification this would require ideally a '.' separator but since that is not a valid character at least '-' would work for me.

[eboasson]

A very brief look at that website suggests there is no "official" mapping to (OMG) IDL, so presumably you'd be free to translate the names? If so, why would an '_' or '/' not work?

[AkhilTThomas]

'_' would be fine for now but it will not be conforming to the OMG guidelines.This can cause interoperability issues with other DDS implementations. Would it be a major change to support '-'?

[eboasson]

I don't think that it would be a major change, the code base doesn't really do anything with the actual topic name (other than check it for validity) but it might well affect tooling, whether that be a scary perl script to help make sense of the trace files Cyclone can write, or things like the cyclonedds tool. If there are some limitations, I'm sure they can be resolved easily.

The biggest problem is probably having to step off my soap-box despite my annoyance at the quality of the OMG specs and the general sloppiness of most implementations, and all the nonsense that causes ...

I'll leave this open, label as it "enhancement" (@thijsmie might be able to comment on whether this is likely to cause a problem, other people might to chime in as well). If you just want run ahead on the reasonable assumption that the character set will be extended: modifying https://github.com/eclipse-cyclonedds/cyclonedds/blob/e54f6eeaa2bc5d7860975507ef60802246ffbb46/src/core/ddsc/src/dds_topic.c#L56 is all I expect to take.

[thijsmie]

This should not cause any issues on any (python) tooling side, it actually just assumes "unicode string" there and makes no attempt to do any parsing or assign special meaning to characters. No complaints from my side to allow this 😁

[clunietp]

Similar to OP, I'm attempting to develop systems which comply with the UMAA specification, which uses colons extensively as a namespacing mechanism. See: https://www.auvsi.org/unmanned-maritime-autonomy-architecture

We're currently using RTI Connext and/or Fast-DDS, and are very interested in Cyclone as well; early interop tests with Cyclone are showing promise, and I'm a fan of your Python implementation as well. However, Cyclone does not support colons in a topic name and that is a hard requirement for my use case (RTI and eProsima do support colons)

Looking at the DDS specification referenced above, the TOPICNAME definition in question is specifically underneath the Annex B - Syntax for Queries and Filters section. I read this topic name definition/restriction as specific to SQL-related operations rather than DDS as a whole. In fact, I can't seem to find a DDS topic name restriction documented anywhere, other than for SQL queries as documented in the DDS spec.

So, two questions: 1 - am I missing the definitive restrictions on the topic name string contents? 2 - If not, are you willing to support the addition of colons to supported topic name strings? I'm happy to create a PR if it helps move things along

Thank you

[eboasson]

@clunietp you're not missing anything: that particular bit is all there is. Our pointing to that fragment to claim "this is what the spec says" is because the specs are no better than this: quite a few important things are only defined — in whole or in part — in an Annex, or sometimes not defined at all.

In a situation like this, the prudent thing for interoperability is to be restrictive in what accept, signalling errors early if there may be a problem. This practice goes back to OpenSplice, and hence goes back 15, 20 years, and it has served us well. However, the ways in which DDS was used was different in those days (more aerospace and defense, more isolated developments), and clearly our stance is now becoming a hindrance rather than a good thing.

There are a few ways in which allowed characters in topic names affect the implementation that I can think of:

• Formatting in trace files and tooling (for further processing them, it is quite convenient if, e.g., " cannot occur in a topic name). This is not a big deal, we can update the output and it won't affect any existing code anyway.
• Some configuration settings. These are unpleasant as they are and it would be much better to have separate XML attributes for partitions and topics anyway. This can be dealt with in a backwards compatible way and allowing more characters doesn't affect any existing code, and so, I'd say this too is not an urgent problem.
• The pattern matching related to the previous doesn't cater for multi-byte sequences. This could be an argument to restrict them to 7-bit clean strings.
• Everything else in the core just treats them as nul-terminated, printable blobs. (I.e. strlen, strcmp, %s in printf ...)
• C++ and Python impose no further restrictions.

So given that it seems we must change our ways ... how about we require isprint is true and not `,"or'` for the time being, then work our way to supporting UTF-8?

As you rightly surmised, there are a ton of things to be done and never enough time ... if you'd be willing to do a PR for this, that'd be fantastic.

Whoever disagrees should speak up or resign oneself to it 😁