OpenDDS
Demarshal problem when using @appendable and types evolve
I have an application v1 that uses the following IDL type
module DDS_Native
{
typedef string<128> color_type;
@nested(FALSE)
@appendable
struct ShapeType {
@key color_type color;
long x;
long y;
long shapesize;
};
}; // module DDS_Native
Now this application evolves into v2 which uses uses the following IDL definition which is the same type but with an additional field
module DDS_Native
{
typedef string<128> color_type;
@nested(FALSE)
@appendable
struct ShapeType {
@key color_type color;
long x;
long y;
long shapesize;
long angle;
};
}; // module DDS_Native
When application v2 publishes a sample v1 does receive the data, the angle is just dropped. But when v1 publishes a sample than v2 receives the following error:
(17330|17339) ERROR DDS_Native::ShapeTypeDataReaderImpl::dds_demarshal deserialization failed, dropping sample.
Just to confirm this is with the default data representation XCDR2 and not XCDR1?
Actually the first log has this:
(15773|15773) DataReaderImpl::setup_deserialization: Setup successfully with the following data representations: Unaligned CDR
Unaligned CDR doesn't support extensibility at all, that's why it didn't work.
Out of curiosity, what does the writer side log for setup_serialization?
Attaching full log
(7167|7167) DataReaderImpl::setup_deserialization: Setup successfully with the following data representations: Unaligned CDR
(7168|7168) DDS_Native::ShapeTypeDataWriterImpl::setup_serialization: Setup successfully with Unaligned CDR data representation.
I'm not sure how you got unaligned CDR in a RTPS setup though...
I am also not certain, this is a new unit test for AXCIOMA where I test extensibility, same test does run with RTI without problems. Any ideas where to look further at this?
(7167|7167) TransportImpl::open()
transport_type: tcp
Ahh, you're using tcp transport. I was confused because I assuming it was rtps_udp since it was RTPS discovery. Defaulting to XCDR2 or unaligned CDR is based on encapsulation, which requires an rtps_udp transport to be enabled..
I don't think we were focused on non-standard RTPS setups when doing the initial XTypes work, but it's probably possible to change it so this works.
Ok, the only thing we do is to force RTPS as default discovery, we don't do any special transport settings (https://github.com/RemedyIT/ciaox11/blob/402e0f11aacc5486096fe2e1f70e554466f28816/ddsx11/vendors/opendds/dds/opendds_specific.cpp#L72)
TheServiceParticipant->set_default_discovery (OpenDDS::DCPS::Discovery::DEFAULT_RTPS);
Do we need to do something special to make sure rtps_udp is used? It looks strange that I explicitly use @appendable and that a transport is used which doesn't support that.
This is what I use in PyOpenDDS:
OpenDDS::DCPS::TransportConfig_rch transport_config =
TheTransportRegistry->create_config("default_rtps_transport_config");
OpenDDS::DCPS::TransportInst_rch transport_inst =
TheTransportRegistry->create_inst("default_rtps_transport", "rtps_udp");
transport_config->instances_.push_back(transport_inst);
TheTransportRegistry->global_config(transport_config);
I think this is based on something in the configuration section in the devguide.
It looks strange that I explicitly use @appendable and that a transport is used which doesn't support that.
Appendable is the default extensibility, so it won't look any different to OpenDDS at runtime unless we explicitly tracked that a extensibility annotation was used. Besides that I think it was assumed that if unsupported setup was used, then we continued as normal.
The OpenDDS devguide doesn't say anything of this mandatory dependency, something should be added there to my idea.
The current behaviour is very nasty, we are using @appendable explicitly in IDL, without manually adding rtps_udp the user will ship an application which is not interoperable when he ships a new version with an extended IDL definition. Can't imagine the user digs into the logs to see that he gets the wrong transport. Not sure what the best setup is, maybe when @appendable is explicitly set we should report an error when there is not a correct transport to fulfill that requirement?
Creating the rtps_udp transport did result in the correct runtime behaviour, but I do now have new errors at shutdown from OpenDDS, now using https://github.com/jwillemsen/ciaox11/blob/7b368f89e94b4dc6ad77eda3cf792fe488284736/ddsx11/vendors/opendds/dds/opendds_specific.cpp#L77
(19080|19080) TransportRegistry::create_config: name=default_rtps_transport_config is already in use.
(19080|19080) TransportRegistry::create_inst: name=default_rtps_transport is already in use.
The OpenDDS devguide doesn't say anything of this mandatory dependency, something should be added there to my idea.
I agree. That or we should make it so this works.
maybe when
@appendableis explicitly set we should report an error when there is not a correct transport to fulfill that requirement?
Maybe, we do error on mutable XCDR1 because we know that doesn't work. It would be more like XCDR1 appendable though, where we warn about, but it works as long as you don't try to actually use the extensibility features.
Maybe, we do error on mutable XCDR1 because we know that doesn't work. It would be more like XCDR1 appendable though, where we warn about, but it works as long as you don't try to actually use the extensibility features.
They should have make final the default, that matches the old behavior, extensibility should be a user decision as it also has some performance costs.
The TransportRegistry errors are caused by our code, some operation which I expected to be called once is executed multiple times, will check that at our side.
So the issue left is that @appendable currently only works with XCDR2 which is only supported by the rtps_udp transport which the user has to manually select. We can force rtps_udp in our code, but that is something we don't prefer to do. What about supporting XCDR2 with all transports?
Yes, I talked with @iguessthislldo about this yesterday. We should be able to change it so that TransportClient::cdr_encapsulation_ is set to true when Mutable/Appendable is in use.
This issue was not fixed, see attached log, when having v1 published v2 can't demarshal it, even with the fixes of #3040
Looks we still get unaligned CDR, anything special we should do in our settings?
(8948|8948) DataReaderImpl::setup_deserialization: Setup successfully with the following data representations: Unaligned CDR
Is the new configuration option from #3040 enabled?
We only set RTPS as discovery (see https://github.com/RemedyIT/ciaox11/blob/1e3347656694c1899773752cb196cfe4b7527d11/ddsx11/vendors/opendds/dds/opendds_specific.cpp#L74), so we need to explicitly enable this? We leave the exact transports to be used up to OpenDDS, we don't really care at the ddsx11 level. We do explicitly tag all IDL types with @appendable, not sure why we also have to configure the transports with some special settings, doesn't feel logical.
Is there a global option to enable cdr_encapsulation for all transports that the OpenDDS core decides to use?
There are more details on #3040 about why it's set up this way. At least for the near term it needs a configuration switch.
However, I thought about it some more and I think this "encapsualtion header" is unnecessary to get Mutable/Appendable support in the non-rtps transports. The DataSampleHeader and DataRepresentationQosPolicy should be sufficient to get the same info.
If this turns out to be correct, we should be able to get the same functionality without needing explicit configuration.
The DataSampleHeader and DataRepresentationQosPolicy should be sufficient to get the same info.
It could depending on the situation, but it's a lot safer to use EncapsulationHeader to actually read what the stream says it is to the get the Encoding object.
I think configuring it could be easier, but ultimately I think we're going to have to configure something to get XCDR2 for the time being. This is because:
- We can't enable encapsulation/XCDR2 for all transports at the moment because it would break the Wireshark sample dissector and it would need a decent amount of work to be able to read XCDR2.
- After thinking about it, it doesn't really make sense to me for an explicit annotation that is already implied to change the transport behavior. It'd be an inconsistency because it'd require an explicit
@appenablewhen it's not required for the RTPS transport.
So if we can't just enable it yet or magically deduce it without making inconsistent rules, then the only option is to require it to be configured manually somehow.
We do generate explicit @appendable in the DDS related IDL. Wasn't aware of the missing xcdr2 support for wireshark.
What about a feature to force XCDR2 for all transports at compile time? We generate the OpenDDS configuration we need as part of the AXCIOMA configure process. For our users we force them to use XCDR2, at the moment they need wireshark they have to sponsor something additionally
@jwillemsen since something needs to be configured, would this work?
DataWriterQos qos;
// get defaults into qos
qos.representation.value.length(1);
qos.representation.value[0] = XCDR2_DATA_REPRESENTATION;
With AXCIOMA we use the QoS XML support, we could document to our users that they need to configure XCDR2 explicitly in their QoS settings, the only problem right now is that representation isn't supported by the QoS XML library, see #2434