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

Data Deserialisation Fails "For reasons unknown" #1769

Closed
Progeny42 opened this issue Jul 17, 2023 · 5 comments
Closed

Data Deserialisation Fails "For reasons unknown" #1769

Progeny42 opened this issue Jul 17, 2023 · 5 comments

Comments

@Progeny42
Copy link

Progeny42 commented Jul 17, 2023

I've been trying to use the dynsub application in order to test our implementation (being aware that it is incomplete). However, I'm receiving the following error:

1689579283.839235 [5]     recvUC: data(application, vendor 1.16): 1107db2:4fbe4261:4c1859db:202 #1: deserialization sensor_track_type_project_topic/org::omg::c4i::Domain_Model::Sensor_Domain::Track_Reporting::sensor_track_type failed (for reasons unknown)

This is occurring in the recvUC thread, and from what I've been able to gather by stepping through the DDS code, appears to be due to the fact that the code is incorrectly processing incoming fields, possibly due to the determined size and alignment of fields being incorrect.

We are using the OMG OARIS v2.0 - beta 1 sensor_track_type. With the following fields of the structure in use, the dynsub application then fails to decode the incoming message, because it believes that the time_of_information field (which is effectively a uint8_t) is a boolean discriminant, so any value other than 0 or 1 causes the deserialisation code to fail.

additional_information - Size of 136
covariance_matrix - Not present
environment - Present
initiation_mode - Not present
jammer_indication - False
max_range_limit - Not present
position - POLAR discriminant
position.elevation_coordinate - Not present
position.range_coordinate - Not present
position.origin - Not present
position_accuracy - Present, position_accuracy_coordinate_type_polar_position_accuracy_kind discriminant
position_accuracy.elevation_accuracy - Not present
position_accuracy.range_accuracy - Not present
position_accuracy.origin - Not present
position_accuracy_coordinate_system - Not present
position_coordinate_system.kind - POLAR
position_coordinate_system.orientation - NORTH_DOWN
position_coordinate_system.origin - SENSOR_REFERENCE_POINT
sensor_track_pre_identification - Present
sensor_track_pre_recognition - Present
simulated - False
time_of_information - 2

That is as far as it gets before it fails, so I've omitted the other fields.

Could this potentially be an issue with the XType layout?

@eboasson
Copy link
Contributor

Are you sure that time_of_information is a uint8_t? I downloaded the IDL definitions from the OMG for easier searching and it looks to me like TimeBase.idl has typedef unsigned long long TimeT, then Common_Types.idl has typedef org::omg::c4i::TimeBase::TimeT time_type and Track_Reporting.idl has org::omg::c4i::Domain_Model::Common_Types::time_type time_of_information.

Not that it matters much, if it says "deserialization failed" then there's a bug somewhere, be it in the IDL compiler, Type Object handling, conversion of Type Object to (de)serialilizer instructions, C++ serialization code if you use C++ ...

A few practical questions:

  • Are all involved processes using Cyclone DDS or is the publisher using a different implementation? If it is, it is harder to debug ...
  • If the publisher is also Cyclone, is it C, C++ or Python code?
  • Do you by any chance have a simple reproducer? These C4I data types are always quite an effort to fill out, and if you happen to have a bit of code that publishes just the sample that causes the error, that'd save me a lot of time.

Thanks!

@kh-aeros
Copy link

Apologies that was a typo, I had meant uint64_t, not uint8_t as you correctly pointed out.

Are all involved processes using Cyclone DDS or is the publisher using a different implementation

Yes, we are using Cyclone in our application.

If the publisher is also Cyclone, is it C, C++ or Python code?

We are using the C++ bindings via CycloneDDS-CXX to publish the data.

Attached is a minimal example, which results in the error being displayed within dynsub.
main.zip

The example:

  • Creates a data writer
  • Sets representative fields of the OARIS sensor_track_type in order to cause the observed issue
  • Writes the data, of which dynsub is listening for

@dpotman
Copy link
Contributor

dpotman commented Jul 19, 2023

Thanks for the reproducer! I found out that the issue was incorrect handling of the discriminant for default union cases, and is fixed in the Cyclone master branch by commit fcc53b8 (part of PR #1433). With this commit in (and the topic descriptors re-generated using the patched IDL compiler), deserialization of the sample succeeds. However, the current dynsub application crashes because it lacks support for alias, enum and union types.

Based on code shared by @eboasson I've created a PR (#1776) that adds support for these types in dynsub. With these changes in, it displays the correct sample data for the sample published by the reproducer code.

@dpotman
Copy link
Contributor

dpotman commented Jul 19, 2023

I just realized that I ran the reproducer using final extensibility for the data types. As the IDL doesn't have extensibility annotations set on the struct and union types, Cyclone's IDL compiler uses (for backwards compatibility) final as the default extensibility. However, the default extensibility in the OMG XTypes 1.3 specification is appendable (which you might have set as default in the IDL compiler command line options or in the idlcxx_generate call in cmake). To be sure that also works (this results in slightly different CDR), I re-ran the tests with appendable extensibility, and that also seems to be working fine.

@kh-aeros
Copy link

Thanks. We've cherry picked that commit, and confirmed ourselves that is is no longer reporting the error within dynsub. I'll close this issue.

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

4 participants